From 8daa5977f9929b8d5c54ab0df6d0a0d7b6812ee5 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 25 Oct 2026 15:10:34 +0100 Subject: [PATCH 1/6] Assigner: a node running a module with no want gets its first want past the ordering rules, and a module on neither list is raised (issue 1945) The node's own composition gives up every module with a fingerprint of the new form, so such a module is applied only through its want. The assigner wrote no want while a rule held it back and the node had none, so clipmenu, screen-lock, i3status-rust and every module requiring x11-display lay on neither list on g14 and shanks: xorg's health is unknown (rule 5) and the canary never ran them healthy (rule 3), and their files were removed and never made again. A node that runs a module and has no want for it now gets its first want by the rules without which there is nothing to want (built, not held, granted, composed); the canary and providers rules order every move after it. Any module still on neither list is raised as an urgent condition naming module, node and rule, written to the journal, and cleared once it has a want (1945/AC19-AC21). --- .../cmd/mesh-delivery-assigner/pass.go | 54 ++++++++++ .../cmd/mesh-delivery-assigner/pass_test.go | 48 +++++++++ .../internal/assign/asks.go | 22 ++++ .../internal/assign/asks_test.go | 16 +++ .../internal/assign/wants.go | 54 +++++++--- .../internal/assign/wants_test.go | 100 ++++++++++++++++++ 6 files changed, 282 insertions(+), 12 deletions(-) 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 0beb8f0..7c7e907 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go @@ -30,6 +30,8 @@ type passer struct { asks *composeAsks // now is the clock; nil is time.Now. now func() time.Time + // neither is, by node and module, each module on neither list whose condition this process raised (issue 1945). + neither map[string]neitherOne } func (p *passer) clock() time.Time { @@ -94,6 +96,8 @@ func (p *passer) wants(ctx context.Context) { for name := range wanted { names[name] = true } + neither := map[string]neitherOne{} + defer p.sayNeither(neither) for name := range names { fp := trunk[name].FP2 name := name @@ -123,6 +127,7 @@ func (p *passer) wants(ctx context.Context) { last[node] = p.w.lastHealthy(node, name, fp) } out := assign.Wants(m, nodes, wanted[name], last) + gatherNeither(neither, name, out) if out.Held != nil { key, err := delivery.HeldKey(name, fp) if err == nil { @@ -155,6 +160,55 @@ func (p *passer) wants(ctx context.Context) { } } +// neitherOne is a module assigned to a node that holds no want for it after a pass, and the rule holding its first want back. +type neitherOne struct { + node, module string + holding assign.Holding +} + +// gatherNeither adds the nodes on which module is on neither list in its outcome, keyed by node and module. +func gatherNeither(into map[string]neitherOne, module string, out assign.Outcome) { + for node, h := range out.Neither { + into[node+"."+module] = neitherOne{node: node, module: module, holding: h} + } +} + +// sayNeither raises, once, the condition of every module on neither list in this pass and writes it to the journal, and clears, +// once, the condition of each one said before that has a want again (issue 1945). A say that fails is made again at the next pass. +func (p *passer) sayNeither(now map[string]neitherOne) { + if p.neither == nil { + p.neither = map[string]neitherOne{} + } + for key, one := range now { + if _, said := p.neither[key]; said { + continue + } + c := assign.NeitherCondition(one.node, one.module, one.holding, true) + if p.say != nil { + if err := p.say(c); err != nil { + p.logf("the condition of %s on neither list on %s could not be said, and is said at the next pass: %v", one.module, one.node, err) + continue + } + } + p.logf("%s is assigned to %s and on neither list: no want, and not in the node's own composition; rule %d holds its first want back: %s", + one.module, one.node, one.holding.Rule, one.holding.Words) + p.neither[key] = one + } + for key, one := range p.neither { + if _, still := now[key]; still { + continue + } + if p.say != nil { + if err := p.say(assign.NeitherCondition(one.node, one.module, assign.Holding{}, false)); err != nil { + p.logf("the condition of %s on neither list on %s could not be cleared, and is cleared at the next pass: %v", one.module, one.node, err) + continue + } + } + p.logf("%s on %s has a want again", one.module, one.node) + delete(p.neither, key) + } +} + // compositions writes the composition check's verdict of every group the indexer holds a candidate for, under the // checker's key of that check: waits, with why, while the indexer holds what it composes from only in part. func (p *passer) compositions(ctx context.Context) { diff --git a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go index 8b48beb..8ba0322 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go @@ -2,12 +2,15 @@ package main import ( "context" + "errors" + "fmt" "strings" "testing" "time" "github.com/nats-io/nats.go/jetstream" + "git.novox.be/novox/mesh-sdk/go/asks" "git.novox.be/novox/mesh-sdk/go/delivery" "mesh-delivery-assigner/internal/assign" @@ -98,3 +101,48 @@ func TestAnAnswerCountsOnlyForTheAskingItAnswered(t *testing.T) { t.Error("an answer to an ask taken away was taken") } } + +// 1945/AC21: given a module on neither list on a node, when passes follow, then the assigner raises its condition once, says it +// again after a say that failed, and clears it once when the module has a want again; a module on neither list is also written to +// the journal, so it is loud with no operator channel. +func TestAModuleOnNeitherListIsRaisedOnceAndClearedOnce(t *testing.T) { + p := newPasser(buckets{}, time.Date(2026, 10, 25, 15, 0, 0, 0, time.UTC)) + var said []asks.Condition + failing := true + p.say = func(c asks.Condition) error { + if failing { + failing = false + return errors.New("the channel is away") + } + said = append(said, c) + return nil + } + var logged []string + p.logf = func(f string, a ...any) { logged = append(logged, fmt.Sprintf(f, a...)) } + one := map[string]neitherOne{"g14.screen-lock": {node: "g14", module: "screen-lock", + holding: assign.Holding{Rule: delivery.RuleProviders, Words: "no provider of x11-display runs healthy on g14 or at its canary"}}} + p.sayNeither(one) // the say fails + p.sayNeither(one) + p.sayNeither(one) + if len(said) != 1 || said[0].State != asks.StateRaised || said[0].Subject.Machine != "g14" || said[0].Subject.ID != "screen-lock" { + t.Fatalf("said = %+v, want one raised condition of screen-lock on g14", said) + } + if !strings.Contains(strings.Join(logged, "\n"), "screen-lock") { + t.Errorf("the journal does not name the module on neither list: %q", logged) + } + p.sayNeither(map[string]neitherOne{}) + p.sayNeither(map[string]neitherOne{}) + if len(said) != 2 || said[1].State != asks.StateCleared || said[1].Key != said[0].Key { + t.Errorf("said = %+v, want the condition cleared once under its key", said) + } +} + +// 1945/AC21: the pass gathers the modules on neither list of every module's outcome, keyed by node and module. +func TestThePassGathersTheModulesOnNeitherList(t *testing.T) { + got := map[string]neitherOne{} + gatherNeither(got, "screen-lock", assign.Outcome{Neither: map[string]assign.Holding{"g14": {Rule: delivery.RuleGrant, Words: "w"}}}) + gatherNeither(got, "rofi", assign.Outcome{}) + if len(got) != 1 || got["g14.screen-lock"].node != "g14" || got["g14.screen-lock"].module != "screen-lock" || got["g14.screen-lock"].holding.Rule != delivery.RuleGrant { + t.Errorf("gathered = %+v", got) + } +} diff --git a/modules/mesh-delivery-assigner/internal/assign/asks.go b/modules/mesh-delivery-assigner/internal/assign/asks.go index 15662c9..87c0b0f 100644 --- a/modules/mesh-delivery-assigner/internal/assign/asks.go +++ b/modules/mesh-delivery-assigner/internal/assign/asks.go @@ -81,6 +81,28 @@ func HeldCondition(module, fp, node, words string, raised, needsYou bool) asks.C return c } +// NeitherCondition is the condition of a module assigned to node that holds no want for it (issue 1945): the node's own composition +// gives up every module with a fingerprint of the new form, so nothing applies it there and its files are gone. It is raised (urgent) +// naming the module, the node and the rule holding its first want back with the rule's words, under the assigner's own name keyed by +// node and module, and cleared (raised false) when the node has a want for it again. +func NeitherCondition(node, module string, h Holding, raised bool) asks.Condition { + key := delivery.AssignerModule + ".neither." + node + "." + module + subject := asks.Scope{Scope: "module", ID: module, Machine: node} + if !raised { + return asks.Condition{Key: key, Subject: subject, State: asks.StateCleared, + Resolved: fmt.Sprintf("%s on %s has a want again, so its own composition is applied there.", module, node)} + } + headline := fmt.Sprintf("%s on %s is on no list", module, node) + if n := len([]rune(headline)); n > asks.HeadlineLength { + headline = string([]rune(headline)[:asks.HeadlineLength]) + } + return asks.Condition{Key: key, Severity: asks.SeverityUrgent, Subject: subject, State: asks.StateRaised, + Summary: headline, Headline: headline, + Explanation: fmt.Sprintf("%s %s is assigned to %s, but the node holds no want for it and the node's own composition does not "+ + "hold it either, so nothing applies it there. The assigner's rule %d holds its first want back: %s. The assigner writes the want once the "+ + "rule holds, and clears this.", asks.NothingToDo, module, node, h.Rule, h.Words)} +} + // LiftedCondition clears the condition of a held fingerprint the assigner lifted by itself, saying why (issue 1906). func LiftedCondition(module, fp string) asks.Condition { c := HeldCondition(module, fp, "", "", false, false) diff --git a/modules/mesh-delivery-assigner/internal/assign/asks_test.go b/modules/mesh-delivery-assigner/internal/assign/asks_test.go index 1342b7d..dbd6523 100644 --- a/modules/mesh-delivery-assigner/internal/assign/asks_test.go +++ b/modules/mesh-delivery-assigner/internal/assign/asks_test.go @@ -147,3 +147,19 @@ func TestTheRecordAndBackupAsksAreValid(t *testing.T) { t.Errorf("the backup condition is refused: %v", err) } } + +// 1945/AC21: the condition of a module on neither list is urgent, names the module, the node and the rule holding its want back, +// under the assigner's own name keyed by node and module, and is cleared under the same key. +func TestTheNeitherConditionNamesModuleNodeAndRule(t *testing.T) { + c := NeitherCondition("g14", "i3status-rust", Holding{Rule: delivery.RuleProviders, Words: "no provider of x11-display runs healthy on g14 or at its canary"}, true) + if _, err := asks.EncodeCondition(c); err != nil { + t.Fatalf("the condition is refused: %v", err) + } + if c.State != asks.StateRaised || c.Severity != asks.SeverityUrgent || c.Subject.ID != "i3status-rust" || c.Subject.Machine != "g14" || + !strings.Contains(c.Explanation, "x11-display") || !strings.Contains(c.Explanation, "rule 5") || c.Key != "mesh-delivery-assigner.neither.g14.i3status-rust" { + t.Errorf("condition = %+v", c) + } + if got := NeitherCondition("g14", "i3status-rust", Holding{}, false); got.State != asks.StateCleared || got.Key != c.Key { + t.Errorf("cleared = %+v", got) + } +} diff --git a/modules/mesh-delivery-assigner/internal/assign/wants.go b/modules/mesh-delivery-assigner/internal/assign/wants.go index f0c83eb..cc2e4da 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants.go @@ -75,22 +75,31 @@ func Canary(nodes []string, online map[string]bool) string { // with why (ADR 0429 §6). Rule 4, at most `step` others ahead, always holds: the mesh keeps no step, and step 0 is no // limit (ADR 0528 clause 3). func Holds(m Module, node string, n Nodes) (bool, int, string) { + return holds(m, node, n, true) +} + +// holds is Holds; with ordering false it leaves out the two rules that order a move among nodes and modules, the canary (3) +// and the providers (5), and keeps those without which there is nothing to want: the build done, not held, the grant and the +// composition. +func holds(m Module, node string, n Nodes, ordering bool) (bool, int, string) { switch { case !m.Built: return false, delivery.RuleBuildDone, fmt.Sprintf("the build of %s at %s is not done", m.Name, short(m.FP)) case m.Held: return false, delivery.RuleNotHeld, fmt.Sprintf("%s is held at %s", m.Name, short(m.FP)) } - canary := Canary(m.Nodes, n.Online) - switch { - case canary == "": - return false, delivery.RuleCanary, fmt.Sprintf("none of the nodes of %s is online, so it has no canary", m.Name) - case node != canary && !healthy(n.Haves[canary][m.Name], m.FP): - return false, delivery.RuleCanary, fmt.Sprintf("the canary %s does not run %s healthy yet", canary, short(m.FP)) - } - for _, provision := range m.Manifest.Requires { - if ok, why := providedHealthy(provision, node, n); !ok { - return false, delivery.RuleProviders, why + if ordering { + canary := Canary(m.Nodes, n.Online) + switch { + case canary == "": + return false, delivery.RuleCanary, fmt.Sprintf("none of the nodes of %s is online, so it has no canary", m.Name) + case node != canary && !healthy(n.Haves[canary][m.Name], m.FP): + return false, delivery.RuleCanary, fmt.Sprintf("the canary %s does not run %s healthy yet", canary, short(m.FP)) + } + for _, provision := range m.Manifest.Requires { + if ok, why := providedHealthy(provision, node, n); !ok { + return false, delivery.RuleProviders, why + } } } if !m.Granted { @@ -154,6 +163,11 @@ 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 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 } // RecordNeed is a move of a module of policy record on a node that waits for the operator's yes: the composition's digest, the @@ -177,7 +191,7 @@ type Holding struct { // module holding data is not set back but asked about (Restore). A // node that wants m but is no longer among its nodes gets its want written removed, never deleted (ADR 0408 §4). func Wants(m Module, n Nodes, wants map[string]delivery.Want, lastHealthy map[string]string) Outcome { - out := Outcome{Wants: map[string]delivery.Want{}, Rules: map[string]Holding{}} + out := Outcome{Wants: map[string]delivery.Want{}, Rules: map[string]Holding{}, Neither: map[string]Holding{}} held := m.Held if held && !m.Kept && m.FP != "" && allRunHealthy(m, n) { out.Lifted, held = true, false @@ -208,7 +222,16 @@ func Wants(m Module, n Nodes, wants map[string]delivery.Want, lastHealthy map[st } continue } - switch ok, rule, words := Holds(mm, node, n); { + ok, rule, words := Holds(mm, node, n) + if !ok && (!had || before.Removed) && (rule == delivery.RuleCanary || rule == delivery.RuleProviders) && runs(n.Haves[node][m.Name]) { + // 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) + } + switch { case ok: d, _ := m.composition(node, m.FP) next = delivery.Want{FP: m.FP, Composed: d.Digest} @@ -245,6 +268,8 @@ 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} continue } next = delivery.Want{FP: before.FP, Composed: before.Composed, Newest: m.FP, Rule: rule, Words: words} @@ -264,6 +289,11 @@ func Wants(m Module, n Nodes, wants map[string]delivery.Want, lastHealthy map[st return out } +// runs says whether a have is of a module the node runs: running, whatever its health. +func runs(h delivery.Have) bool { + return h.Phase == delivery.PhaseRunning +} + // allRunHealthy says whether every node of m that runs the module (has a have of it) runs m.FP healthy, and at least one does: // a hold whose fingerprint runs healthy wherever the module runs holds nothing back any more (issue 1906: seven modules stayed // held on every machine after one transient failed check). A node set back to another fingerprint does not run m.FP, so its diff --git a/modules/mesh-delivery-assigner/internal/assign/wants_test.go b/modules/mesh-delivery-assigner/internal/assign/wants_test.go index 18da1ed..50ac2b8 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants_test.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants_test.go @@ -304,6 +304,7 @@ func TestAMoveJudgedNeitherOrNeverRunOrNotRecordedIsWrittenAtOnce(t *testing.T) } m, n, _ = movingTo(delivery.JudgedRecreates) recording(&m) + m.Nodes = []string{"ace"} // g14 runs it with no want, so its first want is asked (1945/AC19): this case is ace's alone delete(n.Haves["ace"], "mail") if out := Wants(m, n, nil, nil); out.Wants["ace"].FP != fNew || len(out.Record) != 0 { t.Errorf("a node that never ran it: %+v", out) @@ -579,3 +580,102 @@ 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. +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"}}} + 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}, + "xorg": {FP: fpOf('x'), Phase: delivery.PhaseRunning, Health: delivery.HealthUnknown}}, + "shanks": {"screen-lock": {FP: "build-2", 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"}}}, + } + 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) { + 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 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 + 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) + } + // 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) + } +} + +// 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) { + 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) + } + } + 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) + } +} + +// 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) { + 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 _, ok := out.Neither["g14"]; ok { + t.Errorf("g14 named on neither list although it got its want: %+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) + } + } + // 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) + } +} From f7b23033a23b060f0483a9c81c99a094564072d2 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 25 Oct 2026 15:18:07 +0100 Subject: [PATCH 2/6] Assigner: the integration test of wants on the bus proves a running node's first want and the neither condition; held-condition reads filter by key (issue 1945, try-1792937443523744996) --- .../integration_test.go | 49 ++++++++++++------- 1 file changed, 32 insertions(+), 17 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 9ab48e5..40c299f 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 @@ -281,9 +281,11 @@ func (b *bed) get(t *testing.T, bucket, key string) []byte { return nil } -// 1297/B4 on the bus, 1906/D5: the controller sends ace and g14 builds while the canary ace does not run F healthy yet. -// Started whole, the assigner wants F on the canary, holds g14 back with no want, and writes no difference at all: there -// is no send to compare with (ADR 0539 §8). +// 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. func TestWantsAreWrittenAndNoDifferenceIs(t *testing.T) { b := startBed(t, "") fNew, fOld := delivery.SourcePrefix2+strings.Repeat("b", 64), delivery.SourcePrefix2+strings.Repeat("a", 64) @@ -299,16 +301,16 @@ func TestWantsAreWrittenAndNoDifferenceIs(t *testing.T) { gkey, _ := delivery.ControllerGrantKey("web", fNew) grant, _ := delivery.EncodeControllerGrant(delivery.ControllerGrant{}) b.putJSON(t, delivery.ControllerGrantsBucket, gkey, grant) - b.compose(t, "ace", "web", fNew, 'c', delivery.JudgedNeither) - b.compose(t, "g14", "web", fNew, 'c', delivery.JudgedNeither) - for _, node := range []string{"ace", "g14"} { + 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) - have, _ := delivery.EncodeHave(delivery.Have{FP: fOld, Phase: delivery.PhaseRunning, Health: delivery.HealthOK}) - b.putJSON(t, delivery.HaveBucket, node+".web", have) + if node != "g14" { + have, _ := delivery.EncodeHave(delivery.Have{FP: fOld, 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.putJSON(t, provider.AssignmentsBucket, "ace.web", []byte(`{"node":"ace","module":"web","build":"b-8"}`)) - b.putJSON(t, provider.AssignmentsBucket, "g14.web", []byte(`{"node":"g14","module":"web","build":"b-9"}`)) b.start(t) wkey, _ := delivery.WantKey("ace", "web") @@ -316,12 +318,25 @@ func TestWantsAreWrittenAndNoDifferenceIs(t *testing.T) { if err != nil || want.FP != fNew { t.Fatalf("the canary ace's want = %+v, %v", want, err) } - time.Sleep(500 * time.Millisecond) + 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) + } + var neither asks.Condition + for deadline := time.Now().Add(10 * time.Second); time.Now().Before(deadline) && neither.Key == ""; time.Sleep(100 * time.Millisecond) { + for k, c := range b.conditionsSaid(t) { + if strings.Contains(k, ".neither.") && c.State == asks.StateRaised { + neither = c + } + } + } + if neither.Subject.Machine != "g14" || neither.Subject.ID != "web" || neither.Severity != asks.SeverityUrgent || !strings.Contains(neither.Explanation, "canary") { + t.Errorf("the condition of web on neither list on g14 = %+v", neither) + } if keys, err := b.buckets[delivery.AssignerDifferencesBucket].Keys(b.ctx); err == nil && len(keys) != 0 { 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, held back by the canary with no want before, was given one: %s", e.Value()) + t.Errorf("g14, which does not run web and is held back by the canary, was given a want: %s", e.Value()) } } @@ -415,7 +430,7 @@ func TestAFailedDataModuleIsAskedToRestore(t *testing.T) { key := "" for deadline := time.Now().Add(10 * time.Second); time.Now().Before(deadline) && key == ""; time.Sleep(100 * time.Millisecond) { for k, c := range b.conditionsSaid(t) { - if c.State == asks.StateRaised && c.Subject.ID == "mail" { + if c.State == asks.StateRaised && c.Subject.ID == "mail" && strings.Contains(k, ".held.") { if c.Subject.Machine != "ace" || !strings.Contains(c.Explanation, "connection refused") { t.Fatalf("the condition = %+v, want it to name ace and the check's words", c) } @@ -634,8 +649,8 @@ func TestAReleaseAnswerWritesTheWantAgain(t *testing.T) { } for deadline := time.Now().Add(10 * time.Second); time.Now().Before(deadline); time.Sleep(100 * time.Millisecond) { cleared := false - for _, c := range b.conditionsSaid(t) { - cleared = cleared || (c.Subject.ID == "web" && c.State == asks.StateCleared) + for k, c := range b.conditionsSaid(t) { + cleared = cleared || (c.Subject.ID == "web" && c.State == asks.StateCleared && strings.Contains(k, ".held.")) } if cleared { return @@ -805,8 +820,8 @@ func TestAHoldIsLiftedOnceEveryNodeRunsItHealthy(t *testing.T) { if b.hasKey(delivery.AssignerWantsBucket, hkey) || b.hasKey(delivery.AssignerAsksBucket, akey) { continue } - for _, c := range b.conditionsSaid(t) { - if c.Subject.ID == "web" && c.State == asks.StateCleared && strings.Contains(c.Resolved, "lifted its hold by itself") { + for k, c := range b.conditionsSaid(t) { + if c.Subject.ID == "web" && c.State == asks.StateCleared && strings.Contains(k, ".held.") && strings.Contains(c.Resolved, "lifted its hold by itself") { if w := b.wantNow(t, "ace", "web", "the want at the lifted fingerprint", func(w delivery.Want) bool { return w.FP == fNew }); w.Rule != 0 { t.Errorf("the want = %+v", w) } From 81182e9dcda8a646d224a91e4874ab2f579eec27 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 25 Oct 2026 15:26:03 +0100 Subject: [PATCH 3/6] 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) From 3d7d65edce4cdb0a2c85b8815ff33f7e25dc5537 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 25 Oct 2026 15:32:35 +0100 Subject: [PATCH 4/6] Assigner: a node running an old-form build gets its first want at F once the canary runs F healthy (issue 1945) An old-form build (not src2:) has no composition, so no want can ever name what such a node runs. Design limit, decided by the lead (lead-47): its first want names the newest F, by every rule as usual - the canary (rule 3: this node is the canary, or the canary runs F healthy), everything F needs (built, not held, granted, composed) and the providers (rule 5). Otherwise nothing is written and the neither condition stays. The canary stays in force, and old-form modules (clipmenu and screen-lock on g14) leave neither list by themselves, without a hand-written want. The code already behaved so (an old-form have falls through to Holds); the case is now named in Wants, and proven: a unit test for both sides of the canary, the providers and the grant, and the integration test gives g14 F and clears its neither condition once the canary ace runs F healthy. --- .../integration_test.go | 19 +++++- .../internal/assign/wants.go | 7 +- .../internal/assign/wants_test.go | 66 +++++++++++++++++++ 3 files changed, 90 insertions(+), 2 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 241ae99..a8c4365 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 @@ -285,7 +285,8 @@ func (b *bed) get(t *testing.T, bucket, key string) []byte { // 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 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. +// raises the condition naming g14, web and the canary rule; once ace runs F healthy, g14's first want names F and the condition is +// cleared. func TestWantsAreWrittenAndNoDifferenceIs(t *testing.T) { b := startBed(t, "") fNew, fOld := delivery.SourcePrefix2+strings.Repeat("b", 64), delivery.SourcePrefix2+strings.Repeat("a", 64) @@ -340,6 +341,22 @@ func TestWantsAreWrittenAndNoDifferenceIs(t *testing.T) { if e, err := b.buckets[delivery.AssignerWantsBucket].Get(b.ctx, "want.g14.web"); err == nil { t.Errorf("g14, which runs an old-form build and is held back by the canary, was given a want: %s", e.Value()) } + + // 1945/AC21: once the canary ace runs F healthy, g14's first want names F by the rules as usual, and web leaves neither list + // there by itself: the condition is cleared. + healthyF, _ := delivery.EncodeHave(delivery.Have{FP: fNew, Phase: delivery.PhaseRunning, Health: delivery.HealthOK}) + b.putJSON(t, delivery.HaveBucket, "ace.web", healthyF) + if w := b.wantNow(t, "g14", "web", "g14's first want at F once the canary runs it healthy", func(w delivery.Want) bool { return w.FP != "" }); w.FP != fNew || w.Composed != "sha256:"+strings.Repeat("c", 64) || w.Newest != "" { + t.Errorf("g14's want = %+v, want F with its composition", w) + } + cleared := false + for deadline := time.Now().Add(10 * time.Second); time.Now().Before(deadline) && !cleared; time.Sleep(100 * time.Millisecond) { + c, ok := b.conditionsSaid(t)[neither.Key] + cleared = ok && c.State == asks.StateCleared + } + if !cleared { + t.Errorf("the condition of web on neither list on g14 was not cleared once g14 got its want") + } } // 1297/B9 (the assigner's half, ADR 0528 clause 6): started whole, the assigner holds a fingerprint whose health check diff --git a/modules/mesh-delivery-assigner/internal/assign/wants.go b/modules/mesh-delivery-assigner/internal/assign/wants.go index 7ee8b50..264a2e9 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants.go @@ -239,7 +239,12 @@ func Wants(m Module, n Nodes, wants map[string]delivery.Want, lastHealthy map[st 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): + case !strings.HasPrefix(have.FP, delivery.SourcePrefix2): + // An old-form build (not src2:) has no composition, so no want can ever name it: its first want names F, and only by every + // rule as usual, the canary (3) and the providers (5) included, which here hold it back. Nothing is written and the module + // stays on neither list until the canary runs F healthy; then Holds passes and the want names F (issue 1945, a design + // limit set by the lead: the canary stays in force and old-form modules leave neither list by themselves). + case composed && m.GrantedAt != nil && m.GrantedAt(have.FP): ok, target = true, have.FP } } diff --git a/modules/mesh-delivery-assigner/internal/assign/wants_test.go b/modules/mesh-delivery-assigner/internal/assign/wants_test.go index a9b943b..ebaa65f 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants_test.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants_test.go @@ -690,3 +690,69 @@ func TestANewAssignmentWaitingIsNotOnNeitherList(t *testing.T) { t.Errorf("a module of the node's own composition named on neither list: %+v", out.Neither) } } + +// 1945/AC21: a node whose have names an old-form build (no fingerprint of the new form, so no composition of it) can never be +// named in a want at what it runs; its first want names F, under the canary rule as usual and with everything F needs and the +// providers rule. Given the canary runs F healthy, shanks's first want names F; given it does not yet, shanks gets no want and +// stays on neither list. The old-form node that is itself the canary gets F once its providers run healthy. +func TestAnOldFormNodeGetsItsFirstWantAtFOnlyUnderTheCanary(t *testing.T) { + oldForm := func(n Nodes, node string) { + n.Haves[node]["screen-lock"] = delivery.Have{FP: "build-1792260876956849831", Phase: delivery.PhaseRunning, Health: delivery.HealthUnknown} + } + providersHealthy := func(n Nodes) { + for _, node := range []string{"g14", "shanks"} { + n.Haves[node]["xorg"] = delivery.Have{FP: fpOf('x'), Phase: delivery.PhaseRunning, Health: delivery.HealthOK} + } + } + // the canary g14 runs F healthy: shanks's first want names F + m, n := x11() + providersHealthy(n) + oldForm(n, "shanks") + n.Haves["g14"]["screen-lock"] = delivery.Have{FP: fNew, Phase: delivery.PhaseRunning, Health: delivery.HealthOK} + out := Wants(m, n, nil, nil) + if w := out.Wants["shanks"]; w.FP != fNew || w.Composed != dig(fNew) || w.Newest != "" || w.Rule != 0 { + t.Errorf("canary healthy on F: shanks's want = %+v, want F", w) + } + if _, ok := out.Neither["shanks"]; ok { + t.Errorf("canary healthy on F: shanks still on neither list: %+v", out.Neither) + } + // the canary does not run F healthy yet: no want, and shanks stays on neither list by rule 3 + m, n = x11() + providersHealthy(n) + oldForm(n, "shanks") + n.Haves["g14"]["screen-lock"] = delivery.Have{FP: fNew, Phase: delivery.PhaseRunning, Health: delivery.HealthUnknown} + out = Wants(m, n, nil, nil) + if w, ok := out.Wants["shanks"]; ok { + t.Errorf("canary not healthy on F: shanks got a want: %+v", w) + } + if h, ok := out.Neither["shanks"]; !ok || h.Rule != delivery.RuleCanary { + t.Errorf("canary not healthy on F: shanks on neither list = %+v (named %v), want rule 3", h, ok) + } + // the canary healthy on F but the providers not healthy on shanks: rule 5 still holds it back + m, n = x11() + oldForm(n, "shanks") + n.Haves["g14"]["screen-lock"] = delivery.Have{FP: fNew, Phase: delivery.PhaseRunning, Health: delivery.HealthOK} + out = Wants(m, n, nil, nil) + if w, ok := out.Wants["shanks"]; ok { + t.Errorf("providers not healthy: shanks got a want: %+v", w) + } + if h := out.Neither["shanks"]; h.Rule != delivery.RuleProviders { + t.Errorf("providers not healthy: shanks on neither list = %+v, want rule 5", h) + } + // F not granted: nothing, even with the canary healthy + m, n = x11() + providersHealthy(n) + oldForm(n, "shanks") + n.Haves["g14"]["screen-lock"] = delivery.Have{FP: fNew, Phase: delivery.PhaseRunning, Health: delivery.HealthOK} + m.Granted = false + if w, ok := Wants(m, n, nil, nil).Wants["shanks"]; ok { + t.Errorf("F not granted: shanks got a want: %+v", w) + } + // the old-form node is the canary itself: F once its providers run healthy + m, n = x11() + providersHealthy(n) + oldForm(n, "g14") + if w := Wants(m, n, nil, nil).Wants["g14"]; w.FP != fNew || w.Composed != dig(fNew) { + t.Errorf("old-form canary: g14's want = %+v, want F", w) + } +} From 706169aa8a3c0a59b54f3099e50c659145f51b23 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 25 Oct 2026 15:39:45 +0100 Subject: [PATCH 5/6] Assigner: a failed health holds a fingerprint only once the node is done moving (phase running or failed) On novox at 15:27:36 a have reading "health failed, phase applying" at the controller's new fingerprint held its canary two seconds before the new build was healthy: Wants held on a failed health whatever the phase. A failed health while the node backs up, applies or checks is a step of the move, not its outcome, so it holds nothing now. Every other Health read in the assigner (rule 3's healthy, rule 5's providedHealthy, the world's lastHealthy) already asks for phase running. Test from the live row first, seen failing before the fix. --- .../internal/assign/wants.go | 10 ++++++- .../internal/assign/wants_test.go | 26 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/modules/mesh-delivery-assigner/internal/assign/wants.go b/modules/mesh-delivery-assigner/internal/assign/wants.go index 264a2e9..1e2d5ed 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants.go @@ -201,7 +201,7 @@ func Wants(m Module, n Nodes, wants map[string]delivery.Want, lastHealthy map[st } if !held && !out.Lifted && !m.Released && m.FP != "" { for _, node := range slices.Sorted(slices.Values(m.Nodes)) { - if h, ok := n.Haves[node][m.Name]; ok && h.FP == m.FP && h.Health == delivery.HealthFailed { + if h, ok := n.Haves[node][m.Name]; ok && h.FP == m.FP && failed(h) { out.Held = &delivery.Held{Node: node, Words: HeldWords(h, m.Now)} held = true break @@ -315,6 +315,14 @@ func Wants(m Module, n Nodes, wants map[string]delivery.Want, lastHealthy map[st return out } +// failed says whether a have's health check failed once the node is done moving: phase running or failed. A failed health while it +// backs up, applies or checks is a step of the move, not its outcome (on novox at 15:27:36 on 2026-10-25, "health failed, phase +// applying" held the controller's canary two seconds before the new build was healthy). Every healthy read (healthy, providedHealthy, +// the world's lastHealthy) already asks for phase running. +func failed(h delivery.Have) bool { + return h.Health == delivery.HealthFailed && (h.Phase == delivery.PhaseRunning || h.Phase == delivery.PhaseFailed) +} + // runs says whether a have is of a module the node runs: running, whatever its health. func runs(h delivery.Have) bool { return h.Phase == delivery.PhaseRunning diff --git a/modules/mesh-delivery-assigner/internal/assign/wants_test.go b/modules/mesh-delivery-assigner/internal/assign/wants_test.go index ebaa65f..be6c0ee 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants_test.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants_test.go @@ -756,3 +756,29 @@ func TestAnOldFormNodeGetsItsFirstWantAtFOnlyUnderTheCanary(t *testing.T) { t.Errorf("old-form canary: g14's want = %+v, want F", w) } } + +// A failed health counts only once the node is done moving: phase running or failed, never backing-up, applying or checking. +// The live row on novox at 15:27:36 on 2026-10-25 ("health failed, phase applying" at the controller's new fingerprint) held the +// canary two seconds before the new build was healthy. +func TestAFailedHealthWhileTheNodeMovesHoldsNothing(t *testing.T) { + for _, phase := range []delivery.Phase{delivery.PhaseBackingUp, delivery.PhaseApplying, delivery.PhaseChecking} { + m := Module{Name: "mesh-controller", FP: fNew, Built: true, Granted: true, Composed: composedEverywhere, Nodes: []string{"novox"}, + Manifest: catalogue.Manifest{Module: "mesh-controller"}} + n := Nodes{Online: map[string]bool{"novox": true}, Haves: map[string]map[string]delivery.Have{ + "novox": {"mesh-controller": {FP: fNew, Phase: phase, Health: delivery.HealthFailed}}}} + before := map[string]delivery.Want{"novox": {FP: fNew, Composed: dig(fNew)}} + if out := Wants(m, n, before, map[string]string{"novox": fOld}); out.Held != nil { + t.Errorf("phase %s, health failed: held %+v, want no hold while the node moves", phase, out.Held) + } + } + for _, phase := range []delivery.Phase{delivery.PhaseRunning, delivery.PhaseFailed} { + m := Module{Name: "mesh-controller", FP: fNew, Built: true, Granted: true, Composed: composedEverywhere, Nodes: []string{"novox"}, + Manifest: catalogue.Manifest{Module: "mesh-controller"}} + n := Nodes{Online: map[string]bool{"novox": true}, Haves: map[string]map[string]delivery.Have{ + "novox": {"mesh-controller": {FP: fNew, Phase: phase, Health: delivery.HealthFailed}}}} + before := map[string]delivery.Want{"novox": {FP: fNew, Composed: dig(fNew)}} + if out := Wants(m, n, before, map[string]string{"novox": fOld}); out.Held == nil || out.Held.Node != "novox" { + t.Errorf("phase %s, health failed: held %+v, want a hold on novox", phase, out.Held) + } + } +} From 975f0ed65460388ccef716fdaecfe4cc5321c1ed Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 25 Oct 2026 15:47:57 +0100 Subject: [PATCH 6/6] Assigner: a hold lifts over haves not running, the first pass after a start clears stale conditions, and a first want after a removed want is asked (issue 1945, review round 2 of 3d7d65ed) Phase guard, same kind as the failed-health hold. allRunHealthy counted every have whatever its phase, so a node with a removed, held or still moving have kept a hold up for ever: it counts only phase running (healthy or not) and failed (not healthy). The failed-health hold already ignores removed and held haves (phase running or failed only). Stale conditions (B1). The neither and held conditions this process said lived only in its memory, so one raised by a previous process stayed urgent for ever once the module had its want or was no longer held. The first pass after a start clears, once, the neither condition of every assigned module with a want and not on neither list, and the held condition of every module's newest fingerprint that is not held; a clear of a condition that is not open is quiet at the router. A hold raised again forgets that it was cleared, so a later release clears it again. Record ask (review note 3). mustAsk wrote no ask when the want before was removed, so the first-want path could write a recreates or moves-data composition of a policy-record module with no ask. A removed want is now no want, and the first want after it goes through the record ask. Tests first, each seen failing: a hold lifted over removed, held and moving haves; the record ask after a removed want; a fresh pass clearing what a previous process raised, once. --- .../cmd/mesh-delivery-assigner/pass.go | 43 +++++++++++++-- .../cmd/mesh-delivery-assigner/pass_test.go | 49 +++++++++++++++-- .../internal/assign/wants.go | 21 ++++--- .../internal/assign/wants_test.go | 55 +++++++++++++++++++ 4 files changed, 152 insertions(+), 16 deletions(-) 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 6da9587..fed2320 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go @@ -30,7 +30,8 @@ type passer struct { asks *composeAsks // now is the clock; nil is time.Now. now func() time.Time - // neither is, by node and module, each module on neither list whose condition this process raised (issue 1945). + // neither is, by node and module, each module on neither list whose condition this process raised (issue 1945). Nil until the + // first pass after a start, which clears what a previous process may have raised (startClears). neither map[string]neitherOne } @@ -97,7 +98,13 @@ func (p *passer) wants(ctx context.Context) { names[name] = true } neither := map[string]neitherOne{} - defer p.sayNeither(neither) + // The first pass after a start remembers nothing a previous process said: a neither or held condition it raised would stay + // raised for ever once the module has its want or is no longer held. That pass clears, once, the neither condition of every + // assigned module with a want and the held condition of every module not held; a clear of a condition that is not open is + // quiet at the router. + first := p.neither == nil + stale := map[string]neitherOne{} + defer func() { p.sayNeither(neither, stale) }() for name := range names { fp := trunk[name].FP2 name := name @@ -132,6 +139,12 @@ func (p *passer) wants(ctx context.Context) { } out := assign.Wants(m, nodes, wanted[name], last) gatherNeither(neither, name, out) + if first { + startClears(stale, m, wanted[name], out) + if fp != "" && !m.Held && out.Held == nil && !out.Lifted { + p.clearHeld(name, fp) + } + } if out.Held != nil { key, err := delivery.HeldKey(name, fp) if err == nil { @@ -177,11 +190,32 @@ func gatherNeither(into map[string]neitherOne, module string, out assign.Outcome } } +// startClears adds, for the first pass after a start, every node of m that holds a want for it after the pass and is not on +// neither list: a neither condition a previous process raised for it is cleared. +func startClears(into map[string]neitherOne, m assign.Module, before map[string]delivery.Want, out assign.Outcome) { + for _, node := range m.Nodes { + w, ok := out.Wants[node] + if !ok { + w, ok = before[node] + } + if _, on := out.Neither[node]; ok && !w.Removed && !on { + into[node+"."+m.Name] = neitherOne{node: node, module: m.Name} + } + } +} + // sayNeither raises, once, the condition of every module on neither list in this pass and writes it to the journal, and clears, // once, the condition of each one said before that has a want again (issue 1945). A say that fails is made again at the next pass. -func (p *passer) sayNeither(now map[string]neitherOne) { +// stale is, on the first pass after a start, the modules whose condition a previous process may have raised and that have a want: +// each is cleared once. +func (p *passer) sayNeither(now, stale map[string]neitherOne) { if p.neither == nil { p.neither = map[string]neitherOne{} + for key, one := range stale { + if _, on := now[key]; !on { + p.neither[key] = one + } + } } for key, one := range now { if _, said := p.neither[key]; said { @@ -363,7 +397,8 @@ func (p *passer) heldCondition(ctx context.Context, module, fp string, fresh *de return } p.said[c.Key+".raised"] = true - delete(p.said, c.Key+".lifted") // a hold raised again and lifted again is said lifted again + delete(p.said, c.Key+".lifted") // a hold raised again and lifted again is said lifted again + delete(p.said, c.Key+".cleared") // and released again is cleared again } // restoreAsk writes, once, the ask about a module holding data whose held fingerprint failed its health check on node. diff --git a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go index 8ba0322..d62c765 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass_test.go @@ -12,6 +12,7 @@ import ( "git.novox.be/novox/mesh-sdk/go/asks" "git.novox.be/novox/mesh-sdk/go/delivery" + "git.novox.be/novox/mesh-sdk/go/provider" "mesh-delivery-assigner/internal/assign" ) @@ -121,17 +122,17 @@ func TestAModuleOnNeitherListIsRaisedOnceAndClearedOnce(t *testing.T) { p.logf = func(f string, a ...any) { logged = append(logged, fmt.Sprintf(f, a...)) } one := map[string]neitherOne{"g14.screen-lock": {node: "g14", module: "screen-lock", holding: assign.Holding{Rule: delivery.RuleProviders, Words: "no provider of x11-display runs healthy on g14 or at its canary"}}} - p.sayNeither(one) // the say fails - p.sayNeither(one) - p.sayNeither(one) + p.sayNeither(one, nil) // the say fails + p.sayNeither(one, nil) + p.sayNeither(one, nil) if len(said) != 1 || said[0].State != asks.StateRaised || said[0].Subject.Machine != "g14" || said[0].Subject.ID != "screen-lock" { t.Fatalf("said = %+v, want one raised condition of screen-lock on g14", said) } if !strings.Contains(strings.Join(logged, "\n"), "screen-lock") { t.Errorf("the journal does not name the module on neither list: %q", logged) } - p.sayNeither(map[string]neitherOne{}) - p.sayNeither(map[string]neitherOne{}) + p.sayNeither(map[string]neitherOne{}, nil) + p.sayNeither(map[string]neitherOne{}, nil) if len(said) != 2 || said[1].State != asks.StateCleared || said[1].Key != said[0].Key { t.Errorf("said = %+v, want the condition cleared once under its key", said) } @@ -146,3 +147,41 @@ func TestThePassGathersTheModulesOnNeitherList(t *testing.T) { t.Errorf("gathered = %+v", got) } } + +// A neither or held condition a previous process raised is cleared by the first pass after a start: a fresh pass remembers +// nothing it said, so it clears, once, the neither condition of every assigned module that has a want and the held condition of +// every module that is not held. Clearing a condition that is not open is quiet at the router. +func TestTheFirstPassAfterAStartClearsWhatAPreviousProcessRaised(t *testing.T) { + wantsKV := &memKV{values: map[string][]byte{}} + p := newPasser(buckets{delivery.AssignerWantsBucket: wantsKV, delivery.AssignerAsksBucket: &memKV{values: map[string][]byte{}}}, time.Now()) + var said []asks.Condition + p.say = func(c asks.Condition) error { said = append(said, c); return nil } + mod, _ := delivery.EncodeModule(delivery.Module{FP2: fpTest, Repository: "novox/web", Commit: strings.Repeat("1", 40), Path: delivery.RootPath}) + p.w.put(delivery.ModulesBucket, "web", mod) + p.w.put(provider.AssignmentsBucket, "g14.web", []byte(`{"node":"g14","module":"web","build":"b-1"}`)) + on, _ := delivery.EncodePresence(delivery.Presence{Online: true}) + p.w.put(delivery.PresenceBucket, "g14", on) + have, _ := delivery.EncodeHave(delivery.Have{FP: fpTest, Phase: delivery.PhaseRunning, Health: delivery.HealthOK}) + p.w.put(delivery.HaveBucket, "g14.web", have) + key, _ := delivery.WantKey("g14", "web") + want, _ := delivery.EncodeWant(delivery.Want{FP: fpTest, Composed: "sha256:" + strings.Repeat("c", 64)}) + p.w.put(delivery.AssignerWantsBucket, key, want) + p.wants(context.Background()) + cleared := map[string]bool{} + for _, c := range said { + if c.State != asks.StateCleared { + t.Errorf("the first pass raised %+v", c) + } + cleared[c.Key] = true + } + neither := assign.NeitherCondition("g14", "web", assign.Holding{}, false).Key + held := assign.HeldCondition("web", fpTest, "", "", false, false).Key + if !cleared[neither] || !cleared[held] { + t.Errorf("cleared = %v, want %s and %s", cleared, neither, held) + } + n := len(said) + p.wants(context.Background()) + if len(said) != n { + t.Errorf("a second pass said again: %+v", said[n:]) + } +} diff --git a/modules/mesh-delivery-assigner/internal/assign/wants.go b/modules/mesh-delivery-assigner/internal/assign/wants.go index 1e2d5ed..273faf9 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants.go @@ -262,7 +262,12 @@ func Wants(m Module, n Nodes, wants map[string]delivery.Want, lastHealthy map[st 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 := at.mustAsk(node, before, had, ran, running.FP, running.Composed == "", d); wait { + // A removed want is no want: a first want after it goes through the record ask as a first want does. + asked, hadLive := before, had + if before.Removed { + asked, hadLive = delivery.Want{}, false + } + if need, wait := at.mustAsk(node, asked, hadLive, ran, running.FP, running.Composed == "", d); wait { if need != nil { out.Record = append(out.Record, *need) } @@ -315,7 +320,8 @@ func Wants(m Module, n Nodes, wants map[string]delivery.Want, lastHealthy map[st return out } -// failed says whether a have's health check failed once the node is done moving: phase running or failed. A failed health while it +// failed says whether a have's health check failed once the node is done moving: phase running or failed (never removed, held, or +// moving). A failed health while it // backs up, applies or checks is a step of the move, not its outcome (on novox at 15:27:36 on 2026-10-25, "health failed, phase // applying" held the controller's canary two seconds before the new build was healthy). Every healthy read (healthy, providedHealthy, // the world's lastHealthy) already asks for phase running. @@ -328,15 +334,16 @@ func runs(h delivery.Have) bool { return h.Phase == delivery.PhaseRunning } -// allRunHealthy says whether every node of m that runs the module (has a have of it) runs m.FP healthy, and at least one does: -// a hold whose fingerprint runs healthy wherever the module runs holds nothing back any more (issue 1906: seven modules stayed -// held on every machine after one transient failed check). A node set back to another fingerprint does not run m.FP, so its -// hold stays. +// allRunHealthy says whether every node of m that runs the module (its have's phase is running, or failed) runs m.FP healthy, and +// at least one does: a hold whose fingerprint runs healthy wherever the module runs holds nothing back any more (issue 1906: seven +// modules stayed held on every machine after one transient failed check). A node set back to another fingerprint does not run +// m.FP, and a failed one does not run it healthy, so the hold stays; a have that is removed, held for its backup or still moving +// (backing up, applying, checking) is not counted, or it would keep the hold up for ever. func allRunHealthy(m Module, n Nodes) bool { ran := 0 for _, node := range m.Nodes { h, ok := n.Haves[node][m.Name] - if !ok { + if !ok || (h.Phase != delivery.PhaseRunning && h.Phase != delivery.PhaseFailed) { continue } if !healthy(h, m.FP) { diff --git a/modules/mesh-delivery-assigner/internal/assign/wants_test.go b/modules/mesh-delivery-assigner/internal/assign/wants_test.go index be6c0ee..031121e 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants_test.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants_test.go @@ -782,3 +782,58 @@ func TestAFailedHealthWhileTheNodeMovesHoldsNothing(t *testing.T) { } } } + +// A hold is lifted by itself once every node that runs the module runs F healthy: a have of a node that is not running the module +// (removed, held for its backup, or still moving) is not counted, so it does not keep the hold up for ever; a node running another +// fingerprint, or failed, still does. +func TestAHoldIsLiftedOverHavesThatAreNotRunning(t *testing.T) { + setup := func(phase delivery.Phase, fp string, health delivery.Health) (Module, Nodes) { + m := Module{Name: "web", FP: fNew, Built: true, Granted: true, Held: true, Composed: composedEverywhere, Nodes: []string{"ace", "g14"}} + n := Nodes{Online: map[string]bool{"ace": true, "g14": true}, Haves: map[string]map[string]delivery.Have{ + "ace": {"web": {FP: fNew, Phase: delivery.PhaseRunning, Health: delivery.HealthOK}}, + "g14": {"web": {FP: fp, Phase: phase, Health: health}}}} + return m, n + } + for _, phase := range []delivery.Phase{delivery.PhaseRemoved, delivery.PhaseHeld, delivery.PhaseApplying, delivery.PhaseBackingUp, delivery.PhaseChecking} { + m, n := setup(phase, fOld, delivery.HealthUnknown) + if out := Wants(m, n, nil, nil); !out.Lifted { + t.Errorf("g14's have in phase %s kept the hold up", phase) + } + } + for what, h := range map[string]delivery.Have{ + "running another fingerprint": {FP: fOld, Phase: delivery.PhaseRunning, Health: delivery.HealthOK}, + "failed": {FP: fNew, Phase: delivery.PhaseFailed, Health: delivery.HealthFailed}, + } { + m, n := setup(h.Phase, h.FP, h.Health) + if out := Wants(m, n, nil, nil); out.Lifted { + t.Errorf("g14 %s: the hold was lifted", what) + } + } + // a failed health on a removed have holds nothing + m, n := setup(delivery.PhaseRemoved, fNew, delivery.HealthFailed) + m.Held = false + if out := Wants(m, n, nil, nil); out.Held != nil { + t.Errorf("a removed have with a failed health held F: %+v", out.Held) + } +} + +// A module of policy record whose want was removed but which the node still runs gets its first want through the record ask: a +// removed want is no want, so a move the controller judges to recreate or move data waits for the operator's yes. +func TestAFirstWantAfterARemovedWantGoesThroughTheRecordAsk(t *testing.T) { + m, n := x11() + recording(&m) + m.Composed = func(_, fp string) (delivery.ComposedDigest, bool) { + return delivery.ComposedDigest{Digest: dig(fp), Judgement: delivery.JudgedRecreates, Policy: delivery.PolicyRecord}, true + } + out := Wants(m, n, map[string]delivery.Want{"shanks": {Removed: true}}, nil) + if w, ok := out.Wants["shanks"]; ok { + t.Errorf("shanks's first want after a removed want was written with no record ask: %+v", w) + } + asked := false + for _, r := range out.Record { + asked = asked || (r.Node == "shanks" && r.Digest == dig(fOld) && r.Judgement == delivery.JudgedRecreates) + } + if !asked { + t.Errorf("the record asks = %+v, want one for shanks at the composition it runs", out.Record) + } +}