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) + } +}