From 43d1c2456fb22cbdb54a5ffdf07b2bd158f8cb29 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 25 Oct 2026 11:04:08 +0100 Subject: [PATCH] Apply a module again only for a newly missing resource, leave filled files out of the record check, and say when the applied state cannot be read (issue 1945) --- cmd/mesh-host/main.go | 23 +++++++++++++++- cmd/mesh-host/nodeagent.go | 15 +++++++++-- cmd/mesh-host/recorded_test.go | 48 +++++++++++++++++++++++++++++++++- 3 files changed, 82 insertions(+), 4 deletions(-) diff --git a/cmd/mesh-host/main.go b/cmd/mesh-host/main.go index 01ae6ba..7b13709 100644 --- a/cmd/mesh-host/main.go +++ b/cmd/mesh-host/main.go @@ -1334,6 +1334,7 @@ const ( // anyway. A statement the bus did not take is said at the next look. It reads; it never acts (ADR 0240 // rule 6) — but for going on with a take act stopped at its bound (ADR 0328 §7), which runs beside it. func judgeWhatRuns(ctx context.Context, j *liveness.Judge, queue *link.Session, say link.Announce, reapply func(modules ...string)) { + var missingBefore map[string]bool ticker := time.NewTicker(liveness.LookEvery) defer ticker.Stop() var lastSaid time.Time @@ -1421,7 +1422,9 @@ func judgeWhatRuns(ctx context.Context, j *liveness.Judge, queue *link.Session, } // A resource the machine does not have at all is made again: its module's composition applied anew (issue // 1945), once each time the statement changes, never on a clock. - if m := missingModules(st); len(m) > 0 && reapply != nil { + var m []string + m, missingBefore = newlyMissing(missingBefore, st) + if len(m) > 0 && reapply != nil { say(fmt.Sprintf("applying %s again: a resource it declares is not on this machine", strings.Join(m, ", "))) reapply(m...) } @@ -2101,3 +2104,21 @@ func missingModules(st liveness.Statement) []string { sort.Strings(out) return out } + +// newlyMissing are the modules of the resources a statement says missing that the last statement acted on did not, each +// once, sorted, and the missing resources now: a resource still missing is not acted on again, so a module whose +// resource cannot be made is not applied at every change of the statement (the review of issue 1945). +func newlyMissing(last map[string]bool, st liveness.Statement) ([]string, map[string]bool) { + now := map[string]bool{} + var fresh liveness.Statement + for _, r := range st.Resources { + if !r.Missing { + continue + } + now[r.ID] = true + if !last[r.ID] { + fresh.Resources = append(fresh.Resources, r) + } + } + return missingModules(fresh), now +} diff --git a/cmd/mesh-host/nodeagent.go b/cmd/mesh-host/nodeagent.go index e4e902e..7dd18f9 100644 --- a/cmd/mesh-host/nodeagent.go +++ b/cmd/mesh-host/nodeagent.go @@ -293,7 +293,7 @@ func nodeAgent(ctx context.Context, opts options, signer ed25519.PublicKey, sess Composed: bucketWatch{session, composedBucket}, Haves: sessionHaves{session}, Kept: keptByModule(opts, signer), - Recorded: recordedIn(opts.state), + Recorded: recordedIn(opts.state, say), Trusted: trustedKeys(opts, signer), Backup: backupNode(session), Say: say, @@ -519,17 +519,28 @@ func refuseEmptyWrites(d *declaration.Declaration) error { // recordedIn says the engine's applied state holds a record of every resource of a composition: what the agent believes // of a have or the kept declaration only when it does, so a resource removed since is made again (issue 1945). A state // that cannot be read says yes: the agent then believes what it was told, as before. -func recordedIn(statePath string) func(ctx context.Context, c delivery.Composed) bool { +func recordedIn(statePath string, say func(string)) func(ctx context.Context, c delivery.Composed) bool { return func(_ context.Context, c delivery.Composed) bool { known, err := store.Load(statePath) if err != nil { + if say != nil { + say(fmt.Sprintf("the applied state cannot be read, so what runs is believed as told: %v", err)) + } return true } + filled := make(map[string]bool, len(c.Filled)) + for _, p := range c.Filled { + filled[p] = true + } for _, raw := range c.Resources { r, err := declaration.ParseResource(raw) if err != nil { return true } + // A file the provisioner client fills is left to it, and has no record until it writes it. + if f, ok := r.(*declaration.File); ok && filled[f.Path] { + continue + } if _, ok := known.Find(r.Identity()); !ok { return false } diff --git a/cmd/mesh-host/recorded_test.go b/cmd/mesh-host/recorded_test.go index bba28e2..ce7684b 100644 --- a/cmd/mesh-host/recorded_test.go +++ b/cmd/mesh-host/recorded_test.go @@ -23,7 +23,7 @@ func TestACompositionIsRecordedOnlyWhenEveryResourceIs(t *testing.T) { } c := delivery.Composed{Node: "ace", Module: "ca-trust", Resources: []json.RawMessage{ json.RawMessage(`{"id":"ca-trust.trust","type":"file","module":"ca-trust","path":"/etc/ca","mode":"0644","content":"x"}`)}} - recorded := recordedIn(path) + recorded := recordedIn(path, func(string) {}) if !recorded(context.Background(), c) { t.Fatal("a composition whose every resource has a record was not recorded") } @@ -46,3 +46,49 @@ func TestTheModulesAppliedAgainAreThoseWithAMissingResource(t *testing.T) { t.Fatalf("applied again: %v", got) } } + +// A module is applied again only for a resource newly missing: one still missing at the next change of the statement is +// not acted on again, so a resource that cannot be made is not applied (and backed up) at every unrelated change. One +// that comes back and goes again is newly missing again (the review of issue 1945). +func TestOnlyANewlyMissingResourceAppliesItsModuleAgain(t *testing.T) { + missing := func(ids ...string) liveness.Statement { + var st liveness.Statement + for _, id := range ids { + st.Resources = append(st.Resources, liveness.State{Resource: liveness.Resource{Module: "matrix", ID: id}, + State: liveness.Unhealthy, Missing: true}) + } + return st + } + var last map[string]bool + var got []string + got, last = newlyMissing(last, missing("matrix.homeserver")) + if len(got) != 1 { + t.Fatalf("first missing: %v", got) + } + got, last = newlyMissing(last, missing("matrix.homeserver")) + if len(got) != 0 { + t.Fatalf("still missing was acted on again: %v", got) + } + got, last = newlyMissing(last, missing()) + got, last = newlyMissing(last, missing("matrix.homeserver")) + if len(got) != 1 { + t.Fatalf("missing again after it came back: %v", got) + } +} + +// A file the provisioner client fills is left to it by the apply and has no record until the client writes it: it does +// not make its composition unrecorded (the review of issue 1945). +func TestAFilledFileDoesNotMakeItsCompositionUnrecorded(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.json") + var s store.State + s.Record(store.Applied{ID: "ca-trust.trust", Type: "file", Target: "/etc/ca"}) + if err := store.Save(path, s); err != nil { + t.Fatal(err) + } + c := delivery.Composed{Node: "ace", Module: "ca-trust", Filled: []string{"/etc/ca-filled"}, Resources: []json.RawMessage{ + json.RawMessage(`{"id":"ca-trust.trust","type":"file","module":"ca-trust","path":"/etc/ca","mode":"0644","content":"x"}`), + json.RawMessage(`{"id":"ca-trust.filled","type":"file","module":"ca-trust","path":"/etc/ca-filled","mode":"0644","owner-at-runtime":"owned"}`)}} + if !recordedIn(path, func(string) {})(context.Background(), c) { + t.Fatal("a filled file the client has not written made its composition unrecorded") + } +}