From 2d63f0ba62d483ed604d16abcfb6356757ab3440 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 26 Oct 2026 13:55:42 +0100 Subject: [PATCH] Assigner: say the bus bar instead of skipping it silently, and clear a neither condition only in true words (issue 1945) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A bus whose move only the operator's put may make (ADR 0546 §8) got no want and nothing in the outcome: nats on novox, running an old-form build, was on no list while the pass cleared its condition and logged that it had a want again. The bar now names its holding, puts a node without a live want on neither list (its condition needs the operator: assigner_put), and a neither condition is cleared as 'has a want again' only when a live want exists. --- .../cmd/mesh-delivery-assigner/pass.go | 59 ++++++++++++++++--- .../cmd/mesh-delivery-assigner/pass_test.go | 28 +++++++-- .../internal/assign/asks.go | 19 ++++++ .../internal/assign/wants.go | 44 +++++++++++--- .../internal/assign/wants_test.go | 17 ++++++ 5 files changed, 147 insertions(+), 20 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 4e42669..f609b52 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go @@ -3,7 +3,6 @@ package main import ( "context" "errors" - "fmt" "slices" "strings" "time" @@ -114,7 +113,8 @@ func (p *passer) wants(ctx context.Context) { // quiet at the router. first := p.neither == nil stale := map[string]neitherOne{} - defer func() { p.sayNeither(neither, stale) }() + left := map[string]string{} + defer func() { p.sayNeither(neither, stale, left) }() for name := range names { fp := trunk[name].FP2 name := name @@ -154,6 +154,7 @@ func (p *passer) wants(ctx context.Context) { } out := assign.Wants(m, nodes, wanted[name], last) gatherNeither(neither, name, out) + leftNeither(left, m, wanted[name], out) p.sayNoWant(name, m.Nodes, wanted[name], out) if first { startClears(stale, m, out) @@ -210,7 +211,7 @@ func (p *passer) sayNoWant(module string, nodes []string, before map[string]deli } reason := "" if h, held := out.Rules[node]; held && (!written || w.Removed) { - reason = fmt.Sprintf("rule %d: %s", h.Rule, h.Words) + reason = h.Reason() } last, said := p.noWant[key] switch { @@ -242,6 +243,38 @@ func gatherNeither(into map[string]neitherOne, module string, out assign.Outcome } } +// leftNeither says, by node and module, for every node of m not on neither list after the pass, why: "" when the node has a live +// want for it (written before and kept, or written in this pass), else why it has none. sayNeither clears a neither condition in +// these words, so a clear never says a want the node does not hold (issue 1945: "nats on novox has a want again" while the bus bar +// left it with none). +func leftNeither(into map[string]string, m assign.Module, before map[string]delivery.Want, out assign.Outcome) { + for _, node := range m.Nodes { + if _, on := out.Neither[node]; on { + continue + } + w, written := out.Wants[node] + if !written { + w, written = before[node] + } + why := "" + h, ruled := out.Rules[node] + switch { + case written && !w.Removed: + case (m.Held && !out.Lifted) || out.Held != nil: + why = "it is held, and its held condition says so" + case slices.ContainsFunc(out.Record, func(r assign.RecordNeed) bool { return r.Node == node }): + why = "its move waits for the operator's yes on its record ask" + case m.FP == "": + why = "the trunk has no build of it" + case ruled: + why = h.Reason() + default: + why = "the node does not run it" + } + into[node+"."+m.Name] = why + } +} + // startClears adds, for the first pass after a start, every node of m that is not on neither list after the pass: a neither // condition a previous process raised for it is cleared. That covers a node with a want and one with none that is not on neither // list, such as a held module (issue 1945: the hold is said on its own, so a neither raised for it before the restart would stay up @@ -258,7 +291,9 @@ func startClears(into map[string]neitherOne, m assign.Module, out assign.Outcome // 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. // 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) { +// left is, by node and module, why each assigned node not on neither list after this pass has no want ("" when it has one); a key +// it does not hold is no longer assigned. +func (p *passer) sayNeither(now, stale map[string]neitherOne, left map[string]string) { if p.neither == nil { p.neither = map[string]neitherOne{} for key, one := range stale { @@ -278,21 +313,29 @@ func (p *passer) sayNeither(now, stale map[string]neitherOne) { 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.logf("%s is assigned to %s and on neither list: no want, and not in the node's own composition; what holds its first want back: %s", + one.module, one.node, one.holding.Reason()) p.neither[key] = one } for key, one := range p.neither { if _, still := now[key]; still { continue } + why, assigned := left[key] + if !assigned { + why = "it is no longer assigned there" + } if p.say != nil { - if err := p.say(assign.NeitherCondition(one.node, one.module, assign.Holding{}, false)); err != nil { + if err := p.say(assign.NeitherLeftCondition(one.node, one.module, why)); 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) + if why == "" { + p.logf("%s on %s has a want again", one.module, one.node) + } else { + p.logf("%s on %s is no longer on neither list and has no want: %s", one.module, one.node, why) + } delete(p.neither, key) } } 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 2a13988..5a59c5c 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 @@ -357,20 +357,37 @@ 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, nil) // the say fails - p.sayNeither(one, nil) - p.sayNeither(one, nil) + p.sayNeither(one, nil, nil) // the say fails + p.sayNeither(one, nil, nil) + p.sayNeither(one, nil, 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{}, nil) - p.sayNeither(map[string]neitherOne{}, nil) + wants := map[string]string{"g14.screen-lock": ""} + p.sayNeither(map[string]neitherOne{}, nil, wants) + p.sayNeither(map[string]neitherOne{}, nil, wants) 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) } + if last := logged[len(logged)-1]; last != "screen-lock on g14 has a want again" { + t.Errorf("the clear said %q", last) + } + // 1945 (the bus bar): a module that leaves neither list with no want is cleared in words saying so, never "has a want again". + bus := map[string]neitherOne{"novox.nats": {node: "novox", module: "nats", holding: assign.Holding{Bus: true, Words: "the bus moves only by the operator's put"}}} + p.sayNeither(bus, nil, nil) + if c := said[len(said)-1]; c.State != asks.StateRaised || !strings.HasPrefix(c.Explanation, asks.NeedsYou) || !strings.Contains(c.Explanation, "the bus moves only by the operator's put") { + t.Errorf("the bus's neither condition = %+v", c) + } + p.sayNeither(map[string]neitherOne{}, nil, map[string]string{"novox.nats": "it is held, and its held condition says so"}) + if c := said[len(said)-1]; c.State != asks.StateCleared || strings.Contains(c.Resolved, "has a want") { + t.Errorf("a module that left neither list with no want was cleared as %+v", c) + } + if last := logged[len(logged)-1]; last != "nats on novox is no longer on neither list and has no want: it is held, and its held condition says so" { + t.Errorf("the clear without a want said %q", last) + } } // 1945/AC21: the pass gathers the modules on neither list of every module's outcome, keyed by node and module. @@ -399,6 +416,7 @@ func TestThePassGathersTheModulesOnNeitherList(t *testing.T) { {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{}, ""}, + {nil, assign.Outcome{Rules: map[string]assign.Holding{"shanks": {Bus: true, Words: "the bus moves only by the operator's put"}}}, "rofi on shanks gets no want: the bus moves only by the operator's put"}, } { logged = nil p.sayNoWant("rofi", []string{"shanks"}, c.before, c.out) diff --git a/modules/mesh-delivery-assigner/internal/assign/asks.go b/modules/mesh-delivery-assigner/internal/assign/asks.go index 87c0b0f..1c7825a 100644 --- a/modules/mesh-delivery-assigner/internal/assign/asks.go +++ b/modules/mesh-delivery-assigner/internal/assign/asks.go @@ -96,6 +96,14 @@ func NeitherCondition(node, module string, h Holding, raised bool) asks.Conditio if n := len([]rune(headline)); n > asks.HeadlineLength { headline = string([]rune(headline)[:asks.HeadlineLength]) } + if h.Bus { + needs := fmt.Sprintf("put %s on %s with assigner_put", module, node) + return asks.Condition{Key: key, Severity: asks.SeverityUrgent, Subject: subject, State: asks.StateRaised, + Summary: headline, Headline: headline, Needs: needs, + Explanation: fmt.Sprintf("%s%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 never moves the bus: %s. This clears once the node has a want for it.", + asks.NeedsYou, needs, module, node, h.Words)} + } 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 "+ @@ -103,6 +111,17 @@ func NeitherCondition(node, module string, h Holding, raised bool) asks.Conditio "rule holds, and clears this.", asks.NothingToDo, module, node, h.Rule, h.Words)} } +// NeitherLeftCondition clears the neither condition of module on node when it is no longer on neither list: why is "" when the +// node has a live want for it again, else why it has none (it is held, its move waits for an ask, the node no longer runs it), so +// the clear never says a want the node does not hold (issue 1945: "nats on novox has a want again" while it had none). +func NeitherLeftCondition(node, module, why string) asks.Condition { + c := NeitherCondition(node, module, Holding{}, false) + if why != "" { + c.Resolved = fmt.Sprintf("%s on %s is no longer on neither list and has no want: %s.", module, node, why) + } + return c +} + // 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/wants.go b/modules/mesh-delivery-assigner/internal/assign/wants.go index e275c59..452d958 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants.go @@ -192,10 +192,27 @@ type RecordNeed struct { Words string } -// Holding is a rule holding a node back, and why. +// Holding is a rule holding a node back, and why. Bus marks the bar on the bus instead of one of the seven rules (Rule is then +// 0): the bus moves only by the operator's put (ADR 0539 §15, ADR 0546 §8), never by the assigner (issue 1945: nats on novox, +// running an old-form build, got no want and the pass said nothing). type Holding struct { Rule int Words string + Bus bool +} + +// Reason is the holding in words: the rule and its words, or for the bus bar its words alone. +func (h Holding) Reason() string { + if h.Bus { + return h.Words + } + return fmt.Sprintf("rule %d: %s", h.Rule, h.Words) +} + +// BusBarWords say why the assigner writes no want moving the bus on node: it runs prev, the trunk builds fp, and only the +// operator's put moves it. +func BusBarWords(node, prev, fp string) string { + return fmt.Sprintf("the bus moves only by the operator's put (ADR 0546 §8): %s runs %s, the trunk builds %s; ask assigner_put", node, short(prev), short(fp)) } // Wants is the pass of one module m: for each of its nodes, the want by the six rules, starting from the want written @@ -303,6 +320,14 @@ func Wants(m Module, n Nodes, wants map[string]delivery.Want, lastHealthy map[st if need, wait := at.mustAsk(node, asked, hadLive, ran, running.FP, running.Composed == "", d); wait { if need != nil { out.Record = append(out.Record, *need) + } else if prev, barred := at.busBar(asked, hadLive, running.FP); barred { + // The bus is barred: no want moves it but the operator's put. Said, never silent (issue 1945): its holding is named, + // and a node that runs it with no live want has it on neither list, so its condition stays raised. + h := Holding{Bus: true, Words: BusBarWords(node, prev, at.FP)} + out.Rules[node] = h + if !hadLive && running.Phase != delivery.PhaseRemoved && !held { + out.Neither[node] = h + } } continue } @@ -466,12 +491,7 @@ func (m Module) mustAsk(node string, before delivery.Want, had, ran bool, runnin if !had && ran && running == m.FP && haveNamesNoComposition && d.Judgement == delivery.JudgedUnchanged { return nil, false } - // The fingerprint the bus ran before: its want's, or with no readable want the one its have names (ADR 0539 §15 bars a new one). - prev := before.FP - if !had { - prev = running - } - if catalogue.ProvidesBus(m.Manifest) && prev != m.FP { + if _, barred := m.busBar(before, had, running); barred { return nil, true // the bus is never moved by the assigner; the operator writes its want (§15) } if !m.recorded(d) || d.Judgement == delivery.JudgedNeither || d.Judgement == delivery.JudgedUnchanged { @@ -483,6 +503,16 @@ func (m Module) mustAsk(node string, before delivery.Want, had, ran bool, runnin return &RecordNeed{Node: node, Digest: d.Digest, Judgement: d.Judgement, Words: d.Words}, true } +// busBar says whether m is the bus and the move would take it off the fingerprint it ran before, prev: its want's, or with no +// readable want the one its have names (ADR 0539 §15 bars a new one). +func (m Module) busBar(before delivery.Want, had bool, running string) (prev string, barred bool) { + prev = before.FP + if !had { + prev = running + } + return prev, catalogue.ProvidesBus(m.Manifest) && prev != m.FP +} + // recorded says whether the module is of policy record: as the controller holds it beside the composition's digest, the // operator's own choice included (ADR 0539 §14); on a composition made by hand, which carries none, as the manifest derives it. func (m Module) recorded(d delivery.ComposedDigest) bool { diff --git a/modules/mesh-delivery-assigner/internal/assign/wants_test.go b/modules/mesh-delivery-assigner/internal/assign/wants_test.go index 25ae9d1..dc18ce0 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants_test.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants_test.go @@ -427,6 +427,23 @@ func TestTheBusIsNotMovedButItsUserListRevisionIsWrittenAtOnce(t *testing.T) { t.Errorf("a held bus did not take its user-list revision as a put: %+v", w) } m.Puts = nil + // 1945 (the bus bar made loud): a node that runs the bus at an old-form build and holds no want for it gets none, since only + // the operator's put moves the bus, and the pass says so: a holding naming the bar, and the node on neither list so its + // condition stays raised. A node with a live want keeps it and is not on neither list. + m.Composed = composedEverywhere + n.Haves["ace"]["mail"] = delivery.Have{FP: "b-1700000000-oldform", Phase: delivery.PhaseRunning, Health: delivery.HealthOK} + out = Wants(m, n, map[string]delivery.Want{"g14": {FP: fOld, Composed: dig(fOld)}}, nil) + if w, wrote := out.Wants["ace"]; wrote { + t.Errorf("the bus bar wrote ace's want %+v", w) + } + h, on := out.Neither["ace"] + if !on || !h.Bus || out.Rules["ace"] != h || !strings.Contains(h.Words, "only by the operator's put") || !strings.Contains(h.Words, "ace runs b-1700000000-oldform") || + !strings.Contains(h.Words, "assigner_put") { + t.Errorf("the barred bus on ace: neither %+v (%v), rules %+v", h, on, out.Rules["ace"]) + } + if _, on := out.Neither["g14"]; on || out.Wants["g14"].FP == fNew { + t.Errorf("g14 has a live want: neither %+v, its want %+v", out.Neither, out.Wants["g14"]) + } stepsOfAPut(t) }