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 fed2320..5a9402d 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go @@ -100,7 +100,7 @@ func (p *passer) wants(ctx context.Context) { neither := map[string]neitherOne{} // 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 + // assigned module not on neither list 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{} @@ -140,7 +140,7 @@ 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) + startClears(stale, m, out) if fp != "" && !m.Held && out.Held == nil && !out.Lifted { p.clearHeld(name, fp) } @@ -190,15 +190,13 @@ 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) { +// 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 +// for ever) or a move waiting for its record ask. Clearing a condition that is not open is quiet at the router. +func startClears(into map[string]neitherOne, m assign.Module, 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 { + if _, on := out.Neither[node]; !on { into[node+"."+m.Name] = neitherOne{node: node, module: m.Name} } } 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 d62c765..1af98fa 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 @@ -149,7 +149,7 @@ func TestThePassGathersTheModulesOnNeitherList(t *testing.T) { } // 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 +// nothing it said, so it clears, once, the neither condition of every assigned module not on neither list 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{}} @@ -185,3 +185,54 @@ func TestTheFirstPassAfterAStartClearsWhatAPreviousProcessRaised(t *testing.T) { t.Errorf("a second pass said again: %+v", said[n:]) } } + +// 1945/AC24: given a module held at F under rule 2 on a node that runs it with no want, when the assigner passes, then it raises no +// neither condition for it, a neither condition this process raised for it before is cleared, and so is one a previous process +// raised (the first pass after a start), each once. +func TestAHeldModuleRaisesNoNeitherAndClearsAStandingOne(t *testing.T) { + setup := func() (*passer, *[]asks.Condition) { + p := newPasser(buckets{delivery.AssignerWantsBucket: &memKV{values: map[string][]byte{}}, delivery.AssignerAsksBucket: &memKV{values: map[string][]byte{}}}, time.Now()) + 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/resolved", Commit: strings.Repeat("1", 40), Path: delivery.RootPath}) + p.w.put(delivery.ModulesBucket, "systemd-resolved", mod) + p.w.put(provider.AssignmentsBucket, "shanks.systemd-resolved", []byte(`{"node":"shanks","module":"systemd-resolved","build":"b-1"}`)) + on, _ := delivery.EncodePresence(delivery.Presence{Online: true}) + p.w.put(delivery.PresenceBucket, "shanks", on) + have, _ := delivery.EncodeHave(delivery.Have{FP: fpTest, Phase: delivery.PhaseRunning, Health: delivery.HealthFailed}) + p.w.put(delivery.HaveBucket, "shanks.systemd-resolved", have) + hkey, _ := delivery.HeldKey("systemd-resolved", fpTest) + held, _ := delivery.EncodeHeld(delivery.Held{Node: "shanks", Words: "health failed"}) + p.w.put(delivery.AssignerWantsBucket, hkey, held) + return p, said + } + key := assign.NeitherCondition("shanks", "systemd-resolved", assign.Holding{}, false).Key + neithers := func(said []asks.Condition) (raised, cleared int) { + for _, c := range said { + if c.Key != key { + continue + } + if c.State == asks.StateRaised { + raised++ + } else { + cleared++ + } + } + return + } + // a standing neither this process raised + p, said := setup() + p.neither = map[string]neitherOne{"shanks.systemd-resolved": {node: "shanks", module: "systemd-resolved", holding: assign.Holding{Rule: delivery.RuleNotHeld}}} + p.wants(context.Background()) + p.wants(context.Background()) + if r, c := neithers(*said); r != 0 || c != 1 { + t.Errorf("standing in this process: %d raised, %d cleared, want 0 and 1: %+v", r, c, *said) + } + // one a previous process raised: the first pass after a start clears it + p, said = setup() + p.wants(context.Background()) + p.wants(context.Background()) + if r, c := neithers(*said); r != 0 || c != 1 { + t.Errorf("after a start: %d raised, %d cleared, want 0 and 1: %+v", r, c, *said) + } +} diff --git a/modules/mesh-delivery-assigner/internal/assign/wants.go b/modules/mesh-delivery-assigner/internal/assign/wants.go index 273faf9..f49a6ee 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants.go @@ -166,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 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 is, by node, the nodes the module is assigned to that run it (or whose have names it), hold no want for it after this + // pass and are not held (a held module is on the held list), 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 @@ -297,8 +297,10 @@ func Wants(m Module, n Nodes, wants map[string]delivery.Want, lastHealthy map[st out.Rules[node] = Holding{Rule: rule, Words: words} if !had || before.Removed { // 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 { + // 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 { out.Neither[node] = Holding{Rule: rule, Words: words} } continue diff --git a/modules/mesh-delivery-assigner/internal/assign/wants_test.go b/modules/mesh-delivery-assigner/internal/assign/wants_test.go index 031121e..9372367 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants_test.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants_test.go @@ -837,3 +837,30 @@ func TestAFirstWantAfterARemovedWantGoesThroughTheRecordAsk(t *testing.T) { t.Errorf("the record asks = %+v, want one for shanks at the composition it runs", out.Record) } } + +// 1945/AC23: given a module held at F under rule 2 (the operator holds systemd-resolved on shanks, 2026-10-25), or held by the +// assigner in this pass after a failed health check of F, when a node that runs it holds no want for it, then the node is not on +// neither list: a held module is on the held list, whose hold is said and answered on its own. +func TestAHeldModuleWithNoWantIsNotOnNeitherList(t *testing.T) { + for what, change := range map[string]func(*Module, *Nodes){ + "held at F": func(m *Module, _ *Nodes) { m.Held, m.Kept = true, true }, + "held at F, running an old-form build": func(m *Module, n *Nodes) { + m.Held, m.Kept = true, true + n.Haves["shanks"]["screen-lock"] = delivery.Have{FP: "build-2", Phase: delivery.PhaseRunning, Health: delivery.HealthUnknown} + }, + "held by this pass": func(_ *Module, n *Nodes) { + n.Haves["g14"]["screen-lock"] = delivery.Have{FP: fNew, Phase: delivery.PhaseRunning, Health: delivery.HealthFailed} + n.Haves["shanks"]["screen-lock"] = delivery.Have{FP: "build-2", Phase: delivery.PhaseRunning, Health: delivery.HealthUnknown} + }, + } { + m, n := x11() + change(&m, &n) + out := Wants(m, n, nil, nil) + if len(out.Neither) != 0 { + t.Errorf("%s: a held module is on neither list: %+v", what, out.Neither) + } + if out.Rules["shanks"].Rule != delivery.RuleNotHeld { + t.Errorf("%s: shanks is held back by %+v, want rule 2", what, out.Rules["shanks"]) + } + } +}