From 3834c72747d70c005a40d89023cd023e09301443 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 19 Oct 2026 11:57:07 +0200 Subject: [PATCH] Find the delivery's owner on the controller's own path by the seat's claim, not its name (issue 1210) --- cmd/mesh-controller/batches.go | 15 ++-- cmd/mesh-controller/delivery.go | 73 ++++++++++++---- cmd/mesh-controller/fixclass_test.go | 3 +- cmd/mesh-controller/gate.go | 4 +- cmd/mesh-controller/gate_rule_test.go | 15 +++- cmd/mesh-controller/own_path_by_claim_test.go | 86 +++++++++++++++++++ cmd/mesh-controller/plan_retry.go | 16 ++-- cmd/mesh-controller/queue_test.go | 8 +- cmd/mesh-controller/related.go | 12 +-- cmd/mesh-controller/related_test.go | 12 +-- cmd/mesh-controller/release_plan.go | 8 +- 11 files changed, 200 insertions(+), 52 deletions(-) create mode 100644 cmd/mesh-controller/own_path_by_claim_test.go diff --git a/cmd/mesh-controller/batches.go b/cmd/mesh-controller/batches.go index 20240682..403e2956 100644 --- a/cmd/mesh-controller/batches.go +++ b/cmd/mesh-controller/batches.go @@ -367,7 +367,7 @@ func buildsEvery(p inventory.Plan, modules []inventory.Entry) bool { // ownPath says a merge touches a module on the controller's own path, whose walk waits for nobody's word. func ownPath(touches []inventory.Entry) bool { for _, e := range touches { - if _, own := onTheControllersPath[e.Manifest.Module]; own { + if onTheOwnPath(e.Manifest.Module, e.Manifest) { return true } } @@ -945,7 +945,11 @@ func cutUnrelated(ctx context.Context, open *stores, batches, started, waiting [ var read map[string][]inventory.ReadRepository var edges []inventory.Edge judged := false - openWalks, err := openWalksOf(ctx, inv, started) + onPath, err := ownPathNow(ctx, inv) + if err != nil { + return err + } + openWalks, err := openWalksOf(ctx, inv, started, onPath) if err != nil { return err } @@ -978,7 +982,7 @@ func cutUnrelated(ctx context.Context, open *stores, batches, started, waiting [ var fold, keep []inventory.Plan if !b.OwnPath() { for _, w := range waiting { - ww, err := openWalkOf(ctx, inv, w) + ww, err := openWalkOf(ctx, inv, w, onPath) if err != nil { return err } @@ -1013,7 +1017,7 @@ func cutUnrelated(ctx context.Context, open *stores, batches, started, waiting [ if !cut.Open() { continue } - w, err := openWalkOf(ctx, inv, cut) + w, err := openWalkOf(ctx, inv, cut, onPath) if err != nil { return err } @@ -1391,8 +1395,9 @@ func classOfModules(modules []string, entries []inventory.Entry) string { } } } + onPath := ownPathOf(entries) for _, m := range modules { - if _, own := onTheControllersPath[m]; own || resolvers[m] { + if _, own := onPath[m]; own || resolvers[m] { return inventory.ClassCore } } diff --git a/cmd/mesh-controller/delivery.go b/cmd/mesh-controller/delivery.go index 64c5805b..87325a48 100644 --- a/cmd/mesh-controller/delivery.go +++ b/cmd/mesh-controller/delivery.go @@ -42,13 +42,53 @@ import ( // `delivery order` (a group's order from the graph), `delivery check` (a group's heads composed), `delivery // go`, `delivery stop` and `delivery walks`. -// onTheControllersPath are the modules whose walk never waits for the delivery's owner, and what each is. +// onTheControllersPath are the modules, by name, whose walk never waits for the delivery's owner, and what each is. +// The delivery's owner is not among them: it is whichever module claims the seat (ownPathOf, novox/hq ADR 0437 §3). var onTheControllersPath = map[string]string{ - "mesh-controller": "the controller", - "mesh-host": "the node-engine", - "node-tools": "the node tools", - "nats": "the bus", - catalogue.DeliverySeat: "the delivery's owner", + "mesh-controller": "the controller", + "mesh-host": "the node-engine", + "node-tools": "the node tools", + "nats": "the bus", +} + +// claimantsOf are the modules among the catalogued entries that claim a seat, each once, in name order. +func claimantsOf(entries []inventory.Entry, seat string) []string { + var out []string + for _, e := range entries { + if e.Manifest.ClaimsSeat(seat) && !slices.Contains(out, e.Manifest.Module) { + out = append(out, e.Manifest.Module) + } + } + sort.Strings(out) + return out +} + +// ownPathOf is the controller's own path among these entries: the modules by name, and the modules that claim +// the delivery seat, which is how the delivery's owner is found (novox/hq ADR 0437 §3). +func ownPathOf(entries []inventory.Entry) map[string]string { + own := make(map[string]string, len(onTheControllersPath)+1) + for m, what := range onTheControllersPath { + own[m] = what + } + for _, m := range claimantsOf(entries, catalogue.DeliverySeat) { + own[m] = "the delivery's owner" + } + return own +} + +// onTheOwnPath is whether one module, with its manifest at hand, is on the controller's own path. +func onTheOwnPath(module string, m catalogue.Manifest) bool { + _, named := onTheControllersPath[module] + return named || m.ClaimsSeat(catalogue.DeliverySeat) +} + +// ownPathNow is ownPathOf the catalogue as it is. +func ownPathNow(ctx context.Context, inv *inventory.Inventory) (map[string]string, error) { + entries, err := inv.Catalogued(ctx) + if err != nil { + return nil, err + } + return ownPathOf(entries), nil } // deliverySeatHeld is whether a module claiming the delivery seat is assigned somewhere: the seat has a @@ -69,8 +109,9 @@ func awaitsFor(entries []inventory.Entry, modules []string) *inventory.PlanDeliv if !deliverySeatHeld(entries) { return nil } + onPath := ownPathOf(entries) for _, m := range modules { - if _, own := onTheControllersPath[m]; own { + if _, own := onPath[m]; own { return nil } } @@ -327,16 +368,17 @@ func stopInBatch(ctx context.Context, inv *inventory.Inventory, b *inventory.Pla // the stop's one act. The kind is read from what the merges moved; the stopped walk itself is no walk a cut folds. func batchToWalkAgain(ctx context.Context, inv *inventory.Inventory, stopping string, again []inventory.BatchedMerge, now time.Time) (inventory.Plan, []*inventory.Plan, []inventory.MergeMove, error) { - own := false - for _, n := range movesOf(again) { - if _, on := onTheControllersPath[n]; on { - own = true - } - } entries, read, edges, err := theGraph(ctx, inv) if err != nil { return inventory.Plan{}, nil, nil, err } + own := false + onPath := ownPathOf(entries) + for _, n := range movesOf(again) { + if _, on := onPath[n]; on { + own = true + } + } joins, _, _, err := relatedBatches(ctx, inv, again, own, stopping, entries, read, edges) if err != nil { return inventory.Plan{}, nil, nil, err @@ -935,7 +977,7 @@ func deliveryCommand(ctx context.Context, args []string) error { walks[i].Phases = walks[i].WalkPhases(now, appliedFrom(ctx, inv)) } return answer(map[string]any{"held": deliverySeatHeld(entries), "walks": walks, - "own-path": sortedKeysOf(ownPathWords())}) + "own-path": sortedKeysOf(ownPathOf(entries))}) } return fmt.Errorf("delivery %s: plan, order, check, go, stop, backups-again or walks", sub) } @@ -1006,9 +1048,6 @@ func recordWalkPhases(ctx context.Context, inv *inventory.Inventory, now time.Ti return nil } -// ownPathWords is the controller's own path as words, for an answer. -func ownPathWords() map[string]string { return onTheControllersPath } - // theGraph is what the planner reads. func theGraph(ctx context.Context, inv *inventory.Inventory) ([]inventory.Entry, map[string][]inventory.ReadRepository, []inventory.Edge, error) { diff --git a/cmd/mesh-controller/fixclass_test.go b/cmd/mesh-controller/fixclass_test.go index 86e61c44..225089f9 100644 --- a/cmd/mesh-controller/fixclass_test.go +++ b/cmd/mesh-controller/fixclass_test.go @@ -23,6 +23,7 @@ func TestAChangePlanSaysTheClassOfWhatItMoves(t *testing.T) { resolver := entry("dnsmasq", "modules/dnsmasq") resolver.Manifest.Claims = []catalogue.Claim{{Name: resolverSeat}} delivery := entry(catalogue.DeliverySeat, "modules/mesh-delivery") + delivery.Manifest.Claims = []catalogue.Claim{{Name: catalogue.DeliverySeat}} webApp := entry("photos", "modules/photos") entries := []inventory.Entry{gitea, resolver, delivery, webApp} // photos stands on gitea: a change to gitea moves photos as its dependent. @@ -44,7 +45,7 @@ func TestAChangePlanSaysTheClassOfWhatItMoves(t *testing.T) { {"a leaf and a core module together: the slowest-budgeted", []string{"modules/gitea/index.ts", "modules/dnsmasq/module.json"}, inventory.ClassCore}, {"nothing any module's build reads", []string{"README.md"}, "none"}, {"a new leaf module alone", []string{"modules/weather/module.json"}, inventory.ClassLeaf}, - {"a new module on the controller's own path, by its own class", []string{"next/mesh-delivery/module.json"}, inventory.ClassCore}, + {"a new module on the controller's own path, by its own class", []string{"next/mesh-host/module.json"}, inventory.ClassCore}, {"a new leaf module beside a core one moved", []string{"modules/weather/module.json", "modules/dnsmasq/module.json"}, inventory.ClassCore}, } { if got := plan(c.paths...).Class; got != c.want { diff --git a/cmd/mesh-controller/gate.go b/cmd/mesh-controller/gate.go index 45792f00..f0912525 100644 --- a/cmd/mesh-controller/gate.go +++ b/cmd/mesh-controller/gate.go @@ -69,7 +69,7 @@ var ( // own path, or one holding the mesh's resolver. Every other module is a leaf. Its class sets the module's // delivery budget; how many readings its gate takes is gateRuleOf's. func coreModule(module string, m catalogue.Manifest) bool { - if _, own := onTheControllersPath[module]; own { + if onTheOwnPath(module, m) { return true } for _, c := range m.Claims { @@ -106,7 +106,7 @@ func gateRuleOf(module string, m catalogue.Manifest) gateRule { case coreComponent(module) == lease.ComponentNodeTools: return gateRule{passes: 1, witnessed: true, said: "the tool runner's rule: one reading once it answers, " + "after the node-engine's witness window (novox/hq ADR 0351)"} - case module == catalogue.DeliverySeat: + case m.ClaimsSeat(catalogue.DeliverySeat): return gateRule{passes: 1, said: "mesh-delivery's rule: one reading (novox/hq ADR 0351)"} case coreModule(module, m): return gateRule{passes: gatePasses, within: gateWithin, said: fmt.Sprintf("the core's rule: %d readings, "+ diff --git a/cmd/mesh-controller/gate_rule_test.go b/cmd/mesh-controller/gate_rule_test.go index 06777685..69681695 100644 --- a/cmd/mesh-controller/gate_rule_test.go +++ b/cmd/mesh-controller/gate_rule_test.go @@ -39,7 +39,7 @@ func TestEachModuleHasItsGateRule(t *testing.T) { }{ {"mesh-controller", nil, 1, 0, true, "the controller's rule"}, {"node-tools", nil, 1, 0, true, "the tool runner's rule"}, - {catalogue.DeliverySeat, nil, 1, 0, false, "mesh-delivery's rule"}, + {catalogue.DeliverySeat, []catalogue.Claim{{Name: catalogue.DeliverySeat}}, 1, 0, false, "mesh-delivery's rule"}, {"mesh-host", nil, 3, 2 * time.Minute, false, "the core's rule"}, {"dnsmasq", []catalogue.Claim{{Name: resolverSeat}}, 3, 2 * time.Minute, false, "the core's rule"}, {"nats", nil, 3, 2 * time.Minute, false, "the core's rule"}, @@ -56,7 +56,12 @@ func TestEachModuleHasItsGateRule(t *testing.T) { // Every module that was core stays core for its delivery budget and its walk: only its readings change. func TestTheCoreStaysTheCore(t *testing.T) { for _, m := range []string{"mesh-controller", "mesh-host", "node-tools", "nats", catalogue.DeliverySeat} { - if !coreModule(m, catalogue.Manifest{Module: m}) { + // The delivery's owner is core by the claim of the seat, not by its name (novox/hq ADR 0437 §3, issue 1210). + claims := []catalogue.Claim(nil) + if m == catalogue.DeliverySeat { + claims = []catalogue.Claim{{Name: catalogue.DeliverySeat}} + } + if !coreModule(m, catalogue.Manifest{Module: m, Claims: claims}) { t.Errorf("%s is no longer judged as the core for its class", m) } } @@ -78,7 +83,11 @@ func aGateOn(module, node string, since time.Time) *gateOn { } func (o *gateOn) read(at time.Time, h health, why string) { - r := gateRuleOf(o.module, catalogue.Manifest{Module: o.module}) + m := catalogue.Manifest{Module: o.module} + if o.module == catalogue.DeliverySeat { // today's module, which claims the seat + m.Claims = []catalogue.Claim{{Name: catalogue.DeliverySeat}} + } + r := gateRuleOf(o.module, m) floor := gateFloor(o.module, r, *o.g.Since, []string{o.node}, o.g.Applied, o.holder) tallyGate(o.g, gateJudging{modules: []string{o.module}, reading: map[string]health{o.module: h}, worst: h, why: why, rules: map[string]gateRule{o.module: r}, floors: map[string]time.Time{o.module: floor}}, at) diff --git a/cmd/mesh-controller/own_path_by_claim_test.go b/cmd/mesh-controller/own_path_by_claim_test.go new file mode 100644 index 00000000..86eb3230 --- /dev/null +++ b/cmd/mesh-controller/own_path_by_claim_test.go @@ -0,0 +1,86 @@ +package main + +import ( + "strings" + "testing" + + "github.com/novox/mesh-controller/internal/catalogue" + "github.com/novox/mesh-controller/internal/inventory" +) + +// The controller finds the delivery's owner by the claim of the seat mesh-delivery, never by the module's name +// (novox/hq ADR 0437 §3, issue 1210). The gate is a module named mesh-delivery-gate; this is only a fixture name. + +func claimingTheSeat(module string) inventory.Entry { + return inventory.Entry{Manifest: catalogue.Manifest{Module: module, + Claims: []catalogue.Claim{{Name: catalogue.DeliverySeat, Scope: catalogue.ScopeMesh}}}, On: []string{"anchor"}} +} + +func namedOnly(module string) inventory.Entry { + return inventory.Entry{Manifest: catalogue.Manifest{Module: module}, On: []string{"anchor"}} +} + +// 1210/AC1: a module claiming the seat is on the controller's own path whatever its name, awaiting the delivery's +// word as today's module does; a module named mesh-delivery that claims no seat is not on it. +func TestTheModuleClaimingTheSeatIsOnTheControllersOwnPath(t *testing.T) { + gate := []inventory.Entry{claimingTheSeat("mesh-delivery-gate"), namedOnly("mesh-delivery")} + own := ownPathOf(gate) + if _, on := own["mesh-delivery-gate"]; !on { + t.Error("the module claiming the seat is not on the controller's own path") + } + if _, on := own["mesh-delivery"]; on { + t.Error("a module named mesh-delivery that claims no seat is on the controller's own path") + } + if d := awaitsFor(gate, []string{"gitea", "plex"}); d == nil || d.Awaits != catalogue.DeliverySeat { + t.Errorf("an ordinary walk awaits %+v with the gate holding the seat", d) + } + if d := awaitsFor(gate, []string{"mesh-delivery-gate", "gitea"}); d != nil { + t.Errorf("a walk moving the gate awaits %+v; the gate cannot gate itself", d) + } + if d := awaitsFor(gate, []string{"mesh-delivery", "gitea"}); d == nil { + t.Error("a walk moving a module named mesh-delivery that claims no seat starts at once") + } + if !ownWalk(walkOf("novox/one", "notes", "mesh-delivery-gate"), own) || ownWalk(walkOf("novox/one", "notes", "mesh-delivery"), own) { + t.Error("a walk is on the own path by the claim of the module it plans") + } + if ownPath([]inventory.Entry{claimingTheSeat("mesh-delivery-gate")}) != true || ownPath([]inventory.Entry{namedOnly("mesh-delivery")}) { + t.Error("a merge touches the own path by the claim of the module it touches") + } + if classOfModules([]string{"mesh-delivery-gate"}, gate) != inventory.ClassCore { + t.Error("the module claiming the seat is not core") + } +} + +// 1210/AC2: the merge gate's rule of the module claiming the seat is one reading; a module claiming no delivery +// seat gets its ordinary rule. +func TestTheModuleClaimingTheSeatGetsTheDeliverysGateRule(t *testing.T) { + withTheGatesTimes(t) + claimant := claimingTheSeat("mesh-delivery-gate").Manifest + r := gateRuleOf("mesh-delivery-gate", claimant) + if r.passes != 1 || r.within != 0 || !strings.HasPrefix(r.said, "mesh-delivery's rule: one reading (novox/hq ADR 0351)") { + t.Errorf("the gate's rule is %+v", r) + } + r = gateRuleOf("mesh-delivery", catalogue.Manifest{Module: "mesh-delivery"}) + if !strings.HasPrefix(r.said, "a leaf module's rule") { + t.Errorf("a module named mesh-delivery claiming no seat gets %q", r.said) + } +} + +// 1210/AC3: with both today's module and the gate claiming the seat (R3d's group), both are on the own path and +// both get the delivery's rule. +func TestBothClaimantsAreOnTheOwnPathAndGetTheRule(t *testing.T) { + withTheGatesTimes(t) + both := []inventory.Entry{claimingTheSeat("mesh-delivery"), claimingTheSeat("mesh-delivery-gate")} + own := ownPathOf(both) + for _, m := range []string{"mesh-delivery", "mesh-delivery-gate"} { + if _, on := own[m]; !on { + t.Errorf("%s is not on the own path", m) + } + if r := gateRuleOf(m, claimingTheSeat(m).Manifest); r.passes != 1 || !strings.HasPrefix(r.said, "mesh-delivery's rule") { + t.Errorf("%s gets %+v", m, r) + } + if d := awaitsFor(both, []string{m, "gitea"}); d != nil { + t.Errorf("a walk moving %s awaits %+v", m, d) + } + } +} diff --git a/cmd/mesh-controller/plan_retry.go b/cmd/mesh-controller/plan_retry.go index fbc075e8..aa9adf7b 100644 --- a/cmd/mesh-controller/plan_retry.go +++ b/cmd/mesh-controller/plan_retry.go @@ -155,7 +155,7 @@ func newerOpenPlan(p inventory.Plan, plans []inventory.Plan) (inventory.Plan, bo } // retryRefusal is why a plan cannot be retried, or nothing. -func retryRefusal(p inventory.Plan, plans []inventory.Plan) error { +func retryRefusal(p inventory.Plan, plans []inventory.Plan, onPath map[string]string) error { switch { case p.State == inventory.PlanDone: return fmt.Errorf("%s is done; there is nothing to retry", p.ID) @@ -170,7 +170,7 @@ func retryRefusal(p inventory.Plan, plans []inventory.Plan) error { return fmt.Errorf("%s supersedes it: a newer merge of %s (%s at %s) is open, and retrying %s would build "+ "what that one replaced", q.ID, q.Repository, q.ID, short(q.Commit), p.ID) } - return oneWalkAtATime(p, plans) + return oneWalkAtATime(p, plans, onPath) } stopped := stoppedRollouts(p) if len(stopped) == 0 { @@ -194,20 +194,20 @@ func retryRefusal(p inventory.Plan, plans []inventory.Plan) error { "the older build back", m, q.ID, q.State, q.Repository, short(q.Commit), p.ID) } } - return oneWalkAtATime(p, plans) + return oneWalkAtATime(p, plans, onPath) } // oneWalkAtATime refuses a retry while an open walk, started or waiting for its word, is related to it (novox/hq // ADR 0276, ADR 0407 decision 4, issue 973): a walk retried beside a related one would be two walks on the same // things at once. A walk related to none may be retried beside them. The retried walk is judged by the modules it // plans, which already hold their dependents, and the repositories it carries. -func oneWalkAtATime(p inventory.Plan, plans []inventory.Plan) error { +func oneWalkAtATime(p inventory.Plan, plans []inventory.Plan, onPath map[string]string) error { mine := footprintOfWalk(p, nil) for _, q := range plans { if q.ID == p.ID || !q.Open() || q.Release != nil { continue } - if ownWalk(p) || ownWalk(q) { + if ownWalk(p, onPath) || ownWalk(q, onPath) { return fmt.Errorf("%s is open (%s) and one of them is on the controller's own path: one walk at a time — "+ "retry %s once it ended", q.ID, q.Named(), p.ID) } @@ -327,7 +327,11 @@ func retryPlan(ctx context.Context, open *stores, id string) (string, error) { } failed = toRetry(&p, recorded, byID) } - if err := retryRefusal(p, plans); err != nil { + onPath, err := ownPathNow(ctx, inv) + if err != nil { + return "", err + } + if err := retryRefusal(p, plans, onPath); err != nil { return "", err } if again := unjudgedAtGate(p); len(failed) == 0 && len(again) > 0 { diff --git a/cmd/mesh-controller/queue_test.go b/cmd/mesh-controller/queue_test.go index d7dc6cb2..fe1a4674 100644 --- a/cmd/mesh-controller/queue_test.go +++ b/cmd/mesh-controller/queue_test.go @@ -68,7 +68,7 @@ func TestRetryRefusesWhatItCannotResume(t *testing.T) { {plan("plan-o", inventory.PlanBuilding, at), nil, "still building"}, {failed, []inventory.Plan{plan("plan-2", inventory.PlanRolling, at.Add(time.Hour))}, "plan-2 supersedes it"}, } { - err := retryRefusal(c.p, c.others) + err := retryRefusal(c.p, c.others, ownPathOf(nil)) if err == nil || !strings.Contains(err.Error(), c.says) { t.Errorf("%s: %v, wanted it to say %q", c.p.ID, err, c.says) } @@ -76,12 +76,12 @@ func TestRetryRefusesWhatItCannotResume(t *testing.T) { stopped := failed stopped.Modules = map[string]*inventory.PlanModule{"a": {State: "built"}} stopped.Note = "a stopped at its first machine" - if err := retryRefusal(stopped, nil); err == nil || !strings.Contains(err.Error(), "nothing in tier 0") { + if err := retryRefusal(stopped, nil, ownPathOf(nil)); err == nil || !strings.Contains(err.Error(), "nothing in tier 0") { t.Errorf("a plan with nothing failed to build was retried: %v", err) } // A failed or done plan does not supersede it. if err := retryRefusal(failed, []inventory.Plan{plan("plan-4", inventory.PlanFailed, at.Add(time.Hour)), - plan("plan-5", inventory.PlanDone, at.Add(time.Hour))}); err != nil { + plan("plan-5", inventory.PlanDone, at.Add(time.Hour))}, ownPathOf(nil)); err != nil { t.Errorf("refused for a plan that does not supersede it: %v", err) } // Another branch's open walk, or an older one, does not supersede it, and is one walk open: one at a time @@ -89,7 +89,7 @@ func TestRetryRefusesWhatItCannotResume(t *testing.T) { other := plan("plan-3", inventory.PlanBuilding, at.Add(time.Hour)) other.Branch = "release" for _, open := range []inventory.Plan{other, plan("plan-0", inventory.PlanBuilding, at.Add(-time.Hour))} { - if err := retryRefusal(failed, []inventory.Plan{open}); err == nil || !strings.Contains(err.Error(), "one walk at a time") { + if err := retryRefusal(failed, []inventory.Plan{open}, ownPathOf(nil)); err == nil || !strings.Contains(err.Error(), "one walk at a time") { t.Errorf("retried beside the open walk %s: %v", open.ID, err) } } diff --git a/cmd/mesh-controller/related.go b/cmd/mesh-controller/related.go index fe0dc647..7c507af1 100644 --- a/cmd/mesh-controller/related.go +++ b/cmd/mesh-controller/related.go @@ -144,12 +144,12 @@ func relatedOpen(cand footprint, candOwn bool, open []openWalk, edges []inventor // ownWalk says a walk is on the controller's own path: its batch says so, or it plans a module of that path // (issue 973). -func ownWalk(p inventory.Plan) bool { +func ownWalk(p inventory.Plan, onPath map[string]string) bool { if p.OwnPath() { return true } for name := range p.Modules { - if _, own := onTheControllersPath[name]; own { + if _, own := onPath[name]; own { return true } } @@ -173,19 +173,19 @@ func startedWalks(plans []inventory.Plan, id string, created time.Time) []invent // openWalkOf is an open walk as a change is judged against it: what it reaches, from its planned modules and the // merges the record holds for it. -func openWalkOf(ctx context.Context, inv *inventory.Inventory, p inventory.Plan) (openWalk, error) { +func openWalkOf(ctx context.Context, inv *inventory.Inventory, p inventory.Plan, onPath map[string]string) (openWalk, error) { kept, err := inv.MergesOf(ctx, p.ID) if err != nil { return openWalk{}, err } - return openWalk{id: p.ID, fp: footprintOfWalk(p, kept), own: ownWalk(p)}, nil + return openWalk{id: p.ID, fp: footprintOfWalk(p, kept), own: ownWalk(p, onPath)}, nil } // openWalksOf are openWalkOf for each of plans. -func openWalksOf(ctx context.Context, inv *inventory.Inventory, plans []inventory.Plan) ([]openWalk, error) { +func openWalksOf(ctx context.Context, inv *inventory.Inventory, plans []inventory.Plan, onPath map[string]string) ([]openWalk, error) { var out []openWalk for _, p := range plans { - w, err := openWalkOf(ctx, inv, p) + w, err := openWalkOf(ctx, inv, p, onPath) if err != nil { return nil, err } diff --git a/cmd/mesh-controller/related_test.go b/cmd/mesh-controller/related_test.go index 40f80802..d67c0cb9 100644 --- a/cmd/mesh-controller/related_test.go +++ b/cmd/mesh-controller/related_test.go @@ -119,19 +119,19 @@ func TestABatchWhoseMovesAreNotKnownIsRelatedToEverything(t *testing.T) { // A walk is on the controller's own path when its batch says so or it plans a module of that path (issue 973). func TestAWalkOnTheControllersPathIsKnownByItsFlagOrItsModules(t *testing.T) { - if ownWalk(walkOf("novox/one", "notes")) { + if ownWalk(walkOf("novox/one", "notes"), ownPathOf(nil)) { t.Error("an ordinary walk read as on the controller's own path") } flagged := walkOf("novox/one", "notes") flagged.Delivery = &inventory.PlanDelivery{Batch: &inventory.PlanBatch{Own: true}} - if !ownWalk(flagged) { + if !ownWalk(flagged, ownPathOf(nil)) { t.Error("a walk whose batch is own-path read as ordinary") } var module string for m := range onTheControllersPath { module = m } - if !ownWalk(walkOf("novox/one", "notes", module)) { + if !ownWalk(walkOf("novox/one", "notes", module), ownPathOf(nil)) { t.Errorf("a walk planning %s read as ordinary", module) } } @@ -168,15 +168,15 @@ func TestARetryWaitsOnlyForAnOpenWalkItIsRelatedTo(t *testing.T) { p.ID, p.State = id, inventory.PlanBuilding return p } - if err := oneWalkAtATime(failed, []inventory.Plan{open("plan-2", "novox/two", "app")}); err != nil { + if err := oneWalkAtATime(failed, []inventory.Plan{open("plan-2", "novox/two", "app")}, ownPathOf(nil)); err != nil { t.Errorf("refused beside an unrelated walk: %v", err) } - err := oneWalkAtATime(failed, []inventory.Plan{open("plan-3", "novox/three", "notes")}) + err := oneWalkAtATime(failed, []inventory.Plan{open("plan-3", "novox/three", "notes")}, ownPathOf(nil)) if err == nil || !strings.Contains(err.Error(), "plan-3") || !strings.Contains(err.Error(), "both move notes") { t.Errorf("a retry beside a walk moving the same module: %v", err) } failed.Delivery = &inventory.PlanDelivery{Batch: &inventory.PlanBatch{Own: true}} - if err := oneWalkAtATime(failed, []inventory.Plan{open("plan-2", "novox/two", "app")}); err == nil { + if err := oneWalkAtATime(failed, []inventory.Plan{open("plan-2", "novox/two", "app")}, ownPathOf(nil)); err == nil { t.Error("an own-path retry went beside an open walk") } } diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index 3c65d501..e9f1b7bb 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -701,11 +701,15 @@ func startedBeside(ctx context.Context, inv *inventory.Inventory, p inventory.Pl if err != nil { return "", "", err } - open, err := openWalksOf(ctx, inv, started) + onPath, err := ownPathNow(ctx, inv) if err != nil { return "", "", err } - mine, err := openWalkOf(ctx, inv, p) + open, err := openWalksOf(ctx, inv, started, onPath) + if err != nil { + return "", "", err + } + mine, err := openWalkOf(ctx, inv, p, onPath) if err != nil { return "", "", err }