Merge pull request 'Excuse no wait for a move from a build not known, and test the tier path's at-once put-back (hq issue 318 review)' (#152) from fix/318-follow-up-2 into main

cmd/mesh-controller/gate.go

@@ -632,9 +632,14 @@ func movesAddingGroups(ctx context.Context, inv *inventory.Inventory, g *invento
 		if err != nil || !found {
 			target = shelf[j.module]
 		}
+		// What it moved from is not known — no build named (a plan's own module whose previous build was not
+		// recorded), or no manifest kept for it: no wait is excused, rather than one the move did not bring.
+		if from == "" {
+			out[j.module] = false
+			continue
+		}
 		before, had, err := inv.ManifestAt(ctx, j.module, from)
-		if err != nil || (from != "" && !had) {
+		if err != nil || !had {
-			// What it moved from is not known: no wait is excused, rather than one the move did not bring.
 			out[j.module] = false
 			continue
 		}

cmd/mesh-controller/gate_followup_test.go

@@ -421,3 +421,33 @@ func TestABrokenModuleIsPutBackAtOnceAndTheOneBesideItGetsItsOwnVerdict(t *testi
 		t.Fatalf("app is registered at %s; want it kept at c2", current["app"].Commit)
 	}
 }
+
+// **A move from a build not known excuses no wait** (issue 318 review): a plan's own module whose previous
+// build was not recorded, or whose previous manifest is not kept, cannot be shown to have added the group.
+func TestAMoveFromAnUnknownBuildExcusesNoWait(t *testing.T) {
+	open := aMesh(t)
+	ctx := t.Context()
+	inv := open.inventory
+	record := func(commit string, m catalogue.Manifest) {
+		raw, _ := json.Marshal(m)
+		at := time.Now().Add(-time.Hour)
+		if err := inv.RecordBuild(ctx, inventory.Build{ID: "build-lights-" + commit, Module: "lights", Commit: commit,
+			Repository: "novox/mesh-catalog", Path: "modules/lights", Manifest: raw, Asked: at, At: at}); err != nil {
+			t.Fatal(err)
+		}
+	}
+	record("c1", catalogue.Manifest{Module: "lights"})
+	record("c2", catalogue.Manifest{Module: "lights", Resources: []map[string]any{{"id": "operator", "type": "user",
+		"name": "operator", "groups": []any{"lights"}}}})
+	pairs := []judged{{module: "lights", node: "laptop"}}
+	shelf := map[string]catalogue.Manifest{}
+	for _, c := range []struct {
+		from string
+		want bool
+	}{{"c1", true}, {"", false}, {"c0-never-built", false}} {
+		g := &inventory.PlanGate{From: c.from, To: "c2"}
+		if got := movesAddingGroups(ctx, inv, g, pairs, shelf)["lights"]; got != c.want {
+			t.Errorf("a move from %q adds a group: %v; want %v", c.from, got, c.want)
+		}
+	}
+}

cmd/mesh-controller/release.go

@@ -414,7 +414,6 @@ func putBackBroken(ctx context.Context, open *stores, p *inventory.Plan, g *inve
 	batched, back := batchingRollbacks(ctx)
 	var said []string
 	for _, m := range todo {
-		g.Returned = append(g.Returned, m)
 		machines := g.Machines
 		var state *inventory.PlanModule
 		if m == lead && leadState != nil {
@@ -434,6 +433,8 @@ func putBackBroken(ctx context.Context, open *stores, p *inventory.Plan, g *inve
 				machines = []string{c.Node}
 			}
 		}
+		// Counted as put back only once there is something to put back (a carried move or the plan's own).
+		g.Returned = append(g.Returned, m)
 		p.Note = ""
 		gateFailed(batched, open, p, m, state, machines, g.BrokenWhy)
 		if m == lead && leadState != nil {

cmd/mesh-controller/tier_send_test.go

@@ -378,3 +378,60 @@ func TestAModuleItsSendChangedNothingOfIsLeftAsItWasAndRetried(t *testing.T) {
 		t.Fatalf("retried as %q: the plan is %s, app %+v", said, p.State, s)
 	}
 }
+
+// **In a plan's tier path, a broken module is put back at once too** (novox/hq issue 318 review): the node
+// tools' witness reverts them on the first machine. Whether the gate is kept on them or on the module beside
+// them, they are registered back and marked in the judging that finds them broken, the plan goes on judging
+// the other module to its own pass, and only then fails.
+func TestATierPutsABrokenModuleBackAtOnce(t *testing.T) {
+	for _, order := range [][]string{{broker.RuntimeModule, "app1"}, {"app1", broker.RuntimeModule}} {
+		t.Run("gate kept on "+order[0], func(t *testing.T) {
+			tm := aTierMesh(t, order...)
+			ctx := t.Context()
+			inv := tm.open.inventory
+			tierFacts := gatherGateFacts
+			var seen []string
+			gatherGateFacts = func(ctx context.Context, open *stores, component string) (gateFacts, error) {
+				f, err := tierFacts(ctx, open, component)
+				current, _ := open.inventory.CurrentBuilds(ctx)
+				seen = append(seen, current[broker.RuntimeModule].Commit)
+				if current[broker.RuntimeModule].Commit == "c2" {
+					f.rolledBack["anchor"] = []lease.Rollback{{Component: lease.ComponentNodeTools,
+						Outcome: lease.OutcomeRolledBack, From: "c2", To: "c1", At: time.Now(), Why: "the node tools did not answer"}}
+				}
+				return f, err
+			}
+			gateEvery, gateSettle, gateBound = 0, 100*time.Millisecond, 10*time.Second
+			for i := 0; i < 20 && len(seen) < 2; i++ {
+				advancePlans(ctx, tm.open)
+			}
+			if len(seen) < 2 || seen[0] != "c2" || seen[1] != "c1" {
+				t.Fatalf("the node tools' registered build at each judging: %v; want c2, then c1 at once", seen)
+			}
+			if failed, _ := inv.GateFailed(ctx, "build-"+broker.RuntimeModule+"-2"); !failed {
+				t.Fatal("the node tools' build is not marked failed at once")
+			}
+			if p := tm.plan(t); p.State == inventory.PlanFailed {
+				t.Fatalf("the plan failed at once: %s; want it judging app1 first", p.Note)
+			}
+			for deadline := time.Now().Add(5 * time.Second); time.Now().Before(deadline); {
+				advancePlans(ctx, tm.open)
+				if tm.plan(t).State == inventory.PlanFailed {
+					break
+				}
+				time.Sleep(20 * time.Millisecond)
+			}
+			p := tm.plan(t)
+			if p.State != inventory.PlanFailed {
+				t.Fatalf("the plan is %s: %s; want failed, for the node tools", p.State, p.Note)
+			}
+			if v, found, _ := inv.GateOf(ctx, "build-app1-2"); !found || v.Verdict != inventory.GatePassed {
+				t.Fatalf("app1's verdict: %+v; want its own pass (plan: %s)", v, p.Note)
+			}
+			if current, _ := inv.CurrentBuilds(ctx); current["app1"].Commit != "c2" || current[broker.RuntimeModule].Commit != "c1" {
+				t.Fatalf("registered app1 %s, node tools %s; want c2 and c1", current["app1"].Commit,
+					current[broker.RuntimeModule].Commit)
+			}
+		})
+	}
+}