diff --git a/cmd/mesh-controller/recorded_kept.go b/cmd/mesh-controller/recorded_kept.go index 2b65aa4b..bc9560ff 100644 --- a/cmd/mesh-controller/recorded_kept.go +++ b/cmd/mesh-controller/recorded_kept.go @@ -2,6 +2,7 @@ package main import ( "context" + "errors" "fmt" "sort" @@ -93,6 +94,11 @@ func keepingBuildsForSettings(ctx context.Context, node string) context.Context return context.WithValue(ctx, keepForSettingsKey{}, node) } +// errSettingsCannotKeep is a settings walk's send refused because a build of its machine cannot be kept: what the +// machine was last sent is not known, or a build it runs is gone from the records. It does not clear by itself, so +// the walk fails on it rather than trying again (ADR 0344). +var errSettingsCannotKeep = errors.New("the settings cannot be sent with every build kept") + // 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) @@ -123,8 +129,7 @@ func recordedKept(ctx context.Context, open *stores, node string) (map[string]st } 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) + return nil, fmt.Errorf("%w: what %s was last sent is not known (novox/hq ADR 0344)", errSettingsCannotKeep, node) } if every { return nil, fmt.Errorf("what %s was last sent is not known, so the bus's user list cannot be sent "+ @@ -187,8 +192,8 @@ func keepRecorded(ctx context.Context, open *stores, node string, shelf map[stri 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])) + return nil, fmt.Errorf("%w: %s runs %s's build %s, which the build records no longer hold (novox/hq ADR 0344)", + errSettingsCannotKeep, 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 "+ diff --git a/cmd/mesh-controller/settings_walk.go b/cmd/mesh-controller/settings_walk.go index cd756b3e..8c5acb3b 100644 --- a/cmd/mesh-controller/settings_walk.go +++ b/cmd/mesh-controller/settings_walk.go @@ -2,6 +2,7 @@ package main import ( "context" + "errors" "fmt" "slices" "sort" @@ -179,26 +180,37 @@ func advanceSettingsWalk(ctx context.Context, open *stores, p *inventory.Plan) ( } // **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 { + held, err := settingsDataMoves(ctx, open, node) + if errors.Is(err, errSettingsCannotKeep) { + return failBefore(p, node, err.Error()) + } + if err != nil { return false, fmt.Errorf("whether %s's data would move could not be read: %w", node, err) - } else if len(held) > 0 { + } + 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 + return failBefore(p, node, "a running module's data would move there — "+ + strings.Join(strings.Fields(said.String()), " ")) } sent, grants, err := sendJudged(keepingBuildsForSettings(ctx, node), open, node) + if errors.Is(err, errSettingsCannotKeep) { + // It does not clear by itself: tried again on every tick, the walk would stay open and hold the walk + // of waiting builds until a person came. + return failBefore(p, node, err.Error()) + } if err != nil { return false, fmt.Errorf("sending %s %s: %w", node, settingsWalkFor(*s), err) } + // **The module's build is the one the gate judges** (issue 641 review): kept by this send, it is what a + // crossing send — a merge's walk sending the machine a new build of the module — supersedes, so this gate + // never reads that send's report as its own. + kept := "" + if builds, known, err := open.inventory.SentBuilds(ctx, node); err == nil && known { + kept = builds[s.Module] + } r.Gate = &inventory.PlanGate{Component: coreComponent(s.Module), Machines: sent, Since: &now, - Sent: sentNow(ctx, open.inventory, sent), Grants: grants} + From: kept, To: kept, Sent: sentNow(ctx, open.inventory, sent), Grants: grants} 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 != "" { @@ -243,10 +255,30 @@ func advanceSettingsWalk(ctx context.Context, open *stores, p *inventory.Plan) ( } 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, ", ") - } + "replaced is in `settings %s` history", settingsWalkFor(*s), node, g.Why, s.Module) + unreached(r) fmt.Printf("%s: %s\n", p.ID, p.Note) return true, nil } + +// failBefore ends a settings walk failed before it sent a machine, saying why, and that the layer stays. +func failBefore(p *inventory.Plan, node, why string) (bool, error) { + r, s := p.Release, p.Release.Settings + p.State = inventory.PlanFailed + p.Note = fmt.Sprintf("%s not sent to %s: %s; the layer stays as the operator approved it", settingsWalkFor(*s), + node, why) + unreached(r) + fmt.Printf("%s: %s\n", p.ID, p.Note) + return true, nil +} + +// unreached is what a stopped settings walk left without the layer, in words: the machines after the one it stopped +// at, and those passed over as away (ADR 0344 §6). Empty when there are none. +func unreached(r *inventory.PlanRelease) string { + var out string + if r.Next+1 < len(r.Order) { + out += "; not sent: " + strings.Join(r.Order[r.Next+1:], ", ") + } + if len(r.Skipped) > 0 { + out += "; not heard from, left as they were: " + strings.Join(r.Skipped, ", ") + } + return out +} diff --git a/cmd/mesh-controller/settings_walk_test.go b/cmd/mesh-controller/settings_walk_test.go index 72107bfc..f8cca40a 100644 --- a/cmd/mesh-controller/settings_walk_test.go +++ b/cmd/mesh-controller/settings_walk_test.go @@ -3,6 +3,7 @@ package main import ( "context" "encoding/json" + "errors" "fmt" "reflect" "strings" @@ -474,3 +475,91 @@ func TestASettingsWalkStopsBeforeADataMoveAndPassesOverAMachineAway(t *testing.T t.Fatalf("a walk that reached nobody is %s: %s", p.State, p.Note) } } + +// The gate keeps the module's build its send kept, so a crossing send — a merge's walk sending the machine a new +// build of the module while the settings gate judges there — supersedes it, and its report is never read as the +// settings send's (issue 641 review, finding 1). +func TestACrossingSendSupersedesASettingsGate(t *testing.T) { + judgeLeavesAsTheCore(t) + w := aSettledMesh(t) + ctx := t.Context() + inv := w.open.inventory + if _, err := openSettingsWalk(ctx, w.open, inventory.PlanSettings{Module: "app", Ask: "s7"}); err != nil { + t.Fatal(err) + } + advancePlans(ctx, w.open) + p := w.walk(t) + if g := p.Release.Gate; g == nil || g.To != "c1" || g.From != "c1" { + t.Fatalf("the settings gate judges no build: %+v", p.Release.Gate) + } + // Another walk sends anchor app's c2 while the settings gate judges there. + 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.RecordSent(ctx, nodeID(t, w.open, "anchor"), "d-merge", map[string]string{"app": "c2"}); err != nil { + t.Fatal(err) + } + if _, err := inv.RecordDoing(ctx, nodeID(t, w.open, "anchor"), inventory.Doing{Node: "anchor", + Outcome: inventory.OutcomeApplied, Declared: "d-merge", Applied: 1, At: time.Now()}); err != nil { + t.Fatal(err) + } + gateEvery = 0 + advancePlans(ctx, w.open) + p = w.walk(t) + if p.State != inventory.PlanSuperseded || p.Release.Gate == nil || p.Release.Gate.Verdict != inventory.GateSuperseded || + !strings.Contains(p.Note, "superseded on anchor") || len(p.Release.Done) != 0 { + t.Fatalf("the settings walk after a crossing send is %s: %s %+v", p.State, p.Note, p.Release) + } + if !reflect.DeepEqual(w.sent, [][]string{{"anchor"}}) { + t.Fatalf("sent %v: laptop was sent after a superseded judging", w.sent) + } +} + +// A send refusal that cannot clear by itself — what the machine was last sent is not known, or a build it runs is +// gone — fails the walk with why, rather than keeping it open and holding the walk of waiting builds; and a +// failure names the machines passed over as away (issue 641 review, findings 2 and 3). +func TestASettingsWalkFailsOnARefusalThatCannotClear(t *testing.T) { + w := aSettledMesh(t) + ctx := t.Context() + inv := w.open.inventory + // What a send to a machine never sent can keep: nothing, and said as the refusal the walk fails on. + if _, err := inv.AddNode(ctx, "fresh"); err != nil { + t.Fatal(err) + } + if _, err := recordedKept(keepingBuildsForSettings(ctx, "fresh"), w.open, "fresh"); !errors.Is(err, errSettingsCannotKeep) { + t.Fatalf("a settings send to a machine never sent: %v", err) + } + w.heard = map[string]bool{"laptop": true} + settingsDataMoves = func(_ context.Context, _ *stores, node string) ([]dataMove, error) { + return nil, fmt.Errorf("%w: what %s was last sent is not known", errSettingsCannotKeep, node) + } + if _, err := openSettingsWalk(ctx, w.open, inventory.PlanSettings{Module: "app", Ask: "s8"}); err != nil { + t.Fatal(err) + } + advancePlans(ctx, w.open) + p := w.walk(t) + if p.State != inventory.PlanFailed || len(w.sent) != 0 || + !strings.Contains(p.Note, "not sent to laptop: the settings cannot be sent with every build kept: what laptop was last sent is not known") || + !strings.Contains(p.Note, "not heard from, left as they were: anchor") { + t.Fatalf("the walk is %s: %s", p.State, p.Note) + } + // It is not open, so a build waiting for a gate is walked. + 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) + } + if r, err := releaseBacklog(ctx, w.open, ""); err != nil || r == nil { + t.Fatalf("the waiting build is held by the failed settings walk: %+v %v", r, err) + } +}