From 8daa5977f9929b8d5c54ab0df6d0a0d7b6812ee5 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 25 Oct 2026 15:10:34 +0100 Subject: [PATCH] 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) + } +}