Merge pull request 'Hand a whole file to its new owner instead of removing it first (hq ADR 0223)' (#27) from files-forget-when-held into main

2026-10-05 21:50:48 +00:00
parent a192bccf62 762da9e05e
commit 24bada7a6f
3 changed files with 240 additions and 0 deletions
@@ -250,9 +250,24 @@ func ApplyMindingWindows(
 	// forgotten; the unit's found state is the remaining record's to give back.
 	heldUnits := unitsHeld(d.Resources)

+	// **A whole file still declared at its path by another resource is handed to it, never removed
+	// first** (novox/hq ADR 0223). A file can change owners between modules — the machine's resolver
+	// file moving from the module that once wrote it to the uplink's holder — and the two records
+	// meet in one apply, the old undeclared and the new declared at the same path. Removed first, as
+	// every orphan is, the file was deleted (or given back the original the mesh once wrote over)
+	// and stayed so until the new resource's turn came, with every resource in between applied on a
+	// machine without it. The record going is kept until the one declaring the path now is recorded
+	// in its place: then it is forgotten, and what it kept of the original goes with the file.
+	heldFiles := filesHeld(d.Resources)
+	handedFiles := map[string]store.Applied{}
+
 	removeOrphan := func(orphan store.Applied) error {
 		var action, detail string
 		var err error
+		if by, held := heldFiles[fileKey(orphan)]; held && by != orphan.ID {
+			handedFiles[by] = orphan
+			return nil
+		}
 		if declaration.Type(orphan.Type) == declaration.TypeService {
 			if by, held := heldUnits[unitKey(orphan.Scope, orphan.User, orphan.Target)]; held {
 				known.Forget(orphan.ID)
@@ -564,6 +579,11 @@ func ApplyMindingWindows(
 				!known.Recorded(string(declaration.TypeFile), f.Path) {
 				keepFound = keep
 			}
+			// A file handed over is the mesh's already: what the machine holds is judged against what
+			// this host last wrote there, under the record going (novox/hq ADR 0223).
+			if from, handed := handedFiles[resource.Identity()]; handed && was.ID == "" {
+				previous.Wrote = from.Wrote
+			}
 			outcome, err = applyOne(ctx, sys, resource, run, changed, in, previous, unseal, keepFound)
 		}
 		if err != nil {
@@ -668,6 +688,10 @@ func ApplyMindingWindows(
 		if kept == "" && was.Target == outcome.Target {
 			kept, keptMode, keptOwner = was.Kept, was.KeptMode, was.KeptOwner
 		}
+		from, handed := handedFiles[resource.Identity()]
+		if handed && kept == "" && from.Target == outcome.Target {
+			kept, keptMode, keptOwner = from.Kept, from.KeptMode, from.KeptOwner
+		}
 		// Only now. The record follows the fact, never leads it.
 		known.Record(store.Applied{
 			Origin: origin,
@@ -718,6 +742,16 @@ func ApplyMindingWindows(
 			}
 		}
 		report.Outcomes = append(report.Outcomes, outcome)
+		if handed {
+			// Recorded in its place, so the record going is forgotten now — and only now: a file whose
+			// new owner failed or was skipped keeps the old record, and the next apply hands it over.
+			delete(handedFiles, resource.Identity())
+			known.Forget(from.ID)
+			detail := "no longer declared; " + resource.Identity() + " declares the file now, so it was handed to it, not removed"
+			report.Outcomes = append(report.Outcomes, Outcome{ID: from.ID, Type: from.Type, Target: from.Target,
+				Action: "forgotten", Detail: detail})
+			log(fmt.Sprintf("  forgotten %s (%s): %s", from.ID, from.Target, detail))
+		}
 		if outcome.Action == heldStill {
 			// Not changed: nothing about it moved, so nothing that restarts on it restarts.
 			log(fmt.Sprintf("  %s %s (%s): %s", outcome.Action, outcome.ID, outcome.Target, outcome.Detail))
@@ -2863,6 +2897,28 @@ func unitsHeld(resources []declaration.Resource) map[string]string {
 	return held
 }

+// filesHeld is every path a declared file is written whole at, by fileKey, naming the file. A file
+// written into (a block, a JSON document) is not here: its region or members are keyed by its id, and
+// another resource's are a different region of the same file.
+func filesHeld(resources []declaration.Resource) map[string]string {
+	held := map[string]string{}
+	for _, r := range resources {
+		if f, ok := r.(*declaration.File); ok && f.Into == "" {
+			held[filepath.Clean(f.Path)] = f.ID
+		}
+	}
+	return held
+}
+
+// fileKey is a record's key in filesHeld: its path, for a file the host wrote whole, and nothing for
+// anything else.
+func fileKey(a store.Applied) string {
+	if declaration.Type(a.Type) != declaration.TypeFile || a.Into != nil || a.Target == "" {
+		return ""
+	}
+	return filepath.Clean(a.Target)
+}
+
 // unitKey names a unit by the manager it is in and its name (novox/hq ADR 0177): the machine's
 // manager, or one account's. A system unit is the same key whether its record says "system" or
 // nothing, as every record before the scope existed does.
@@ -0,0 +1,179 @@
+package apply
+
+import (
+	"context"
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/novox/mesh-host/internal/store"
+)
+
+// novox/hq ADR 0223: the machine's resolver file moves from the module that wrote it to the uplink's
+// holder, and the two records meet in one apply — the old undeclared, the new declared at the same
+// path. Every orphan is removed before anything is applied, so without a handover the file was
+// deleted (or given back the original the mesh once wrote over) and every resource applied before the
+// new one ran on a machine without it. Handed over, the file is never absent, the record going is
+// forgotten only once the new one is recorded, and what was kept of the machine's original goes with
+// the file to its new owner.
+
+const (
+	theOriginal = "nameserver 192.0.2.53\n"
+	firstOwners = "# the mesh, as the first module wrote it\nnameserver 10.42.0.1\n"
+	newOwners   = "# the mesh, as the uplink writes it\nnameserver 10.42.0.1\nnameserver 10.42.0.3\n"
+)
+
+// heldBy is a declaration whose file at path is the module's, with a service declared ahead of it
+// so the apply does something on the machine before the file's turn comes.
+func heldBy(t *testing.T, path, module, content string) string {
+	t.Helper()
+	return fmt.Sprintf(`{"declaration":1,"resources":[
+	  {"id":"%[1]s.service","type":"service","unit":"%[1]s.service","state":"running"},
+	  {"id":"%[1]s.fact-resolvers","type":"file","path":%[2]q,"mode":"0644","content":%[3]q}
+	]}`, module, path, content)
+}
+
+// watching is a service manager that answers every unit running, and fails the test whenever the
+// file is not there while it is asked something — the moment between an orphan's removal and the
+// new owner's turn.
+func watching(t *testing.T, path string) Runner {
+	t.Helper()
+	var commands []string
+	services := recordingServices(&commands)
+	return func(ctx context.Context, name string, args ...string) (string, error) {
+		if _, err := os.Stat(path); err != nil {
+			t.Errorf("the file was not there while %s %s ran: %v", name, strings.Join(args, " "), err)
+		}
+		return services(ctx, name, args...)
+	}
+}
+
+func TestAWholeFileIsHandedToItsNewOwnerNotRemoved(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "resolv.conf")
+	if err := os.WriteFile(path, []byte(theOriginal), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	keep := KeepIn(filepath.Join(dir, "kept"))
+	apply := func(raw string, known store.State) (Report, store.State) {
+		t.Helper()
+		report, state, err := ApplyKeeping(context.Background(), archHost(t), parse(t, raw), known,
+			store.OriginDeclared, watching(t, path), nil, nil, keep)
+		if err != nil {
+			t.Fatalf("apply failed: %v", err)
+		}
+		return report, state
+	}
+
+	// The first owner writes over the machine's file, keeping the original.
+	_, state := apply(heldBy(t, path, "resolv-conf", firstOwners), store.State{})
+	first, _ := state.Find("resolv-conf.fact-resolvers")
+	if first.Kept == "" {
+		t.Fatal("the machine's original was not kept before the first write")
+	}
+
+	// The uplink takes it over in one apply.
+	report, state := apply(heldBy(t, path, "networkmanager", newOwners), state)
+	if got, _ := os.ReadFile(path); string(got) != newOwners {
+		t.Fatalf("after the handover the file holds %q", got)
+	}
+	for _, o := range report.Outcomes {
+		if o.ID == "resolv-conf.fact-resolvers" && o.Action != "forgotten" {
+			t.Errorf("the record going was %q, not forgotten: %s", o.Action, o.Detail)
+		}
+	}
+	if _, still := state.Find("resolv-conf.fact-resolvers"); still {
+		t.Error("the record going is still recorded after its file was handed over")
+	}
+	now, _ := state.Find("networkmanager.fact-resolvers")
+	if now.Kept != first.Kept {
+		t.Errorf("the new owner keeps the original at %q; the first kept it at %q", now.Kept, first.Kept)
+	}
+	if kept, _ := os.ReadFile(now.Kept); string(kept) != theOriginal {
+		t.Errorf("what the new owner would give back is %q, not the machine's original", kept)
+	}
+	if aside, _ := filepath.Glob(path + ".removed-*"); len(aside) > 0 {
+		t.Errorf("the file was moved aside on the way: %v", aside)
+	}
+
+	// Steady from here, and undeclared the machine's original comes back — not the first owner's.
+	report, state = apply(heldBy(t, path, "networkmanager", newOwners), state)
+	for _, o := range report.Outcomes {
+		if o.ID == "networkmanager.fact-resolvers" && o.Action != "unchanged" {
+			t.Errorf("a second apply was %q: %s", o.Action, o.Detail)
+		}
+	}
+	if _, _, err := ApplyKeeping(context.Background(), archHost(t), somethingElse(t), state,
+		store.OriginDeclared, recordingServices(new([]string)), nil, nil, keep); err != nil {
+		t.Fatal(err)
+	}
+	if got, _ := os.ReadFile(path); string(got) != theOriginal {
+		t.Errorf("undeclaring the new owner left %q; the machine's original goes back", got)
+	}
+}
+
+// A file the mesh made where there was none is handed over the same way, and is still the mesh's —
+// undeclared, it goes.
+func TestAFileTheMeshMadeIsHandedOverAndStillGoesWithItsLastOwner(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "resolv.conf")
+	run := watching(t, path)
+	_, state, err := Apply(context.Background(), archHost(t), parse(t, heldBy(t, path, "resolv-conf", firstOwners)),
+		store.State{}, store.OriginDeclared, recordingServices(new([]string)), nil, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, state, err = Apply(context.Background(), archHost(t), parse(t, heldBy(t, path, "dhcpcd", newOwners)),
+		state, store.OriginDeclared, run, nil, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got, _ := os.ReadFile(path); string(got) != newOwners {
+		t.Fatalf("after the handover the file holds %q", got)
+	}
+	if _, _, err := Apply(context.Background(), archHost(t), somethingElse(t), state, store.OriginDeclared,
+		recordingServices(new([]string)), nil, nil); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := os.Stat(path); !os.IsNotExist(err) {
+		t.Errorf("the mesh's own file stayed after its last owner was undeclared: %v", err)
+	}
+}
+
+// A file edited on the machine since the first owner wrote it is still corrected and said so by the
+// new owner: the handover judges drift against what this host last wrote there.
+func TestAHandedOverFileChangedOnTheMachineIsSaidCorrected(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "resolv.conf")
+	_, state, err := Apply(context.Background(), archHost(t), parse(t, heldBy(t, path, "resolv-conf", firstOwners)),
+		store.State{}, store.OriginDeclared, recordingServices(new([]string)), nil, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	_ = os.WriteFile(path, []byte("nameserver 1.1.1.1\n"), 0o644)
+	report, _, err := Apply(context.Background(), archHost(t), parse(t, heldBy(t, path, "systemd-networkd", newOwners)),
+		state, store.OriginDeclared, recordingServices(new([]string)), nil, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, o := range report.Outcomes {
+		if o.ID == "systemd-networkd.fact-resolvers" && o.Action != "corrected" {
+			t.Errorf("a handed-over file changed on the machine was %q: %s", o.Action, o.Detail)
+		}
+	}
+}
+
+// The preview says the same before it happens.
+func TestThePlanHandsAFileOverRatherThanRemovingIt(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "resolv.conf")
+	_, state, err := Apply(context.Background(), archHost(t), parse(t, heldBy(t, path, "resolv-conf", firstOwners)),
+		store.State{}, store.OriginDeclared, recordingServices(new([]string)), nil, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, step := range Plan(parse(t, heldBy(t, path, "networkmanager", newOwners)), state, store.OriginDeclared) {
+		if step.ID == "resolv-conf.fact-resolvers" && (step.Verb != "forget" || !strings.Contains(step.Why, "networkmanager.fact-resolvers")) {
+			t.Errorf("the plan would %s the file being handed over: %s", step.Verb, step.Why)
+		}
+	}
+}
@@ -85,11 +85,16 @@ func Plan(d *declaration.Declaration, known store.State, origin string) []Step {
 	var protecting, orphans []Step
 	made := meshMadeUnits(known)
 	heldUnits := unitsHeld(d.Resources)
+	heldFiles := filesHeld(d.Resources)
 	for _, orphan := range known.Orphans(declared, origin) {
 		step := Step{Verb: "remove", Type: orphan.Type, ID: orphan.ID, Target: orphan.Target,
 			Why: "recorded here and no longer declared"}
 		held := heldUnits[unitKey(orphan.Scope, orphan.User, orphan.Target)]
+		by, handed := heldFiles[fileKey(orphan)]
 		switch {
+		case handed:
+			// In ApplyKeeping's words (novox/hq ADR 0223).
+			step.Verb, step.Why = "forget", "no longer declared; "+by+" declares the file now, so it is handed to it, not removed"
 		case orphan.Type == string(declaration.TypeService) && held != "":
 			// In removeOrphan's words (novox/hq issue 190).
 			step.Verb, step.Why = "forget", "no longer declared; "+held+" still holds the unit, so it is left as it is"