Stray-copy fix: the capped second-drive copy container never outlives the copy — fixed name + label, removed by name on any error (the timeout included), every leftover removed at start before the scheduler and web server; a restore is refused while a copy runs or may still run; --mount (refuses a path the daemon does not see) and the image ID with --pull never (review); red-proved; unreleased

controller/internal/backup/tier2_copy_lifecycle_test.go

@@ -0,0 +1,212 @@
+package backup
+
+import (
+	"context"
+	"errors"
+	"os"
+	"path/filepath"
+	"strings"
+	"sync"
+	"testing"
+	"time"
+)
+
+// The stray-copy fix (tier2_copy_lifecycle.go): the capped copy's container never outlives the copy, a leftover from a
+// lost controller process is removed at start, and a restore is refused while a copy runs or may still run.
+// RED-PROOF (2026-10-22 night): each test below was seen failing with its mechanism removed — the rm -f after an
+// error (TimeoutRemoves, ErrorRemoves), the --name/--label args (NamedAndLabelled), the start-up loop (StartUpRemoves),
+// the stray flag (FailedStartUp, Unremovable), the in-flight record (RefusedWhileACopyRuns).
+
+type dockerRec struct {
+	mu    sync.Mutex
+	calls []string
+}
+
+func (r *dockerRec) all() string {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+	return strings.Join(r.calls, "\n")
+}
+
+// strayCopySeams stubs every Docker seam; docker answers for `rm`/`ps` come from fn.
+func strayCopySeams(t *testing.T, root string, run func(ctx context.Context, args []string) ([]byte, int, error),
+	fn func(args []string) ([]byte, error)) (*dockerRec, *dockerRec) {
+	t.Helper()
+	runs, lifecycle := &dockerRec{}, &dockerRec{}
+	oldRoot, oldImg, oldRun, oldLog, oldDocker, oldTO := tier2CappedRoot, tier2SelfImage, tier2RunCapped, tier2Logf, tier2Docker, tier2CopyTimeout
+	tier2CappedRoot = root
+	tier2SelfImage = func() string { return "sha256:abc" }
+	tier2RunCapped = func(ctx context.Context, args []string) ([]byte, int, error) {
+		runs.mu.Lock()
+		runs.calls = append(runs.calls, strings.Join(args, " "))
+		runs.mu.Unlock()
+		return run(ctx, args)
+	}
+	tier2Docker = func(_ context.Context, args ...string) ([]byte, error) {
+		lifecycle.mu.Lock()
+		lifecycle.calls = append(lifecycle.calls, strings.Join(args, " "))
+		lifecycle.mu.Unlock()
+		return fn(args)
+	}
+	tier2Logf = func(string, ...any) {}
+	tier2SetStray(false)
+	t.Cleanup(func() {
+		tier2CappedRoot, tier2SelfImage, tier2RunCapped, tier2Logf, tier2Docker, tier2CopyTimeout = oldRoot, oldImg, oldRun, oldLog, oldDocker, oldTO
+		tier2SetStray(false)
+	})
+	return runs, lifecycle
+}
+
+func appDirs(t *testing.T) (root, src, dst string) {
+	root = t.TempDir() + "/"
+	src = filepath.Join(root, "d1", "backups", "primary", "immich")
+	dst = filepath.Join(root, "d2", "backups", "secondary", "immich", "recovery-unit")
+	os.MkdirAll(src, 0755)
+	return
+}
+
+func TestStrayCopy_TheCopyContainerIsNamedAndLabelled(t *testing.T) {
+	root, src, dst := appDirs(t)
+	runs, life := strayCopySeams(t, root, func(context.Context, []string) ([]byte, int, error) { return nil, 0, nil },
+		func([]string) ([]byte, error) { return nil, nil })
+	if err := rsyncMirror(src, dst); err != nil {
+		t.Fatal(err)
+	}
+	name, app := tier2CopyName(dst)
+	if app != "immich" || !strings.HasPrefix(name, "felhom-tier2-copy-immich-") {
+		t.Fatalf("name %q app %q", name, app)
+	}
+	got := runs.all()
+	for _, want := range []string{"--name " + name, "--label felhom.tier2-copy=1", "--label felhom.tier2-copy.app=immich"} {
+		if !strings.Contains(got, want) {
+			t.Fatalf("the copy container lacks %q:\n%s", want, got)
+		}
+	}
+	if life.all() != "rm -f "+name {
+		t.Fatalf("a leftover of the same copy must be removed before it starts; lifecycle calls:\n%s", life.all())
+	}
+}
+
+func TestStrayCopy_ATimeoutRemovesTheContainerByName(t *testing.T) {
+	root, src, dst := appDirs(t)
+	_, life := strayCopySeams(t, root, func(ctx context.Context, _ []string) ([]byte, int, error) {
+		<-ctx.Done() // the docker CLI is killed by the context; the container would keep running
+		return nil, -1, errors.New("signal: killed")
+	}, func([]string) ([]byte, error) { return nil, nil })
+	tier2CopyTimeout = 50 * time.Millisecond
+	err := rsyncMirror(src, dst)
+	if err == nil || !strings.Contains(err.Error(), "timed out") {
+		t.Fatalf("a timed-out copy must be an error that says so: %v", err)
+	}
+	name, _ := tier2CopyName(dst)
+	if c := strings.Count(life.all(), "rm -f "+name); c != 2 {
+		t.Fatalf("the container must be removed by name after the timeout (rm calls %d):\n%s", c, life.all())
+	}
+}
+
+func TestStrayCopy_AnErrorRemovesTheContainerByName(t *testing.T) {
+	root, src, dst := appDirs(t)
+	_, life := strayCopySeams(t, root, func(context.Context, []string) ([]byte, int, error) {
+		return []byte("rsync error"), 23, errors.New("exit 23")
+	}, func([]string) ([]byte, error) { return nil, nil })
+	if err := rsyncMirror(src, dst); err == nil {
+		t.Fatal("an rsync failure must be an error")
+	}
+	name, _ := tier2CopyName(dst)
+	if c := strings.Count(life.all(), "rm -f "+name); c != 2 {
+		t.Fatalf("the container must be removed by name after an error (rm calls %d)", c)
+	}
+}
+
+func TestStrayCopy_AnUnremovableLeftoverStopsTheCopy(t *testing.T) {
+	root, src, dst := appDirs(t)
+	runs, _ := strayCopySeams(t, root, func(context.Context, []string) ([]byte, int, error) { return nil, 0, nil },
+		func([]string) ([]byte, error) {
+			return []byte("Error response from daemon: cannot remove"), errors.New("exit 1")
+		})
+	if err := rsyncMirror(src, dst); err == nil {
+		t.Fatal("a copy must not start beside a leftover it could not remove")
+	}
+	if runs.all() != "" {
+		t.Fatalf("the copy started anyway: %s", runs.all())
+	}
+	if _, blocked := (&Manager{}).Tier2CopyBlocksRestore(); !blocked {
+		t.Fatal("a leftover that may still run must refuse restores")
+	}
+}
+
+func TestStrayCopy_ANotStartedContainerThatIsGoneStillFallsBack(t *testing.T) {
+	root, src, dst := appDirs(t)
+	os.WriteFile(filepath.Join(src, "f"), []byte("x"), 0644)
+	strayCopySeams(t, root, func(context.Context, []string) ([]byte, int, error) {
+		return []byte("no such image"), 125, errors.New("exit 125")
+	},
+		func(args []string) ([]byte, error) {
+			return []byte("Error: No such container: x"), errors.New("exit 1")
+		})
+	if err := rsyncMirror(src, dst); err != nil {
+		t.Fatalf("exit 125 and no container left → the in-process copy: %v", err)
+	}
+	if _, err := os.Stat(filepath.Join(dst, "f")); err != nil {
+		t.Fatal("the fallback copy did not run")
+	}
+}
+
+func TestStrayCopy_StartUpRemovesEveryLeftover(t *testing.T) {
+	_, life := strayCopySeams(t, t.TempDir()+"/", nil, func(args []string) ([]byte, error) {
+		if args[0] == "ps" {
+			return []byte("felhom-tier2-copy-immich-0a0b0c0d\nfelhom-tier2-copy-nextcloud-01020304\n"), nil
+		}
+		return nil, nil
+	})
+	if err := ClearLeftoverTier2Copies(context.Background()); err != nil {
+		t.Fatal(err)
+	}
+	got := life.all()
+	for _, want := range []string{"ps -a --filter label=felhom.tier2-copy=1 --format {{.Names}}",
+		"rm -f felhom-tier2-copy-immich-0a0b0c0d", "rm -f felhom-tier2-copy-nextcloud-01020304"} {
+		if !strings.Contains(got, want) {
+			t.Fatalf("start-up lacks %q:\n%s", want, got)
+		}
+	}
+	if _, blocked := (&Manager{}).Tier2CopyBlocksRestore(); blocked {
+		t.Fatal("after a clean start-up removal restores must be allowed")
+	}
+}
+
+func TestStrayCopy_AFailedStartUpCheckRefusesRestores(t *testing.T) {
+	strayCopySeams(t, t.TempDir()+"/", nil, func([]string) ([]byte, error) { return nil, errors.New("docker down") })
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel() // ends the retry loop
+	if err := ClearLeftoverTier2Copies(ctx); err == nil {
+		t.Fatal("a failed check must be reported")
+	}
+	if _, blocked := (&Manager{}).Tier2CopyBlocksRestore(); !blocked {
+		t.Fatal("until the start-up check succeeds, restores must be refused")
+	}
+}
+
+func TestStrayCopy_ARestoreIsRefusedWhileACopyRuns(t *testing.T) {
+	root, src, dst := appDirs(t)
+	inside := make(chan struct{})
+	release := make(chan struct{})
+	strayCopySeams(t, root, func(context.Context, []string) ([]byte, int, error) {
+		close(inside)
+		<-release
+		return nil, 0, nil
+	}, func([]string) ([]byte, error) { return nil, nil })
+	done := make(chan error)
+	go func() { done <- rsyncMirror(src, dst) }()
+	<-inside
+	m := &Manager{}
+	if app, blocked := m.Tier2CopyBlocksRestore(); !blocked || app != "immich" {
+		t.Fatalf("during a copy a restore must be refused (app %q, blocked %v)", app, blocked)
+	}
+	close(release)
+	if err := <-done; err != nil {
+		t.Fatal(err)
+	}
+	if _, blocked := m.Tier2CopyBlocksRestore(); blocked {
+		t.Fatal("after the copy a restore must be allowed")
+	}
+}