carlos / platform Public

appconfig: reap drop-ins for units no route names, recording the reload owed first

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Paul Campbell pushed by paul@keymail.dev 4ce1b888d2eb46b2ed4beac8f53e16a01bb3d75e parent f1d93b5
3 files changed, +140 −0
  • internal/appconfig/sync.go +1 −0
  • internal/appconfig/unitsync.go +36 −0
  • internal/appconfig/unitsync_test.go +103 −0
diff --git a/internal/appconfig/sync.go b/internal/appconfig/sync.go
index 4c2dd609b..27de091e9 100644
--- a/internal/appconfig/sync.go
+++ b/internal/appconfig/sync.go
@@ -203,6 +203,7 @@ func SyncUnits(ctx context.Context, reg *registry.Registry, s release.Store, pri
syncHost(rt, m, r.defaultMissing, priv, envDir, restart)
}
if ops != nil {
+ reapDropIns(routes, ops)
applyStamped(envDir, ops, plan, work)
}
// An empty route list is never authority to delete anything:
diff --git a/internal/appconfig/unitsync.go b/internal/appconfig/unitsync.go
index a5e217ab1..9e819e8f8 100644
--- a/internal/appconfig/unitsync.go
+++ b/internal/appconfig/unitsync.go
@@ -185,6 +185,42 @@ func prepareStamped(rt registry.Route, unit string, m *Manifest, defaultMissing
return w
}
+// reapDropIns (spec §3.2 reaping, step 2): for every marker-bearing drop-in
+// whose unit is named by no route row on the box, record the reload it
+// owes FIRST, then remove the drop-in (codex R3-2). Keyed off the full
+// route list, never the delivered subset; an empty list is never authority
+// to delete anything (the same rule as reap for env files). Unmarked files
+// and symlinked directories are never touched.
+func reapDropIns(routes []registry.Route, ops *UnitOps) {
+ if len(routes) == 0 {
+ return
+ }
+ named := map[string]bool{}
+ for _, rt := range routes {
+ if rt.Unit != "" {
+ named[rt.Unit] = true
+ }
+ }
+ units, skipped := listMarkedDropIns(ops.DropInRoot)
+ for _, unit := range skipped {
+ fmt.Printf(" %s: %s is not a plain directory; not entered\n", unit, dropInDir(ops.DropInRoot, unit))
+ }
+ for _, unit := range units {
+ if named[unit] {
+ continue
+ }
+ if err := writePending(ops.DropInRoot, unit); err != nil {
+ fmt.Printf(" %s: record reload owed: %v (drop-in kept)\n", unit, err)
+ continue
+ }
+ if err := removeDropIn(ops.DropInRoot, unit); err != nil {
+ fmt.Printf(" %s: remove drop-in: %v\n", unit, err)
+ continue
+ }
+ fmt.Printf(" %s: no route names this unit, removed its drop-in; reload owed\n", unit)
+ }
+}
+
// envFilesMatch: the unit's effective EnvironmentFile list must be EXACTLY
// carlos's one file (spec §3.2 step 4, codex R3-3). Presence or last
// position is not enough: a later drop-in that resets the list and re-adds
diff --git a/internal/appconfig/unitsync_test.go b/internal/appconfig/unitsync_test.go
index 530789309..83335207b 100644
--- a/internal/appconfig/unitsync_test.go
+++ b/internal/appconfig/unitsync_test.go
@@ -469,6 +469,109 @@ func TestSyncUnitsUnreadableDropInKeepsMarker(t *testing.T) {
}
}
+func TestSyncUnitsReapsDropInForAnUnstampedRouteWithMarkerFirst(t *testing.T) {
+ ctx, s, reg, envDir, priv := unitKit(t)
+ f := newFakeUnits(t)
+ f.loaded("relay.service", envDir, "relay.example.com")
+ upsertStamped(t, reg, "relay.example.com", "relay.service", nil)
+ syncOnce(t, ctx, reg, s, priv, envDir, f)
+ // Unstamp: the row stays, the unit is no longer named by any route.
+ if err := reg.SetUnit("relay.example.com", ""); err != nil {
+ t.Fatal(err)
+ }
+ f.reloads, f.restarted = 0, nil
+ f.reloadErr = errors.New("busy")
+ syncOnce(t, ctx, reg, s, priv, envDir, f)
+ if _, exists, _, _ := readDropIn(f.root, "relay.service"); exists {
+ t.Fatal("drop-in must be removed for a unit no route names")
+ }
+ if !hasPending(f.root, "relay.service") {
+ t.Fatal("the reload owed must be recorded when the reload fails")
+ }
+ if len(f.restarted) != 0 {
+ t.Fatal("an unowned unit is never restarted")
+ }
+ f.reloadErr = nil
+ syncOnce(t, ctx, reg, s, priv, envDir, f)
+ if hasPending(f.root, "relay.service") || len(f.restarted) != 0 {
+ t.Fatalf("after a good reload: pending=%v restarted=%v", hasPending(f.root, "relay.service"), f.restarted)
+ }
+ if _, err := os.Stat(dropInDir(f.root, "relay.service")); err != nil {
+ t.Fatal("the directory is never removed")
+ }
+ // The env file stays while the row exists (today's reap policy).
+ if _, err := os.Stat(filepath.Join(envDir, "relay.example.com.env")); err != nil {
+ t.Fatal("env file must stay while the route row exists")
+ }
+}
+
+func TestSyncUnitsReaperNeverRemovesUnmarkedOrSymlinked(t *testing.T) {
+ ctx, s, reg, envDir, priv := unitKit(t)
+ f := newFakeUnits(t)
+ os.MkdirAll(dropInDir(f.root, "foreign.service"), 0o755)
+ foreign := []byte("[Service]\nEnvironmentFile=/x\n")
+ os.WriteFile(filepath.Join(dropInDir(f.root, "foreign.service"), DropInFile), foreign, 0o644)
+ elsewhere := t.TempDir()
+ os.WriteFile(filepath.Join(elsewhere, DropInFile), renderDropIn(envDir, "h"), 0o644)
+ os.Symlink(elsewhere, dropInDir(f.root, "linked.service"))
+ // One unrelated carlos-app@ route so the route list is not empty.
+ if err := reg.Upsert(registry.Route{Host: "km.example.com", Kind: "instance", App: "relay", Account: "bab",
+ Status: "ready", Backing: registry.BackingUnit}); err != nil {
+ t.Fatal(err)
+ }
+ if err := SyncUnits(ctx, reg, s, priv, envDir, func(string) error { return nil }, f.ops()); err != nil {
+ t.Fatal(err)
+ }
+ if cur, _, _, _ := readDropIn(f.root, "foreign.service"); string(cur) != string(foreign) {
+ t.Fatal("unmarked file touched")
+ }
+ if _, err := os.Stat(filepath.Join(elsewhere, DropInFile)); err != nil {
+ t.Fatal("file behind a symlinked directory removed")
+ }
+ if f.reloads != 0 {
+ t.Fatal("nothing carlos owned changed; no reload")
+ }
+}
+
+func TestSyncUnitsEmptyRegistryReapsNoDropIns(t *testing.T) {
+ ctx, s, reg, envDir, priv := unitKit(t)
+ f := newFakeUnits(t)
+ ensureDropInDir(f.root, "relay.service")
+ writeDropIn(f.root, "relay.service", renderDropIn(envDir, "relay.example.com"))
+ if err := SyncUnits(ctx, reg, s, priv, envDir, func(string) error { return nil }, f.ops()); err != nil {
+ t.Fatal(err)
+ }
+ if _, exists, _, _ := readDropIn(f.root, "relay.service"); !exists {
+ t.Fatal("an empty route list is never authority to delete")
+ }
+}
+
+func TestSyncUnitsReStampMovesTheDropIn(t *testing.T) {
+ ctx, s, reg, envDir, priv := unitKit(t)
+ f := newFakeUnits(t)
+ f.loaded("a.service", envDir, "relay.example.com")
+ f.loaded("b.service", envDir, "relay.example.com")
+ upsertStamped(t, reg, "relay.example.com", "a.service", nil)
+ syncOnce(t, ctx, reg, s, priv, envDir, f)
+ if err := reg.SetUnit("relay.example.com", "b.service"); err != nil {
+ t.Fatal(err)
+ }
+ f.restarted = nil
+ syncOnce(t, ctx, reg, s, priv, envDir, f)
+ if _, exists, _, _ := readDropIn(f.root, "a.service"); exists {
+ t.Fatal("old unit's drop-in must be reaped")
+ }
+ if _, exists, marked, _ := readDropIn(f.root, "b.service"); !exists || !marked {
+ t.Fatal("new unit's drop-in must be written")
+ }
+ if len(f.restarted) != 1 || f.restarted[0] != "b.service" {
+ t.Fatalf("restarted = %v, want [b.service] only", f.restarted)
+ }
+ if hasPending(f.root, "a.service") || hasPending(f.root, "b.service") {
+ t.Fatal("both markers must be cleared after one good reload and restart")
+ }
+}
+
func TestEnvFilesMatch(t *testing.T) {
want := "/etc/carlos/apps/h.env"
if !envFilesMatch([]string{want}, want) {