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)

2026-10-25 11:04:08 +01:00
parent c46937da04
commit 43d1c2456f
3 changed files with 82 additions and 4 deletions
@@ -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
+}
@@ -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
 			}
@@ -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")
+	}
+}