From 81182e9dcda8a646d224a91e4874ab2f579eec27 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 25 Oct 2026 15:26:03 +0100 Subject: [PATCH] Assigner: a running node's first want names the fingerprint it runs, never a newer one, and only a node that runs the module is on neither list (issue 1945, review of f7b23033) A first want at the newest fingerprint passed over the canary for every module right after the switch. It now names what the node runs: F when it runs F (by the rules without which there is nothing to want), or an older fingerprint of the new form whose composition and grant exist, held back from F by the rule; otherwise nothing is written and the node stays on neither list. A new assignment, with no have, waits for the rules and raises no condition (1945/AC19-AC21). --- .../integration_test.go | 28 +++-- .../cmd/mesh-delivery-assigner/pass.go | 4 + .../internal/assign/wants.go | 59 ++++++--- .../internal/assign/wants_test.go | 113 ++++++++++-------- 4 files changed, 121 insertions(+), 83 deletions(-) diff --git a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/integration_test.go b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/integration_test.go index 40c299f..241ae99 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/integration_test.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/integration_test.go @@ -283,9 +283,9 @@ func (b *bed) get(t *testing.T, bucket, key string) []byte { // 1297/B4 on the bus, 1906/D5: the controller sends ace, g14 and shanks builds while the canary ace does not run F healthy yet. // Started whole, the assigner wants F on the canary and writes no difference at all: there is no send to compare with (ADR 0539 -// §8). 1945/AC19, AC21: shanks, which runs the module with no want, gets its first want past the canary rule; g14, which does not -// run it, is held back with no want, so web is on neither list there and the assigner raises the condition naming g14, web and -// the canary rule. +// §8). 1945/AC19, AC21: shanks, which runs the older fingerprint (composed and granted) with no want, gets its first want at that +// fingerprint, never at F; g14, which runs a build of the old form, gets none, so web is on neither list there and the assigner +// raises the condition naming g14, web and the canary rule. func TestWantsAreWrittenAndNoDifferenceIs(t *testing.T) { b := startBed(t, "") fNew, fOld := delivery.SourcePrefix2+strings.Repeat("b", 64), delivery.SourcePrefix2+strings.Repeat("a", 64) @@ -298,19 +298,21 @@ func TestWantsAreWrittenAndNoDifferenceIs(t *testing.T) { } bkey, _ := delivery.BuilderBuildKey("web", fNew) b.putJSON(t, delivery.BuilderBuildsBucket, bkey, built) - gkey, _ := delivery.ControllerGrantKey("web", fNew) - grant, _ := delivery.EncodeControllerGrant(delivery.ControllerGrant{}) - b.putJSON(t, delivery.ControllerGrantsBucket, gkey, grant) + for _, fp := range []string{fNew, fOld} { + gkey, _ := delivery.ControllerGrantKey("web", fp) + grant, _ := delivery.EncodeControllerGrant(delivery.ControllerGrant{}) + b.putJSON(t, delivery.ControllerGrantsBucket, gkey, grant) + } + runs := map[string]string{"ace": fOld, "g14": "build-9", "shanks": fOld} for _, node := range []string{"ace", "g14", "shanks"} { b.compose(t, node, "web", fNew, 'c', delivery.JudgedNeither) on, _ := delivery.EncodePresence(delivery.Presence{Online: true}) b.putJSON(t, delivery.PresenceBucket, node, on) - if node != "g14" { - have, _ := delivery.EncodeHave(delivery.Have{FP: fOld, Phase: delivery.PhaseRunning, Health: delivery.HealthOK}) - b.putJSON(t, delivery.HaveBucket, node+".web", have) - } + have, _ := delivery.EncodeHave(delivery.Have{FP: runs[node], Phase: delivery.PhaseRunning, Health: delivery.HealthOK}) + b.putJSON(t, delivery.HaveBucket, node+".web", have) b.putJSON(t, provider.AssignmentsBucket, node+".web", []byte(`{"node":"`+node+`","module":"web","build":"b-`+node+`"}`)) } + b.compose(t, "shanks", "web", fOld, 'a', delivery.JudgedNeither) b.start(t) wkey, _ := delivery.WantKey("ace", "web") @@ -318,8 +320,8 @@ func TestWantsAreWrittenAndNoDifferenceIs(t *testing.T) { if err != nil || want.FP != fNew { t.Fatalf("the canary ace's want = %+v, %v", want, err) } - if w := b.wantNow(t, "shanks", "web", "shanks' first want past the canary rule", func(w delivery.Want) bool { return w.FP == fNew }); w.Composed != "sha256:"+strings.Repeat("c", 64) { - t.Errorf("shanks' want = %+v, want F naming its composition", w) + if w := b.wantNow(t, "shanks", "web", "shanks' first want at what it runs", func(w delivery.Want) bool { return w.FP != "" }); w.FP != fOld || w.Composed != "sha256:"+strings.Repeat("a", 64) || w.Newest != fNew { + t.Errorf("shanks' want = %+v, want the fingerprint it runs naming its composition, held back from F", w) } var neither asks.Condition for deadline := time.Now().Add(10 * time.Second); time.Now().Before(deadline) && neither.Key == ""; time.Sleep(100 * time.Millisecond) { @@ -336,7 +338,7 @@ func TestWantsAreWrittenAndNoDifferenceIs(t *testing.T) { t.Errorf("differences were written: %v", keys) } if e, err := b.buckets[delivery.AssignerWantsBucket].Get(b.ctx, "want.g14.web"); err == nil { - t.Errorf("g14, which does not run web and is held back by the canary, was given a want: %s", e.Value()) + t.Errorf("g14, which runs an old-form build and is held back by the canary, was given a want: %s", e.Value()) } } diff --git a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go index 7c7e907..6da9587 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go @@ -103,6 +103,10 @@ func (p *passer) wants(ctx context.Context) { name := name m := assign.Module{Name: name, FP: fp, Manifest: manifests[name], Nodes: nodesOf[name], Now: time.Now(), Composed: func(node, fp string) (delivery.ComposedDigest, bool) { return p.w.composed(node, name, fp) }, + GrantedAt: func(fp string) bool { + key, err := delivery.ControllerGrantKey(name, fp) + return err == nil && p.w.has(delivery.ControllerGrantsBucket, key) + }, Restored: func(node string) bool { key, err := delivery.RestoreAskKey(node, name, fp) return err == nil && p.answeredWith(key, assign.OptionRestore) diff --git a/modules/mesh-delivery-assigner/internal/assign/wants.go b/modules/mesh-delivery-assigner/internal/assign/wants.go index cc2e4da..7ee8b50 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants.go @@ -15,13 +15,16 @@ import ( // trunk (the indexer's modules), whether the builder's build of F is done, whether F is held, whether the controller // issued its credential for F, its manifest at F, and the nodes it is assigned to. type Module struct { - Name string - FP string - Built bool - Held bool - Granted bool - Manifest catalogue.Manifest - Nodes []string + Name string + FP string + Built bool + Held bool + Granted bool + // GrantedAt says whether the controller issued the module's credential for another fingerprint than FP: the first want of a node + // that runs an older one names it (issue 1945). Nil: none. + GrantedAt func(fp string) bool + Manifest catalogue.Manifest + Nodes []string // Composed answers the controller's composition of the module for a node at a fingerprint (mesh-controller_composed): its // digest and the judgement of the move to it, and whether it exists. Nil: none exists (ADR 0539 §1, §6). Composed func(node, fp string) (delivery.ComposedDigest, bool) @@ -163,8 +166,8 @@ type Outcome struct { Restore []string // Rules is, by node, the rule holding the node back from m.FP and why. Rules map[string]Holding - // Neither is, by node, the nodes the module is assigned to that hold no want for it after this pass, with the rule holding the - // first want back: the node's own composition does not hold a module with a fingerprint of the new form, so such a module is on + // Neither is, by node, the nodes the module is assigned to that run it (or whose have names it) and hold no want for it after this + // pass, with the rule holding the first want back: the node's own composition does not hold a module with a fingerprint of the new form, so such a module is on // neither list and nothing applies it on that node (issue 1945). A module with no fingerprint of the new form goes with the node's // own composition and is never here. Neither map[string]Holding @@ -223,23 +226,38 @@ func Wants(m Module, n Nodes, wants map[string]delivery.Want, lastHealthy map[st continue } ok, rule, words := Holds(mm, node, n) - if !ok && (!had || before.Removed) && (rule == delivery.RuleCanary || rule == delivery.RuleProviders) && runs(n.Haves[node][m.Name]) { + target := m.FP + if have, ran := n.Haves[node][m.Name]; !ok && (!had || before.Removed) && (rule == delivery.RuleCanary || rule == delivery.RuleProviders) && runs(have) { // A node that runs the module and has no want for it has no want the ordering rules could hold it back to: holding its first // want back leaves the module on neither list, since the node's own composition gives up every module with a fingerprint of // the new form (issue 1945: on g14 the bundles of clipmenu, screen-lock and every module requiring x11-display were removed - // and never made again, because xorg's health is unknown and screen-lock's canary never ran it healthy). Its first want is - // written by the rules without which there is nothing to want; the ordering rules order every move after it. - ok, rule, words = holds(mm, node, n, false) + // and never made again, because xorg's health is unknown and screen-lock's canary never ran it healthy). Its first want names + // what it runs, never a newer fingerprint, so the canary is never passed over: F when it runs F (by the rules without which + // there is nothing to want), or an older fingerprint of the new form whose composition and grant exist. Rules 3 and 5 then + // order the move from it as from any want. + switch _, composed := m.composition(node, have.FP); { + case !ran: + case have.FP == m.FP: + ok, rule, words = holds(mm, node, n, false) + case strings.HasPrefix(have.FP, delivery.SourcePrefix2) && composed && m.GrantedAt != nil && m.GrantedAt(have.FP): + ok, target = true, have.FP + } } switch { case ok: - d, _ := m.composition(node, m.FP) - next = delivery.Want{FP: m.FP, Composed: d.Digest} - if had && !before.Removed && before.FP == m.FP && before.Composed == d.Digest { + d, _ := m.composition(node, target) + next = delivery.Want{FP: target, Composed: d.Digest} + at := m + if target != m.FP { + // the first want at the older fingerprint the node runs: held back from F by the rule, and judged as a want at it + next.Newest, next.Rule, next.Words = m.FP, rule, words + at.FP = target + } + if had && !before.Removed && before.FP == target && before.Composed == d.Digest { next.Backup, next.Words = before.Backup, before.Words // the answer to a failed backup stays while its target stays (ADR 0539 §16) } running, ran := n.Haves[node][m.Name] - if need, wait := m.mustAsk(node, before, had, ran, running.FP, running.Composed == "", d); wait { + if need, wait := at.mustAsk(node, before, had, ran, running.FP, running.Composed == "", d); wait { if need != nil { out.Record = append(out.Record, *need) } @@ -268,8 +286,11 @@ func Wants(m Module, n Nodes, wants map[string]delivery.Want, lastHealthy map[st default: out.Rules[node] = Holding{Rule: rule, Words: words} if !had || before.Removed { - // The node gets no want for a module assigned to it: the module is on neither list (issue 1945), and the pass says so. - out.Neither[node] = Holding{Rule: rule, Words: words} + // The node runs the module, or its have names it, and gets no want for it: the module is on neither list (issue 1945), and + // the pass says so. A new assignment, with no have, waits for the rules and is on no list yet by design. + if h, there := n.Haves[node][m.Name]; there && h.Phase != delivery.PhaseRemoved { + out.Neither[node] = Holding{Rule: rule, Words: words} + } continue } next = delivery.Want{FP: before.FP, Composed: before.Composed, Newest: m.FP, Rule: rule, Words: words} diff --git a/modules/mesh-delivery-assigner/internal/assign/wants_test.go b/modules/mesh-delivery-assigner/internal/assign/wants_test.go index 50ac2b8..a9b943b 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants_test.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants_test.go @@ -581,18 +581,19 @@ func TestAHoldIsLiftedOnceEveryNodeRunsItHealthy(t *testing.T) { } } -// x11 is screen-lock as on g14 and shanks on 2026-10-25 (issue 1945): built, granted and composed at F on both, requiring -// x11-display, whose provider xorg runs on each node with its health unknown; each node runs screen-lock from before the switch -// (a have, health unknown) and holds no want for it. +// x11 is screen-lock as on g14 and shanks on 2026-10-25 (issue 1945): built, granted and composed at F and at the older fOld +// on both, requiring x11-display, whose provider xorg runs on each node with its health unknown; each node runs screen-lock at fOld +// (health unknown) and holds no want for it. The canary g14 does not run F. func x11() (Module, Nodes) { m := Module{Name: "screen-lock", FP: fNew, Built: true, Granted: true, Composed: composedEverywhere, Nodes: []string{"g14", "shanks"}, - Manifest: catalogue.Manifest{Module: "screen-lock", Requires: []string{"x11-display"}}} + GrantedAt: func(fp string) bool { return fp == fNew || fp == fOld }, + Manifest: catalogue.Manifest{Module: "screen-lock", Requires: []string{"x11-display"}}} n := Nodes{ Online: map[string]bool{"g14": true, "shanks": true}, Haves: map[string]map[string]delivery.Have{ - "g14": {"screen-lock": {FP: "build-1", Phase: delivery.PhaseRunning, Health: delivery.HealthUnknown}, + "g14": {"screen-lock": {FP: fOld, Phase: delivery.PhaseRunning, Health: delivery.HealthUnknown}, "xorg": {FP: fpOf('x'), Phase: delivery.PhaseRunning, Health: delivery.HealthUnknown}}, - "shanks": {"screen-lock": {FP: "build-2", Phase: delivery.PhaseRunning, Health: delivery.HealthUnknown}, + "shanks": {"screen-lock": {FP: fOld, Phase: delivery.PhaseRunning, Health: delivery.HealthUnknown}, "xorg": {FP: fpOf('x'), Phase: delivery.PhaseRunning, Health: delivery.HealthUnknown}}, }, Providers: map[string]map[string][]string{"x11-display": {"xorg": {"g14", "shanks"}}}, @@ -600,80 +601,90 @@ func x11() (Module, Nodes) { return m, n } -// 1945/AC19: given a module each of its nodes runs with no want, held back only by the providers rule (its provider's health is -// unknown) or by the canary rule (the canary does not run F healthy), when the assigner passes, then each node gets its first want -// at F naming its composition, and no node is left on neither list. -func TestANodeRunningAModuleWithNoWantGetsItsFirstWantPastTheOrderingRules(t *testing.T) { +// 1945/AC19: given a node that runs an older fingerprint of the new form, composed and granted, with no want, held back from F by +// the providers or the canary rule, when the assigner passes, then its first want names the fingerprint it runs with that +// composition, never the newest, and says the newest it is held back from; the canary is never passed over. +func TestANodeRunningAModuleWithNoWantGetsItsFirstWantAtWhatItRuns(t *testing.T) { m, n := x11() out := Wants(m, n, nil, nil) for _, node := range []string{"g14", "shanks"} { - if w := out.Wants[node]; w.FP != fNew || w.Composed != dig(fNew) || w.Rule != 0 { - t.Errorf("%s's want = %+v, want its first want at F naming its composition", node, w) + if w := out.Wants[node]; w.FP != fOld || w.Composed != dig(fOld) || w.Newest != fNew || w.Rule == 0 { + t.Errorf("%s's want = %+v, want its first want at the fingerprint it runs, held back from F", node, w) } } if len(out.Neither) != 0 { t.Errorf("left on neither list: %+v", out.Neither) } - // with the provider healthy, only the canary holds shanks back (g14 does not run F healthy yet): the same + // with the provider healthy, only the canary holds shanks back: still at what it runs m, n = x11() n.Haves["g14"]["xorg"] = delivery.Have{FP: fpOf('x'), Phase: delivery.PhaseRunning, Health: delivery.HealthOK} n.Haves["shanks"]["xorg"] = n.Haves["g14"]["xorg"] - if w := Wants(m, n, nil, nil).Wants["shanks"]; w.FP != fNew { - t.Errorf("shanks held back by the canary with no want: %+v", w) + if w := Wants(m, n, nil, nil).Wants["shanks"]; w.FP != fOld || w.Rule != delivery.RuleCanary { + t.Errorf("shanks held back by the canary with no want: %+v, want at fOld by rule 3", w) } - // a removed want is no want either + // a node already running F gets its want at F; a removed want is no want either m, n = x11() - if w := Wants(m, n, map[string]delivery.Want{"g14": {Removed: true}}, nil).Wants["g14"]; w.FP != fNew || w.Removed { - t.Errorf("g14 with a removed want = %+v, want F", w) + n.Haves["g14"]["screen-lock"] = delivery.Have{FP: fNew, Phase: delivery.PhaseRunning, Health: delivery.HealthUnknown} + out = Wants(m, n, map[string]delivery.Want{"g14": {Removed: true}}, nil) + if w := out.Wants["g14"]; w.FP != fNew || w.Composed != dig(fNew) || w.Removed { + t.Errorf("g14 running F with a removed want = %+v, want F", w) } } -// 1945/AC19: the ordering rules still order every move after the first want: a node holding a want stays on it while the provider's -// health is unknown, and a node that does not run the module waits for them. -func TestTheOrderingRulesStillHoldAMoveFromAWant(t *testing.T) { +// 1945/AC19: a node running a fingerprint that is not of the new form, or one whose composition or grant does not exist, gets no +// want past the rules, and is named on neither list; the ordering rules still order every move from a want. +func TestNoFirstWantWithoutTheCompositionAndGrantOfWhatItRuns(t *testing.T) { + for what, change := range map[string]func(*Module, *Nodes){ + "an old-form build": func(_ *Module, n *Nodes) { + n.Haves["shanks"]["screen-lock"] = delivery.Have{FP: "build-2", Phase: delivery.PhaseRunning, Health: delivery.HealthUnknown} + }, + "no grant": func(m *Module, _ *Nodes) { m.GrantedAt = func(fp string) bool { return fp == fNew } }, + "no composition": func(m *Module, _ *Nodes) { + m.Composed = func(_, fp string) (delivery.ComposedDigest, bool) { + return delivery.ComposedDigest{Digest: dig(fp), Judgement: delivery.JudgedNeither}, fp == fNew + } + }, + } { + m, n := x11() + change(&m, &n) + out := Wants(m, n, nil, nil) + if w, ok := out.Wants["shanks"]; ok { + t.Errorf("%s: shanks got a want: %+v", what, w) + } + if h, ok := out.Neither["shanks"]; !ok || h.Rule == 0 { + t.Errorf("%s: shanks on neither list = %+v (named %v)", what, h, ok) + } + } m, n := x11() before := map[string]delivery.Want{"g14": {FP: fOld, Composed: dig(fOld)}, "shanks": {FP: fOld, Composed: dig(fOld)}} out := Wants(m, n, before, nil) - for _, node := range []string{"g14", "shanks"} { - if w := out.Wants[node]; w.FP != fOld || w.Newest != fNew || (w.Rule != delivery.RuleProviders && w.Rule != delivery.RuleCanary) { - t.Errorf("%s's want = %+v, want it kept at the old fingerprint, held by rule 3 or 5", node, w) - } + if len(out.Neither) != 0 { + t.Errorf("nodes holding a want named on neither list: %+v", out.Neither) } - m, n = x11() - delete(n.Haves["shanks"], "screen-lock") - if w, ok := Wants(m, n, nil, nil).Wants["shanks"]; ok { - t.Errorf("a node that does not run the module got its want past the providers rule: %+v", w) + for node, w := range out.Wants { + if w.FP != fOld { + t.Errorf("%s's want moved from its want past the ordering rules: %+v", node, w) + } } } -// 1945/AC20: given a module assigned to a node that holds no want for it and that no rule lets have one (it does not run it and its -// provider is not healthy; or it runs it and the grant is missing), when the assigner passes, then the outcome names that node in -// Neither with the rule and its words; a node with a want, or a module with no fingerprint of the new form, is never named. -func TestAModuleOnNeitherListIsNamed(t *testing.T) { +// 1945/AC20: a new assignment (a node with no have of the module) waiting for the canary, the build or the grant gets no want and is +// never named on neither list: only a node that runs the module, or whose have names it, is; a module with no fingerprint of the new +// form goes with the node's own composition and is never named. +func TestANewAssignmentWaitingIsNotOnNeitherList(t *testing.T) { m, n := x11() delete(n.Haves["shanks"], "screen-lock") - n.Haves["g14"]["screen-lock"] = delivery.Have{FP: fNew, Phase: delivery.PhaseRunning, Health: delivery.HealthOK} // the canary passes out := Wants(m, n, nil, nil) - if h, ok := out.Neither["shanks"]; !ok || h.Rule != delivery.RuleProviders || !strings.Contains(h.Words, "x11-display") { - t.Errorf("shanks on neither list = %+v (named %v), want rule 5 naming x11-display", h, ok) + if w, ok := out.Wants["shanks"]; ok { + t.Errorf("a new assignment waiting for the canary got a want: %+v", w) } - if _, ok := out.Neither["g14"]; ok { - t.Errorf("g14 named on neither list although it got its want: %+v", out.Neither) + if _, ok := out.Neither["shanks"]; ok { + t.Errorf("a new assignment waiting for the canary is named on neither list: %+v", out.Neither) } - m, n = x11() - m.Granted = false - out = Wants(m, n, nil, nil) - for _, node := range []string{"g14", "shanks"} { - if h := out.Neither[node]; h.Rule != delivery.RuleGrant { - t.Errorf("%s on neither list = %+v, want rule 6", node, h) - } + m.Built = false + if out := Wants(m, n, nil, nil); len(out.Neither) != 1 || out.Neither["g14"].Rule != delivery.RuleBuildDone { + t.Errorf("with no build: neither = %+v, want g14 alone (it runs the module), by rule 1", out.Neither) } - // a node holding a want is on a list - out = Wants(m, n, map[string]delivery.Want{"g14": {FP: fOld, Composed: dig(fOld)}}, nil) - if _, ok := out.Neither["g14"]; ok { - t.Errorf("g14 holds a want and is named on neither list") - } - // a module with no fingerprint of the new form goes with the node's own composition m.FP = "" if out := Wants(m, n, nil, nil); len(out.Neither) != 0 { t.Errorf("a module of the node's own composition named on neither list: %+v", out.Neither)