From d4a054bfed3a058882799fedb7c6acfb906c3569 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 25 Oct 2026 21:45:43 +0100 Subject: [PATCH] Assigner: say why a node gets no want, in the journal and in its removed want (issue 1945) A new assignment whose trunk build was not done fell through the rules with nothing said: no want, no neither-list entry, no log line, and the rule holding it back was computed and dropped. The pass now logs the rule when a node's reason appears or changes and when it has a want again, one line per change. A node whose want is removed and that does not run the module gets the reason in that removed want's words, so assigner_want answers why; a node with no key gets no removed want, since its node agent would then remove the module and stop holding its resources back. --- .../cmd/mesh-delivery-assigner/pass.go | 41 +++++++++++++++++++ .../cmd/mesh-delivery-assigner/pass_test.go | 25 +++++++++++ .../internal/assign/wants.go | 17 +++++++- .../internal/assign/wants_test.go | 27 ++++++++++++ 4 files changed, 109 insertions(+), 1 deletion(-) 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 1bbe91f..32a1dcc 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go @@ -3,6 +3,7 @@ package main import ( "context" "errors" + "fmt" "strings" "time" @@ -35,6 +36,9 @@ type passer struct { neither map[string]neitherOne // acts are the puts' acts (ADR 0546); nil: the puts are not taken further (no bus to call, or no puts bucket). acts *putActs + // noWant is, by node and module, the reason last said why the node gets no want for the module (issue 1945): a reason is + // said in the journal when it appears or changes, and its clearing when the node has a want again; one line per change. + noWant map[string]string } func (p *passer) clock() time.Time { @@ -148,6 +152,7 @@ func (p *passer) wants(ctx context.Context) { } out := assign.Wants(m, nodes, wanted[name], last) gatherNeither(neither, name, out) + p.sayNoWant(name, m.Nodes, wanted[name], out) if first { startClears(stale, m, out) if fp != "" && !m.Held && out.Held == nil && !out.Lifted { @@ -186,6 +191,42 @@ func (p *passer) wants(ctx context.Context) { } } +// sayNoWant says in the journal why a node of module gets no want after this pass, from the rule the outcome names for it +// (issue 1945: a new assignment whose trunk build was not done was held back with nothing said), once per reason, and says +// when the node has a want again. A node with a live want, written before and kept or written in this pass, has one. +func (p *passer) sayNoWant(module string, nodes []string, before map[string]delivery.Want, out assign.Outcome) { + if p.noWant == nil { + p.noWant = map[string]string{} + } + assigned := map[string]bool{} + for _, node := range nodes { + assigned[node] = true + key := node + "." + module + w, written := out.Wants[node] + if !written { + w, written = before[node] + } + reason := "" + if h, held := out.Rules[node]; held && (!written || w.Removed) { + reason = fmt.Sprintf("rule %d: %s", h.Rule, h.Words) + } + last, said := p.noWant[key] + switch { + case reason != "" && reason != last: + p.logf("%s on %s gets no want: %s", module, node, reason) + p.noWant[key] = reason + case reason == "" && said && written && !w.Removed: + p.logf("%s on %s has a want", module, node) + delete(p.noWant, key) + } + } + for key := range p.noWant { + if node, m, _ := strings.Cut(key, "."); m == module && !assigned[node] { + delete(p.noWant, key) // no longer assigned there: its removed want says so + } + } +} + // 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 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 0458ebc..ff438f1 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 @@ -380,6 +380,31 @@ func TestThePassGathersTheModulesOnNeitherList(t *testing.T) { 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) } + // 1945 (a first want held back silently): the journal says why a node gets no want when the reason appears or changes, and + // when it has a want again: one line per change, not per pass. + p := newPasser(buckets{}, time.Date(2026, 10, 25, 21, 30, 0, 0, time.UTC)) + var logged []string + p.logf = func(f string, a ...any) { logged = append(logged, fmt.Sprintf(f, a...)) } + notDone := assign.Outcome{Rules: map[string]assign.Holding{"shanks": {Rule: delivery.RuleBuildDone, Words: "the build of rofi at 4e7b118e is not done"}}} + wanted := assign.Outcome{Wants: map[string]delivery.Want{"shanks": {FP: fpTest, Composed: "sha256:" + strings.Repeat("c", 64)}}} + for i, c := range []struct { + before map[string]delivery.Want + out assign.Outcome + line string // the line said in this pass, "" for none + }{ + {nil, notDone, "rofi on shanks gets no want: rule 1: the build of rofi at 4e7b118e is not done"}, + {nil, notDone, ""}, + {map[string]delivery.Want{"shanks": {Removed: true}}, notDone, ""}, + {nil, assign.Outcome{Rules: map[string]assign.Holding{"shanks": {Rule: delivery.RuleGrant, Words: "no credential"}}}, "rofi on shanks gets no want: rule 6: no credential"}, + {nil, wanted, "rofi on shanks has a want"}, + {map[string]delivery.Want{"shanks": wanted.Wants["shanks"]}, assign.Outcome{}, ""}, + } { + logged = nil + p.sayNoWant("rofi", []string{"shanks"}, c.before, c.out) + if got := strings.Join(logged, "\n"); got != c.line { + t.Errorf("pass %d said %q, want %q", i, got, c.line) + } + } } // A neither or held condition a previous process raised is cleared by the first pass after a start: a fresh pass remembers diff --git a/modules/mesh-delivery-assigner/internal/assign/wants.go b/modules/mesh-delivery-assigner/internal/assign/wants.go index 8d0daff..e275c59 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants.go @@ -333,8 +333,18 @@ func Wants(m Module, n Nodes, wants map[string]delivery.Want, lastHealthy map[st // the pass says so. A new assignment, with no have, waits for the rules and is on no list yet by design. A held module (rule 2, // held before or by this pass) is on the held list instead, whose hold is said and answered on its own: it is never on neither // list (issue 1945: systemd-resolved, held on shanks by the operator, was said as urgent every hour). - if h, there := n.Haves[node][m.Name]; there && h.Phase != delivery.PhaseRemoved && !held { + h, there := n.Haves[node][m.Name] + switch { + case there && h.Phase != delivery.PhaseRemoved && !held: out.Neither[node] = Holding{Rule: rule, Words: words} + case had && (!there || h.Phase == delivery.PhaseRemoved): + // A removed want of a node that does not run the module says why no want follows it (issue 1945), so the view's + // assigner_want answers it; it stays removed. A node with no key gets none: a removed want written where none was + // would have its node agent remove the module and stop holding its composition's resources back. + next = delivery.Want{Removed: true, Words: NoWantWords(rule, words)} + if !delivery.SameWant(normal(before), normal(next)) { + out.Wants[node] = next + } } continue } @@ -370,6 +380,11 @@ func Wants(m Module, n Nodes, wants map[string]delivery.Want, lastHealthy map[st return out } +// NoWantWords say why a node gets no want for a module: the rule that holds its first want back and why (issue 1945). +func NoWantWords(rule int, words string) string { + return fmt.Sprintf("no want: rule %d: %s", rule, words) +} + // 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 diff --git a/modules/mesh-delivery-assigner/internal/assign/wants_test.go b/modules/mesh-delivery-assigner/internal/assign/wants_test.go index 69deb19..25ae9d1 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants_test.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants_test.go @@ -867,6 +867,33 @@ func TestANewAssignmentWaitingIsNotOnNeitherList(t *testing.T) { 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) } + // 1945 (the assigner holds a first want back silently): with no build, a node whose want was removed and that does not run + // the module has the reason written into its removed want, once; a node with no key gets no want written, and a node that + // runs the module is on neither list, its removed want left as it is. + gone := delivery.Want{Removed: true, Words: "screen-lock is no longer assigned to shanks"} + for _, c := range []struct { + name string + before map[string]delivery.Want + want map[string]delivery.Want + }{ + {"no key: nothing written", nil, map[string]delivery.Want{}}, + {"a removed want: it says why", map[string]delivery.Want{"shanks": gone}, + map[string]delivery.Want{"shanks": {Removed: true, Words: "no want: rule 1: the build of screen-lock at " + short(m.FP) + " is not done"}}}, + {"a removed want that says why already: nothing written", map[string]delivery.Want{"shanks": {Removed: true, Words: "no want: rule 1: the build of screen-lock at " + short(m.FP) + " is not done"}}, + map[string]delivery.Want{}}, + {"a node that runs it: on neither list, its removed want left", map[string]delivery.Want{"g14": gone}, map[string]delivery.Want{}}, + } { + out := Wants(m, n, c.before, nil) + if len(out.Wants) != len(c.want) { + t.Errorf("%s: wants = %+v, want %+v", c.name, out.Wants, c.want) + continue + } + for node, w := range c.want { + if !delivery.SameWant(normal(out.Wants[node]), w) { + t.Errorf("%s: want of %s = %+v, want %+v", c.name, node, out.Wants[node], w) + } + } + } 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)