From 1219ec822943e1f15b1f89a4b9ac15831e7560db Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 13 Oct 2026 16:50:10 +0200 Subject: [PATCH] A settings walk sends the layer alone, every build kept, and stops before a data move (issue 641, ADR 0344) A judged send carries what waits on its machine, which would have released builds held after a failed walk without a person's word. The settings walk now keeps every build of its machine, refuses a send that would move a running module's data, and never says delivered when it reached no machine. --- cmd/mesh-controller/proposals.go | 7 +- cmd/mesh-controller/recorded_kept.go | 26 ++++- cmd/mesh-controller/settings_walk.go | 86 +++++++++------ cmd/mesh-controller/settings_walk_test.go | 127 ++++++++++++++++++++-- 4 files changed, 197 insertions(+), 49 deletions(-) diff --git a/cmd/mesh-controller/proposals.go b/cmd/mesh-controller/proposals.go index f3c0de6d..fc5db024 100644 --- a/cmd/mesh-controller/proposals.go +++ b/cmd/mesh-controller/proposals.go @@ -518,7 +518,7 @@ func (pr proposer) propose(ctx context.Context, in proposeInput) (string, error) fmt.Fprintf(&b, " until %s nothing changes: the layer is set only on Approve, and discarded on Decline or at expiry\n", q.Expires.Local().Format("2006-01-02 15:04")) fmt.Fprintf(&b, " `settings proposals %s` shows it whole; once approved, a walk sends it to %s at once, one machine at "+ - "a time, each judged, and the operator is told when it has landed (novox/hq ADR 0344)", id, p.reaches()) + "a time, each judged, and its end is told as every walk's is (novox/hq ADR 0344)", id, p.reaches()) return b.String(), nil } @@ -537,8 +537,9 @@ func (p settingsProposal) onApprove() string { if p.Clear { what = "the layer is removed" } - return fmt.Sprintf("Approved, %s and sent at once to %s, one machine at a time, each judged; you are told when it "+ - "has landed. A module that reads its settings only when it starts takes them at its next start.", what, p.reaches()) + return fmt.Sprintf("Approved, %s and sent at once to %s, one machine at a time, each judged, every build there "+ + "kept; its end is told as every walk's is. A module that reads its settings only when it starts takes them at "+ + "its next start.", what, p.reaches()) } func newProposalID() string { diff --git a/cmd/mesh-controller/recorded_kept.go b/cmd/mesh-controller/recorded_kept.go index ecf9e25c..2b65aa4b 100644 --- a/cmd/mesh-controller/recorded_kept.go +++ b/cmd/mesh-controller/recorded_kept.go @@ -83,6 +83,22 @@ func everyBuildKept(ctx context.Context, node string) bool { return on != "" && on == node } +type keepForSettingsKey struct{} + +// keepingBuildsForSettings is a context whose sends compose every module of one machine — recorded or rolling out — +// at the build that machine was last sent, as keepingEveryBuild does, for a settings walk's send (novox/hq ADR 0344): +// the layer the operator approved, and nothing of any module's new code. A build held for a person, or waiting +// for a gate, stays where it is; the walk that is its own carries it. +func keepingBuildsForSettings(ctx context.Context, node string) context.Context { + return context.WithValue(ctx, keepForSettingsKey{}, node) +} + +// settingsKeptOn is whether this context is a settings walk's send to this machine. +func settingsKeptOn(ctx context.Context, node string) bool { + on, _ := ctx.Value(keepForSettingsKey{}).(string) + return on != "" && on == node +} + // recordedKept is, for a send under keepingRecorded, every recorded module the machine was last sent a // build of that the mesh's build is not identical to: module → the commit it keeps. Nil when the context // keeps nothing, or when what the machine was last sent is not known (it is then held whole elsewhere — @@ -92,7 +108,7 @@ func everyBuildKept(ctx context.Context, node string) bool { // 490), and a machine whose last send is not known refuses the send: there is nothing to keep it at, and // composing the mesh's builds would be the very move the grants step must not make. func recordedKept(ctx context.Context, open *stores, node string) (map[string]string, error) { - every := everyBuildKept(ctx, node) + every := everyBuildKept(ctx, node) || settingsKeptOn(ctx, node) skip, on := keptExcept(ctx) if !on && !every { return nil, nil @@ -106,6 +122,10 @@ func recordedKept(ctx context.Context, open *stores, node string) (map[string]st return nil, err } if !known { + if settingsKeptOn(ctx, node) { + return nil, fmt.Errorf("what %s was last sent is not known, so its settings cannot be sent there with "+ + "every build kept (novox/hq ADR 0344)", node) + } if every { return nil, fmt.Errorf("what %s was last sent is not known, so the bus's user list cannot be sent "+ "there alone with every build kept (novox/hq issue 490)", node) @@ -166,6 +186,10 @@ func keepRecorded(ctx context.Context, open *stores, node string, shelf map[stri if err != nil { return nil, err } + if !found && settingsKeptOn(ctx, node) { + return nil, fmt.Errorf("%s runs %s's build %s, which the build records no longer hold: its settings cannot "+ + "be sent there with it kept (novox/hq ADR 0344)", node, m, short(kept[m])) + } if !found && everyBuildKept(ctx, node) { 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])) diff --git a/cmd/mesh-controller/settings_walk.go b/cmd/mesh-controller/settings_walk.go index d55469c5..cd756b3e 100644 --- a/cmd/mesh-controller/settings_walk.go +++ b/cmd/mesh-controller/settings_walk.go @@ -2,7 +2,6 @@ package main import ( "context" - "errors" "fmt" "slices" "sort" @@ -21,9 +20,9 @@ import ( // opens a **settings walk**: the walk ADR 0236 opens for builds waiting for a gate, with a layer in place of the // builds. Its order is every machine running the module that the layer reaches, the control node last; each is // sent and its module judged there, from the machine's report on that send, before the next is sent. The send is -// a gated send's, so whatever else waits on that machine for a gate goes with it and is judged with it, and every -// recorded module stays at the build its machine was last sent (ADR 0242): a policy of `record` holds a build, -// never a setting. The walk says its start and end on `plan-moved` (ADR 0308), and the holder of the operator's +// the layer alone: every module there stays at the build its machine was last sent (keepingBuildsForSettings), so a +// build held for a person (ADR 0242) or waiting for a gate stays where it is — a policy of `record` holds a build, +// never a setting — and a send that would move a running module's data stops the walk (ADR 0217 §3). The walk says its start and end on `plan-moved` (ADR 0308), and the holder of the operator's // channel tells its end once. A failure stops the walk and leaves the layer as approved; it holds back nothing // else. @@ -93,6 +92,29 @@ var openSettingsWalk = func(ctx context.Context, open *stores, s inventory.PlanS return fmt.Sprintf("%s sends it to %s, one machine at a time, each judged", p.ID, strings.Join(order, ", ")), nil } +// settingsDataMoves is what a settings walk's send to a machine would move of a running module's data: composed as +// that send composes it, every build kept, and compared with what the machine was last sent (ADR 0217 §3). +var settingsDataMoves = func(ctx context.Context, open *stores, node string) ([]dataMove, error) { + keeping := keepingBuildsForSettings(keepingRecorded(ctx), node) + gens, err := generators(keeping, open) + if err != nil { + return nil, err + } + plan, settings, err := planFor(keeping, open, node) + if err != nil { + return nil, err + } + declared, err := declarationWith(keeping, open, node, plan, settings, gens, Reading) + if err != nil { + return nil, err + } + body, err := declared.Body() + if err != nil { + return nil, err + } + return heldMoves(keeping, open.inventory, node, body, nil) +} + // judgingOn is the open walk, other than the one named, whose gate is judging on a machine; empty when none is. A // settings walk sends a machine only when no other walk judges there: its send would set aside the declaration // that walk's gate reads the machine's report against. @@ -155,34 +177,43 @@ func advanceSettingsWalk(ctx context.Context, open *stores, p *inventory.Plan) ( } else if id != "" { return wait(fmt.Sprintf("waiting before %s: %s is judging there", node, id)) } - moves, sent, grants, err := gatedSend(ctx, open, node, nil) - if errors.Is(err, errWalkedElsewhere) { - return wait(fmt.Sprintf("waiting before %s: %v", node, err)) - } - if err == nil && len(sent) == 0 { - // Nothing waited there for a gate: the machine is sent for the layer alone, judged all the same. - sent, grants, err = sendJudged(ctx, open, node) + // **A running module's data that would move is a person's word** (ADR 0217 §3): the walk stops before + // that machine, failed and said, and `push --move` sends it. + if held, err := settingsDataMoves(ctx, open, node); err != nil { + return false, fmt.Errorf("whether %s's data would move could not be read: %w", node, err) + } else if len(held) > 0 { + var said strings.Builder + sayDataHeld(&said, node, held) + p.State = inventory.PlanFailed + p.Note = fmt.Sprintf("%s not sent to %s: a running module's data would move there — %s; the layer "+ + "stays as the operator approved it", settingsWalkFor(*s), node, strings.Join(strings.Fields(said.String()), " ")) + if rest := r.Order[r.Next+1:]; len(rest) > 0 { + p.Note += "; not sent: " + strings.Join(rest, ", ") + } + fmt.Printf("%s: %s\n", p.ID, p.Note) + return true, nil } + sent, grants, err := sendJudged(keepingBuildsForSettings(ctx, node), open, node) if err != nil { return false, fmt.Errorf("sending %s %s: %w", node, settingsWalkFor(*s), err) } - r.Gate = &inventory.PlanGate{Component: coreComponent(s.Module), Machines: sent, Since: &now, Carried: moves, + r.Gate = &inventory.PlanGate{Component: coreComponent(s.Module), Machines: sent, Since: &now, Sent: sentNow(ctx, open.inventory, sent), Grants: grants} - p.Note = fmt.Sprintf("sent %s %s (ask %s); judging %s there", node, settingsWalkFor(*s), s.Ask, s.Module) - if len(moves) > 0 { - p.Note += fmt.Sprintf(", and %d build(s) that waited there for a gate", len(moves)) - } + p.Note = fmt.Sprintf("sent %s %s (ask %s), every build there kept; judging %s there", node, + settingsWalkFor(*s), s.Ask, s.Module) if said := grantsSaid(grants); said != "" { p.Note += "; " + said } - if said := recreationsSaid(moves); said != "" { - p.Note += "; " + said - } fmt.Printf("%s: %s\n", p.ID, p.Note) return true, nil } p.State, p.Tier = inventory.PlanDone, len(p.Tiers) - p.Note = fmt.Sprintf("%s (ask %s) delivered on %s", settingsWalkFor(*s), s.Ask, orNone(strings.Join(r.Done, ", "))) + p.Note = fmt.Sprintf("%s (ask %s) delivered on %s", settingsWalkFor(*s), s.Ask, strings.Join(r.Done, ", ")) + if len(r.Done) == 0 { + // Sent nowhere is not delivered: said as the walk's failure, so the operator is not told it landed. + p.State = inventory.PlanFailed + p.Note = fmt.Sprintf("%s (ask %s) was sent to no machine", settingsWalkFor(*s), s.Ask) + } if len(r.Skipped) > 0 { p.Note += "; not heard from, left as they were, their next send carries it: " + strings.Join(r.Skipped, ", ") } @@ -197,14 +228,8 @@ func advanceSettingsWalk(ctx context.Context, open *stores, p *inventory.Plan) ( } switch verdict { case "": - // What the send carried and broke is put back at once, as a judged send's is; the module whose settings - // changed has no build to put back and is judged to its verdict. - if returned := len(g.Returned); putBackBroken(ctx, open, p, g, "", nil) && len(g.Returned) > returned { - return true, nil - } return wait(fmt.Sprintf("judging %s on %s: %s", s.Module, node, gateLine(g))) case inventory.GatePassed: - passCarried(ctx, open, p, g, "") r.Done = append(r.Done, node) r.Next++ r.Gate = nil @@ -216,19 +241,12 @@ func advanceSettingsWalk(ctx context.Context, open *stores, p *inventory.Plan) ( fmt.Printf("%s: %s\n", p.ID, p.Note) return true, nil } - p.Note = "" - batched, back := batchingRollbacks(ctx) - failCarried(batched, open, p, g, "") - sendRollbacks(ctx, open, p, back) - putBack := p.Note + p.State = inventory.PlanFailed p.Note = fmt.Sprintf("%s failed its gate on %s: %s — the layer stays as the operator approved it, and the one it "+ "replaced is in `settings %s` history", settingsWalkFor(*s), node, g.Why, s.Module) if rest := r.Order[r.Next+1:]; len(rest) > 0 { p.Note += "; not sent: " + strings.Join(rest, ", ") } - if putBack != "" { - p.Note += "; " + putBack - } fmt.Printf("%s: %s\n", p.ID, p.Note) return true, nil } diff --git a/cmd/mesh-controller/settings_walk_test.go b/cmd/mesh-controller/settings_walk_test.go index 8cf8e580..0c7e81f9 100644 --- a/cmd/mesh-controller/settings_walk_test.go +++ b/cmd/mesh-controller/settings_walk_test.go @@ -57,8 +57,8 @@ func TestASettingsWalkWaitsWhileAnotherWalkJudgesOnTheMachine(t *testing.T) { } } -// The ask says, before the Approve, that the layer is sent at once and judged, and that the operator is told when -// it has landed; neither the ask nor the proposer's answer names a push any more. +// The ask says, before the Approve, that the layer is sent at once and judged, and that its end is told as every +// walk's is; neither the ask nor the proposer's answer names a push any more. func TestTheAskSaysAnApprovedLayerIsSentAtOnce(t *testing.T) { for _, c := range []struct { in proposeInput @@ -81,8 +81,8 @@ func TestTheAskSaysAnApprovedLayerIsSentAtOnce(t *testing.T) { if err := q.Check(r.now); err != nil { t.Fatalf("the ask is refused: %v", err) } - if !strings.Contains(q.Explanation, "and sent at once to "+c.where+", one machine at a time, each judged; you "+ - "are told when it has landed") { + if !strings.Contains(q.Explanation, "and sent at once to "+c.where+", one machine at a time, each judged, "+ + "every build there kept; its end is told as every walk's is") { t.Errorf("the ask does not say the layer is sent at once to %s:\n%s", c.where, q.Explanation) } approve := q.Options[0] @@ -113,6 +113,13 @@ func TestAnApprovedLayerOpensTheWalkThatSendsIt(t *testing.T) { if _, err := assign(ctx, open, "laptop", "notes"); err != nil { t.Fatal(err) } + // A layer set by hand opens no walk: it still waits for a push (ADR 0344 §7). + if err := atTheTerminal(t, "settings", "set", "notes", `{"x":"0"}`, "--node", "laptop"); err != nil { + t.Fatal(err) + } + if plans, _ := open.inventory.OpenPlans(ctx); len(plans) != 0 { + t.Fatalf("a layer set at the terminal opened %d walk(s)", len(plans)) + } set := setLayerIn(open) now := time.Now() values := map[string]any{"x": "1"} @@ -154,8 +161,9 @@ func TestAnApprovedLayerOpensTheWalkThatSendsIt(t *testing.T) { } t.Cleanup(func() { openSettingsWalk = was }) again := map[string]any{"x": "2"} - r := settingsProposal{Module: "notes", Node: "laptop", Values: again, From: "g14/claude-code", At: now, - Digest: layerDigest(again), BeforeDigest: layerDigest(nil)} + byHand := map[string]any{"x": "0"} + r := settingsProposal{Module: "notes", Node: "laptop", Values: again, Before: byHand, HadLayer: true, + From: "g14/claude-code", At: now, Digest: layerDigest(again), BeforeDigest: layerDigest(byHand)} if _, err := set(ctx, r, "s11", "approved"); err == nil || !strings.Contains(err.Error(), "is set, and the walk that "+ "sends it could not be opened: the plans are not to be had — `push laptop` sends it") { t.Errorf("an unopened walk: %v", err) @@ -168,6 +176,10 @@ type walkMesh struct { open *stores sent [][]string broken map[string]bool + // kept says, per send, whether it kept every build of its machine; moves is the data a machine's send would move. + kept []bool + moves map[string][]dataMove + heard map[string]bool } func aSettledMesh(t *testing.T) *walkMesh { @@ -175,7 +187,8 @@ func aSettledMesh(t *testing.T) *walkMesh { open := aMesh(t) ctx := t.Context() inv := open.inventory - w := &walkMesh{open: open, broken: map[string]bool{}} + w := &walkMesh{open: open, broken: map[string]bool{}, moves: map[string][]dataMove{}, + heard: map[string]bool{"anchor": true, "laptop": true}} withConditionsInMemory(t) was := doctorFrom doctorFrom = nil @@ -199,7 +212,8 @@ func aSettledMesh(t *testing.T) *walkMesh { t.Fatal(err) } } - wasMoves, wasHeard, wasSend, wasGather := machineMoves, releaseHeard, sendRollout, gatherGateFacts + wasMoves, wasHeard, wasSend, wasGather, wasData := machineMoves, releaseHeard, sendRollout, gatherGateFacts, settingsDataMoves + settingsDataMoves = func(_ context.Context, _ *stores, node string) ([]dataMove, error) { return w.moves[node], nil } machineMoves = func(ctx context.Context, open *stores, f moveFacts, node string, all bool) ([]inventory.CarriedMove, error) { modules, err := open.inventory.Assigned(ctx, node) if err != nil { @@ -211,12 +225,11 @@ func aSettledMesh(t *testing.T) *walkMesh { } return f.moves(node, modules, sent, known, all), nil } - releaseHeard = func(context.Context, *stores) (map[string]bool, error) { - return map[string]bool{"anchor": true, "laptop": true}, nil - } + releaseHeard = func(context.Context, *stores) (map[string]bool, error) { return w.heard, nil } n := 0 sendRollout = func(ctx context.Context, open *stores, names []string) ([]string, error) { w.sent = append(w.sent, append([]string(nil), names...)) + w.kept = append(w.kept, len(names) == 1 && settingsKeptOn(ctx, names[0])) current, err := open.inventory.CurrentBuilds(ctx) if err != nil { return nil, err @@ -258,6 +271,7 @@ func aSettledMesh(t *testing.T) *walkMesh { gateSettle, gateEvery = 0, time.Hour t.Cleanup(func() { machineMoves, releaseHeard, sendRollout, gatherGateFacts = wasMoves, wasHeard, wasSend, wasGather + settingsDataMoves = wasData gateSettle, gateEvery, gateBound = wasSettle, wasEvery, wasBound }) return w @@ -325,6 +339,9 @@ func TestAFailedSettingsWalkStopsAndHoldsNothingBack(t *testing.T) { ctx := t.Context() inv := w.open.inventory w.broken["anchor"] = true + if err := inv.KeepSettings(ctx, "", "app", map[string]any{"x": "1"}); err != nil { + t.Fatal(err) + } if _, err := openSettingsWalk(ctx, w.open, inventory.PlanSettings{Module: "app", Ask: "s2"}); err != nil { t.Fatal(err) } @@ -340,6 +357,9 @@ func TestAFailedSettingsWalkStopsAndHoldsNothingBack(t *testing.T) { if !reflect.DeepEqual(w.sent, [][]string{{"anchor"}}) { t.Fatalf("sent %v: anchor alone, nothing to laptop", w.sent) } + if layer, has, err := inv.Layer(ctx, "", "app"); err != nil || !has || layer["x"] != "1" { + t.Fatalf("the layer after the failure: %v %v %v", layer, has, err) + } // A build that waits for a gate is still walked: the failed settings walk holds nothing back. w.broken["anchor"] = false now := time.Now().Add(-time.Minute) @@ -357,3 +377,88 @@ func TestAFailedSettingsWalkStopsAndHoldsNothingBack(t *testing.T) { t.Fatalf("the waiting build is held back by a failed settings walk: %+v %v", r, err) } } + +// Each send of a settings walk keeps every build of its machine: the layer goes, and a build held for a person or +// waiting for a gate stays where it is (ADR 0344 §3). +func TestASettingsWalkSendsTheLayerAloneEveryBuildKept(t *testing.T) { + w := aSettledMesh(t) + ctx := t.Context() + inv := w.open.inventory + gateEvery = 0 + if _, err := openSettingsWalk(ctx, w.open, inventory.PlanSettings{Module: "app", Ask: "s3"}); err != nil { + t.Fatal(err) + } + for i := 0; i < 4; i++ { + advancePlans(ctx, w.open) + } + if !reflect.DeepEqual(w.kept, []bool{true, true}) || w.walk(t).State != inventory.PlanDone { + t.Fatalf("the sends kept every build: %v; the walk: %s", w.kept, w.walk(t).Note) + } + // What such a send keeps, on the store: a rolled-out build waiting there is kept at the one last sent. + now := time.Now().Add(-time.Minute) + manifest, _ := json.Marshal(catalogue.Manifest{Module: "app", Version: "1"}) + if err := inv.RecordBuild(ctx, inventory.Build{ID: "build-app-c2", Module: "app", Commit: "c2", + Repository: "novox/mesh-catalog", Path: "modules/app", Manifest: manifest, Asked: now, At: now, + Made: []inventory.Artifact{{Name: "x", Kind: "bundle", Reference: "sha256:app-2"}}}); err != nil { + t.Fatal(err) + } + if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "app", Version: "1"}, inventory.Source{ + Repository: "novox/mesh-catalog", Seat: "git", Path: "modules/app", BuiltFrom: "c2", Head: "c2", Asked: now}); err != nil { + t.Fatal(err) + } + kept, err := recordedKept(keepingBuildsForSettings(ctx, "anchor"), w.open, "anchor") + if err != nil || kept["app"] != "c1" { + t.Fatalf("a settings send to anchor keeps %v (%v), want app at c1", kept, err) + } + if kept, _ := recordedKept(ctx, w.open, "anchor"); kept["app"] != "" { + t.Fatalf("an ordinary send keeps a rolled-out build: %v", kept) + } +} + +// A machine whose send would move a running module's data stops the walk there, failed and said with the push that +// sends it on a person's word (ADR 0217 §3); one not heard from is passed over and named; and a walk that reached +// no machine is not said delivered. +func TestASettingsWalkStopsBeforeADataMoveAndPassesOverAMachineAway(t *testing.T) { + w := aSettledMesh(t) + ctx := t.Context() + gateEvery = 0 + w.moves["laptop"] = []dataMove{{Container: "app-server", Inside: "/data", From: "/srv/a", To: "/srv/b"}} + if _, err := openSettingsWalk(ctx, w.open, inventory.PlanSettings{Module: "app", Ask: "s4"}); err != nil { + t.Fatal(err) + } + for i := 0; i < 4; i++ { + advancePlans(ctx, w.open) + } + p := w.walk(t) + if p.State != inventory.PlanFailed || !reflect.DeepEqual(w.sent, [][]string{{"anchor"}}) || + !strings.Contains(p.Note, "not sent to laptop: a running module's data would move there") || + !strings.Contains(p.Note, "push laptop --move app") { + t.Fatalf("sent %v; the walk is %s: %s", w.sent, p.State, p.Note) + } + + v := aSettledMesh(t) + v.heard = map[string]bool{"laptop": true} + gateEvery = 0 + if _, err := openSettingsWalk(ctx, v.open, inventory.PlanSettings{Module: "app", Ask: "s5"}); err != nil { + t.Fatal(err) + } + for i := 0; i < 4; i++ { + advancePlans(ctx, v.open) + } + p = v.walk(t) + if p.State != inventory.PlanDone || !reflect.DeepEqual(p.Release.Skipped, []string{"anchor"}) || + !strings.Contains(p.Note, "delivered on laptop; not heard from, left as they were, their next send carries it: anchor") { + t.Fatalf("the walk with anchor away is %s: %s", p.State, p.Note) + } + + u := aSettledMesh(t) + u.heard = map[string]bool{} + if _, err := openSettingsWalk(ctx, u.open, inventory.PlanSettings{Module: "app", Ask: "s6"}); err != nil { + t.Fatal(err) + } + advancePlans(ctx, u.open) + if p := u.walk(t); p.State != inventory.PlanFailed || p.Outcome() != inventory.WalkFailed || + !strings.Contains(p.Note, "was sent to no machine") || len(u.sent) != 0 { + t.Fatalf("a walk that reached nobody is %s: %s", p.State, p.Note) + } +}