From d17049125655568a90396626c682df23f25e58bd Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 18:51:17 +0200 Subject: [PATCH] Keep the core over an older record instead of refusing the send (issue 201, review 90) Refusing the whole send wedged a machine whose gate put back the controller and whose put-back send was refused: every later automatic send there was refused, retried each tick, and held up every other walk. A send now composes the controller and the node-engine at the build the machine was last sent when the mesh's record is older, and carries the rest; nothing is refused or retried, and a plan whose own build is the older one ends with its gate superseded. A sent build that failed its gate is never kept, so the next send completes a put-back. The condition names the two ways out and clears on any send that carries the record, a person's push included. --- cmd/mesh-controller/core_downgrade.go | 201 ++++++++++++++++---------- cmd/mesh-controller/issue201_test.go | 188 ++++++++++++++---------- cmd/mesh-controller/plain_words.go | 12 +- cmd/mesh-controller/push.go | 3 + cmd/mesh-controller/recorded_kept.go | 17 +++ cmd/mesh-controller/release.go | 46 ++---- 6 files changed, 267 insertions(+), 200 deletions(-) diff --git a/cmd/mesh-controller/core_downgrade.go b/cmd/mesh-controller/core_downgrade.go index e66f11c9..e4b9885d 100644 --- a/cmd/mesh-controller/core_downgrade.go +++ b/cmd/mesh-controller/core_downgrade.go @@ -2,7 +2,6 @@ package main import ( "context" - "errors" "fmt" "sort" "time" @@ -12,28 +11,31 @@ import ( "github.com/novox/mesh-controller/internal/lease" ) -// **A send never moves the mesh's own core to a build older than the one its machine runs** (novox/hq issue -// 201). On 2026-10-02 two merges a minute apart made two plans; the controller's plan rolled the new controller, -// which widened its seat's row, and the node-engine's plan then sent the control node the controller build -// before it. The older binary read the row it could not serve and the mesh lost its voice for ten minutes — the -// thing that would have corrected it was the thing that was down. ADR 0185 took the outage out of it (a -// controller behind its row serves what it can); the race is closed here, without needing its trigger: whatever -// composed the older build, the send that would carry it is refused. +// **No send but a person's push and a gate's own put-back moves the mesh's own core to a build older than the +// one its machine was last sent** (novox/hq issue 201). On 2026-10-02 two merges a minute apart made two plans; +// the controller's plan rolled the new controller, which widened its seat's row, and the node-engine's plan then +// sent the control node the controller build before it. The older binary read the row it could not serve and the +// mesh lost its voice for ten minutes — the thing that would have corrected it was the thing that was down. ADR +// 0185 took the outage out of it (a controller behind its row serves what it can); the race is closed here, +// without needing its trigger: whatever made the mesh's record older, the send keeps the build. // -// Older is by the builds' registration order — when each was asked (novox/hq issue 219) — never by the -// commits' dates. A gate's own put-back is the one send that may carry it, for the modules it puts back: it says -// so (sendScope.puttingBack), and the controller's put-back is held to the store's schema already (issue 352). -// A person's push carries what it carries, as every other refusal of a send leaves it. +// **Kept, not refused.** The send composes the controller or the node-engine at the build the machine was last +// sent, as a recorded module is kept (ADR 0245), and carries everything else: no other module, secret, healer or +// walk waits on it, and nothing is retried. A plan whose own build is the older one finds its gate superseded by +// the build kept there (issue 352) and ends. The mesh's record stays older, and a warning condition says so until +// a send carries the record — a person's `push ` accepts the older build — or the record moves forward: a +// newer merge, or `rebuild mesh-controller`. // -// The refusal is said in the plan's words — the send's error, which the plan keeps as its note and tries again -// — and as a warning condition on the machine, cleared by the first send there that carries no such move. +// Older is by the builds' registration order — when each was asked (novox/hq issue 219) — never by the commits' +// dates. So a retry or a rebuild of an old commit, asked anew, counts as newer: it is registered as the module's +// current build by the same order, and the guard does not stop that move. A build that failed its gate is never +// ahead: the machine was sent it and the record was put back before it (gateFailed), and if the put-back's own +// send was refused, the next send completes it rather than keeping the failed build there. A gate's own put-back +// carries its modules back whatever the order (sendScope.puttingBack); the controller's is held to the store's +// schema already (issue 352). -// errCoreDowngrade is a send refused because it would put a core module on a machine back behind the build that -// machine runs. -var errCoreDowngrade = errors.New("a send would put the mesh's own core back behind the build its machine runs") - -// kindCoreDowngrade is that refusal, as a condition. -const kindCoreDowngrade = "core-downgrade-refused" +// kindCoreDowngrade is a core module kept at the build its machine was last sent, over an older record. +const kindCoreDowngrade = "core-downgrade-kept" // coreDowngradeSource is the condition's source: the sends, which raise and clear it. const coreDowngradeSource = "sends" @@ -42,67 +44,84 @@ const coreDowngradeSource = "sends" // store and its seat's row, and the node-engine, which reads what the controller declares. var downgradeGuarded = []string{lease.ComponentController, lease.ComponentEngine} -// buildAsked is when the newest build of a module from a commit that worked was asked — or recorded, when the -// request time is not known — and false when the records hold none. A variable, so a test reads its own order. -var buildAsked = func(ctx context.Context, inv *inventory.Inventory, module, commit string) (time.Time, bool, error) { +// askedBuild is what the order needs of one build of a module: when it was asked, and whether it failed its gate. +type askedBuild struct { + asked time.Time + failedGate bool +} + +// buildAsked is the newest build of a module from a commit that worked — when it was asked (or recorded, when +// the request time is not known), and whether it failed its gate — and false when the records hold none. A +// variable, so a test reads its own order. +var buildAsked = func(ctx context.Context, inv *inventory.Inventory, module, commit string) (askedBuild, bool, error) { builds, err := inv.Builds(ctx, module, askedLookBack) if err != nil { - return time.Time{}, false, err + return askedBuild{}, false, err } for _, b := range builds { if b.Worked() && sameCommit(b.Commit, commit) { - return b.AskedOrAt(), true, nil + failed, err := inv.GateFailed(ctx, b.ID) + if err != nil { + return askedBuild{}, false, err + } + return askedBuild{asked: b.AskedOrAt(), failedGate: failed}, true, nil } } - return time.Time{}, false, nil + return askedBuild{}, false, nil } // askedLookBack is how many of a module's newest builds are read for when a build was asked. const askedLookBack = 50 -// coreDowngrade is one move of a core module to an older build than its machine runs. +// coreDowngrade is one core module a machine was last sent a build of that is newer than the mesh's record. type coreDowngrade struct { - move inventory.CarriedMove - component string - fromAsked, asked time.Time + module, node, component string + // sent is the build the machine was last sent, record the one the mesh holds. + sent, record string + sentAsked, recordAsked time.Time } func (d coreDowngrade) String() string { - return fmt.Sprintf("%s would be sent %s %s, older than the %s it runs (asked %s, before %s) — no send but a "+ - "gate's own put-back moves the %s back (novox/hq issue 201)", d.move.Node, d.move.Module, short(d.move.To), - short(d.move.From), d.asked.UTC().Format(time.RFC3339), d.fromAsked.UTC().Format(time.RFC3339), d.component) + return fmt.Sprintf("%s was last sent %s %s, and the mesh holds the older %s (asked %s, before %s): a send there "+ + "keeps %s; `push %s` sends the older build, and a newer merge or `rebuild %s` moves the record forward "+ + "(novox/hq issue 201)", d.node, d.module, short(d.sent), short(d.record), + d.recordAsked.UTC().Format(time.RFC3339), d.sentAsked.UTC().Format(time.RFC3339), short(d.sent), d.node, + d.module) } -// coreDowngrades is the moves of a machine's send that put a core module back to a build asked before the one it -// runs, leaving out what a put-back puts back. A move whose either build the records do not hold is not proved -// older, and is left to the gate that judges it. Pure but for asked. -func coreDowngrades(moves []inventory.CarriedMove, scope sendScope, - asked func(module, commit string) (time.Time, bool, error)) ([]coreDowngrade, error) { +// coreDowngrades is, for one machine, each guarded core module whose record is older than the build the machine +// was last sent, leaving out what a gate's own put-back puts back and a sent build that failed its gate. A move +// whose either build the records do not hold is not proved older and is left to the gate that judges it. Pure +// but for asked. +func coreDowngrades(node string, sent map[string]string, record map[string]string, scope sendScope, + asked func(module, commit string) (askedBuild, bool, error)) ([]coreDowngrade, error) { var out []coreDowngrade - for _, mv := range moves { - component := coreComponent(mv.Module) - if !guarded(component) || mv.From == "" || mv.To == "" || sameCommit(mv.From, mv.To) { + for module, was := range sent { + component := coreComponent(module) + now := record[module] + if !guarded(component) || was == "" || now == "" || sameCommit(was, now) { continue } - if scope.puttingBack && scope.modules[mv.Module] { + if scope.puttingBack && scope.modules[module] { continue } - from, known, err := asked(mv.Module, mv.From) - if err != nil || !known { - if err != nil { - return nil, fmt.Errorf("when %s's build %s was asked cannot be read: %w", mv.Module, short(mv.From), err) - } - continue - } - to, known, err := asked(mv.Module, mv.To) + from, known, err := asked(module, was) if err != nil { - return nil, fmt.Errorf("when %s's build %s was asked cannot be read: %w", mv.Module, short(mv.To), err) + return nil, fmt.Errorf("when %s's build %s was asked cannot be read: %w", module, short(was), err) } - if known && to.Before(from) { - out = append(out, coreDowngrade{move: mv, component: component, fromAsked: from, asked: to}) + if !known || from.failedGate { + continue + } + to, known, err := asked(module, now) + if err != nil { + return nil, fmt.Errorf("when %s's build %s was asked cannot be read: %w", module, short(now), err) + } + if known && to.asked.Before(from.asked) { + out = append(out, coreDowngrade{module: module, node: node, component: component, sent: was, record: now, + sentAsked: from.asked, recordAsked: to.asked}) } } - sort.Slice(out, func(i, j int) bool { return out[i].move.Module < out[j].move.Module }) + sort.Slice(out, func(i, j int) bool { return out[i].module < out[j].module }) return out, nil } @@ -115,32 +134,29 @@ func guarded(component string) bool { return false } -// refuseCoreDowngrades says, for one machine's moves, each that would put a core module back, raising its -// condition; and clears the condition of each guarded component that machine's send does not move back. -func refuseCoreDowngrades(ctx context.Context, inv *inventory.Inventory, scope sendScope, node string, - moves []inventory.CarriedMove) ([]string, error) { - found, err := coreDowngrades(moves, scope, func(module, commit string) (time.Time, bool, error) { +// coreKept is, for a send that keeps (sendKeeps), each guarded core module of a machine to keep at the build it +// was last sent, module → commit, each said and raised as a condition. +func coreKept(ctx context.Context, inv *inventory.Inventory, node string, sent map[string]string, + current map[string]inventory.CurrentBuild) (map[string]string, error) { + record := map[string]string{} + for m, c := range current { + record[m] = c.Commit + } + found, err := coreDowngrades(node, sent, record, scopeOf(ctx), func(module, commit string) (askedBuild, bool, error) { return buildAsked(ctx, inv, module, commit) }) - if err != nil { + if err != nil || len(found) == 0 { return nil, err } - var said []string - refused := map[string]bool{} + out := map[string]string{} for _, d := range found { - said = append(said, d.String()) - refused[d.component] = true + out[d.module] = d.sent sayCoreDowngrade(ctx, d) } - for _, c := range downgradeGuarded { - if !refused[c] { - clearCoreDowngrade(ctx, c, node) - } - } - return said, nil + return out, nil } -// coreDowngradeObservation is a refused downgrade as a condition: `core...`. +// coreDowngradeObservation is a kept core build as a condition: `core...`. func coreDowngradeObservation(component, node, summary string) conditions.Observation { return conditions.Observation{Scope: conditions.ScopeCore, ID: component + "." + node, Kind: kindCoreDowngrade, Token: kindCoreDowngrade, Machine: node, Severity: conditions.Warning, Source: coreDowngradeSource, @@ -148,24 +164,49 @@ func coreDowngradeObservation(component, node, summary string) conditions.Observ } func sayCoreDowngrade(ctx context.Context, d coreDowngrade) { - fmt.Printf("NOT SENT: %s\n", d) + fmt.Printf("KEPT: %s\n", d) keeper := conditionsFrom if keeper == nil { return } - if _, err := keeper.Observe(ctx, coreDowngradeObservation(d.component, d.move.Node, d.String())); err != nil { - fmt.Printf("the condition for %s's refused downgrade on %s could not be raised: %v\n", d.move.Module, - d.move.Node, err) + if _, err := keeper.Observe(ctx, coreDowngradeObservation(d.component, d.node, d.String())); err != nil { + fmt.Printf("the condition for %s kept on %s could not be raised: %v\n", d.module, d.node, err) } } -func clearCoreDowngrade(ctx context.Context, component, node string) { +// settleCoreDowngrades clears, after a send to a machine is away, the condition of each guarded core module the +// send carried at the mesh's record — a person's push that accepted the older build, or a send after the record +// moved forward — or did not carry at all. A send that kept the newer build leaves it open. +func settleCoreDowngrades(ctx context.Context, inv *inventory.Inventory, node string, carried map[string]string) { keeper := conditionsFrom - if keeper == nil { + if keeper == nil || carried == nil { return } - key := coreDowngradeObservation(component, node, "").Key() - if _, err := keeper.Clear(ctx, key, "a send to "+node+" carries no older "+component); err != nil { - fmt.Printf("the condition %s could not be cleared: %v\n", key, err) + var open []string + for _, c := range downgradeGuarded { + if _, found, err := keeper.Get(ctx, coreDowngradeObservation(c, node, "").Key()); err == nil && found { + open = append(open, c) + } + } + if len(open) == 0 { + return + } + current, err := inv.CurrentBuilds(ctx) + if err != nil { + fmt.Printf("whether %s still keeps a core build over the mesh's record cannot be read: %v\n", node, err) + return + } + for module, now := range current { + component := coreComponent(module) + if !guarded(component) { + continue + } + if was, has := carried[module]; has && !sameCommit(was, now.Commit) { + continue + } + key := coreDowngradeObservation(component, node, "").Key() + if _, err := keeper.Clear(ctx, key, node+" was sent the "+component+" the mesh holds"); err != nil { + fmt.Printf("the condition %s could not be cleared: %v\n", key, err) + } } } diff --git a/cmd/mesh-controller/issue201_test.go b/cmd/mesh-controller/issue201_test.go index 9ec8f3bd..b601ecda 100644 --- a/cmd/mesh-controller/issue201_test.go +++ b/cmd/mesh-controller/issue201_test.go @@ -3,7 +3,6 @@ package main import ( "context" "encoding/json" - "errors" "strings" "testing" "time" @@ -15,104 +14,76 @@ import ( // novox/hq issue 201: on 2026-10-02 a plan sent the control node a controller older than the one running there, // whose seat row the newer one had already widened; the older binary refused the row and the mesh went quiet for -// ten minutes. No send but a gate's own put-back moves the controller or the node-engine back behind the build its -// machine runs, by the builds' registration order. +// ten minutes. No send but a person's push and a gate's own put-back moves the controller or the node-engine back +// behind the build its machine was last sent: the send keeps it there, and carries the rest. -// askedIn answers when a build was asked from a fixed table: module@commit → when. -func askedIn(table map[string]time.Time) func(module, commit string) (time.Time, bool, error) { - return func(module, commit string) (time.Time, bool, error) { +// askedIn answers a build from a fixed table: module@commit → when it was asked; failed names those that failed +// their gate. +func askedIn(table map[string]time.Time, failed ...string) func(module, commit string) (askedBuild, bool, error) { + return func(module, commit string) (askedBuild, bool, error) { at, known := table[module+"@"+commit] - return at, known, nil + b := askedBuild{asked: at} + for _, f := range failed { + if f == module+"@"+commit { + b.failedGate = true + } + } + return b, known, nil } } -func TestACoreModuleIsNeverMovedBehindTheBuildItsMachineRuns(t *testing.T) { +func TestACoreModuleIsKeptOverAnOlderRecord(t *testing.T) { old, now := t0, t0.Add(time.Minute) - asked := askedIn(map[string]time.Time{ + table := map[string]time.Time{ "mesh-controller@c1": old, "mesh-controller@c2": now, "mesh-host@h1": old, "mesh-host@h2": now, "app@a1": old, "app@a2": now, - }) - back := []inventory.CarriedMove{ - {Module: "mesh-controller", Node: "anchor", From: "c2", To: "c1"}, - {Module: "mesh-host", Node: "anchor", From: "h2", To: "h1"}, - {Module: "app", Node: "anchor", From: "a2", To: "a1"}, } - found, err := coreDowngrades(back, sendScope{judged: map[string]bool{"anchor": true}}, asked) + sent := map[string]string{"mesh-controller": "c2", "mesh-host": "h2", "app": "a2"} + record := map[string]string{"mesh-controller": "c1", "mesh-host": "h1", "app": "a1"} + found, err := coreDowngrades("anchor", sent, record, sendScope{judged: map[string]bool{"anchor": true}}, askedIn(table)) if err != nil { t.Fatal(err) } - if len(found) != 2 || found[0].move.Module != "mesh-controller" || found[0].component != lease.ComponentController || - found[1].move.Module != "mesh-host" || found[1].component != lease.ComponentEngine { - t.Fatalf("the controller and the node-engine sent back, and nothing else, are refused: %+v", found) + if len(found) != 2 || found[0].module != "mesh-controller" || found[0].component != lease.ComponentController || + found[0].sent != "c2" || found[1].module != "mesh-host" || found[1].component != lease.ComponentEngine { + t.Fatalf("the controller and the node-engine over an older record are kept, and nothing else: %+v", found) } - if said := found[0].String(); !strings.Contains(said, "anchor would be sent mesh-controller c1") || - !strings.Contains(said, "issue 201") { - t.Fatalf("the refusal says the machine, the module and the build: %s", said) + if said := found[0].String(); !strings.Contains(said, "anchor was last sent mesh-controller c2") || + !strings.Contains(said, "`push anchor`") || !strings.Contains(said, "`rebuild mesh-controller`") { + t.Fatalf("it says the machine, the build kept and the two ways out: %s", said) } - // Forward, the same build, or a build the records do not hold: not refused. - for _, mv := range []inventory.CarriedMove{ - {Module: "mesh-controller", Node: "anchor", From: "c1", To: "c2"}, - {Module: "mesh-controller", Node: "anchor", From: "c2", To: "c2"}, - {Module: "mesh-controller", Node: "anchor", From: "c2", To: "c9"}, - {Module: "mesh-controller", Node: "anchor", From: "c9", To: "c1"}, - } { - if found, err := coreDowngrades([]inventory.CarriedMove{mv}, sendScope{}, asked); err != nil || len(found) != 0 { - t.Fatalf("%+v is no downgrade proved: %+v %v", mv, found, err) + // The record forward, the same build, or a build the records do not hold: nothing kept. + for _, c := range [][2]string{{"c1", "c2"}, {"c2", "c2"}, {"c2", "c9"}, {"c9", "c1"}} { + found, err := coreDowngrades("anchor", map[string]string{"mesh-controller": c[0]}, + map[string]string{"mesh-controller": c[1]}, sendScope{}, askedIn(table)) + if err != nil || len(found) != 0 { + t.Fatalf("sent %s, record %s: no downgrade proved, nothing kept: %+v %v", c[0], c[1], found, err) } } + // A sent build that failed its gate is never ahead: the record put back before it is sent. + if found, _ := coreDowngrades("anchor", sent, record, sendScope{}, askedIn(table, "mesh-controller@c2")); len(found) != 1 || + found[0].module != "mesh-host" { + t.Fatalf("a controller that failed its gate was kept over the build put back: %+v", found) + } + // A gate's own put-back carries its own modules back, and nothing else. putBack := sendScope{modules: map[string]bool{"mesh-controller": true}, puttingBack: true} - found, err = coreDowngrades(back, putBack, asked) - if err != nil { - t.Fatal(err) - } - if len(found) != 1 || found[0].move.Module != "mesh-host" { + if found, _ := coreDowngrades("anchor", sent, record, putBack, askedIn(table)); len(found) != 1 || found[0].module != "mesh-host" { t.Fatalf("a put-back of the controller carries it back, and not the node-engine: %+v", found) } - // Naming a module is not a put-back. - if found, _ := coreDowngrades(back[:1], sendScope{modules: map[string]bool{"mesh-controller": true}}, asked); len(found) != 1 { + if found, _ := coreDowngrades("anchor", sent, record, sendScope{modules: map[string]bool{"mesh-controller": true}}, + askedIn(table)); len(found) != 2 { t.Fatalf("a send for the controller that is no put-back carries it back: %+v", found) } } -// The refusal is a warning condition on the machine, which the next send there carrying no such move clears. -func TestARefusedCoreDowngradeIsSaidAndClearedByTheNextSend(t *testing.T) { - k, _ := withConditionsInMemory(t) - ctx := t.Context() - was := buildAsked - buildAsked = func(_ context.Context, _ *inventory.Inventory, module, commit string) (time.Time, bool, error) { - return askedIn(map[string]time.Time{"mesh-controller@c1": t0, "mesh-controller@c2": t0.Add(time.Minute)})(module, commit) - } - t.Cleanup(func() { buildAsked = was }) - - said, err := refuseCoreDowngrades(ctx, nil, sendScope{}, "anchor", - []inventory.CarriedMove{{Module: "mesh-controller", Node: "anchor", From: "c2", To: "c1"}}) - if err != nil || len(said) != 1 { - t.Fatalf("the downgrade is refused: %v %v", said, err) - } - key := coreDowngradeObservation(lease.ComponentController, "anchor", "").Key() - c, open, err := k.Get(ctx, key) - if err != nil || !open || !strings.Contains(c.Summary, "mesh-controller c1") { - t.Fatalf("the condition %s is raised, saying the build: %+v %v %v", key, c, open, err) - } - - said, err = refuseCoreDowngrades(ctx, nil, sendScope{}, "anchor", - []inventory.CarriedMove{{Module: "mesh-controller", Node: "anchor", From: "c1", To: "c2"}}) - if err != nil || len(said) != 0 { - t.Fatalf("a move forward is sent: %v %v", said, err) - } - if _, open, err := k.Get(ctx, key); err != nil || open { - t.Fatalf("the condition is cleared by a send carrying no older controller: %v %v", open, err) - } -} - -// TestReplay201 replays the race on a store: the control node runs the newer controller, the mesh's record of -// the controller is the build before it, and a plan's gated send there — the node-engine's plan of 2026-10-02 — -// is refused, said as the plan's words; the gate's own put-back of the controller is not. -func TestReplay201(t *testing.T) { +// core201 is a mesh whose control node, anchor, was sent the newer controller c2 while the mesh's record of the +// controller is the build before it, c1, which passed its gate when it rolled out. +func core201(t *testing.T) *stores { + t.Helper() open := aMesh(t) ctx := t.Context() inv := open.inventory @@ -128,6 +99,10 @@ func TestReplay201(t *testing.T) { return b } before := record("c1aaaaaaaaaa", old) + if err := inv.RecordGate(ctx, inventory.GateVerdict{Build: before.ID, Module: "mesh-controller", Commit: before.Commit, + Machines: []string{"anchor"}, Verdict: inventory.GatePassed}); err != nil { + t.Fatal(err) + } record("c2bbbbbbbbbb", now) if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "mesh-controller", Version: "1"}, inventory.Source{ Repository: "novox/mesh-controller", BuiltFrom: "c2bbbbbbbbbb", Head: "c2bbbbbbbbbb", Asked: now}); err != nil { @@ -139,7 +114,6 @@ func TestReplay201(t *testing.T) { if err := inv.RecordSent(ctx, nodeID(t, open, "anchor"), "d-anchor", map[string]string{"mesh-controller": "c2bbbbbbbbbb"}); err != nil { t.Fatal(err) } - // The mesh's record goes back to the build before, as the record went on 2026-10-02. if err := inv.RestoreModule(ctx, before); err != nil { t.Fatal(err) } @@ -156,14 +130,70 @@ func TestReplay201(t *testing.T) { return f.moves(node, modules, sent, known, all), nil } t.Cleanup(func() { machineMoves = wasMoves }) + return open +} + +// TestReplay201 replays the race on a store: the control node runs the newer controller and the mesh's record is +// the build before it. A plan's send there — the node-engine's plan of 2026-10-02 — goes, keeps the newer +// controller, and says so as a condition; the gate's own put-back carries it back; a send that carries the record, +// as a person's push does, clears the condition. +func TestReplay201(t *testing.T) { + open := core201(t) + ctx := t.Context() + k, _ := withConditionsInMemory(t) judged := withScope(ctx, sendScope{judged: map[string]bool{"anchor": true}}) - if _, err := ungatedIn(judged, open, []string{"anchor"}, ""); !errors.Is(err, errCoreDowngrade) || - !strings.Contains(err.Error(), "mesh-controller c1aaaaaa") { - t.Fatalf("a plan's send of the older controller to the machine running the newer is refused: %v", err) + if kept, err := ungatedIn(judged, open, []string{"anchor"}, ""); err != nil || len(kept) != 1 { + t.Fatalf("a plan's send to the machine is made, the rest of it with it: %v %v", kept, err) } - putBack := withScope(ctx, sendScope{modules: map[string]bool{"mesh-controller": true}, puttingBack: true}) - if kept, err := ungatedIn(putBack, open, []string{"anchor"}, ""); err != nil || len(kept) != 1 { - t.Fatalf("the gate's own put-back of the controller is sent: %v %v", kept, err) + keeps, err := recordedKept(keepingRecorded(judged), open, "anchor") + if err != nil || keeps["mesh-controller"] != "c2bbbbbbbbbb" { + t.Fatalf("the send keeps the newer controller there: %v %v", keeps, err) + } + shelf := map[string]catalogue.Manifest{"mesh-controller": {Module: "mesh-controller", Version: "old"}} + if _, err := keepRecorded(keepingRecorded(judged), open, "anchor", shelf); err != nil || shelf["mesh-controller"].Version != "1" { + t.Fatalf("the send composes the kept build's manifest: %+v %v", shelf, err) + } + key := coreDowngradeObservation(lease.ComponentController, "anchor", "").Key() + if c, found, err := k.Get(ctx, key); err != nil || !found || !strings.Contains(c.Summary, "c2bbbbbb") { + t.Fatalf("the condition %s is raised, saying the build kept: %+v %v %v", key, c, found, err) + } + + putBack := keepingRecorded(withScope(ctx, sendScope{modules: map[string]bool{"mesh-controller": true}, puttingBack: true})) + if keeps, err := recordedKept(putBack, open, "anchor"); err != nil || len(keeps) != 0 { + t.Fatalf("the gate's own put-back of the controller carries it back: %v %v", keeps, err) + } + + settleCoreDowngrades(ctx, open.inventory, "anchor", map[string]string{"mesh-controller": "c2bbbbbbbbbb"}) + if _, found, _ := k.Get(ctx, key); !found { + t.Fatal("a send that kept the newer controller cleared the condition") + } + settleCoreDowngrades(ctx, open.inventory, "anchor", map[string]string{"mesh-controller": "c1aaaaaaaaaa"}) + if _, found, _ := k.Get(ctx, key); found { + t.Fatal("a send carrying the mesh's record — a person's push — leaves the condition open") + } +} + +// The review of #273: a gate failed the newer controller, put the record back, and the put-back's own send was +// refused. The next automatic send — a healer's, which judges nothing — completes the put-back: a build that +// failed its gate is never kept. +func TestARollbackWhoseSendWasRefusedIsCompletedByTheNextSend(t *testing.T) { + open := core201(t) + ctx := t.Context() + k, _ := withConditionsInMemory(t) + if err := open.inventory.RecordGate(ctx, inventory.GateVerdict{Build: "build-mesh-controller-c2bbbbbbbbbb", + Module: "mesh-controller", Commit: "c2bbbbbbbbbb", Previous: "c1aaaaaaaaaa", Machines: []string{"anchor"}, + Verdict: inventory.GateFailed, Rollback: inventory.NotRolledBack}); err != nil { + t.Fatal(err) + } + healer := withScope(ctx, sendScope{}) + if kept, err := ungatedIn(healer, open, []string{"anchor"}, ""); err != nil || len(kept) != 1 { + t.Fatalf("the healer's send is made: %v %v", kept, err) + } + if keeps, err := recordedKept(keepingRecorded(healer), open, "anchor"); err != nil || len(keeps) != 0 { + t.Fatalf("the failed controller was kept over the build put back: %v %v", keeps, err) + } + if open, _ := k.Open(ctx); len(open) != 0 { + t.Fatalf("a completed put-back raised a condition: %+v", open) } } diff --git a/cmd/mesh-controller/plain_words.go b/cmd/mesh-controller/plain_words.go index e2caf2d4..a5f705b1 100644 --- a/cmd/mesh-controller/plain_words.go +++ b/cmd/mesh-controller/plain_words.go @@ -182,7 +182,7 @@ var plainWordings = map[string]func(conditions.Observation) words{ "register it, so it was not made.", module, m), Resolved: "Resolved: " + module + " on " + m + " is settled"} }), - // A send that would have put the controller or the node-engine back behind what a machine runs (novox/hq + // The controller or the node-engine kept at the build a machine was last sent, over an older record (novox/hq // issue 201). kindCoreDowngrade: worded(func(o conditions.Observation) words { m := machineOr(o, "a machine") @@ -190,11 +190,11 @@ var plainWordings = map[string]func(conditions.Observation) words{ if o.Scope != conditions.ScopeCore || !guarded(component) { component = "core component" } - return words{Headline: "An older " + component + " was not sent to " + m, - Needs: "decide which build of the " + component + " the mesh should hold; the details say which.", - Explanation: fmt.Sprintf("A send would have put the mesh's own %s on %s back to an older build than "+ - "the one it runs, so it was not sent and %s keeps its build.", component, m, m), - Resolved: "Resolved: sends to " + m + " carry no older " + component} + return words{Headline: m + " keeps a newer " + component + " than the mesh holds", + Needs: fmt.Sprintf("push %s to take the older build, or a newer merge or rebuild moves the record forward.", m), + Explanation: fmt.Sprintf("The mesh's record of its own %s is older than the build %s was last sent, so "+ + "the mesh's sends keep the newer one there rather than put it back.", component, m), + Resolved: "Resolved: " + m + " runs the " + component + " the mesh holds"} }), kindBindingKept: worded(func(o conditions.Observation) words { m := machineOr(o, "a machine") diff --git a/cmd/mesh-controller/push.go b/cmd/mesh-controller/push.go index 8aef75c2..7ff94186 100644 --- a/cmd/mesh-controller/push.go +++ b/cmd/mesh-controller/push.go @@ -1027,6 +1027,9 @@ func (b overTheBus) declare(ctx context.Context, s readyNode, body []byte) (stri if err != nil { return "", err } + // A send that carried the mesh's record of the controller and the node-engine — a person's push included — + // ends the condition of one kept over it (novox/hq issue 201). + settleCoreDowngrades(ctx, b.open.inventory, s.node, s.declared.Builds) if len(s.declared.Bindings) > 0 { // And where it bound each consumer of a provision that keeps its data (novox/hq ADR 0232): // what the next resolution keeps it at. On the same outliving context as the send's record. diff --git a/cmd/mesh-controller/recorded_kept.go b/cmd/mesh-controller/recorded_kept.go index a8e8cf5b..ecf9e25c 100644 --- a/cmd/mesh-controller/recorded_kept.go +++ b/cmd/mesh-controller/recorded_kept.go @@ -135,6 +135,17 @@ func recordedKept(ctx context.Context, open *stores, node string) (map[string]st } out[m] = was } + // **And the controller and the node-engine at the build the machine was last sent, over an older record** + // (novox/hq issue 201, core_downgrade.go). Under keepingEveryBuild they are kept already. + if !every { + core, err := coreKept(ctx, inv, node, sent, current) + if err != nil { + return nil, err + } + for m, was := range core { + out[m] = was + } + } return out, nil } @@ -159,6 +170,12 @@ func keepRecorded(ctx context.Context, open *stores, node string, shelf map[stri return nil, fmt.Errorf("%s runs %s's build %s, which the build records no longer hold: the bus's user "+ "list cannot be sent there alone with it kept (novox/hq issue 490)", node, m, short(kept[m])) } + if !found && guarded(coreComponent(m)) { + return nil, fmt.Errorf("%s was last sent %s's build %s, newer than the one the mesh holds, and the build records "+ + "no longer hold it: this send cannot keep it and does not move it back — `push %s` sends the older build on "+ + "a person's word, and a newer merge or `rebuild %s` moves the record forward (novox/hq issue 201)", + node, m, short(kept[m]), node, m) + } if !found { return nil, fmt.Errorf("%s records rather than rolls out, and %s runs its build %s, which the build "+ "records no longer hold: this send cannot keep it and does not move it — `push %s` sends the new "+ diff --git a/cmd/mesh-controller/release.go b/cmd/mesh-controller/release.go index 5f23d856..ab21b05a 100644 --- a/cmd/mesh-controller/release.go +++ b/cmd/mesh-controller/release.go @@ -251,47 +251,26 @@ func ungatedIn(ctx context.Context, open *stores, names []string, addedHolder st if err != nil { return nil, err } - var kept, refused, older []string + var kept, refused []string for _, n := range names { - // Every move, gated or not: a move to a build that passed a gate elsewhere is no gate's concern here, and - // may still be a core module's downgrade (novox/hq issue 201). - every, err := machineMoves(ctx, open, f, n, true) + moves, err := machineMoves(ctx, open, f, n, false) if err != nil { return nil, err } - // The grants step composes every module at the build its machine was last sent (novox/hq issue 490): - // a move it keeps is not made. Read, not assumed, so a move it could not keep is still refused here. - var keeps map[string]string - if everyBuildKept(ctx, n) { - if keeps, err = recordedKept(ctx, open, n); err != nil { - return nil, err - } + // The grants step composes every module at the build its machine was last sent (novox/hq issue 490), + // and every send keeps the controller and the node-engine over an older record (issue 201): a move it + // keeps is not made. Read, not assumed, so a move it could not keep is still refused here. + keeping := ctx + if _, on := keptExcept(ctx); !on { + keeping = keepingRecorded(ctx) // as the send composes it (sendKeeps), which is set after this check } - var made []inventory.CarriedMove - for _, mv := range every { - if was, kept := keeps[mv.Module]; kept && sameCommit(was, mv.From) { - continue - } - made = append(made, mv) - } - // **No send but a gate's own put-back moves the controller or the node-engine to an older build than - // the one its machine runs** (novox/hq issue 201). - downgrades, err := refuseCoreDowngrades(ctx, open.inventory, scope, n, made) + keeps, err := recordedKept(keeping, open, n) if err != nil { return nil, err } - if len(downgrades) > 0 { - if n == addedHolder { - fmt.Printf("%s holds the bus and its user list changed, and it is not sent now: %s\n", n, - strings.Join(downgrades, "; ")) - continue - } - older = append(older, downgrades...) - continue - } var waiting []string - for _, mv := range made { - if f.gated(mv.Module, mv.To) { + for _, mv := range moves { + if was, kept := keeps[mv.Module]; kept && sameCommit(was, mv.From) { continue } if !scope.judged[n] && !scope.modules[mv.Module] { @@ -308,9 +287,6 @@ func ungatedIn(ctx context.Context, open *stores, names []string, addedHolder st refused = append(refused, fmt.Sprintf("%s (%s)", n, strings.Join(waiting, ", "))) } } - if len(older) > 0 { - return nil, fmt.Errorf("%w: %s", errCoreDowngrade, strings.Join(older, "; ")) - } if len(refused) > 0 { return nil, fmt.Errorf("%w: %s — a walk sends them, one machine at a time, each judged (novox/hq ADR "+ "0236); `upgrade backlog` lists them", errUngated, strings.Join(refused, "; "))