diff --git a/cmd/mesh-host/applied_store.go b/cmd/mesh-host/applied_store.go index 60df951..75243ed 100644 --- a/cmd/mesh-host/applied_store.go +++ b/cmd/mesh-host/applied_store.go @@ -30,6 +30,16 @@ func (s appliedStore) path(module string) (string, error) { return filepath.Join(s.dir, module+".json"), nil } +// holds says whether the store keeps the module's composition. +func (s appliedStore) holds(module string) bool { + path, err := s.path(module) + if err != nil { + return false + } + _, err = os.Stat(path) + return err == nil +} + func (s appliedStore) Load() (map[string][]byte, error) { entries, err := os.ReadDir(s.dir) if errors.Is(err, os.ErrNotExist) { diff --git a/cmd/mesh-host/authority_test.go b/cmd/mesh-host/authority_test.go index 84d8ea9..ac7bdcb 100644 --- a/cmd/mesh-host/authority_test.go +++ b/cmd/mesh-host/authority_test.go @@ -127,6 +127,38 @@ func TestTheAppliedAuthorityIsKeptForTheRecord(t *testing.T) { if err := certifyThisNode(context.Background(), state, "n1", trust, ask, func() { written++ }); err == nil || written != 1 { t.Errorf("a certificate of another authority was kept: %v", err) } + + // Issue 774: what the engine applied is kept beside the state too, one file per module, 0600 in a 0700 directory, + // written by a rename so a crash leaves the one before; a name that is no module's is refused, never a path. + t.Run("the compositions the engine applied are kept beside the state", func(t *testing.T) { + applied := appliedStoreFor(filepath.Join(t.TempDir(), "state", "state.json")) + if m, err := applied.Load(); err != nil || len(m) != 0 || applied.holds("node") { + t.Fatalf("an empty store loads %v, %v", m, err) + } + for _, tc := range []struct { + module string + takes bool + }{{"node", true}, {"nats", true}, {"mesh-delivery-view", true}, {"../state", false}, {"a/b", false}, {"..", false}, {"", false}} { + if err := applied.Keep(tc.module, []byte(`{"v":1}`)); (err == nil) != tc.takes { + t.Errorf("keeping %q: %v, want taken %v", tc.module, err, tc.takes) + } + } + if err := applied.Keep("node", []byte(`{"v":2}`)); err != nil { + t.Fatal(err) + } + entries, _ := os.ReadDir(applied.dir) + dirInfo, _ := os.Stat(applied.dir) + fileInfo, _ := os.Stat(filepath.Join(applied.dir, "node.json")) + if len(entries) != 3 || dirInfo.Mode().Perm() != 0o700 || fileInfo.Mode().Perm() != 0o600 { + t.Fatalf("the store holds %d files (want 3, no temporary one left), directory %v, file %v", len(entries), dirInfo.Mode(), fileInfo.Mode()) + } + if err := applied.Drop("nats"); err != nil || applied.Drop("nats") != nil || applied.holds("nats") { + t.Fatalf("a dropped module is still held, or dropping twice fails: %v", err) + } + if m, err := applied.Load(); err != nil || string(m["node"]) != `{"v":2}` || len(m) != 2 { + t.Fatalf("the store loads %v, %v", m, err) + } + }) } // testAuthority is a self-signed authority certificate for key, as the vault makes its own. diff --git a/cmd/mesh-host/nodeagent.go b/cmd/mesh-host/nodeagent.go index 07fc79c..00531d8 100644 --- a/cmd/mesh-host/nodeagent.go +++ b/cmd/mesh-host/nodeagent.go @@ -454,8 +454,15 @@ func nodeAgent(ctx context.Context, opts options, signer ed25519.PublicKey, trus var ag *agent.Agent // The kept release-5 declaration is no longer read once every module the seat's placement puts here has a have of its // own (ADR 0550 §2: its signature is the controller's, trusted no more after the lease write). - if retired, why := placedAllHave(linked, session, node); retired && !keptRetired.Swap(true) { - say("the kept declaration is no longer read: " + why) + // Nor before this engine keeps the node's own as it runs (applied/node.json): until then the kept declaration is the one + // record of what the node's own composition carries, its folded modules among them (issue 774, review of the + // re-land, round 1: novox's first start would compose its own with nats running nothing). + if retired, why := placedAllHave(linked, session, node); retired && !keptRetired.Load() { + if !appliedStoreFor(opts.state).holds(agent.NodeModule) { + say("the kept declaration is still read: " + why + ", but this engine keeps no node's own composition yet") + } else if !keptRetired.Swap(true) { + say("the kept declaration is no longer read: " + why) + } } kept := keptOnce(opts, signer) // The mesh's own certificate the declaration kept from before carries, kept for the record before this machine @@ -529,7 +536,7 @@ func nodeAgent(ctx context.Context, opts options, signer ed25519.PublicKey, trus if herr != nil { res = failedWith(herr.Error()) } else { - res = applyComposition(actx, opts, held, others, ag.Placed(scopeName(c)), sched, say) + res = applyComposition(actx, opts, held, others, ag.Placed(scopeName(c)), ag.RemovedByWant(), sched, say) } done() // The mesh's own certificate it carries is kept for this machine's record (ADR 0555 §3). @@ -690,7 +697,10 @@ func (c *agentNow) get() *agent.Agent { // // placed is the modules a want still places here besides this one: none of their resources is removed by this apply, // which is refused instead (issue 774). -func applyComposition(ctx context.Context, opts options, c delivery.Composed, others []delivery.Composed, placed map[string]bool, +// +// removed is the modules whose want says removed: in the node's own apply, a resource of any other module goes only when +// its module is among them (issue 774). +func applyComposition(ctx context.Context, opts options, c delivery.Composed, others []delivery.Composed, placed, removed map[string]bool, sched *apply.Scheduler, say link.Announce) agent.Result { name := scopeName(c) if c.Module == "" { @@ -707,6 +717,12 @@ func applyComposition(ctx context.Context, opts options, c delivery.Composed, ot return failedWith(err.Error()) } scope := apply.Scope{Module: name, Others: resourcesOf(others), Placed: placed} + if name == agent.NodeModule { + scope.RemovedByWant = removed + if scope.RemovedByWant == nil { + scope.RemovedByWant = map[string]bool{} + } + } got := applyModuleHeld(ctx, opts, d, &scope, sched, say) if got.Refused != "" { return failedWith(got.Refused) diff --git a/internal/agent/agent.go b/internal/agent/agent.go index 0b47ede..7b6494c 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -904,7 +904,7 @@ func (a *Agent) loadHad(ctx context.Context) { // loadApplied takes, once at the start, what this engine last applied of each module as what runs of it, ahead of the // bucket and the kept declaration (Env.Applied, issue 774): it is what the machine holds, whatever the bus still keeps. -// One whose have names another composition that runs (applied since by another engine) is left to the have; one whose +// One whose have names another composition runs as applied until loadHad reads that one from the bucket; one whose // have cannot be read, or that has none, is kept as applied, so a module is never composed as running nothing. func (a *Agent) loadApplied(ctx context.Context) { if a.env.Applied == nil { @@ -914,6 +914,30 @@ func (a *Agent) loadApplied(ctx context.Context) { if err != nil { a.env.Say(fmt.Sprintf("the compositions this engine applied cannot be read, so what runs is read from the haves alone: %v", err)) } + // At an engine's first start nothing is kept yet: the node's own runs as the kept declaration has it, which the store + // keeps now, so the node never composes its own as if its folded modules ran nothing once the declaration retires + // (issue 774, review of the re-land, round 1). Taken whatever digest the node's have names: the composition that names + // is in no bucket, and the declaration is the one record of what runs on disk. + seeded := false + if _, own := kept[NodeModule]; !own && err == nil && a.env.Kept != nil { + if rs, ok := a.env.Kept()[NodeModule]; ok && len(rs) > 0 { + c, _ := a.keptComposed(NodeModule, rs) + c.Module = "" + raw, eerr := delivery.EncodeComposed(c) + if eerr == nil { + eerr = a.env.Applied.Keep(NodeModule, raw) + } + if eerr != nil { + a.env.Say(fmt.Sprintf("the node's own as the kept declaration has it cannot be kept: %v", eerr)) + } else { + if kept == nil { + kept = map[string][]byte{} + } + kept[NodeModule], seeded = raw, true + a.env.Say("the node's own runs as the kept declaration has it, kept now as what this engine runs") + } + } + } names := make([]string, 0, len(kept)) for m := range kept { names = append(names, m) @@ -933,10 +957,13 @@ func (a *Agent) loadApplied(ctx context.Context) { continue } h, found, unread := a.readHave(ctx, m) - if found && h.Composed != "" && h.Composed != dig && haveRuns(h) { - a.env.Say(fmt.Sprintf("%s's have names the composition %s, not %s this engine applied last: what runs of it is read from its have", - m, short(h.Composed), short(dig))) - continue + if seeded && m == NodeModule { + found, unread = true, false // what the kept declaration has runs, whatever the have names + } else if found && h.Composed != "" && h.Composed != dig && haveRuns(h) { + // The composition the have names replaces this once loadHad finds it in the bucket; until then, and when + // the bucket keeps it no more, what this engine applied runs (issue 774, review of the re-land, round 1). + a.env.Say(fmt.Sprintf("%s's have names the composition %s, not %s this engine applied last: it runs as applied "+ + "until the one its have names is read", m, short(h.Composed), short(dig))) } if m == NodeModule { c.Module, c.FP = "", "" @@ -1151,6 +1178,20 @@ func (a *Agent) Placed(except string) map[string]bool { return out } +// RemovedByWant is the modules whose want says removed (apply.Scope.RemovedByWant, issue 774): in the node's own apply, +// only their resources may go. Called without a.mu held. +func (a *Agent) RemovedByWant() map[string]bool { + a.mu.Lock() + defer a.mu.Unlock() + out := map[string]bool{} + for m, w := range a.wants { + if m != NodeModule && w.Present && w.Want.Removed { + out[m] = true + } + } + return out +} + // HoldsNode says whether the agent runs the node's own composition, so the node's parts are its and not the kept // declaration's. func (a *Agent) HoldsNode() bool { diff --git a/internal/agent/agent_test.go b/internal/agent/agent_test.go index 4aa7d9f..9ec0741 100644 --- a/internal/agent/agent_test.go +++ b/internal/agent/agent_test.go @@ -835,6 +835,56 @@ func TestEverythingIsWhatTheAgentRunsAndWhatIsKeptForTheRest(t *testing.T) { b.wants.put(wk, rraw) eventually(t, "the removed module's composition was not forgotten", func() bool { return store.get("web") == nil }) }) + + // Issue 774, review of the re-land, round 1: at the re-landed engine's first start nothing is kept yet, and the node's + // have names a composition the node composed itself, in no bucket. The node's own runs as the kept declaration has it + // and is kept then, so the next start, the declaration retired, still runs nats. + t.Run("at the first start nothing is kept: the node's own runs as the kept declaration has it", func(t *testing.T) { + natsConf, _ := json.Marshal(map[string]any{"id": "nats.conf", "type": "file", "path": "/tmp/r6/nats/conf", "content": "listen 4222"}) + store := &memApplied{kept: map[string][]byte{}} + h := &haves{} + h.Write(context.Background(), Have{Module: NodeModule, Composed: "sha256:" + strings.Repeat("c", 64)}) + firstPass := func(kept map[string][]json.RawMessage) []delivery.Composed { + b := newBed(t) + b.env.SelfComposed, b.env.Trusted, b.kept, b.env.Applied, b.env.Haves = true, nil, kept, store, h + firstCh := make(chan []delivery.Composed, 1) + comps := b.comps + var ag *Agent + b.env.Composed = watchFunc(func(ctx context.Context, p string) (<-chan Update, error) { + firstCh <- ag.Everything() + return comps.Watch(ctx, p) + }) + ag = New(b.env) + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + go func() { defer close(done); _ = ag.Run(ctx) }() + defer func() { cancel(); <-done }() + select { + case got := <-firstCh: + return got + case <-time.After(5 * time.Second): + t.Fatal("the composer was not watched") + } + return nil + } + runsNats := func(cs []delivery.Composed) bool { + for _, c := range cs { + if c.Module == "" && len(c.Resources) == 1 && strings.Contains(string(c.Resources[0]), "4222") { + return true + } + } + return false + } + if got := firstPass(map[string][]json.RawMessage{NodeModule: {natsConf}}); !runsNats(got) { + t.Fatalf("the first start does not run nats in the node's own (%d compositions)", len(got)) + } + if store.get(NodeModule) == nil { + t.Fatal("the first start kept no node's own composition") + } + if got := firstPass(nil); !runsNats(got) { + t.Fatalf("the next start, the kept declaration retired, does not run nats in the node's own (%d compositions)", len(got)) + } + }) } // watchFunc is a Watcher from a function. diff --git a/internal/apply/apply.go b/internal/apply/apply.go index 24f93c5..8eb83d8 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -316,23 +316,34 @@ type Scope struct { // the module's own removes a resource of one of them (issue 774: on 2026-10-29 the node's own apply on the bus node, // composed without what its folded modules ran, removed the bus server and four held modules). Nil guards nothing. Placed map[string]bool + // RemovedByWant are the modules whose want says removed. In the node's own apply (Module NodeModule) with this set + // non-nil, a resource of any other module is removed only when that module is here: a module the node runs folded + // in its own composition has no want, so Placed does not name it (issue 774, review of the re-land, round 1: nats on + // the bus node has none, and its nats.conf was guessed as nats's and removed). Nil guards by Placed alone. + RemovedByWant map[string]bool } // placedOwner is the module a want still places on the node that the record belongs to, other than the scope's own: // the module recorded with it, else the one its id names (a resource the node's own composition carried for a folded // module is recorded as the node's, its id naming the module). Empty when there is none. func (sc *Scope) placedOwner(a store.Applied) string { - if sc == nil || len(sc.Placed) == 0 { + if sc == nil || (len(sc.Placed) == 0 && sc.RemovedByWant == nil) { return "" } m := a.Module if m == "" || m == store.NodeModule { m, _ = declaration.ModuleGuess(a.ID) } - if m == "" || m == sc.Module || !sc.Placed[m] { + if m == "" || m == sc.Module || m == store.NodeModule { return "" } - return m + if sc.Placed[m] { + return m + } + if sc.Module == store.NodeModule && sc.RemovedByWant != nil && !sc.RemovedByWant[m] { + return m // a module the node's own composition carried, which no want removed + } + return "" } // owns reports whether a record belongs to the scope's module. diff --git a/internal/apply/module_scope_test.go b/internal/apply/module_scope_test.go index 14b95f4..51e42a5 100644 --- a/internal/apply/module_scope_test.go +++ b/internal/apply/module_scope_test.go @@ -131,10 +131,16 @@ func TestAResourceThatMovedToItsModulesCompositionIsLeftByTheNodesOwnApply(t *te placed bool // a want still places its module here refused bool // the node's own apply is refused whole stays bool // the resource is still on the machine and recorded + // wants names the modules whose want says removed (Scope.RemovedByWant); nil leaves the guard to Placed alone. + wants []string }{ {name: "moved to its module's own composition", id: "systemd-resolved.fact-suffix", moved: true, placed: true, stays: true}, {name: "a held folded module, in no composition", id: "nats.server-conf", placed: true, refused: true, stays: true}, {name: "its module's want removed", id: "nats.server-conf"}, + // Issue 774, review of the re-land, round 1: a module folded in the node's own has no want, so Placed does not name + // it; in the node's own apply only a module a want removed loses its resources. + {name: "a folded module with no want", id: "nats.server-conf", wants: []string{}, refused: true, stays: true}, + {name: "a folded module a want removed", id: "nats.server-conf", wants: []string{"nats", "old"}}, } { t.Run(tc.name, func(t *testing.T) { dir := t.TempDir() @@ -156,6 +162,12 @@ func TestAResourceThatMovedToItsModulesCompositionIsLeftByTheNodesOwnApply(t *te if tc.placed { scope.Placed = map[string]bool{module: true} } + if tc.wants != nil { + scope.RemovedByWant = map[string]bool{} + for _, m := range tc.wants { + scope.RemovedByWant[m] = true + } + } report, after, err := ApplyModule(context.Background(), archHost(t), &declaration.Declaration{}, scope, state, store.OriginDeclared, noServices, nil, nil, nil, nil) if tc.refused != (err != nil) {