v0.189.0 — desired state + the app-stop crash marker (R-166 / D-b)

The box stops inferring the customer's intent from a container count and reads what they actually asked for. Part 1 — desired state. AppConfig gains a tri-state `desired_state` (""/running/stopped), written ONLY by the customer's own action: the API action switch, DeployStack, UpdateOptionalConfig's redeploy branch, and the .fab import. Intent is written BEFORE the act and a failed write REFUSES the act. StartStack/StopStack are deliberately not writers — 14 callers, only 2 are the customer. bootrecon.isBootOrphan now reads intent instead of len(Containers)>0, which closes R-157 mechanism B (a power cut or interrupted deploy left an app with zero containers, read as a deliberate stop, and stranded silently). ABSENT MEANS UNKNOWN, NEVER "running": every pre-v0.189.0 app.yaml reads absent, so the legacy fallback is byte-identical to the old rule. A running-only startup backfill converges the unambiguous cases; `stopped` is never inferred. Part 2 — backup.AppStopGuard, a persisted marker over every stop→work→start window (volume dump, offbox reconstitute, .fab export). Its own file, never quiesce's. Written before the stop, cleared only after a restart that succeeded, kept when one fails. Recover() completes before the boot reconciler is launched and returns its outcome, which main.go reports on the existing backup_failed event once the notifier exists. A defer is not the mechanism — a SIGKILL runs none (Campaign 8 fault 10). Also: SaveAppConfig rebuilt AppConfig field-by-field (the R-100 shape) and would have dropped desired_state on every save across nine call sites. Replaced with copy-and-overlay. Measured: app.yaml does not round-trip unknown YAML keys. No hub change, no agent coupling, no user-visible string. 27/27 packages green; 7 red-proofs observed FAIL then restored.
2026-08-02 18:40:17 +02:00
parent e7c44c0e0f
commit dbcb306fcf
17 changed files with 2211 additions and 33 deletions
@@ -0,0 +1,395 @@
+package backup
+
+import (
+	"encoding/json"
+	"errors"
+	"io"
+	"log"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// R-166 part 2 — the app-stop crash marker.
+//
+// THE DISCIPLINE THAT MATTERS HERE (§10): a `defer` is not crash-safety, so a test that lets the
+// deferred cleanup run proves nothing about a crash. Every "interrupted" test below simulates a
+// SIGKILL by never reaching the restart — the marker is written, the process conceptually dies, and
+// a FRESH guard over the SAME file does the recovering. That is exactly what Campaign 8 fault 10
+// established on live hardware: a SIGKILL runs no deferred function, and what brought the stacks
+// back was the marker read at startup.
+
+type fakeStarter struct {
+	starts   []string
+	failWith map[string]error
+}
+
+func (f *fakeStarter) StartStack(name string) error {
+	f.starts = append(f.starts, name)
+	if err := f.failWith[name]; err != nil {
+		return err
+	}
+	return nil
+}
+
+func newGuard(t *testing.T, dir string) (*AppStopGuard, *fakeStarter) {
+	t.Helper()
+	s := &fakeStarter{}
+	g := NewAppStopGuard(filepath.Join(dir, "appstop-state.json"), log.New(io.Discard, "", 0))
+	g.SetStarter(s)
+	return g, s
+}
+
+func markerPath(dir string) string { return filepath.Join(dir, "appstop-state.json") }
+
+func markerExists(t *testing.T, dir string) bool {
+	t.Helper()
+	_, err := os.Stat(markerPath(dir))
+	if err != nil && !os.IsNotExist(err) {
+		t.Fatal(err)
+	}
+	return err == nil
+}
+
+// --- Scenario E — a crash mid-backup brings the app back -----------------------------------------
+
+func TestRecover_InterruptedVolumeDump_RestartsTheAppAndClearsTheMarker(t *testing.T) {
+	dir := t.TempDir()
+
+	// --- process 1: an operation stops the app and is KILLED. No End(), no defer, no cleanup. ---
+	g1, _ := newGuard(t, dir)
+	if err := g1.Begin("volume-dump:immich", ReasonVolumeDump, []string{"immich"}); err != nil {
+		t.Fatalf("Begin: %v", err)
+	}
+	if !markerExists(t, dir) {
+		t.Fatal("Begin did not write a marker — nothing would survive the kill")
+	}
+	// <SIGKILL here> — g1 is abandoned deliberately; nothing else is called on it.
+
+	// --- process 2: a fresh controller starts and recovers from the file alone. ---
+	g2, starter := newGuard(t, dir)
+	res := g2.Recover()
+
+	if len(starter.starts) != 1 || starter.starts[0] != "immich" {
+		t.Fatalf("started %v, want exactly [immich] — the app was left stranded by the interrupted backup", starter.starts)
+	}
+	if res == nil || len(res.Restarted) != 1 || res.Restarted[0] != "immich" {
+		t.Fatalf("recovery result = %+v, want immich restarted", res)
+	}
+	if res.Reason != ReasonVolumeDump {
+		t.Fatalf("reason = %q, want %q — the operator must be told WHICH operation was interrupted", res.Reason, ReasonVolumeDump)
+	}
+	if markerExists(t, dir) {
+		t.Fatal("the marker survived a successful recovery — the next boot would restart the app again")
+	}
+	// The operator-facing text must name the interruption, not merely report a restart.
+	if msg := res.Message(); msg == "" || !strings.Contains(msg, "interrupted") {
+		t.Fatalf("operator message %q does not say the operation was interrupted", msg)
+	}
+}
+
+func TestRecover_NoMarker_IsASilentNoOp(t *testing.T) {
+	dir := t.TempDir()
+	g, starter := newGuard(t, dir)
+	if res := g.Recover(); res != nil {
+		t.Fatalf("Recover reported %+v on a box with no marker", res)
+	}
+	if len(starter.starts) != 0 {
+		t.Fatalf("started %v with no marker present", starter.starts)
+	}
+}
+
+func TestRecover_FailedRestart_KEEPSTheMarkerForTheNextStartup(t *testing.T) {
+	// The single most important failure behaviour: clearing a marker whose restart failed would
+	// erase the only durable record that an app is owed one. The app is genuinely still down.
+	dir := t.TempDir()
+	g1, _ := newGuard(t, dir)
+	if err := g1.Begin("volume-dump:immich", ReasonVolumeDump, []string{"immich", "nextcloud"}); err != nil {
+		t.Fatal(err)
+	}
+
+	g2, starter := newGuard(t, dir)
+	starter.failWith = map[string]error{"immich": errors.New("compose up: no such image")}
+	res := g2.Recover()
+
+	if len(res.Failed) != 1 || res.Failed[0] != "immich" {
+		t.Fatalf("failed=%v, want [immich]", res.Failed)
+	}
+	if len(res.Restarted) != 1 || res.Restarted[0] != "nextcloud" {
+		t.Fatalf("restarted=%v, want [nextcloud] — one app failing must not abort the others", res.Restarted)
+	}
+	if !markerExists(t, dir) {
+		t.Fatal("the marker was cleared even though a restart FAILED — the next startup would not retry")
+	}
+	if msg := res.Message(); !strings.Contains(msg, "NOT be restarted") {
+		t.Fatalf("operator message %q does not report the failure", msg)
+	}
+	if d := res.Detail(); !strings.Contains(d, "restart_failed") || !strings.Contains(d, "immich") {
+		t.Fatalf("detail %q does not name which app failed", d)
+	}
+}
+
+func TestRecover_IsIdempotentAcrossRepeatedStartups(t *testing.T) {
+	dir := t.TempDir()
+	g1, _ := newGuard(t, dir)
+	if err := g1.Begin("op", ReasonOffboxReconstitute, []string{"immich"}); err != nil {
+		t.Fatal(err)
+	}
+	g2, s2 := newGuard(t, dir)
+	g2.Recover()
+	g3, s3 := newGuard(t, dir)
+	g3.Recover()
+
+	if len(s2.starts) != 1 {
+		t.Fatalf("first recovery started %v", s2.starts)
+	}
+	if len(s3.starts) != 0 {
+		t.Fatalf("a SECOND startup restarted %v again — the marker was not cleared", s3.starts)
+	}
+}
+
+func TestRecover_CorruptMarkerIsQuarantinedNotSilentlySkipped(t *testing.T) {
+	// §9.4: never a silent skip. A corrupt marker cannot be acted on, but it must leave a trace —
+	// otherwise a genuinely interrupted operation vanishes without evidence.
+	dir := t.TempDir()
+	if err := os.WriteFile(markerPath(dir), []byte("{not json"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	g, starter := newGuard(t, dir)
+	if res := g.Recover(); res != nil {
+		t.Fatalf("a corrupt marker produced a recovery result %+v", res)
+	}
+	if len(starter.starts) != 0 {
+		t.Fatalf("apps were started from a corrupt marker: %v", starter.starts)
+	}
+	if markerExists(t, dir) {
+		t.Fatal("the corrupt marker was left in place — it would be re-read forever")
+	}
+	quarantined, _ := filepath.Glob(markerPath(dir) + ".corrupt-*")
+	if len(quarantined) != 1 {
+		t.Fatalf("the corrupt marker was not quarantined (found %d) — it was silently dropped", len(quarantined))
+	}
+}
+
+func TestRecover_NoStarterWiredKeepsTheMarker(t *testing.T) {
+	// D-b's safety rule: never worse than not having the file. With no starter the guard cannot act,
+	// so it must keep the record for a startup that can, rather than clear it and lose the app.
+	dir := t.TempDir()
+	g1, _ := newGuard(t, dir)
+	if err := g1.Begin("op", ReasonVolumeDump, []string{"immich"}); err != nil {
+		t.Fatal(err)
+	}
+	g2 := NewAppStopGuard(markerPath(dir), log.New(io.Discard, "", 0)) // deliberately no SetStarter
+	if res := g2.Recover(); res != nil {
+		t.Fatalf("recovered without a starter: %+v", res)
+	}
+	if !markerExists(t, dir) {
+		t.Fatal("the marker was cleared with no starter wired — the app would never come back")
+	}
+}
+
+func TestNilGuardIsInert(t *testing.T) {
+	// A caller that was never wired must degrade to pre-v0.189.0 behaviour, not panic.
+	var g *AppStopGuard
+	if err := g.Begin("op", ReasonVolumeDump, []string{"x"}); err != nil {
+		t.Fatalf("nil guard Begin returned %v", err)
+	}
+	g.End()
+	if res := g.Recover(); res != nil {
+		t.Fatalf("nil guard recovered %+v", res)
+	}
+}
+
+func TestMarkerContentsAreDiagnosable(t *testing.T) {
+	dir := t.TempDir()
+	g, _ := newGuard(t, dir)
+	if err := g.Begin("volume-dump:immich", ReasonVolumeDump, []string{"immich"}); err != nil {
+		t.Fatal(err)
+	}
+	raw, err := os.ReadFile(markerPath(dir))
+	if err != nil {
+		t.Fatal(err)
+	}
+	var m AppStopMarker
+	if err := json.Unmarshal(raw, &m); err != nil {
+		t.Fatalf("the marker on disk is not readable JSON: %v", err)
+	}
+	if !m.Active || m.OpID != "volume-dump:immich" || m.Reason != ReasonVolumeDump ||
+		len(m.Stacks) != 1 || m.Stacks[0] != "immich" || m.StartedAt.IsZero() {
+		t.Fatalf("the marker does not record enough to diagnose the interruption: %+v", m)
+	}
+	// 0600 — it names customer apps.
+	fi, err := os.Stat(markerPath(dir))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if fi.Mode().Perm() != 0o600 {
+		t.Fatalf("marker mode = %v, want 0600", fi.Mode().Perm())
+	}
+}
+
+func TestBeginWithNoStacksWritesNothing(t *testing.T) {
+	dir := t.TempDir()
+	g, _ := newGuard(t, dir)
+	if err := g.Begin("op", ReasonVolumeDump, nil); err != nil {
+		t.Fatal(err)
+	}
+	if markerExists(t, dir) {
+		t.Fatal("a marker was written for an operation that stops nothing")
+	}
+}
+
+// --- Scenarios E/F — DumpAppVolumesSafe, the primary site ----------------------------------------
+
+// inspectingProvider is the StackDataProvider slice DumpAppVolumesSafe touches. It records whether
+// the marker file EXISTED at each step — the positive observable for the ordering property. An
+// absent log line is not evidence (standing rule 3); the file's presence at the moment of the stop
+// is.
+//
+// GetDockerVolumes returns nothing, so the dump itself is a no-op and no Docker is involved — the
+// stop/start bracket around it is what is under test.
+type inspectingProvider struct {
+	StackDataProvider
+	markerFile          string
+	events              []string
+	stopErr             error
+	startErr            error
+	markerPresentAtStop bool
+	markerAtStartCall   bool
+	// panicOnVolumes simulates a hard abort (SIGKILL/power cut) at the point the dump begins: the
+	// unwind skips the restart statement, exactly as a kill would.
+	panicOnVolumes bool
+}
+
+func (p *inspectingProvider) GetDockerVolumes(string) []string {
+	if p.panicOnVolumes {
+		panic("simulated hard abort mid-dump")
+	}
+	return nil
+}
+
+func (p *inspectingProvider) StopStack(name string) error {
+	_, err := os.Stat(p.markerFile)
+	p.markerPresentAtStop = err == nil
+	p.events = append(p.events, "stop:"+name)
+	return p.stopErr
+}
+
+func (p *inspectingProvider) StartStack(name string) error {
+	_, err := os.Stat(p.markerFile)
+	p.markerAtStartCall = err == nil
+	p.events = append(p.events, "start:"+name)
+	return p.startErr
+}
+
+func newDumpManager(t *testing.T, dir string, p *inspectingProvider) *Manager {
+	t.Helper()
+	lg := log.New(io.Discard, "", 0)
+	m := &Manager{logger: lg, stackProvider: p, systemDataPath: dir}
+	m.appStop = NewAppStopGuard(markerPath(dir), lg)
+	return m
+}
+
+func TestDumpAppVolumesSafe_MarkerCoversTheWholeStopStartWindow(t *testing.T) {
+	// Scenario F, the happy path: the marker is on disk BEFORE the stop, still on disk for the whole
+	// time the app is down, and GONE once the restart succeeds.
+	dir := t.TempDir()
+	p := &inspectingProvider{markerFile: markerPath(dir)}
+	m := newDumpManager(t, dir, p)
+
+	if err := m.DumpAppVolumesSafe("immich"); err != nil {
+		t.Fatalf("DumpAppVolumesSafe: %v", err)
+	}
+
+	if !p.markerPresentAtStop {
+		t.Fatal("the marker was NOT on disk when the app was stopped — a crash one instruction later " +
+			"strands the app, which is the entire failure this marker exists to prevent")
+	}
+	if !p.markerAtStartCall {
+		t.Fatal("the marker was already gone while the app was still down")
+	}
+	if markerExists(t, dir) {
+		t.Fatal("the marker survived a dump whose restart succeeded — the next boot would restart the app again")
+	}
+	if len(p.events) != 2 || p.events[0] != "stop:immich" || p.events[1] != "start:immich" {
+		t.Fatalf("events=%v, want [stop:immich start:immich]", p.events)
+	}
+}
+
+func TestDumpAppVolumesSafe_Interrupted_RecoveryBringsTheAppBack(t *testing.T) {
+	// Scenario E end-to-end THROUGH THE PRODUCTION PATH, and WITHOUT running any cleanup.
+	//
+	// The abort is real: GetDockerVolumes panics, which unwinds out of DumpAppVolumesSafe AFTER the
+	// marker was written and the app stopped, and BEFORE the restart statement — and because that
+	// restart is a plain statement, not a defer, it never runs. That is the shape of a hard kill.
+	//
+	// The earlier version of this test called m.appStop.Begin itself, which meant it proved the
+	// marker type worked and NOT that DumpAppVolumesSafe uses it — it survived the red-proof that
+	// deleted the production Begin call. Driving the real function is what makes the proof bite.
+	//
+	// RED-PROOF: delete the `m.appStop.Begin(...)` call from DumpAppVolumesSafe and this test fails —
+	// nothing is written, so nothing is recovered. Demonstrated in REPORT.md §5.
+	dir := t.TempDir()
+	p := &inspectingProvider{markerFile: markerPath(dir), panicOnVolumes: true}
+	m := newDumpManager(t, dir, p)
+
+	func() {
+		defer func() {
+			if recover() == nil {
+				t.Error("the simulated abort did not fire — this test proves nothing")
+			}
+		}()
+		_ = m.DumpAppVolumesSafe("immich")
+	}()
+
+	if !p.markerPresentAtStop {
+		t.Fatal("the app was stopped before any marker existed")
+	}
+	if p.markerAtStartCall {
+		t.Fatal("the restart ran despite the abort — the simulation is wrong, not the code")
+	}
+
+	// <the controller is gone> — a fresh one starts and recovers from the file alone.
+	g, starter := newGuard(t, dir)
+	res := g.Recover()
+
+	if len(starter.starts) != 1 || starter.starts[0] != "immich" {
+		t.Fatalf("started %v — the app stopped by the interrupted dump was not brought back", starter.starts)
+	}
+	if res == nil || res.Reason != ReasonVolumeDump {
+		t.Fatalf("recovery did not name the volume dump as the interrupted operation: %+v", res)
+	}
+	if markerExists(t, dir) {
+		t.Fatal("the marker was not cleared after a successful recovery")
+	}
+}
+
+func TestDumpAppVolumesSafe_FailedRestartKeepsTheMarker(t *testing.T) {
+	dir := t.TempDir()
+	p := &inspectingProvider{markerFile: markerPath(dir), startErr: errors.New("compose up failed")}
+	m := newDumpManager(t, dir, p)
+
+	if err := m.DumpAppVolumesSafe("immich"); err == nil {
+		t.Fatal("a failed restart must surface as an error")
+	}
+	if !markerExists(t, dir) {
+		t.Fatal("the marker was cleared even though the restart FAILED — the app is still down and " +
+			"nothing records that it is owed a restart")
+	}
+}
+
+func TestDumpAppVolumesSafe_FailedStopClearsTheMarker(t *testing.T) {
+	// Nothing was stopped, so nothing is owed a restart. A stranded marker here would cost a
+	// spurious restart at the next startup AND a false "a backup was interrupted" alert.
+	dir := t.TempDir()
+	p := &inspectingProvider{markerFile: markerPath(dir), stopErr: errors.New("stack is protected")}
+	m := newDumpManager(t, dir, p)
+
+	if err := m.DumpAppVolumesSafe("traefik"); err == nil {
+		t.Fatal("a failed stop must surface as an error")
+	}
+	if markerExists(t, dir) {
+		t.Fatal("a marker was left behind for an app that was never stopped")
+	}
+}