From 9f072c110c14c7f00fc20d7d18988cbebfb0a6a1 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 14 Oct 2026 23:58:11 +0200 Subject: [PATCH] Take no rebuild into a waiting walk, and ask what a tier left unasked (hq issue 754) A rebuild that landed while a walk waited for mesh-delivery's word was taken as the walk's build of that module; once let go, the walk asked a tier only when every module of it was unasked, so it never asked the rest and stood still. A walk now takes only the outcomes of builds it asked, and none while it waits; a rebuild does not join a waiting walk and says it runs beside it; and a tier asked in part is asked the rest, including a module held built that the walk never asked for, which is somebody else's build and not sent as the walk's. --- cmd/mesh-controller/batches.go | 2 +- cmd/mesh-controller/plan_retry.go | 7 + cmd/mesh-controller/queue.go | 9 +- cmd/mesh-controller/queue_test.go | 5 +- cmd/mesh-controller/release_plan.go | 89 +++++++++---- .../walk_wait_rebuild_754_test.go | 122 ++++++++++++++++++ 6 files changed, 206 insertions(+), 28 deletions(-) create mode 100644 cmd/mesh-controller/walk_wait_rebuild_754_test.go diff --git a/cmd/mesh-controller/batches.go b/cmd/mesh-controller/batches.go index 2399cad3..e5d3ea50 100644 --- a/cmd/mesh-controller/batches.go +++ b/cmd/mesh-controller/batches.go @@ -1049,7 +1049,7 @@ func planBatch(ctx context.Context, open *stores, batch *inventory.Plan, carry [ return nil } // Written and its first tier asked as one act on the plans (novox/hq issue 213), under the caller's hold. - if err := askTier(ctx, inv, &plan); err != nil { + if err := askTier(ctx, inv, &plan, plan.Tiers[plan.Tier]); err != nil { return err } if err := inv.SavePlan(ctx, &plan); err != nil { diff --git a/cmd/mesh-controller/plan_retry.go b/cmd/mesh-controller/plan_retry.go index 93a3bcdc..acb722c8 100644 --- a/cmd/mesh-controller/plan_retry.go +++ b/cmd/mesh-controller/plan_retry.go @@ -372,6 +372,13 @@ func joinAPlan(ctx context.Context, open *stores, module string) (bool, string, if !found { return false, "", nil } + // **A walk that waits for its delivery's word asks nothing** (novox/hq ADR 0239, issue 754): a rebuild does not + // join it, and its outcome is not the walk's. The walk asks its own tier once it has its word. + if p.Waiting() { + return false, fmt.Sprintf("not joining %s (%s at %s): it waits for %s's word, so this build is not its "+ + "outcome; once let go, the walk asks %s itself", p.ID, p.Repository, short(p.Commit), p.Delivery.Awaits, + module), nil + } entries, err := inv.Catalogued(ctx) if err != nil { return false, "", err diff --git a/cmd/mesh-controller/queue.go b/cmd/mesh-controller/queue.go index 651dbcdf..e6bd065d 100644 --- a/cmd/mesh-controller/queue.go +++ b/cmd/mesh-controller/queue.go @@ -521,7 +521,9 @@ func rebuildCommand(ctx context.Context, args []string) error { } // **A module a plan holds unbuilt, or failed, joins that plan** rather than running beside it: the - // plan would ask it again, or stay failed on an outcome a rebuild has replaced. + // plan would ask it again, or stay failed on an outcome a rebuild has replaced. A walk waiting for its word + // is not joined (novox/hq issue 754): the rebuild runs beside it, and says so. + var beside string if module != "" { joined, said, err := joinAPlan(ctx, open, module) if err != nil { @@ -531,6 +533,7 @@ func rebuildCommand(ctx context.Context, args []string) error { fmt.Println(said) return nil } + beside = said } id, err := askABuild(ctx, source, path, ref) if err != nil { @@ -538,6 +541,10 @@ func rebuildCommand(ctx context.Context, args []string) error { } recordAsked(ctx, inventory.BuildRequest{ID: id, Repository: source.Repository, Seat: source.Seat, Path: path, Ref: ref, For: "rebuild"}) + if beside != "" { + fmt.Printf("rebuild asked as %s, beside the walk: %s\n", id, beside) + return nil + } fmt.Printf("rebuild asked as %s\n", id) return nil } diff --git a/cmd/mesh-controller/queue_test.go b/cmd/mesh-controller/queue_test.go index dfd656d3..0a4a13fd 100644 --- a/cmd/mesh-controller/queue_test.go +++ b/cmd/mesh-controller/queue_test.go @@ -637,8 +637,9 @@ func TestAPlanStoppedAtItsFirstMachineIsRetried(t *testing.T) { stopped := inventory.Plan{ID: "plan-rollout", Repository: "novox/a", Branch: "main", Commit: "c0ffee", Created: long, State: inventory.PlanFailed, Tiers: [][]string{{"a"}, {"b"}}, Note: "a stopped at its first machine in tier 0: laptop refused what it was sent", - Modules: map[string]*inventory.PlanModule{"a": {State: "built", BuiltAt: &long, Commit: "c0ffee", - First: []string{"laptop"}, FirstAt: &long, Why: "laptop refused what it was sent"}}} + // Asked by the plan, as every module a plan holds built is (novox/hq issue 754). + Modules: map[string]*inventory.PlanModule{"a": {State: "built", AskedAt: &long, Build: "build-a", BuiltAt: &long, + Commit: "c0ffee", First: []string{"laptop"}, FirstAt: &long, Why: "laptop refused what it was sent"}}} if err := open.inventory.SavePlan(ctx, &stopped); err != nil { t.Fatal(err) } diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index 63430ce0..bb49ccad 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -259,10 +259,11 @@ func applied(module string, builtAt time.Time, running []string, reports []inven return len(waiting) == 0, waiting } -// askTier asks the build machine for every module of the tier, and marks each asked. A module +// askTier asks the build machine for the named modules of the plan's tier — all of it, or what of it is still +// unasked (novox/hq issue 754) — and marks each asked. A module // the catalogue no longer holds, or whose ask could not be made, is a failure of the plan: a tier // half asked is a tier that will never complete. -func askTier(ctx context.Context, inv *inventory.Inventory, p *inventory.Plan) error { +func askTier(ctx context.Context, inv *inventory.Inventory, p *inventory.Plan, names []string) error { entries, err := inv.Catalogued(ctx) if err != nil { return err @@ -271,12 +272,62 @@ func askTier(ctx context.Context, inv *inventory.Inventory, p *inventory.Plan) e for _, e := range entries { byName[e.Manifest.Module] = e } - for _, name := range p.Tiers[p.Tier] { + for _, name := range names { askModule(ctx, p, name, byName) } return nil } +// unaskedOf are the modules of a plan's current tier it has still to ask: those it never asked, and one it holds +// built that it never asked for (novox/hq issue 754) — a build taken in by a controller from before a walk took +// only its own asks, somebody else's, which the walk would otherwise send as its own (ADR 0219). A tier partly asked is asked the rest, not +// waited on for ever: a walk that waits for every build of a tier it never asked whole never moves again. +func unaskedOf(p inventory.Plan) []string { + if p.Tier >= len(p.Tiers) { + return nil + } + var out []string + for _, m := range p.Tiers[p.Tier] { + s := p.Modules[m] + switch { + case s == nil || s.State == "": + out = append(out, m) + case s.State == "built" && s.Build == "" && s.AskedAt == nil: + out = append(out, m) + } + } + return out +} + +// askedInTier says the plan asked a module of its current tier itself: under an id, or at a time it kept. +func askedInTier(p inventory.Plan) bool { + if p.Tier >= len(p.Tiers) { + return false + } + for _, m := range p.Tiers[p.Tier] { + if s := p.Modules[m]; s != nil && (s.Build != "" || s.AskedAt != nil) { + return true + } + } + return false +} + +// takesOutcome says whether a build's outcome is the plan's for a module of its current tier (novox/hq ADR 0219, +// issues 219 and 754). **A walk that waits for its delivery's word asks nothing, and so is answered by nothing** +// (ADR 0239): a rebuild that lands in the wait is somebody else's build. **Nor is a module the plan never asked +// answered**: taken, the plan held a module built that it never asked, and waited for the rest of the tier for +// ever. Asked under an id, the plan takes that id's outcome and no other; asked before ids were kept, a build +// asked at or after the plan's ask. +func takesOutcome(p inventory.Plan, state *inventory.PlanModule, asked time.Time, id string) bool { + if p.Waiting() || state == nil || (state.Build == "" && state.AskedAt == nil) { + return false + } + if state.Build != "" { + return state.Build == id + } + return !askedBefore(asked, state.AskedAt) +} + // askABuild is how a plan asks for one build, not waited for, and learns the id it asked under. A // variable so a test of what a plan does around an ask needs no build machine. // @@ -526,22 +577,15 @@ func planBuiltSaying(ctx context.Context, open *stores, module, commit, failed s if !inTier { continue } - state := p.Modules[module] - if state == nil { - state = &inventory.PlanModule{} - p.Modules[module] = state - } // **The plan's own ask is its outcome, by id** (novox/hq ADR 0219); another build of the module // is, as before, when it was asked at or after the plan's ask (issue 219). // **A module asked under an id is answered by that id's outcome and no other** (novox/hq ADR // 0219): a replay, a rebuild beside the plan, or an older ask finishing late is somebody else's // build, made from other source, and settling the plan with it would send that. A plan from - // before ids were kept is matched as it was: by when the build was asked (issue 219). - if state.Build != "" { - if state.Build != id { - continue - } - } else if askedBefore(asked, state.AskedAt) { + // before ids were kept is matched as it was: by when the build was asked (issue 219). A walk + // waiting for its word, or a module it never asked, takes no outcome at all (issue 754). + state := p.Modules[module] + if !takesOutcome(*p, state, asked, id) { continue } if failed != "" && deletedAtSource(failed) { @@ -747,17 +791,11 @@ func advanceOnce(ctx context.Context, open *stores, p *inventory.Plan, return true, nil } tier := p.Tiers[p.Tier] - // Not yet asked: ask. - unasked := 0 - for _, m := range tier { - if s := p.Modules[m]; s == nil || s.State == "" { - unasked++ - } - } - if unasked == len(tier) { + // Not yet asked: ask — the whole tier, or what of it is still unasked (novox/hq issue 754). + if unasked := unaskedOf(*p); len(unasked) > 0 { // **One walk at a time** (novox/hq ADR 0276): a walk about to ask its first tier while another started // walk is open waits for that one to end, let go or not, and says so. - if p.Tier == 0 { + if p.Tier == 0 && !askedInTier(*p) { if behind, err := startedBeside(ctx, inv, p.ID, p.Created); err != nil { return false, err } else if behind != "" { @@ -767,7 +805,10 @@ func advanceOnce(ctx context.Context, open *stores, p *inventory.Plan, return changed, nil } } - if err := askTier(ctx, inv, p); err != nil { + if len(unasked) < len(tier) { + fmt.Printf("%s: tier %d was asked in part; asking the rest: %s\n", p.ID, p.Tier, strings.Join(unasked, ", ")) + } + if err := askTier(ctx, inv, p, unasked); err != nil { return false, err } return true, nil diff --git a/cmd/mesh-controller/walk_wait_rebuild_754_test.go b/cmd/mesh-controller/walk_wait_rebuild_754_test.go new file mode 100644 index 00000000..962fcebd --- /dev/null +++ b/cmd/mesh-controller/walk_wait_rebuild_754_test.go @@ -0,0 +1,122 @@ +package main + +import ( + "reflect" + "testing" + "time" + + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/link" +) + +// novox/hq issue 754: a walk waiting for its delivery's word takes no build's outcome, nor does a walk take one for +// a module it never asked; asked under an id, it takes that id's alone. +func TestAWalkTakesOnlyTheOutcomesOfWhatItAsked(t *testing.T) { + asked := time.Date(2026, 10, 14, 23, 30, 0, 0, time.UTC) + before := asked.Add(-time.Minute) + waiting := inventory.Plan{Delivery: &inventory.PlanDelivery{Awaits: "mesh-delivery"}} + letGo := inventory.Plan{Delivery: &inventory.PlanDelivery{Awaits: "mesh-delivery", Go: &asked}} + own := &inventory.PlanModule{State: "asked", AskedAt: &asked, Build: "build-own"} + old := &inventory.PlanModule{State: "asked", AskedAt: &asked} + for _, c := range []struct { + what string + p inventory.Plan + state *inventory.PlanModule + at time.Time + id string + takes bool + }{ + {"a rebuild while the walk waits", waiting, nil, asked, "build-rebuild", false}, + {"its own id while it waits", waiting, own, asked, "build-own", false}, + {"a module it never asked", letGo, nil, asked, "build-rebuild", false}, + {"a module it never asked, state kept empty", letGo, &inventory.PlanModule{}, asked, "build-rebuild", false}, + {"another id", letGo, own, asked, "build-rebuild", false}, + {"its own id", letGo, own, asked, "build-own", true}, + {"a plan from before ids, asked after it", letGo, old, asked.Add(time.Second), "x", true}, + {"a plan from before ids, asked before it", letGo, old, before, "x", false}, + {"a plan with no delivery, its own id", inventory.Plan{}, own, asked, "build-own", true}, + } { + if got := takesOutcome(c.p, c.state, c.at, c.id); got != c.takes { + t.Errorf("%s: takes %v, want %v", c.what, got, c.takes) + } + } +} + +// novox/hq issue 754: a tier partly asked asks the rest; a module held built that the plan never asked — the live +// shape, a hand rebuild taken in during the wait, built after the plan was made — is asked again; one asked is kept. +func TestATierPartlyAskedAsksTheRest(t *testing.T) { + created := time.Date(2026, 10, 14, 23, 30, 0, 0, time.UTC) + earlier, later := created.Add(-time.Hour), created.Add(7*time.Minute) + p := inventory.Plan{Created: created, Tiers: [][]string{{"build-agent", "gitea", "route-proxy", "stale", "kept", "own"}}, + Modules: map[string]*inventory.PlanModule{ + "gitea": {}, + "route-proxy": {State: "built", BuiltAt: &later, Commit: "c51357f8"}, + "stale": {State: "built", BuiltAt: &earlier}, + "kept": {State: "asked", AskedAt: &created, Build: "build-kept"}, + "own": {State: "built", AskedAt: &created, BuiltAt: &later, Build: "build-own"}, + }} + if got, want := unaskedOf(p), []string{"build-agent", "gitea", "route-proxy", "stale"}; !reflect.DeepEqual(got, want) { + t.Fatalf("unasked %v, want %v", got, want) + } + if !askedInTier(p) { + t.Fatal("a tier with a module asked under an id reads as never asked") + } + p.Modules["kept"], p.Modules["own"] = nil, nil + if askedInTier(p) { + t.Fatal("a tier the plan never asked reads as asked") + } +} + +// novox/hq issue 754, as seen on 2026-10-14: a rebuild lands while the walk waits for its word; the walk is let go; +// it asks every module of its tier, and completes it once those builds come in. +func TestARebuildInTheWaitIsNotTheWalksBuild(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + asked := asksRecorded(t) + twoTiers(t, open) + created := time.Now().UTC().Add(-10 * time.Minute) + plan := inventory.Plan{ID: "plan-754", Repository: "novox/a", Branch: "main", Commit: "c0ffee", Created: created, + State: inventory.PlanBuilding, Tiers: [][]string{{"a", "b"}}, Modules: map[string]*inventory.PlanModule{}, + Delivery: &inventory.PlanDelivery{Awaits: "mesh-delivery"}} + if err := open.inventory.SavePlan(ctx, &plan); err != nil { + t.Fatal(err) + } + + // A rebuild of a, beside the waiting walk: not joined, and its outcome not taken. + joined, said, err := joinAPlan(ctx, open, "a") + if err != nil || joined || said == "" { + t.Fatalf("a rebuild joined a waiting walk: %v %q %v", joined, said, err) + } + planBuilt(ctx, open, "a", "c0ffee", "", time.Now().UTC(), "build-rebuild") + p, _ := open.inventory.PlanByID(ctx, plan.ID) + if s := p.Modules["a"]; s != nil && s.State != "" { + t.Fatalf("the waiting walk took a rebuild as its build: %+v", s) + } + if len(*asked) != 0 { + t.Fatalf("a waiting walk asked %v", *asked) + } + + // Let go: both are asked, and their outcomes complete the tier. + if _, err := letGo(ctx, open.inventory, plan.ID, "operator", "test"); err != nil { + t.Fatal(err) + } + advancePlans(ctx, open) + p, _ = open.inventory.PlanByID(ctx, plan.ID) + for _, m := range []string{"a", "b"} { + if s := p.Modules[m]; s == nil || s.State != "asked" || s.Build == "" { + t.Fatalf("let go, %s is %+v", m, s) + } + } + if len(*asked) != 2 { + t.Fatalf("asked %v", *asked) + } + for _, m := range []string{"a", "b"} { + id := p.Modules[m].Build + at, _ := link.BuildAskedAt(id) + planBuilt(ctx, open, m, "c0ffee", "", at, id) + } + advancePlans(ctx, open) + if p, _ = open.inventory.PlanByID(ctx, plan.ID); p.Open() && p.Tier == 0 { + t.Fatalf("the walk did not get past its tier: %s at %d, %q", p.State, p.Tier, p.Note) + } +} -- 2.54.0