From 4347983a72136061595c3c02b4db4455cf5587b3 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Mon, 5 Oct 2026 22:02:03 +0200 Subject: [PATCH] R-682: a Remove cut off by a controller restart is finished at boot RemoveStack journals itself before compose down and clears on every return; at start a found journal finishes the remove through the same RemoveStack (once, before the boot reconciler), keeping drive data and backups even if the household had asked to delete them (no unattended deletion at boot; logged). Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS --- controller/cmd/controller/main.go | 5 + controller/internal/stacks/delete.go | 7 ++ .../stacks/r682_remove_interrupted_test.go | 71 ++++++++++++++ .../internal/stacks/remove_interrupted.go | 94 +++++++++++++++++++ 4 files changed, 177 insertions(+) create mode 100644 controller/internal/stacks/r682_remove_interrupted_test.go create mode 100644 controller/internal/stacks/remove_interrupted.go diff --git a/controller/cmd/controller/main.go b/controller/cmd/controller/main.go index f237618..50392b2 100644 --- a/controller/cmd/controller/main.go +++ b/controller/cmd/controller/main.go @@ -497,6 +497,11 @@ func main() { // sweep could bring up a half-updated app with no record of why. Pin-backs for updates interrupted // before anything ran happen here too. The resumed health wait itself is launched further down, // once the backup side is wired, because a resumed update that fails must be able to HOLD. + // R-682: a Remove a restart cut off is finished first (drive data and backups kept) — before the + // boot reconciler, which would otherwise start the half-removed app again. + if names := stackMgr.RecoverInterruptedRemoves(); len(names) > 0 { + log.Printf("[WARN] [stacks] %d remove(s) interrupted by the restart were finished: %v", len(names), names) + } if resumed := stackMgr.RecoverUpdates(); len(resumed) > 0 { logger.Printf("[WARN] [update] %d interrupted update(s) will resume after the backup side is wired: %v", len(resumed), resumed) } diff --git a/controller/internal/stacks/delete.go b/controller/internal/stacks/delete.go index b6a4ea7..1352d03 100644 --- a/controller/internal/stacks/delete.go +++ b/controller/internal/stacks/delete.go @@ -667,6 +667,13 @@ func (m *Manager) RemoveStack(name string, removeHDDData bool, backupPathsToRemo // 2026-09-30: v0.284.0 wired only DeleteStack, and the household's Remove button runs THIS function. removedRepos := appImageRepos(stackDir, LoadAppConfig(stackDir)) + // R-682: journal the remove before anything is torn down; every return below clears it, so a + // marker found at start means the process died mid-remove (remove_interrupted.go). + if err := markRemovePending(stackDir, removeHDDData, len(backupPathsToRemove)); err != nil { + m.logger.Printf("[WARN] [stacks] RemoveStack %s: cannot journal the remove (a restart mid-remove would leave it half-done): %v", name, err) + } + defer clearRemovePending(stackDir) + // Step 2: Run docker compose down --volumes env := m.stackEnv(stackDir) // R-489 (v0.242.0): the volumes are listed BEFORE and AFTER; the difference is what was removed. diff --git a/controller/internal/stacks/r682_remove_interrupted_test.go b/controller/internal/stacks/r682_remove_interrupted_test.go new file mode 100644 index 0000000..07b97b9 --- /dev/null +++ b/controller/internal/stacks/r682_remove_interrupted_test.go @@ -0,0 +1,71 @@ +package stacks + +import ( + "os" + "path/filepath" + "testing" +) + +// R-682 — the consequence: a remove the process died in is FINISHED at the next start (the app no +// longer reads installed, its record is gone), and the drive data the household asked to delete is +// KEPT (no unattended deletion at boot); the marker is gone afterwards. +func TestR682_InterruptedRemoveIsFinishedAtBootKeepingData(t *testing.T) { + drive := t.TempDir() + m, dir, composeRan := newR442Manager(t, "app", driveCompose, driveAppYAML(drive), drive) + plantFile(t, filepath.Join(drive, "appdata", "app", "keep.txt")) // a path the compose binds: removable + // The state a kill mid-remove leaves: journal written (asking for drive data too), app.yaml present. + if err := markRemovePending(dir, true, 2); err != nil { + t.Fatal(err) + } + + got := m.RecoverInterruptedRemoves() + if len(got) != 1 || got[0] != "app" { + t.Fatalf("RecoverInterruptedRemoves = %v, want [app]", got) + } + if _, err := os.Stat(composeRan); err != nil { + t.Error("the finishing remove must run compose down") + } + if _, err := os.Stat(filepath.Join(dir, "app.yaml")); !os.IsNotExist(err) { + t.Errorf("app.yaml survived: the app still reads installed (err=%v)", err) + } + if st, ok := m.GetStack("app"); ok && st.Deployed { + t.Error("the app still reads deployed after the recovery") + } + if _, err := os.Stat(filepath.Join(drive, "appdata", "app", "keep.txt")); err != nil { + t.Errorf("drive data was deleted unattended at boot: %v", err) + } + if _, err := os.Stat(filepath.Join(dir, removePendingFile)); !os.IsNotExist(err) { + t.Error("the remove marker survived the recovery") + } +} + +// RemoveStack journals before compose down and clears on return; a clean start with no marker +// touches nothing. +func TestR682_RemoveJournalsAndClears(t *testing.T) { + drive := t.TempDir() + m, dir, _ := newR442Manager(t, "app", ssdCompose, driveAppYAML(drive), drive) + // The stub compose records whether the journal existed when `down` ran. + seen := filepath.Join(t.TempDir(), "journal-seen") + stub := t.TempDir() + script := "#!/bin/sh\ntest -f " + filepath.Join(dir, removePendingFile) + " && touch " + seen + "\nexit 0\n" + if err := os.WriteFile(filepath.Join(stub, "docker-compose"), []byte(script), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", stub+string(os.PathListSeparator)+os.Getenv("PATH")) + + if got := m.RecoverInterruptedRemoves(); len(got) != 0 { + t.Fatalf("no marker: nothing to recover, got %v", got) + } + if _, err := os.Stat(filepath.Join(dir, "app.yaml")); err != nil { + t.Fatal("a start with no marker removed the app") + } + if _, err := m.RemoveStack("app", false, nil); err != nil { + t.Fatalf("RemoveStack: %v", err) + } + if _, err := os.Stat(seen); err != nil { + t.Error("the remove journal did not exist while compose down ran") + } + if _, err := os.Stat(filepath.Join(dir, removePendingFile)); !os.IsNotExist(err) { + t.Error("the journal survived a completed remove") + } +} diff --git a/controller/internal/stacks/remove_interrupted.go b/controller/internal/stacks/remove_interrupted.go new file mode 100644 index 0000000..ce6c134 --- /dev/null +++ b/controller/internal/stacks/remove_interrupted.go @@ -0,0 +1,94 @@ +package stacks + +import ( + "encoding/json" + "os" + "path/filepath" + "time" +) + +// ── A Remove cut off by a controller restart is finished at boot (R-682) ───────────────────────────── +// +// MEASURED 2026-09-24 on 9202 (chaos round 9): a kill 2 s after the Remove press answered the household +// 502; after the restart the app read deployed and stopped with NO container left — half-removed, and +// nothing told the household to press Remove again. An install (R-681) and an update journal themselves +// and are finished at boot; a remove did not. +// +// THE MECHANISM: RemoveStack writes removePendingFile after every refusal and BEFORE `compose down`, and +// removes it on every return (success or a reported failure — the household saw that answer). So a +// marker found at start means the process died mid-remove, and RecoverInterruptedRemoves finishes it: +// - app.yaml already gone → the remove had reached its last step; leftovers of the applied record and +// the marker go; +// - otherwise → the SAME RemoveStack a second press runs, but KEEPING the drive data and the backups +// even when the household had asked to delete them. Deleting customer data unattended at boot, while +// drives may still be settling, is the one step this recovery does not take (R-442's fail-closed +// direction: when unsure, the data stays). The kept request is logged; the app reads not installed, +// and its leftover data stays reachable through the not-installed app's own delete action. +// Pinned by r682_remove_interrupted_test.go. + +const removePendingFile = ".felhom-remove-pending" + +type removeJournal struct { + At string `json:"at"` + RemoveHDDData bool `json:"remove_hdd_data"` + BackupPathsAsked int `json:"backup_paths_asked"` +} + +func markRemovePending(stackDir string, removeHDDData bool, backupPaths int) error { + b, _ := json.Marshal(removeJournal{At: time.Now().UTC().Format(time.RFC3339), RemoveHDDData: removeHDDData, BackupPathsAsked: backupPaths}) + return os.WriteFile(filepath.Join(stackDir, removePendingFile), append(b, '\n'), 0o644) +} + +func clearRemovePending(stackDir string) { + _ = os.Remove(filepath.Join(stackDir, removePendingFile)) +} + +// RecoverInterruptedRemoves runs once at start, after the initial scan and BEFORE the boot reconciler +// (which would otherwise start a half-removed app). It returns the names it finished. +func (m *Manager) RecoverInterruptedRemoves() []string { + m.mu.RLock() + type cand struct{ name, dir string } + var cands []cand + for name, st := range m.stacks { + if st.ComposePath == "" { + continue + } + dir := filepath.Dir(st.ComposePath) + if _, err := os.Stat(filepath.Join(dir, removePendingFile)); err == nil { + cands = append(cands, cand{name, dir}) + } + } + m.mu.RUnlock() + + var out []string + for _, c := range cands { + var j removeJournal + if b, err := os.ReadFile(filepath.Join(c.dir, removePendingFile)); err == nil { + _ = json.Unmarshal(b, &j) // an unreadable journal finishes the safe way (data kept) + } + if _, err := os.Stat(filepath.Join(c.dir, "app.yaml")); os.IsNotExist(err) { + for _, p := range []string{AppliedComposePath(c.dir), filepath.Join(c.dir, appliedMetaDir)} { + _ = os.RemoveAll(p) + } + clearRemovePending(c.dir) + m.logger.Printf("[INFO] [stacks] remove %s: had reached its last step before the restart; marker cleared (R-682)", c.name) + out = append(out, c.name) + continue + } + m.logger.Printf("[WARN] [stacks] remove %s was INTERRUPTED by a controller restart (pressed %s) — finishing it now, keeping drive data and backups (R-682)", c.name, j.At) + if j.RemoveHDDData || j.BackupPathsAsked > 0 { + m.logger.Printf("[WARN] [stacks] remove %s: the household had asked to delete drive data=%v and %d backup path(s) — NOT done unattended; the data stays (R-682)", c.name, j.RemoveHDDData, j.BackupPathsAsked) + } + if _, err := m.RemoveStack(c.name, false, nil); err != nil { + // ONE attempt, never a retry at a later start: a refusal here (the app came back running, or + // is busy) leaves it as the household sees it, and a marker kept for later would turn a + // deliberate stop next week into a removal nobody asked for. A refusal returns before the + // journal is written, so the marker is cleared here explicitly. + clearRemovePending(c.dir) + m.logger.Printf("[ERROR] [stacks] remove %s: finishing the interrupted remove failed: %v — the Remove button finishes it", c.name, err) + continue + } + out = append(out, c.name) + } + return out +}