diff --git a/internal/agent/agent.go b/internal/agent/agent.go index 465ee94..40f6fe4 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -16,6 +16,7 @@ import ( "encoding/json" "fmt" "sort" + "strings" "sync" "git.novox.be/novox/mesh-sdk/go/delivery" @@ -189,6 +190,8 @@ func (a *Agent) Run(ctx context.Context) error { compsCaught = true } else if m := a.takeComposition(u); m != "" { dirty[m] = true + // The node's own may wait for this one to read (unreadableModules): looked at again with it. + dirty[NodeModule] = true } } // Nothing is applied until what was there at the start is all in: a module's composition may arrive after its @@ -389,6 +392,90 @@ func (a *Agent) others(except string) []delivery.Composed { return out } +// unreadableModules are the keys of the module compositions in the node's bucket that cannot be read or are refused, of a +// module the assigner has not removed: for each module only the composition its want names, else its newest, since a +// stale one nobody applies (written before a rule the SDK now holds) must not hold the node's own back. Called holding +// a.mu. +func (a *Agent) unreadableModules() []string { + type pick struct { + key string + c compNow + } + current := map[string]pick{} + for key, c := range a.comps { + _, m, fp, isNode, err := delivery.ParseComposedKey(key) + if err != nil || isNode { + continue + } + w := a.wants[m] + if w.Present && w.Want.Removed { + continue + } + if w.Present && w.Want.FP != "" { + if w.Want.FP == fp { + current[m] = pick{key, c} + } + continue + } + if prev, ok := current[m]; !ok || c.Rev > prev.c.Rev { + current[m] = pick{key, c} + } + } + var out []string + for _, p := range current { + if p.c.Err != nil { + out = append(out, p.key) + } + } + sort.Strings(out) + return out +} + +// heldOut is what an apply or a removal holds out: others, and every module whose own composition the node's bucket holds +// and the agent does not run yet. A module that left the node's own composition for one of its own keeps its resources +// until that one is applied, so the node's own apply never removes them in between (issue 1945, live on 2026-10-25: +// shanks lost systemd-resolved's guard while its composition waited for a credential). The composition its want names, +// else its newest. Not part of Everything: what has not been applied is not what runs. +func (a *Agent) heldOut(except string) []delivery.Composed { + out := a.others(except) + held := map[string]bool{} + for _, c := range out { + held[c.Module] = true + } + waiting := map[string]compNow{} + for key, c := range a.comps { + _, m, fp, isNode, err := delivery.ParseComposedKey(key) + if err != nil || isNode || c.Err != nil || m == except || held[m] { + continue + } + if h, ok := a.running[m]; ok && h.Phase != delivery.PhaseRemoved { + continue + } + if w := a.wants[m]; w.Present && w.Want.Removed { + continue // a module the assigner removed holds nothing back + } + if w := a.wants[m]; w.Present && !w.Want.Removed && w.Want.FP == fp { + waiting[m] = c + continue + } + if w := a.wants[m]; w.Present && !w.Want.Removed && waiting[m].Rev != 0 && waiting[m].C.FP == w.Want.FP { + continue + } + if prev, ok := waiting[m]; !ok || c.Rev > prev.Rev { + waiting[m] = c + } + } + names := make([]string, 0, len(waiting)) + for m := range waiting { + names = append(names, m) + } + sort.Strings(names) + for _, m := range names { + out = append(out, waiting[m].C) + } + return out +} + // Everything is what the node runs: the compositions the agent runs, and the kept declaration's resources for each module // the agent does not hold yet. What the node judges its health by, and what the filling, the take act and the judgement // of directories read, is built from it. @@ -457,7 +544,17 @@ func (a *Agent) settleOne(ctx context.Context, module string) { a.mu.Unlock() return // held for a failed backup, and no answer since } - others := a.others(module) + if module == NodeModule { + // Not while a module's composition cannot be read or is refused: what it carries may be what left the node's own, + // and the node's own apply would remove it, held out by nothing (issue 1945, the review's gap 2). + if bad := a.unreadableModules(); len(bad) > 0 { + a.sayOnce("node-waits:"+strings.Join(bad, ","), fmt.Sprintf("the node's own composition is not applied while "+ + "%s cannot be read or is refused: what it carries would be removed", strings.Join(bad, ", "))) + a.mu.Unlock() + return + } + } + others := a.heldOut(module) a.mu.Unlock() a.apply(ctx, module, w, comp, others) } @@ -559,7 +656,7 @@ func (a *Agent) remove(ctx context.Context, module string, w wantNow) { a.mu.Unlock() return } - others := a.others(module) + others := a.heldOut(module) a.mu.Unlock() res := a.env.Remove(ctx, module, others) a.mu.Lock() diff --git a/internal/agent/agent_test.go b/internal/agent/agent_test.go index 3b7608a..793d9a3 100644 --- a/internal/agent/agent_test.go +++ b/internal/agent/agent_test.go @@ -111,6 +111,7 @@ type bed struct { backup func(module string) Backup kept map[string][]json.RawMessage others [][]delivery.Composed + said []string env Env } @@ -148,7 +149,12 @@ func newBed(t *testing.T) *bed { }, Kept: func() map[string][]json.RawMessage { return b.kept }, Trusted: func() []ed25519.PublicKey { return []ed25519.PublicKey{controllerPub, operatorPub} }, - Say: func(s string) { t.Log(s) }, + Say: func(s string) { + t.Log(s) + b.mu.Lock() + b.said = append(b.said, s) + b.mu.Unlock() + }, } return b } @@ -640,3 +646,139 @@ func TestAHaveWithNoCompositionDefersToTheKeptDeclaration(t *testing.T) { t.Fatalf("applied %v", got) } } + +// A module that leaves the node's own composition for a composition of its own keeps its resources until that one is +// applied: the node's own apply holds out every module whose composition the node's bucket holds and the agent does not +// run yet (issue 1945, live on 2026-10-25 at 02:32: shanks' node-engine applied a node's own composition that no longer +// carried systemd-resolved, whose own composition waited for its credential, and removed its guard, so the machine lost +// its resolver). +func TestTheNodesOwnApplyHoldsOutAModuleThatMovedToItsOwnComposition(t *testing.T) { + b := newBed(t) + fp := fpOf("b") + // systemd-resolved's own composition is written; no want names it yet. + rk, rraw, _ := composition(t, "systemd-resolved", fp, false, file("systemd-resolved", "guard", "g")) + b.comps.before(rk, rraw) + nk, nraw, ndig := composition(t, NodeModule, "", false) + nwk, err := delivery.WantNodeKey(node) + if err != nil { + t.Fatal(err) + } + nwraw, err := delivery.EncodeNodeWant(delivery.NodeWant{Composed: ndig}) + if err != nil { + t.Fatal(err) + } + b.comps.before(nk, nraw) + b.wants.before(nwk, nwraw) + stop := b.run() + defer stop() + eventually(t, "the node's own not applied", func() bool { return len(b.appliedList()) == 1 }) + b.mu.Lock() + defer b.mu.Unlock() + var names []string + for _, c := range b.others[0] { + names = append(names, c.Module) + } + if !strings.Contains(strings.Join(names, ","), "systemd-resolved") { + t.Fatalf("the others of the node's own apply were %v: systemd-resolved's resources would be removed", names) + } +} + +// A module the assigner removed holds nothing back, its compositions still in the bucket or not: its resources are its +// removal's to take (issue 1945). +func TestARemovedModulesCompositionHoldsNothingBack(t *testing.T) { + b := newBed(t) + fp := fpOf("b") + rk, rraw, _ := composition(t, "gone", fp, false, file("gone", "x", "1")) + b.comps.before(rk, rraw) + gk, err := delivery.WantKey(node, "gone") + if err != nil { + t.Fatal(err) + } + graw, err := delivery.EncodeWant(delivery.Want{Removed: true}) + if err != nil { + t.Fatal(err) + } + b.wants.before(gk, graw) + nk, nraw, ndig := composition(t, NodeModule, "", false) + nwk, _ := delivery.WantNodeKey(node) + nwraw, _ := delivery.EncodeNodeWant(delivery.NodeWant{Composed: ndig}) + b.comps.before(nk, nraw) + b.wants.before(nwk, nwraw) + stop := b.run() + defer stop() + eventually(t, "the node's own not applied", func() bool { + for _, m := range b.appliedList() { + if m == NodeModule { + return true + } + } + return false + }) + b.mu.Lock() + defer b.mu.Unlock() + for _, others := range b.others { + for _, c := range others { + if c.Module == "gone" { + t.Fatal("a removed module's composition was held out") + } + } + } +} + +// The node's own composition is not applied while a module's composition in the node's bucket cannot be read or is not +// signed by a trusted key: what that one carries may be what left the node's own, and the node's own apply would remove +// it (issue 1945, the review's gap 2). It is said, naming the composition, and applied once that one reads. +func TestTheNodesOwnWaitsWhileAModulesCompositionCannotBeRead(t *testing.T) { + b := newBed(t) + bad, err := delivery.ComposedKey(node, "broken", fpOf("c")) + if err != nil { + t.Fatal(err) + } + b.comps.before(bad, []byte(`{"v":1,"not":"a composition"}`)) + nk, nraw, ndig := composition(t, NodeModule, "", false) + nwk, _ := delivery.WantNodeKey(node) + nwraw, _ := delivery.EncodeNodeWant(delivery.NodeWant{Composed: ndig}) + b.comps.before(nk, nraw) + b.wants.before(nwk, nwraw) + stop := b.run() + defer stop() + eventually(t, "the wait not said", func() bool { + b.mu.Lock() + defer b.mu.Unlock() + for _, s := range b.said { + if strings.Contains(s, bad) { + return true + } + } + return false + }) + if got := b.appliedList(); len(got) != 0 { + t.Fatalf("applied %v while a composition could not be read", got) + } + // Read again, the node's own is applied. + gk, graw, _ := composition(t, "broken", fpOf("c"), false, file("broken", "x", "1")) + b.comps.put(gk, graw) + eventually(t, "the node's own not applied", func() bool { return len(b.appliedList()) == 1 }) +} + +// A stale refused composition beside a newer readable one of the same module does not hold the node's own apply back: +// only the composition the module's want names, else its newest, counts (the review of issue 1945: the bucket keeps +// compositions written before the filled paths, which the SDK now refuses, and nobody applies them). +func TestAStaleRefusedCompositionDoesNotHoldTheNodesOwnBack(t *testing.T) { + b := newBed(t) + stale, err := delivery.ComposedKey(node, "power", fpOf("a")) + if err != nil { + t.Fatal(err) + } + b.comps.before(stale, []byte(`{"v":1,"not":"a composition"}`)) + gk, graw, _ := composition(t, "power", fpOf("b"), false, file("power", "x", "1")) + b.comps.before(gk, graw) + nk, nraw, ndig := composition(t, NodeModule, "", false) + nwk, _ := delivery.WantNodeKey(node) + nwraw, _ := delivery.EncodeNodeWant(delivery.NodeWant{Composed: ndig}) + b.comps.before(nk, nraw) + b.wants.before(nwk, nwraw) + stop := b.run() + defer stop() + eventually(t, "the node's own not applied", func() bool { return len(b.appliedList()) == 1 }) +} diff --git a/internal/apply/apply.go b/internal/apply/apply.go index 03c57b6..731d432 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -430,9 +430,32 @@ func applyScoped( // And what a record going hands to a resource that still asks for its group (novox/hq ADR 0252 ยง3). handed := Handed{} + // **A resource another module's composition now holds is that module's, and is left** (issue 1945): a module that + // leaves the node's own composition for one of its own takes its resources with it, by their identity, and the node's + // own apply leaves each as it is, record and all, until that module's apply takes it. Removed, a node lost its + // resolver guard, its sudoers file and a running container between two applies. + heldByOthers := map[string]string{} + if scope != nil { + for _, r := range scope.Others { + heldByOthers[r.Identity()] = r.DeclaredModule() + } + } + removeOrphan := func(orphan store.Applied) error { var action, detail string var err error + if module, held := heldByOthers[orphan.ID]; held && !declared[orphan.ID] { + if module == "" { + module = "another module" + } + detail := "no longer declared here; the composition of " + module + " holds it, so it was left as it is" + report.Outcomes = append(report.Outcomes, Outcome{ + ID: orphan.ID, Type: orphan.Type, Target: orphan.Target, + Action: "kept", Detail: detail, + }) + log(fmt.Sprintf(" kept %s (%s): %s", orphan.ID, orphan.Target, detail)) + return nil + } if by, held := heldFiles[fileKey(orphan)]; held && by != orphan.ID { if scope != nil && !declared[by] { // Another module declares the path: it is that module's, so this record is let go and the file is diff --git a/internal/apply/module_scope_test.go b/internal/apply/module_scope_test.go index 2ad9d20..67b494a 100644 --- a/internal/apply/module_scope_test.go +++ b/internal/apply/module_scope_test.go @@ -111,3 +111,38 @@ func TestARecordWithoutAnOwnerBelongsToTheModuleItsIdNames(t *testing.T) { t.Fatalf("a's file exists %v, b's file exists %v", exists(fa), exists(fb)) } } + +// A resource that leaves the node's own composition for its module's own is that module's now: the node's own apply +// leaves it as it is, whatever its type, until the module's own apply takes it (issue 1945, live on 2026-10-25 at 02:32: +// shanks and g14 lost systemd-resolved's guard, its resolver files and sudo's operator file, ace qbittorrent's server, +// because the node's own apply removed every record no longer in it, the others holding the very same resources or not). +func TestAResourceThatMovedToItsModulesCompositionIsLeftByTheNodesOwnApply(t *testing.T) { + dir := t.TempDir() + fact := filepath.Join(dir, "suffix") + gone := filepath.Join(dir, "gone.conf") + own := declare(t, `{"id":"systemd-resolved.fact-suffix","type":"file","path":"`+fact+`","content":"mesh\n"}, + {"id":"old.gone","type":"file","path":"`+gone+`","content":"g\n"}`) + _, state, err := ApplyModule(context.Background(), archHost(t), own, Scope{Module: store.NodeModule}, + state0(), store.OriginDeclared, noServices, nil, nil, nil, nil) + if err != nil { + t.Fatal(err) + } + // systemd-resolved now has a composition of its own, not applied yet; old.gone is no module's any more. + moved := declare(t, `{"id":"systemd-resolved.fact-suffix","type":"file","path":"`+fact+`","content":"mesh\n","module":"systemd-resolved"}`) + report, state, err := ApplyModule(context.Background(), archHost(t), &declaration.Declaration{}, + Scope{Module: store.NodeModule, Others: moved.Resources}, state, store.OriginDeclared, noServices, nil, nil, nil, nil) + if err != nil { + t.Fatal(err) + } + if !exists(fact) { + t.Fatalf("the moved resource was removed: %+v", outcomeOf(report, "systemd-resolved.fact-suffix")) + } + if exists(gone) { + t.Fatal("a resource no composition holds any more was kept") + } + if _, still := state.Find("systemd-resolved.fact-suffix"); !still { + t.Fatal("the moved resource's record was dropped, so its module's apply would not know it") + } +} + +func state0() store.State { return store.State{} }