From d54eab4052d132a120b83356092f1d5e7db738ca Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 29 Oct 2026 15:57:27 +0100 Subject: [PATCH] Never keep a want back for an unhealthy provider, nor a first want for the canary where nothing runs: the module is wanted and its provider down is said (issue 1960) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On novox on 2026-10-29 the assigner wrote no want for de-spiegel and link2pay (rule 5: no provider of route runs healthy) nor photos (rule 5: no provider of s3-bucket), and none where no node of a module was online (rule 3). A module with no want is on no list and nothing applies it; a site runs while the proxy in front of it is down. - Quick fix: skip rules 3 and 5 for the three modules' first wants. Leaves rule 5 holding every move and the canary holding new assignments. - Thorough fix, chosen: rule 5 holds no want; it is said per node (Outcome.Unreached, journaled once per words by the pass). The canary (rule 3) no longer holds the first want of a node that does not run the module. A node that runs it keeps the canary order: at an older composed and granted fingerprint of the new form its first want stays at what it runs; at an old-form build it waits for the canary as before (issue 1945); at an older fingerprint with no composition or grant it gets F. This departs from ADR 0429 §6, which makes every provider healthy a rule of every want. Tests: rows changed in the existing rule tests, and rows for today's modules (route-proxy down: de-spiegel and link2pay wanted; minio not healthy: photos moves; novox offline: a first want written, a move kept back). --- .../cmd/mesh-delivery-assigner/pass.go | 24 ++++ .../internal/assign/wants.go | 73 +++++++----- .../internal/assign/wants_test.go | 107 ++++++++++++++---- 3 files changed, 151 insertions(+), 53 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 fbcb74f..ad767b0 100644 --- a/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go +++ b/modules/mesh-delivery-assigner/cmd/mesh-delivery-assigner/pass.go @@ -40,6 +40,9 @@ type passer struct { // 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 + // unreached is, by node and module, the words last said why a wanted module may not be reached there (issue 1960: its + // provider is down, which holds no want): said once per words, and once more when it is reached again. + unreached map[string]string // nodeFailed is, by node and module, the words of the condition last raised for a failure that is the node's own (ADR 0557 // §1, issue 1496): raised once per words, cleared once when it ends. nodeFailed map[string]string @@ -196,6 +199,7 @@ func (p *passer) wants(ctx context.Context) { gatherNeither(neither, name, out) leftNeither(left, m, wanted[name], out) p.sayNoWant(name, m.Nodes, wanted[name], out) + p.sayUnreached(name, m.Nodes, out) p.sayNodeFailed(name, fp, m.Nodes, nodes, out, first) if first { startClears(stale, m, out) @@ -274,6 +278,26 @@ func (p *passer) sayNoWant(module string, nodes []string, before map[string]deli } } +// sayUnreached says in the journal, once per words, that a module is wanted on a node and may not be reached there, since +// no provider of what it requires runs healthy (rule 5, which holds no want: issue 1960), and once when it is reached again. +func (p *passer) sayUnreached(module string, nodes []string, out assign.Outcome) { + if p.unreached == nil { + p.unreached = map[string]string{} + } + for _, node := range nodes { + key := node + "." + module + why, last := out.Unreached[node], p.unreached[key] + switch { + case why != "" && why != last: + p.logf("%s on %s is wanted and may not be reached: %s", module, node, why) + p.unreached[key] = why + case why == "" && last != "": + p.logf("%s on %s is reached: its providers run healthy", module, node) + delete(p.unreached, key) + } + } +} + // sayNodeFailed raises, once per node and words, the condition of a failed health check of a module's newest fingerprint fp // that is the node's own, since fp runs healthy elsewhere and is not held (ADR 0557 §1, issue 1496), and says it in the journal; // it clears the condition once when the node no longer fails alone, or is no longer one of the module's nodes. The first pass diff --git a/modules/mesh-delivery-assigner/internal/assign/wants.go b/modules/mesh-delivery-assigner/internal/assign/wants.go index 3923a4f..6d54820 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants.go @@ -89,14 +89,15 @@ func Canary(nodes []string, online map[string]bool) string { // Holds is whether every rule holds for moving node's want for m to m.FP, and if not, the first rule that does not, // 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). +// limit (ADR 0528 clause 3). Rule 5, every provider healthy, holds no want (issue 1960): a module whose provider is down +// is wanted all the same, and said as not reached (Unreached), since a site runs while the proxy in front of it is +// down and one module never blocks another. 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. +// holds is Holds; with ordering false it leaves out the canary (3), which orders a move among nodes, 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: @@ -112,11 +113,6 @@ func holds(m Module, node string, n Nodes, ordering bool) (bool, int, string) { 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 { return false, delivery.RuleGrant, fmt.Sprintf("the controller has not issued the credential of %s for %s", m.Name, short(m.FP)) @@ -157,6 +153,17 @@ func providedHealthy(provision, node string, n Nodes) (bool, string) { return false, fmt.Sprintf("no provider of %s runs healthy on %s or at its canary", provision, node) } +// unreached is why m on node may be wanted and not reached: the first provision it requires that no provider runs +// healthy on node or at its canary (rule 5, said and never holding a want; issue 1960); empty when all do. +func unreached(m Module, node string, n Nodes) string { + for _, provision := range m.Manifest.Requires { + if ok, why := providedHealthy(provision, node, n); !ok { + return why + } + } + return "" +} + // keepsData says whether a module holds data a set-back would lose: an own item of a class that keeps (irreplaceable, // valuable or rebuildable; cache and none do not). func keepsData(m catalogue.Manifest) bool { @@ -186,6 +193,9 @@ type Outcome struct { Restore []string // Rules is, by node, the rule holding the node back from m.FP and why. Rules map[string]Holding + // Unreached is, by node, why the module may be wanted there and not reached: no provider of a provision it requires runs + // healthy (rule 5, issue 1960). It holds no want back; the pass says it. + Unreached map[string]string // 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 @@ -301,26 +311,33 @@ func Wants(m Module, n Nodes, wants map[string]delivery.Want, lastHealthy map[st } ok, rule, words := Holds(mm, node, n) target := m.FP - if have, ran := n.Haves[node][m.Name]; !ok && (!had || before.Removed) && (rule == delivery.RuleCanary || rule == delivery.RuleProviders) && runs(have) { - // 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 names - // what it runs, never a newer fingerprint, so the canary is never passed over: F when it runs F (by the rules without which - // there is nothing to want), or an older fingerprint of the new form whose composition and grant exist. Rules 3 and 5 then - // order the move from it as from any want. - switch _, composed := m.composition(node, have.FP); { - case !ran: - case have.FP == m.FP: - ok, rule, words = holds(mm, node, n, false) - case !strings.HasPrefix(have.FP, delivery.SourcePrefix2): - // An old-form build (not src2:) has no composition, so no want can ever name it: its first want names F, and only by every - // rule as usual, the canary (3) and the providers (5) included, which here hold it back. Nothing is written and the module - // stays on neither list until the canary runs F healthy; then Holds passes and the want names F (issue 1945, a design - // limit set by the lead: the canary stays in force and old-form modules leave neither list by themselves). - case composed && m.GrantedAt != nil && m.GrantedAt(have.FP): - ok, target = true, have.FP + if have, ran := n.Haves[node][m.Name]; !ok && (!had || before.Removed) && rule == delivery.RuleCanary { + // The canary orders a move, never the first want of a node that does not run the module (issue 1960: de-spiegel, + // link2pay and photos on novox were left with no want). A node that runs an older fingerprint of the new form, + // composed and granted, gets its first want at what it runs, and rule 3 then orders its move to F as from any want. + // A node that runs an old-form build, which no want can name, stays under the canary (issue 1945, a design limit): + // its first want names F once the canary runs F healthy. Any other node that runs it (F itself, or an older + // fingerprint with no composition or grant) gets F by the rules without which there is nothing to want. + older := ran && runs(have) && have.FP != m.FP && strings.HasPrefix(have.FP, delivery.SourcePrefix2) + if older { + _, composed := m.composition(node, have.FP) + older = composed && m.GrantedAt != nil && m.GrantedAt(have.FP) } + oldForm := ran && runs(have) && have.FP != m.FP && !strings.HasPrefix(have.FP, delivery.SourcePrefix2) + switch { + case older: + ok, target = true, have.FP + case oldForm: + // held back by rule 3 as written: on neither list until the canary runs F healthy + default: + ok, rule, words = holds(mm, node, n, false) + } + } + if why := unreached(m, node, n); why != "" { + if out.Unreached == nil { + out.Unreached = map[string]string{} + } + out.Unreached[node] = why } if ok && target == m.FP && len(out.NodeFailed) > 0 && !(had && !before.Removed && before.FP == m.FP) && n.Haves[node][m.Name].FP != m.FP { diff --git a/modules/mesh-delivery-assigner/internal/assign/wants_test.go b/modules/mesh-delivery-assigner/internal/assign/wants_test.go index 56fdc87..8d75165 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants_test.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants_test.go @@ -65,9 +65,6 @@ func TestEachRuleHoldsTheWantBack(t *testing.T) { delivery.RuleCanary: func(_ *Module, n *Nodes) { n.Haves["ace"]["mail"] = delivery.Have{FP: fOld, Phase: delivery.PhaseRunning, Health: delivery.HealthOK} }, - delivery.RuleProviders: func(_ *Module, n *Nodes) { - n.Haves["ace"]["store"] = delivery.Have{FP: fpOf('s'), Phase: delivery.PhaseRunning, Health: delivery.HealthWaits} - }, delivery.RuleGrant: func(m *Module, _ *Nodes) { m.Granted = false }, delivery.RuleComposed: func(m *Module, _ *Nodes) { m.Composed = func(string, string) (delivery.ComposedDigest, bool) { return delivery.ComposedDigest{}, false } @@ -86,6 +83,50 @@ func TestEachRuleHoldsTheWantBack(t *testing.T) { t.Errorf("rule %d broken: g14's want is %+v", rule, w) } } + // 1960: rule 5 holds no want, and no module is left without a first want for a provider down or a node offline; the + // provider down is said (Unreached). novox on 2026-10-29: route-proxy down held de-spiegel and link2pay, minio not + // healthy held photos, and novox offline left a module with no canary. + for _, c := range []struct { + name, module, provision, provider string + have *delivery.Have // what novox runs of the module; nil: a new assignment + before map[string]delivery.Want + online bool + want string + }{ + {"de-spiegel, route-proxy down, old form", "de-spiegel", "route", "route-proxy", + &delivery.Have{FP: "build-1", Phase: delivery.PhaseRunning, Health: delivery.HealthUnknown}, nil, true, fNew}, + {"link2pay, route-proxy down, new assignment", "link2pay", "route", "route-proxy", nil, nil, true, fNew}, + {"photos, minio not healthy, moving its want", "photos", "s3-bucket", "minio", + &delivery.Have{FP: fOld, Phase: delivery.PhaseRunning, Health: delivery.HealthOK}, + map[string]delivery.Want{"novox": {FP: fOld, Composed: dig(fOld)}}, true, fNew}, + {"novox offline, no canary, new assignment", "link2pay", "route", "route-proxy", nil, nil, false, fNew}, + {"novox offline, no canary, moving its want", "photos", "s3-bucket", "minio", + &delivery.Have{FP: fOld, Phase: delivery.PhaseRunning, Health: delivery.HealthOK}, + map[string]delivery.Want{"novox": {FP: fOld, Composed: dig(fOld)}}, false, fOld}, + } { + m := Module{Name: c.module, FP: fNew, Built: true, Granted: true, Composed: composedEverywhere, Nodes: []string{"novox"}, + Manifest: catalogue.Manifest{Module: c.module, Requires: []string{c.provision}}} + n := Nodes{Online: map[string]bool{"novox": c.online}, Haves: map[string]map[string]delivery.Have{"novox": { + c.provider: {FP: fpOf('p'), Phase: delivery.PhaseFailed, Health: delivery.HealthFailed}}}, + Providers: map[string]map[string][]string{c.provision: {c.provider: {"novox"}}}} + if c.have != nil { + n.Haves["novox"][c.module] = *c.have + } + out := Wants(m, n, c.before, nil) + w, written := out.Wants["novox"] + if !written { + w = c.before["novox"] + } + if w.FP != c.want || w.Removed { + t.Errorf("%s: novox's want = %+v, want %s", c.name, w, short(c.want)) + } + if _, on := out.Neither["novox"]; on { + t.Errorf("%s: on neither list: %+v", c.name, out.Neither) + } + if c.online && !strings.Contains(out.Unreached["novox"], c.provision) { + t.Errorf("%s: the provider down is not said: %q", c.name, out.Unreached["novox"]) + } + } // 1126/AC-SW1, AC-SW2, AC-SW3: a want written Self while g14 was placed, kept by a rule after the placement is gone, names // the controller's composition of the fingerprint the rule keeps (on 2026-10-27 g14's systemd-resolved, held, and forticlient, @@ -108,7 +149,7 @@ func TestEachRuleHoldsTheWantBack(t *testing.T) { {name: "rule 5 at F", before: fNew, set: func(m *Module, n *Nodes) { atF(m, n) n.Haves["ace"]["store"] = delivery.Have{FP: fpOf('s'), Phase: delivery.PhaseRunning, Health: delivery.HealthWaits} - }, want: delivery.Want{FP: fNew, Composed: dig(fNew)}, rule: delivery.RuleProviders}, + }, want: delivery.Want{FP: fNew, Composed: dig(fNew)}}, {name: "rule 3 at F", before: fNew, set: func(m *Module, n *Nodes) { atF(m, n) n.Haves["ace"]["mail"] = delivery.Have{FP: fOld, Phase: delivery.PhaseRunning, Health: delivery.HealthOK} @@ -962,10 +1003,12 @@ func x11() (Module, Nodes) { func TestANodeRunningAModuleWithNoWantGetsItsFirstWantAtWhatItRuns(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 != fOld || w.Composed != dig(fOld) || w.Newest != fNew || w.Rule == 0 { - t.Errorf("%s's want = %+v, want its first want at the fingerprint it runs, held back from F", node, w) - } + // 1960: the providers rule holds no want: the canary g14 gets F, its provider's health unknown said + if w := out.Wants["g14"]; w.FP != fNew || out.Unreached["g14"] == "" { + t.Errorf("g14's want = %+v (unreached %q), want F, the provider said", w, out.Unreached["g14"]) + } + if w := out.Wants["shanks"]; w.FP != fOld || w.Composed != dig(fOld) || w.Newest != fNew || w.Rule != delivery.RuleCanary { + t.Errorf("shanks's want = %+v, want its first want at the fingerprint it runs, held back from F by the canary", w) } if len(out.Neither) != 0 { t.Errorf("left on neither list: %+v", out.Neither) @@ -1003,11 +1046,22 @@ func TestNoFirstWantWithoutTheCompositionAndGrantOfWhatItRuns(t *testing.T) { m, n := x11() change(&m, &n) out := Wants(m, n, nil, nil) - if w, ok := out.Wants["shanks"]; ok { - t.Errorf("%s: shanks got a want: %+v", what, w) + if what == "an old-form build" { + // the canary g14 does not run F healthy: no want, on neither list by rule 3 (issue 1945) + if w, ok := out.Wants["shanks"]; ok { + t.Errorf("%s: shanks got a want: %+v", what, w) + } + if h, ok := out.Neither["shanks"]; !ok || h.Rule != delivery.RuleCanary { + t.Errorf("%s: shanks on neither list = %+v (named %v), want rule 3", what, h, ok) + } + continue } - if h, ok := out.Neither["shanks"]; !ok || h.Rule == 0 { - t.Errorf("%s: shanks on neither list = %+v (named %v)", what, h, ok) + // 1960: no want can name what it runs, so its first want names F rather than none: never on neither list + if w := out.Wants["shanks"]; w.FP != fNew { + t.Errorf("%s: shanks's want = %+v, want F", what, w) + } + if h, ok := out.Neither["shanks"]; ok { + t.Errorf("%s: shanks on neither list = %+v", what, h) } } m, n := x11() @@ -1016,10 +1070,12 @@ func TestNoFirstWantWithoutTheCompositionAndGrantOfWhatItRuns(t *testing.T) { if len(out.Neither) != 0 { t.Errorf("nodes holding a want named on neither list: %+v", out.Neither) } - for node, w := range out.Wants { - if w.FP != fOld { - t.Errorf("%s's want moved from its want past the ordering rules: %+v", node, w) - } + // the canary g14 moves to F (its provider's health no rule); shanks waits for it + if w := out.Wants["shanks"]; w.FP != "" && w.FP != fOld { + t.Errorf("shanks's want moved from its want past the canary: %+v", w) + } + if w := out.Wants["g14"]; w.FP != fNew { + t.Errorf("the canary g14's want = %+v, want F", w) } } @@ -1030,11 +1086,12 @@ func TestANewAssignmentWaitingIsNotOnNeitherList(t *testing.T) { m, n := x11() delete(n.Haves["shanks"], "screen-lock") out := Wants(m, n, nil, nil) - if w, ok := out.Wants["shanks"]; ok { - t.Errorf("a new assignment waiting for the canary got a want: %+v", w) + // 1960: the canary orders moves, never a first want: the new assignment is wanted at F + if w := out.Wants["shanks"]; w.FP != fNew { + t.Errorf("a new assignment while the canary does not run F: %+v, want F", w) } if _, ok := out.Neither["shanks"]; ok { - t.Errorf("a new assignment waiting for the canary is named on neither list: %+v", out.Neither) + t.Errorf("a new assignment is named on neither list: %+v", out.Neither) } m.Built = false if out := Wants(m, n, nil, nil); len(out.Neither) != 1 || out.Neither["g14"].Rule != delivery.RuleBuildDone { @@ -1076,7 +1133,7 @@ func TestANewAssignmentWaitingIsNotOnNeitherList(t *testing.T) { // 1945/AC21: a node whose have names an old-form build (no fingerprint of the new form, so no composition of it) can never be // named in a want at what it runs; its first want names F, under the canary rule as usual and with everything F needs and the // providers rule. Given the canary runs F healthy, shanks's first want names F; given it does not yet, shanks gets no want and -// stays on neither list. The old-form node that is itself the canary gets F once its providers run healthy. +// stays on neither list; the providers hold no want (issue 1960). The old-form node that is itself the canary gets F once its providers run healthy. func TestAnOldFormNodeGetsItsFirstWantAtFOnlyUnderTheCanary(t *testing.T) { oldForm := func(n Nodes, node string) { n.Haves[node]["screen-lock"] = delivery.Have{FP: "build-1792260876956849831", Phase: delivery.PhaseRunning, Health: delivery.HealthUnknown} @@ -1110,16 +1167,16 @@ func TestAnOldFormNodeGetsItsFirstWantAtFOnlyUnderTheCanary(t *testing.T) { if h, ok := out.Neither["shanks"]; !ok || h.Rule != delivery.RuleCanary { t.Errorf("canary not healthy on F: shanks on neither list = %+v (named %v), want rule 3", h, ok) } - // the canary healthy on F but the providers not healthy on shanks: rule 5 still holds it back + // 1960: the canary healthy on F but the providers not healthy on shanks: F all the same, the provider down said m, n = x11() oldForm(n, "shanks") n.Haves["g14"]["screen-lock"] = delivery.Have{FP: fNew, Phase: delivery.PhaseRunning, Health: delivery.HealthOK} out = Wants(m, n, nil, nil) - if w, ok := out.Wants["shanks"]; ok { - t.Errorf("providers not healthy: shanks got a want: %+v", w) + if w := out.Wants["shanks"]; w.FP != fNew || out.Unreached["shanks"] == "" { + t.Errorf("providers not healthy: shanks's want = %+v (unreached %q), want F, the provider said", w, out.Unreached["shanks"]) } - if h := out.Neither["shanks"]; h.Rule != delivery.RuleProviders { - t.Errorf("providers not healthy: shanks on neither list = %+v, want rule 5", h) + if _, on := out.Neither["shanks"]; on { + t.Errorf("providers not healthy: shanks on neither list: %+v", out.Neither) } // F not granted: nothing, even with the canary healthy m, n = x11() -- 2.54.0