diff --git a/cmd/mesh-host/nodeagent.go b/cmd/mesh-host/nodeagent.go index 09e9b31..b30421e 100644 --- a/cmd/mesh-host/nodeagent.go +++ b/cmd/mesh-host/nodeagent.go @@ -123,8 +123,8 @@ func (h sessionHaves) Write(ctx context.Context, x agent.Have) { Sealed: x.Sealed, Server: x.Server}) } -func (h sessionHaves) Removed(ctx context.Context, module string) { - h.session.NoteRemoved(ctx, module, "") +func (h sessionHaves) Removed(ctx context.Context, module string, words ...string) { + h.session.NoteRemoved(ctx, module, "", words...) } // shorten keeps a have's detail within what a have value holds: its head, which says what failed, and its tail, which is @@ -564,9 +564,9 @@ func nodeAgent(ctx context.Context, opts options, signer ed25519.PublicKey, trus sayFacts(ctx, ag) return res }, - Remove: func(ctx context.Context, module string, others []delivery.Composed) agent.Result { + Remove: func(ctx context.Context, module string, others []delivery.Composed, keep map[string]bool) agent.Result { actx, done := applyContext(session.WithLink(ctx)) - res := removeComposition(actx, opts, module, others, ag.Placed(module), sched, say) + res := removeComposition(actx, opts, module, others, ag.Placed(module), keep, sched, say) done() afterApply() sayFacts(ctx, ag) @@ -732,13 +732,13 @@ func applyComposition(ctx context.Context, opts options, c delivery.Composed, ot if got.Probe != nil { return failedWith(got.Probe.partial().Error()) } - return agent.Result{} + return agent.Result{Kept: got.Outcome.Kept} } // removeComposition removes one module's own resources and nothing of another's. -func removeComposition(ctx context.Context, opts options, module string, others []delivery.Composed, placed map[string]bool, +func removeComposition(ctx context.Context, opts options, module string, others []delivery.Composed, placed, keep map[string]bool, sched *apply.Scheduler, say link.Announce) agent.Result { - scope := apply.Scope{Module: module, Others: resourcesOf(others), Placed: placed} + scope := apply.Scope{Module: module, Others: resourcesOf(others), Placed: placed, Keep: keep} got := applyModuleHeld(ctx, opts, &declaration.Declaration{Version: declaration.Version}, &scope, sched, say) switch { case got.Refused != "": @@ -746,7 +746,7 @@ func removeComposition(ctx context.Context, opts options, module string, others case got.Err != nil: return agent.Result{Failed: true, Detail: shorten(got.Err.Error())} } - return agent.Result{} + return agent.Result{Kept: got.Outcome.Kept} } // busyModules are the modules whose last apply left a container running because it said busy. diff --git a/internal/agent/agent.go b/internal/agent/agent.go index a6d9892..aebc25d 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -77,8 +77,9 @@ type Haves interface { Read(ctx context.Context, module string) (Have, bool) // Write writes a module's have where it differs from the bucket's. Write(ctx context.Context, h Have) - // Removed writes the module's have with the phase removed, once, if a key of it exists. - Removed(ctx context.Context, module string) + // Removed writes the module's have with the phase removed, once, if a key of it exists; words, when given, are its + // detail. + Removed(ctx context.Context, module string, words ...string) } // Result is how one apply or removal came out. @@ -88,6 +89,8 @@ type Result struct { // JetStream API error while the bus reloads), not the module's: the apply is tried again without a new revision. Transient bool Detail string + // Kept is the resources of the keep set the apply left as they ran, by id (apply.Report.Kept, novox/hq ADR 0594 §4). + Kept []string } // Backup is the answer of the backup asked before a module moves. @@ -117,8 +120,9 @@ type Env struct { // resources (catalogue.BusServerOf, novox/hq ADR 0594 §7), empty for any other module; known says whether the // module's manifest could be read. Nil names no bus module. BusServer func(ctx context.Context, module string) (ids []string, known bool) - // Remove removes the module's own resources and nothing of another's. - Remove func(ctx context.Context, module string, others []delivery.Composed) Result + // Remove removes the module's own resources and nothing of another's, and leaves those of keep as they run (the bus + // server's, ADR 0594 §2). + Remove func(ctx context.Context, module string, others []delivery.Composed, keep map[string]bool) Result // Backup backs a module up before a new composition of it is applied; nil backs up nothing. Backup func(ctx context.Context, module string) Backup // Kept is the resources the node last applied, by module, from the declaration it kept before the switch (ADR 0539 @@ -612,6 +616,10 @@ func (a *Agent) writeHave(ctx context.Context, h Have) { // serverOf is what the module's have says the bus server runs (Have.Server): kept since this agent read or wrote it, // else read from the have once. Empty for every module but the bus. +// +// A have written before this node agent kept the server names none: the server then runs what that have says the module +// runs, its fingerprint, else its old-form build (the take-over of a server the node's own composition made). Read so on +// the first read, before this agent writes the have, so every have it writes carries it. func (a *Agent) serverOf(ctx context.Context, module string) string { a.mu.Lock() s, known := a.server[module] @@ -619,8 +627,17 @@ func (a *Agent) serverOf(ctx context.Context, module string) string { if known { return s } + if server, _ := a.busKeep(ctx, module); len(server) == 0 { + return "" // not the bus, or not known to be yet: nothing kept, so a later read asks again + } if h, ok := a.env.Haves.Read(ctx, module); ok { s = h.Server + if s == "" { + s = h.FP + } + if s == "" { + s = h.Build + } } a.mu.Lock() if _, written := a.server[module]; !written { @@ -1496,14 +1513,6 @@ func (a *Agent) apply(ctx context.Context, module string, w wantNow, comp compNo ranServer, madeServer := "", "" if len(keep) > 0 { ranServer = a.serverOf(ctx, module) - if ranServer == "" { - if h, ok := a.env.Haves.Read(ctx, module); ok { - ranServer = h.FP - if ranServer == "" { - ranServer = h.Build - } - } - } if ranServer == "" { ranServer = fp // a bus module this node never ran: its server is made at this fingerprint } @@ -1606,6 +1615,10 @@ func (a *Agent) apply(ctx context.Context, module string, w wantNow, comp compNo if a.env.Settled != nil { a.env.Settled(module) } + if len(keep) > 0 && len(res.Kept) == 0 { + // Nothing of the server was there to keep: the apply made it at this fingerprint (ADR 0594 §4). + madeServer = fp + } a.writeHave(ctx, Have{Module: module, FP: fp, Composed: dig, Phase: string(delivery.PhaseRunning), Detail: res.Detail, Server: madeServer}) } @@ -1624,7 +1637,16 @@ func (a *Agent) remove(ctx context.Context, module string, w wantNow) { } others := a.heldOut(module) a.mu.Unlock() - res := a.env.Remove(ctx, module, others) + // A removed want of the bus module never removes its server (novox/hq ADR 0594 §2): a removal carries no warrant, and + // removing the server is not built. One whose manifest cannot be read keeps what would be the server's, said. + keep, known := a.busKeep(ctx, module) + if !known { + keep = map[string]bool{} + for _, id := range catalogue.BusServer { + keep[module+"."+id] = true + } + } + res := a.env.Remove(ctx, module, others, keep) a.mu.Lock() if res.Failed { a.attempt[module] = attempt{FP: "removed", WantRev: w.Rev} @@ -1644,6 +1666,16 @@ func (a *Agent) remove(ctx context.Context, module string, w wantNow) { if a.env.Settled != nil { a.env.Settled(module) } + if len(res.Kept) > 0 { + words := fmt.Sprintf("removed, but for %s: the bus server stays as it runs until a want carrying the operator's "+ + "warrant removes it (novox/hq ADR 0594 §2)", strings.Join(res.Kept, ", ")) + if !known { + words += "; its manifest could not be read, so what would be its server was left" + } + a.env.Say(module + " is " + words) + a.env.Haves.Removed(ctx, module, words) + return + } a.env.Haves.Removed(ctx, module) } diff --git a/internal/agent/agent_test.go b/internal/agent/agent_test.go index d65e75e..34846be 100644 --- a/internal/agent/agent_test.go +++ b/internal/agent/agent_test.go @@ -65,10 +65,11 @@ func (f *feed) put(key string, value []byte) { } type haves struct { - mu sync.Mutex - stored map[string]Have - log []Have - removed []string + mu sync.Mutex + stored map[string]Have + log []Have + removed []string + removedWords []string // unreadable is each module whose have the bus cannot read now (ReadErr). unreadable map[string]bool } @@ -99,10 +100,11 @@ func (h *haves) Write(_ context.Context, x Have) { h.stored[x.Module] = x h.log = append(h.log, x) } -func (h *haves) Removed(_ context.Context, m string) { +func (h *haves) Removed(_ context.Context, m string, words ...string) { h.mu.Lock() defer h.mu.Unlock() h.removed = append(h.removed, m) + h.removedWords = append(h.removedWords, strings.Join(words, "")) } func (h *haves) last(module string) Have { h.mu.Lock() @@ -127,7 +129,9 @@ type bed struct { kept map[string][]json.RawMessage others [][]delivery.Composed keeps []map[string]bool // what each apply was told to keep as it runs (ADR 0594 §2) - said []string + // serverGone is a bus server not on the machine: an apply keeping it keeps nothing and makes it. + serverGone bool + said []string // settled is each module Settled was told, and settledRan whether what runs held it then. settled []string settledRan []bool @@ -143,6 +147,13 @@ func newBed(t *testing.T) *bed { defer b.mu.Unlock() b.others = append(b.others, others) b.keeps = append(b.keeps, keep) + var kept []string // what of keep was there to keep: all of it, unless the test took the server away + for id := range keep { + if !b.serverGone { + kept = append(kept, id) + } + } + slices.Sort(kept) name := c.Module if name == "" { name = NodeModule @@ -151,13 +162,19 @@ func newBed(t *testing.T) *bed { if b.failWith != "" && (b.failTimes == 0 || len(b.applied) <= b.failTimes) { return Result{Failed: true, Transient: b.transient, Detail: b.failWith} } - return Result{} + return Result{Kept: kept} }, - Remove: func(_ context.Context, m string, _ []delivery.Composed) Result { + Remove: func(_ context.Context, m string, _ []delivery.Composed, keep map[string]bool) Result { b.mu.Lock() defer b.mu.Unlock() b.removed = append(b.removed, m) - return Result{} + b.keeps = append(b.keeps, keep) + var kept []string + for id := range keep { + kept = append(kept, id) + } + slices.Sort(kept) + return Result{Kept: kept} }, Backup: func(_ context.Context, m string) Backup { b.mu.Lock() @@ -1582,8 +1599,22 @@ func TestTheNodesOwnCompositionIsAppliedWhenItDiffersAndARecordMoveWaitsForAYes( return b.keeps[len(b.keeps)-1] } fpA, fpB := fpOf("a"), fpOf("b") + // A have written before the first apply (the node cannot compose yet) keeps the old build's name as what the + // server runs: the writer names no build, so it is read before the first write. + want("nats", fpA, "") + eventually(t, "the bus's want was not taken", func() bool { + b.ag.mu.Lock() + defer b.ag.mu.Unlock() + return b.ag.wants["nats"].Present + }) + b.ag.NotComposed(context.Background(), "a stand-in for the bus's secret") + if h := b.haves.last("nats"); h.Build != "" || h.Server != took { + t.Fatalf("a have written before the first apply: %+v", h) + } + b.ag.NotComposed(context.Background(), "") for i, row := range []struct { name, fp, image, users, warrant string + gone bool // the server is not on the machine: made, at the want's fingerprint applies bool keep map[string]bool server string @@ -1595,7 +1626,12 @@ func TestTheNodesOwnCompositionIsAppliedWhenItDiffersAndARecordMoveWaitsForAYes( applies: true, server: fpB}, {name: "the same warrant again", fp: fpB, image: "2", users: "u2", warrant: delivery.WarrantOperator, server: fpB}, {name: "a build after the warrant", fp: fpA, image: "3", users: "u2", applies: true, keep: server, server: fpB}, + {name: "a build whose server is not there", fp: fpOf("e"), image: "4", users: "u2", gone: true, applies: true, + keep: server, server: fpOf("e")}, } { + b.mu.Lock() + b.serverGone = row.gone + b.mu.Unlock() before := len(b.appliedList()) key, raw, dig := bus(row.fp, row.image, row.users) b.comps.put(key, raw) @@ -1645,9 +1681,28 @@ func TestTheNodesOwnCompositionIsAppliedWhenItDiffersAndARecordMoveWaitsForAYes( } } time.Sleep(100 * time.Millisecond) - if got := b.appliedList(); len(got) != 7 { + if got := b.appliedList(); len(got) != 8 { t.Errorf("applied %v, not once for each change", got) } + // A removed want removes the rest of the bus and never its server: it is kept, said in the journal and in the + // removed have's words. + rkey, _ := delivery.WantKey(node, "nats") + rraw, _ := delivery.EncodeWant(delivery.Want{Removed: true}) + b.wants.put(rkey, rraw) + eventually(t, "the bus's removal was not asked", func() bool { + b.haves.mu.Lock() + defer b.haves.mu.Unlock() + return slices.Contains(b.haves.removed, "nats") + }) + if got := lastKeep(); !maps.Equal(got, server) { + t.Errorf("the bus's removal kept %v, not its server %v", got, server) + } + b.haves.mu.Lock() + words := b.haves.removedWords[len(b.haves.removedWords)-1] + b.haves.mu.Unlock() + if !strings.Contains(words, "the bus server stays") || !strings.Contains(words, "nats.server") { + t.Errorf("the removed have does not say the server stays: %q", words) + } }) } diff --git a/internal/apply/apply.go b/internal/apply/apply.go index ff94eda..c1027f0 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -120,6 +120,9 @@ type Report struct { MovedOut []MovedOut `json:"moved-out,omitempty"` // Left is every socket, pipe or device a move-out directory holds and did not move (ADR 0422 §4). Left []LeftInPlace `json:"left,omitempty"` + // Kept is every resource the scope keeps (Scope.Keep) that this apply left as it runs, by id (novox/hq ADR 0594 + // §2, §4): what says whether the bus server still runs what it ran, or was made by this apply. + Kept []string `json:"kept,omitempty"` } // Waiting says whether a container of this apply waits to be replaced because it is busy: what makes the @@ -416,6 +419,11 @@ func applyScoped( if err := refuseRootIdentities(d); err != nil { return report, known, &Error{Resource: "the declaration's identities", Err: err, Done: report} } + // What of the bus server is kept as it runs, asked of the runtime before anything moves (ADR 0594 §2). + keepNow, err := keptServer(ctx, d, scope, run) + if err != nil { + return report, known, &Error{Resource: "the bus server", Err: err, Done: report} + } declared := map[string]bool{} for _, r := range d.Resources { @@ -544,6 +552,17 @@ func applyScoped( removeOrphan := func(orphan store.Applied) error { var action, detail string var err error + if keepNow[orphan.ID] { + // A removed want of the bus module removes the rest of it and never its server (ADR 0594 §2): only a + // warranted want replaces or removes the server. Its record stays, the module's. + detail := "no longer declared; the bus server is left as it runs: only a want carrying the operator's " + + "warrant removes it (novox/hq ADR 0594 §2)" + report.Outcomes = append(report.Outcomes, Outcome{ID: orphan.ID, Type: orphan.Type, Target: orphan.Target, + Action: keptAsItRuns, Detail: detail}) + report.Kept = append(report.Kept, orphan.ID) + log(fmt.Sprintf(" %s %s (%s): %s", keptAsItRuns, orphan.ID, orphan.Target, detail)) + return nil + } if module, held := heldByOthers[orphan.ID]; held && !declared[orphan.ID] { if module == "" { module = "another module" @@ -1071,8 +1090,9 @@ func applyScoped( if from, handed := handedFiles[resource.Identity()]; handed && was.ID == "" { previous.Wrote = from.Wrote } - if o, kept := keepAsItRuns(ctx, scope, resource, run, previous); kept { + if o, kept := keepAsItRuns(keepNow, resource, previous); kept { outcome = o + report.Kept = append(report.Kept, o.ID) } else { outcome, err = applyOne(ctx, sys, resource, run, changed, in, previous, unseal, keepFound) } @@ -3041,8 +3061,8 @@ const keptAsItRuns = "kept-as-it-runs" // unpacked at its path before. The outcome carries what the resource was last recorded reading and holding, so its // record is the module's from here on (taken over from the node's own composition at the bus module's first want) and // a later warranted apply still sees what changed under it. -func keepAsItRuns(ctx context.Context, scope *Scope, r declaration.Resource, run Runner, previous store.Applied) (Outcome, bool) { - if scope == nil || !scope.Keep[r.Identity()] { +func keepAsItRuns(keep map[string]bool, r declaration.Resource, previous store.Applied) (Outcome, bool) { + if !keep[r.Identity()] { return Outcome{}, false } out := begin(r) @@ -3053,9 +3073,7 @@ func keepAsItRuns(ctx context.Context, scope *Scope, r declaration.Resource, run if res.Schedule != "" || res.RunOnce { return Outcome{}, false } - if _, err := containerState(ctx, res.Name, run); err != nil { - return Outcome{}, false - } + // Found running by keptServer before the loop: it is left whatever its declaration says now. out.reads = previous.Reads out.busySince = previous.BusySince out.Detail = why @@ -3079,6 +3097,36 @@ func keepAsItRuns(ctx context.Context, scope *Scope, r declaration.Resource, run return Outcome{}, false } +// keptServer is what of the scope's Keep this apply keeps (novox/hq ADR 0594 §2), decided once before anything is +// applied, as the archives that restart the server come before it: every kept container the declaration names is +// asked of the runtime once. All found: the Keep stands. One the runtime says is not there: nothing is kept, so the +// server is made at what the declaration says now and its archives are applied with it. A runtime that does not +// answer, or answers anything else, is an error, never taken as an absent server: replacing a running bus server +// because one inspect failed is the harm the Keep exists to prevent. +func keptServer(ctx context.Context, d *declaration.Declaration, scope *Scope, run Runner) (map[string]bool, error) { + if scope == nil || len(scope.Keep) == 0 { + return nil, nil + } + for _, r := range d.Resources { + c, ok := r.(*declaration.Container) + if !ok || !scope.Keep[r.Identity()] || c.Schedule != "" || c.RunOnce { + continue + } + cri, err := containerRuntime(ctx, run) + if err != nil { + return nil, fmt.Errorf("%w, so whether the bus server %s runs cannot be told; nothing was applied", err, c.Name) + } + if _, err := run(ctx, cri, "container", "inspect", "--format", "{{.State.Running}}", c.Name); err != nil { + if absent(err) { + return nil, nil // made, at what the declaration says now + } + return nil, fmt.Errorf("the container runtime did not say whether the bus server %s runs (%v); nothing was "+ + "applied, so it is not replaced on an unanswered look", c.Name, err) + } + } + return scope.Keep, nil +} + // heldStill is the action of a container left alone because a maintenance window holds it stopped // (novox/hq issue 224). Its own word rather than "unchanged": a person reading a machine that looks // half-stopped needs to see why, and "unchanged" beside a stopped server says the opposite. diff --git a/internal/apply/module_scope_test.go b/internal/apply/module_scope_test.go index 4ac1aee..9f60560 100644 --- a/internal/apply/module_scope_test.go +++ b/internal/apply/module_scope_test.go @@ -2,9 +2,10 @@ package apply import ( "context" - "fmt" + "errors" "os" "path/filepath" + "slices" "strings" "testing" @@ -225,8 +226,24 @@ func TestAResourceThatMovedToItsModulesCompositionIsLeftByTheNodesOwnApply(t *te // while the bus was folded. Kept (a want without the warrant's mark), the server's container and its program archive // are left as they run, whatever the build changed in them, and become nats's; its files and the tools' account file // are written. Not kept, the same apply replaces the container. - for _, keep := range []bool{true, false} { - t.Run(fmt.Sprintf("the bus server kept %v", keep), func(t *testing.T) { + // Review of 774, round 1: the keep is decided once before anything moves, by the runtime's answer for the server. A + // server not there is made and its archives applied with it, none said kept; a runtime that does not answer fails the + // apply, nothing written and nothing replaced; a removal of the bus removes the rest and leaves the server. + for _, tc := range []struct { + name string + keep bool + gone bool // the server is not on the machine + mute bool // the runtime does not answer the server's inspect + remove bool // the bus module's want is removed + }{ + {name: "kept", keep: true}, + {name: "not kept"}, + {name: "kept, the server not there", keep: true, gone: true}, + {name: "kept, the runtime does not answer", keep: true, mute: true}, + {name: "kept, the bus removed", keep: true, remove: true}, + } { + t.Run("the bus server "+tc.name, func(t *testing.T) { + keep := tc.keep && !tc.gone dir := t.TempDir() conf, account, program := filepath.Join(dir, "nats.conf"), filepath.Join(dir, "tools", "account"), filepath.Join(dir, "program") bus := func(env, confBody, programBody, module string) *declaration.Declaration { @@ -250,29 +267,54 @@ func TestAResourceThatMovedToItsModulesCompositionIsLeftByTheNodesOwnApply(t *te if made == nil { t.Fatalf("the node's own made no server: %v", m.asked) } + if tc.gone { + delete(m.containers, "mesh-broker-nats") + } m.asked = nil + run := m.run + if tc.mute { + run = func(ctx context.Context, name string, args ...string) (string, error) { + if name == "docker" && len(args) > 1 && args[0] == "container" && args[1] == "inspect" { + m.asked = append(m.asked, "docker "+strings.Join(args, " ")) + return "", errors.New("Cannot connect to the Docker daemon at unix:///var/run/docker.sock") + } + return m.run(ctx, name, args...) + } + } next := bus("2", "b", "two", "nats") next.Resources = append(next.Resources, declare(t, `{"id":"nats.tools-account","type":"file","path":"`+account+ `","content":"tools","module":"nats"}`).Resources...) scope := Scope{Module: "nats", Placed: map[string]bool{"nats": true}} - if keep { + if tc.keep { scope.Keep = map[string]bool{"nats.server": true, "nats.program-snapshot": true} } + moved := func() []string { + var out []string + for _, a := range m.asked { + for _, verb := range []string{"docker rm", "docker run", "docker create", "docker stop", "docker restart", "docker rename"} { + if strings.HasPrefix(a, verb+" ") { + out = append(out, a) + } + } + } + return out + } report, after, err := ApplyModule(context.Background(), archHost(t), next, scope, state, - store.OriginDeclared, m.run, nil, nil, nil, nil) + store.OriginDeclared, run, nil, nil, nil, nil) + if tc.mute { + if err == nil || !strings.Contains(err.Error(), "did not say whether the bus server") { + t.Fatalf("an unanswered inspect did not fail the apply: %v", err) + } + if got, _ := os.ReadFile(conf); string(got) != "a" || len(moved()) > 0 { + t.Errorf("an apply that could not tell whether the server runs wrote %q and moved %v", got, moved()) + } + return + } if err != nil { t.Fatal(err) } - var moved []string - for _, a := range m.asked { - for _, verb := range []string{"docker rm", "docker run", "docker create", "docker stop", "docker restart"} { - if strings.HasPrefix(a, verb+" ") { - moved = append(moved, a) - } - } - } - if keep == (len(moved) > 0) { - t.Fatalf("kept %v, and the server was moved by %v", keep, moved) + if keep == (len(moved()) > 0) { + t.Fatalf("kept %v, and the server was moved by %v", keep, moved()) } if got, _ := os.ReadFile(conf); string(got) != "b" { t.Errorf("the server's file holds %q", got) @@ -283,7 +325,8 @@ func TestAResourceThatMovedToItsModulesCompositionIsLeftByTheNodesOwnApply(t *te if got, _ := os.ReadFile(filepath.Join(program, "snapshot")); keep != (string(got) == "one") { t.Errorf("kept %v, and the server's program is %q", keep, got) } - for _, id := range []string{"nats.server", "nats.program-snapshot"} { + var kept []string + for _, id := range []string{"nats.program-snapshot", "nats.server"} { rec, ok := after.Find(id) if !ok || rec.Module != "nats" { t.Errorf("%s is recorded %+v, not as nats's", id, rec) @@ -291,10 +334,35 @@ func TestAResourceThatMovedToItsModulesCompositionIsLeftByTheNodesOwnApply(t *te if o := outcomeOf(report, id); keep != (o.Action == keptAsItRuns) { t.Errorf("kept %v, and %s was %s: %s", keep, id, o.Action, o.Detail) } + if keep { + kept = append(kept, id) + } } - if keep && report.Changed() == false { + if !slices.Equal(report.Kept, kept) && !(len(kept) == 0 && len(report.Kept) == 0) { + t.Errorf("the report says it kept %v, want %v", report.Kept, kept) + } + if tc.keep && report.Changed() == false { t.Error("an apply that wrote the server's files says nothing changed") } + if !tc.remove { + return + } + m.asked = nil + gone, last, err := ApplyModule(context.Background(), archHost(t), &declaration.Declaration{}, scope, after, + store.OriginDeclared, run, nil, nil, nil, nil) + if err != nil { + t.Fatal(err) + } + if len(moved()) > 0 || m.containers["mesh-broker-nats"] == nil || !exists(filepath.Join(program, "snapshot")) { + t.Errorf("the bus's removal removed its server: %v", moved()) + } + if exists(conf) || exists(account) { + t.Errorf("the bus's removal left the rest of it: conf %v, account %v", exists(conf), exists(account)) + } + if _, ok := last.Find("nats.server"); !ok || !slices.Equal(gone.Kept, []string{"nats.program-snapshot", "nats.server"}) && + !slices.Equal(gone.Kept, []string{"nats.server", "nats.program-snapshot"}) { + t.Errorf("the removal kept %v, the server recorded %v", gone.Kept, ok) + } }) } } diff --git a/internal/link/have.go b/internal/link/have.go index 46b017c..79c3820 100644 --- a/internal/link/have.go +++ b/internal/link/have.go @@ -59,6 +59,8 @@ type haveState struct { // nodeKeeps is what the node agent keeps as it runs on a want without the warrant's mark (HaveNode.Keeps, novox/hq // ADR 0594 §8), written into its key. nodeKeeps []string + // removedWords is, by module in removed, what its removed have says when it is more than that it was removed. + removedWords map[string]string // written is the build of each module this node-engine wrote a have key for and that the declaration it applied // still holds; removed is each such module the latest declaration no longer holds, with the build its key last // said, until the key is written once with the phase removed (issue 1369, novox/hq ADR 0408 §4: never deleted). @@ -508,6 +510,9 @@ func (q *Session) writeRemoved(ctx context.Context, b HaveBucket, node string) b } h := delivery.Have{V: delivery.Version, Build: s.removed[module], Phase: delivery.PhaseRemoved, Health: delivery.HealthUnknown, Detail: "the applied declaration no longer holds the module"} + if words := s.removedWords[module]; words != "" { + h.Detail = words + } raw, err := delivery.EncodeHave(h) if err != nil { q.sayHave(fmt.Sprintf("cannot encode %s: %v", key, err)) @@ -699,13 +704,23 @@ func (q *Session) NoteUserList(ctx context.Context, module string, w delivery.Us // NoteRemoved takes that a module's own resources were removed, and writes its have with the phase removed once (issue // 1369): the key is never deleted. -func (q *Session) NoteRemoved(ctx context.Context, module, build string) { +// +// words, when given, are the removed have's detail (the bus server left as it runs, novox/hq ADR 0594 §2). +func (q *Session) NoteRemoved(ctx context.Context, module, build string, words ...string) { q.init() s := &q.haves s.mu.Lock() if s.removed == nil { s.removed = map[string]string{} } + if said := strings.Join(words, ""); said != "" { + if s.removedWords == nil { + s.removedWords = map[string]string{} + } + s.removedWords[module] = said + } else { + delete(s.removedWords, module) + } if f, ok := s.modules[module]; ok && build == "" { build = f.Build } diff --git a/internal/link/have_test.go b/internal/link/have_test.go index f6fc2c7..e681bf9 100644 --- a/internal/link/have_test.go +++ b/internal/link/have_test.go @@ -586,6 +586,12 @@ func TestAnAppliedModulesHaveNamesTheFingerprintAndTheDigestAndFollowsItsHealth( if got := l.have(t, "n1.nats"); got.FP != fp || got.Server != "build-1792674595610176744" { t.Fatalf("the bus's have: %+v", got) } + // Its removal leaves the server, and its removed have says so (review of 774, round 1). + q.NoteRemoved(context.Background(), "nats", "", "removed, but for nats.server: the bus server stays as it runs") + if got := l.have(t, "n1.nats"); got.Phase != delivery.PhaseRemoved || !strings.Contains(got.Detail, "the bus server stays") { + t.Fatalf("the bus's removed have: %+v", got) + } + q.NoteModule(context.Background(), ModuleFact{Module: "nats", FP: fp, Composed: dig, Server: "build-1792674595610176744"}) q.NoteComposes(context.Background(), delivery.ComposesOwn, "it composes its own", delivery.KeepsBusServer) if n, err := delivery.DecodeHaveNode(l.kv["n1.node"].value); err != nil || !slices.Equal(n.Keeps, []string{delivery.KeepsBusServer}) || n.Composes != delivery.ComposesOwn {