diff --git a/cmd/mesh-controller/gate.go b/cmd/mesh-controller/gate.go index 54b26eb0..d9d71bcc 100644 --- a/cmd/mesh-controller/gate.go +++ b/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) { - // What it moved from is not known: no wait is excused, rather than one the move did not bring. + if err != nil || !had { out[j.module] = false continue } diff --git a/cmd/mesh-controller/gate_followup_test.go b/cmd/mesh-controller/gate_followup_test.go index bd17ac8d..54022686 100644 --- a/cmd/mesh-controller/gate_followup_test.go +++ b/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) + } + } +} diff --git a/cmd/mesh-controller/release.go b/cmd/mesh-controller/release.go index ac9967c8..c1667ec2 100644 --- a/cmd/mesh-controller/release.go +++ b/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 { diff --git a/cmd/mesh-controller/tier_send_test.go b/cmd/mesh-controller/tier_send_test.go index a6e6b5fe..34be200d 100644 --- a/cmd/mesh-controller/tier_send_test.go +++ b/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) + } + }) + } +}