diff --git a/CHANGELOG.md b/CHANGELOG.md index 997042d..0f240b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,20 @@ +## Unreleased — the second-drive copy can no longer outlive the controller (the stray-copy fix), 2026-10-22 night + +**MinAgent: 0.158.1** (unchanged). Committed, not released. + +- **The stray-copy fix (night brief 2026-10-23 Part 1).** The capped second-drive copy (R-1214, below) runs `rsync -a + --delete` in its own container; killing the `docker` CLI — the copy's 60-minute limit, a controller restart or a + self-update — did NOT stop it, so the copy went on deleting outside every guard the controller holds (R-1180). Now + every copy container has a fixed name (`felhom-tier2-copy--<8 hex of the destination>`) and the label + `felhom.tier2-copy=1` (+ `felhom.tier2-copy.app`); a leftover of the same copy is removed before a new one starts; + on any error, the timeout included, it is removed by name; at controller start, BEFORE the scheduler and the web + server, every labelled container is removed (each logged; on failure retried every minute). A restore is refused + while any second-drive copy runs, capped or in-process, or while a leftover may still run („Éppen fut a második + meghajtóra készülő mentési másolat…"). The same night's review also moved the capped copy to `--mount` (a path the + Docker daemon does not see is refused — exit 125, the in-process copy — instead of created empty, where `--delete` + would empty the copy) and to the controller's image ID with `--pull never`. Tests `TestStrayCopy_*` (backup, web, + wiring; Docker-free seams), each red-proved; 9202 proof in felhom.eu `audits/night-2026-10-23/P1-straycopy/`. + ## Unreleased — the second-drive copy no longer squeezes the apps' memory (R-1214), 2026-10-22 night **MinAgent: 0.158.1** (unchanged). Committed, not released. diff --git a/controller/cmd/controller/main.go b/controller/cmd/controller/main.go index 52ac18a..9ffa19e 100644 --- a/controller/cmd/controller/main.go +++ b/controller/cmd/controller/main.go @@ -576,6 +576,12 @@ func main() { } if cfg.Backup.Enabled { backupMgr = backup.NewManager(cfg, sett, logger) + // The stray-copy fix: a second-drive copy container a previous controller process lost (restart, self-update) + // still runs `rsync --delete`. Remove every one BEFORE the scheduler or the web server can start a backup or a + // restore; until that succeeds, restores are refused. Pinned by TestStrayCopy_ClearIsWiredBeforeTheScheduler. + if err := backup.ClearLeftoverTier2Copies(ctx); err != nil { + logger.Printf("[ERROR] [backup] leftover second-drive copy check: %v", err) + } // R-550: load the persisted restore record BEFORE any page can ask for it. var putBack []string if appStopRecovery != nil { diff --git a/controller/cmd/controller/straycopy_wiring_test.go b/controller/cmd/controller/straycopy_wiring_test.go new file mode 100644 index 0000000..d786ac4 --- /dev/null +++ b/controller/cmd/controller/straycopy_wiring_test.go @@ -0,0 +1,27 @@ +package main + +import ( + "os" + "strings" + "testing" +) + +// The stray-copy fix: main.go removes every leftover second-drive copy container BEFORE the scheduler and the web +// server start (either could begin a backup or a restore beside a lost copy). RED-PROOF (2026-10-22 night): the call +// removed, or moved below sched.Start, → this fails. +func TestStrayCopy_ClearIsWiredBeforeTheScheduler(t *testing.T) { + src, err := os.ReadFile("main.go") + if err != nil { + t.Fatal(err) + } + s := string(src) + c := strings.Index(s, "backup.ClearLeftoverTier2Copies(ctx)") + st := strings.Index(s, "sched.Start(ctx)") + ws := strings.Index(s, "web.NewServer(") + if c < 0 || st < 0 || ws < 0 { + t.Fatalf("missing: clear=%d sched.Start=%d web.NewServer=%d", c, st, ws) + } + if c > st || c > ws { + t.Fatal("the leftover-copy removal must run before the scheduler and the web server start") + } +} diff --git a/controller/internal/backup/r1214_capped_copy_test.go b/controller/internal/backup/r1214_capped_copy_test.go index 2ca74a9..299b487 100644 --- a/controller/internal/backup/r1214_capped_copy_test.go +++ b/controller/internal/backup/r1214_capped_copy_test.go @@ -20,7 +20,9 @@ type cappedRec struct{ calls [][]string } func withSeams(t *testing.T, root string, img string, run func(args []string) ([]byte, int, error)) *cappedRec { t.Helper() rec := &cappedRec{} - oldRoot, oldImg, oldRun, oldLog := tier2CappedRoot, tier2SelfImage, tier2RunCapped, tier2Logf + oldRoot, oldImg, oldRun, oldLog, oldDocker := tier2CappedRoot, tier2SelfImage, tier2RunCapped, tier2Logf, tier2Docker + // The copy's lifecycle commands (rm -f by name) never reach Docker from a test (they would act on the host). + tier2Docker = func(context.Context, ...string) ([]byte, error) { return nil, nil } tier2CappedRoot = root tier2SelfImage = func() string { return img } tier2RunCapped = func(_ context.Context, args []string) ([]byte, int, error) { @@ -28,7 +30,9 @@ func withSeams(t *testing.T, root string, img string, run func(args []string) ([ return run(args) } tier2Logf = func(string, ...any) {} - t.Cleanup(func() { tier2CappedRoot, tier2SelfImage, tier2RunCapped, tier2Logf = oldRoot, oldImg, oldRun, oldLog }) + t.Cleanup(func() { + tier2CappedRoot, tier2SelfImage, tier2RunCapped, tier2Logf, tier2Docker = oldRoot, oldImg, oldRun, oldLog, oldDocker + }) return rec } @@ -44,8 +48,9 @@ func TestR1214_MntPathsAreCopiedCapped(t *testing.T) { t.Fatalf("docker calls = %d, want 1", len(rec.calls)) } got := strings.Join(rec.calls[0], " ") - for _, want := range []string{"run --rm", "--memory 512m", "--memory-swap 512m", "--network none", - "-v " + src + ":" + src + ":ro", "-v " + dst + ":" + dst + " ", "--entrypoint rsync gitea.dooplex.hu/admin/felhom-controller:9.9.9 -a --delete " + src + "/ " + dst + "/"} { + for _, want := range []string{"run --rm", "--memory 512m", "--memory-swap 512m", "--network none", "--pull never", + "--mount type=bind,src=" + src + ",dst=" + src + ",readonly", "--mount type=bind,src=" + dst + ",dst=" + dst + " ", + "--entrypoint rsync gitea.dooplex.hu/admin/felhom-controller:9.9.9 -a --delete " + src + "/ " + dst + "/"} { if !strings.Contains(got, want) { t.Fatalf("the capped copy lacks %q:\n%s", want, got) } diff --git a/controller/internal/backup/tier2.go b/controller/internal/backup/tier2.go index 11e3f97..d0542a7 100644 --- a/controller/internal/backup/tier2.go +++ b/controller/internal/backup/tier2.go @@ -782,26 +782,53 @@ func (m *Manager) tier2NoTargetReason(err error) string { // differs for the daemon: a capped `--delete` there would mirror an EMPTY folder over the copy. Those paths, a box // whose image cannot be read, and a container that cannot START (docker's own exit 125) take the plain rsync, as // before. Pinned by TestR1214_*. +// +// The container must never outlive the copy (the stray-copy fix, tier2_copy_lifecycle.go): it has a fixed name and a +// label; a leftover of the same copy is removed first; on any error — the timeout included, which kills only the +// docker CLI — it is removed by name. Every copy, capped or in-process, is in the controller's record of copies while +// it runs, and a restore is refused meanwhile. Pinned by TestStrayCopy_*. func rsyncMirror(src, dst string) error { if err := os.MkdirAll(dst, 0755); err != nil { return fmt.Errorf("mkdir %s: %w", dst, err) } - ctx, cancel := context.WithTimeout(context.Background(), 60*time.Minute) + ctx, cancel := context.WithTimeout(context.Background(), tier2CopyDeadline()) defer cancel() // Trailing slashes: copy the CONTENTS of src into dst. s, d := strings.TrimRight(src, "/"), strings.TrimRight(dst, "/") rs := []string{"-a", "--delete", s + "/", d + "/"} + name, app := tier2CopyName(d) + tier2CopyBegin(app) + defer tier2CopyEnd(app) if tier2CappedPath(s) && tier2CappedPath(d) { if img := tier2SelfImage(); img != "" { - args := append([]string{"run", "--rm", "--memory", tier2CopyMemCap, "--memory-swap", tier2CopyMemCap, - "--network", "none", "-v", s + ":" + s + ":ro", "-v", d + ":" + d, "--entrypoint", "rsync", img}, rs...) + // A leftover of this same copy (one a removal could not stop) never runs beside a new one. + if !tier2RemoveCopy(name, "a leftover of the same copy, before a new one") { + return fmt.Errorf("a leftover copy container %s could not be removed — not copying beside it", name) + } + args := append([]string{"run", "--rm", "--name", name, "--label", tier2CopyLabel + "=1", + "--label", tier2CopyAppLabel + "=" + tier2DockerSafe(app), + "--memory", tier2CopyMemCap, "--memory-swap", tier2CopyMemCap, + "--network", "none", "--pull", "never", + // --mount (not -v): a path the daemon does not see is REFUSED (exit 125 → the in-process copy) instead of + // created empty — an empty source would let `--delete` empty the copy (review 2026-10-22 night, C4). + "--mount", "type=bind,src=" + s + ",dst=" + s + ",readonly", "--mount", "type=bind,src=" + d + ",dst=" + d, + "--entrypoint", "rsync", img}, rs...) out, code, err := tier2RunCapped(ctx, args) if err == nil { return nil } + // The docker CLI ending does not stop the container (a timeout kills only the CLI): remove it by name. + removed := tier2RemoveCopy(name, "the copy ended with an error") + if ctx.Err() != nil { + tier2Logf("[WARN] [tier2] the copy into %s ran past its time limit (%s) — its container %s was removed (removed=%v)", d, tier2CopyDeadline(), name, removed) + return fmt.Errorf("copy timed out after %s: %v: %s", tier2CopyDeadline(), err, strings.TrimSpace(string(out))) + } if code != 125 { return fmt.Errorf("%v: %s", err, strings.TrimSpace(string(out))) } + if !removed { + return fmt.Errorf("the copy container %s did not start and could not be removed: %s", name, strings.TrimSpace(string(out))) + } tier2Logf("[WARN] [tier2] the capped copy container did not start (%s) — copying in-process, as before (R-1214)", strings.TrimSpace(string(out))) } } @@ -835,7 +862,9 @@ var tier2CappedRoot = "/mnt/" // Seams (tests never reach Docker — tests that do act on the host they run on). var ( tier2SelfImage = func() string { - out, err := dockerexec.Command("docker", "inspect", "felhom-controller", "--format", "{{.Config.Image}}").Output() + // The image ID, not its tag: a tag can be removed or re-pointed; with --pull never the copy runs exactly the + // image the controller runs (review 2026-10-22 night, C7). + out, err := dockerexec.Command("docker", "inspect", "felhom-controller", "--format", "{{.Image}}").Output() if err != nil { return "" } diff --git a/controller/internal/backup/tier2_copy_lifecycle.go b/controller/internal/backup/tier2_copy_lifecycle.go new file mode 100644 index 0000000..0fecdc6 --- /dev/null +++ b/controller/internal/backup/tier2_copy_lifecycle.go @@ -0,0 +1,212 @@ +package backup + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "fmt" + "strings" + "sync" + "time" + + "gitea.dooplex.hu/admin/felhom-controller/internal/dockerexec" +) + +// The stray-copy fix (night brief 2026-10-23, Part 1). The capped second-drive copy (R-1214) runs `rsync -a --delete` +// in its own container. Killing the `docker` CLI — the copy's timeout, a controller restart, a self-update — does NOT +// stop that container: the copy goes on with `--delete`, outside every guard the controller holds in memory (R-1180: +// no mirror during a restore of that app). So: +// +// - every copy container has a fixed name (one per destination) and the label tier2CopyLabel; +// - a leftover of the same copy is removed before a new one starts, and the container is removed by name when the +// copy ends with an error (the timeout included); +// - at controller start, before the scheduler or the web server can start a backup or a restore, every labelled +// container is removed (ClearLeftoverTier2Copies, each one logged); +// - a restore is refused while a copy runs or while a leftover may still run (Tier2CopyBlocksRestore). +// +// Pinned by TestStrayCopy_* (tier2_copy_lifecycle_test.go) and the web test TestStrayCopy_RestoreRefused*. + +// tier2CopyLabel marks every second-drive copy container; its value is "1". tier2CopyAppLabel names the app. +const ( + tier2CopyLabel = "felhom.tier2-copy" + tier2CopyAppLabel = "felhom.tier2-copy.app" + tier2CopyPrefix = "felhom-tier2-copy-" +) + +// tier2CopyTimeout bounds one copy. tier2CopyTimeoutOverride is set ONLY by a test build's ldflags +// (-X …/internal/backup.tier2CopyTimeoutOverride=2m) to prove the timeout path on a scratch box; a release leaves it +// empty and an unparseable value is ignored. +var ( + tier2CopyTimeout = 60 * time.Minute + tier2CopyTimeoutOverride = "" +) + +func tier2CopyDeadline() time.Duration { + if tier2CopyTimeoutOverride != "" { + if d, err := time.ParseDuration(tier2CopyTimeoutOverride); err == nil && d > 0 { + return d + } + } + return tier2CopyTimeout +} + +// tier2CopyName returns the fixed container name for a copy into dst and the app it belongs to. The app is the path +// segment after backups/secondary/ or backups/primary/ ("" when the path has neither); the name carries 8 hex of the +// destination's hash, so two legs of one app never share a name. +func tier2CopyName(dst string) (name, app string) { + for _, marker := range []string{"/backups/secondary/", "/backups/primary/"} { + if i := strings.Index(dst, marker); i >= 0 { + app = strings.SplitN(dst[i+len(marker):], "/", 2)[0] + break + } + } + sum := sha256.Sum256([]byte(dst)) + return tier2CopyPrefix + tier2DockerSafe(app) + "-" + hex.EncodeToString(sum[:4]), app +} + +// tier2DockerSafe keeps [a-z0-9_.-] (a Docker name/label value), max 40 characters; "x" when nothing is left. +func tier2DockerSafe(s string) string { + var b strings.Builder + for _, r := range strings.ToLower(s) { + if (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9') || r == '_' || r == '.' || r == '-' { + b.WriteRune(r) + } + if b.Len() >= 40 { + break + } + } + if b.Len() == 0 { + return "x" + } + return b.String() +} + +// tier2Docker runs one docker command for the copy's lifecycle (rm, ps). A seam: tests never reach Docker. +var tier2Docker = func(ctx context.Context, args ...string) ([]byte, error) { + return dockerexec.CommandContext(ctx, "docker", args...).CombinedOutput() +} + +// The controller's own record of copies (package-level: one controller, one backup manager per process). +var tier2Copies = struct { + sync.Mutex + inFlight map[string]int // app → copies running now (capped or in-process) + stray bool // a container this process could not remove may still run, or the start-up removal has not succeeded +}{inFlight: map[string]int{}} + +func tier2CopyBegin(app string) { + tier2Copies.Lock() + tier2Copies.inFlight[app]++ + tier2Copies.Unlock() +} + +func tier2CopyEnd(app string) { + tier2Copies.Lock() + if tier2Copies.inFlight[app]--; tier2Copies.inFlight[app] <= 0 { + delete(tier2Copies.inFlight, app) + } + tier2Copies.Unlock() +} + +func tier2SetStray(v bool) { + tier2Copies.Lock() + tier2Copies.stray = v + tier2Copies.Unlock() +} + +// tier2RemoveCopy removes the copy container `name` (no error when it does not exist). It reports whether the +// container is surely gone; when not, the stray flag is set, so restores stay refused until a removal succeeds. +func tier2RemoveCopy(name, why string) bool { + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + defer cancel() + out, err := tier2Docker(ctx, "rm", "-f", name) + if err != nil && !strings.Contains(strings.ToLower(string(out)), "no such container") { + tier2Logf("[ERROR] [tier2] could not remove the copy container %s (%s): %v %s — restores stay refused until it is removed", + name, why, err, strings.TrimSpace(string(out))) + tier2SetStray(true) + return false + } + return true +} + +// ClearLeftoverTier2Copies removes every labelled copy container — a copy a previous controller process started and +// lost (restart, self-update, crash). It runs at controller start, BEFORE the scheduler and the web server; each +// removal is logged. Until it succeeds, restores are refused; on failure it retries every minute in the background +// until it succeeds or ctx ends. +func ClearLeftoverTier2Copies(ctx context.Context) error { + tier2SetStray(true) + err := clearLeftoverTier2CopiesOnce(ctx) + if err == nil { + tier2SetStray(false) + return nil + } + tier2Logf("[ERROR] [tier2] the start-up check for a leftover second-drive copy failed: %v — restores are refused until it succeeds; retrying every minute", err) + go func() { + t := time.NewTicker(time.Minute) + defer t.Stop() + for { + select { + case <-ctx.Done(): + return + case <-t.C: + if clearLeftoverTier2CopiesOnce(ctx) == nil { + tier2SetStray(false) + tier2Logf("[INFO] [tier2] the leftover second-drive copy check succeeded — restores allowed again") + return + } + } + } + }() + return err +} + +func clearLeftoverTier2CopiesOnce(ctx context.Context) error { + lctx, cancel := context.WithTimeout(ctx, 60*time.Second) + defer cancel() + out, err := tier2Docker(lctx, "ps", "-a", "--filter", "label="+tier2CopyLabel+"=1", "--format", "{{.Names}}") + if err != nil { + return fmt.Errorf("docker ps: %v %s", err, strings.TrimSpace(string(out))) + } + names := strings.Fields(string(out)) + if len(names) == 0 { + tier2Logf("[INFO] [tier2] start-up: no leftover second-drive copy container") + return nil + } + failed := 0 + for _, n := range names { + rctx, rcancel := context.WithTimeout(ctx, 60*time.Second) + rout, rerr := tier2Docker(rctx, "rm", "-f", n) + rcancel() + if rerr != nil { + failed++ + tier2Logf("[ERROR] [tier2] start-up: could not remove the leftover copy container %s: %v %s", n, rerr, strings.TrimSpace(string(rout))) + continue + } + tier2Logf("[WARN] [tier2] start-up: removed a leftover second-drive copy container %s — a copy a previous controller process lost; the copy is redone at the next run", n) + } + if failed > 0 { + return fmt.Errorf("%d of %d leftover copy container(s) not removed", failed, len(names)) + } + return nil +} + +// Tier2CopyBlocksRestore reports whether a restore must wait: a second-drive copy runs now (its `--delete` would remove +// files the restore reads or writes), or a copy container may still run that this process could not remove. The +// returned app is "" when the cause is not one app. +func (m *Manager) Tier2CopyBlocksRestore() (app string, blocked bool) { + tier2Copies.Lock() + defer tier2Copies.Unlock() + if tier2Copies.stray { + return "", true + } + for a := range tier2Copies.inFlight { + return a, true + } + return "", false +} + +// MarkTier2CopyForTest records a copy of app as running until the returned func is called — a test-only seam (another +// package's test exercises the restore refusal without a real copy), in the spirit of AcquireRunningForTest. +func MarkTier2CopyForTest(app string) func() { + tier2CopyBegin(app) + return func() { tier2CopyEnd(app) } +} diff --git a/controller/internal/backup/tier2_copy_lifecycle_test.go b/controller/internal/backup/tier2_copy_lifecycle_test.go new file mode 100644 index 0000000..3712533 --- /dev/null +++ b/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") + } +} diff --git a/controller/internal/web/restore_wizard.go b/controller/internal/web/restore_wizard.go index f049ba9..f66297e 100644 --- a/controller/internal/web/restore_wizard.go +++ b/controller/internal/web/restore_wizard.go @@ -221,6 +221,15 @@ func (s *Server) restoreOpBlocked() (string, bool) { return "Egy mentési művelet már fut, ezért most nem indítható visszaállítás. Az állapotát " + "ezen az oldalon követheted; amint befejeződik, újra indíthatsz visszaállítást.", true } + // The stray-copy fix: a second-drive copy (rsync --delete) runs now, or one a previous controller process lost may + // still run — a restore beside it could lose the files it brings back (R-1180's other direction). + if app, blocked := s.backupMgr.Tier2CopyBlocksRestore(); blocked { + if s.logger != nil { + s.logger.Printf("[WARN] [restore] restore refused — a second-drive copy is running or may still run (app=%q)", app) + } + return "Éppen fut a második meghajtóra készülő mentési másolat, ezért most nem indítható visszaállítás. " + + "Néhány perc múlva próbáld újra.", true + } return "", false } diff --git a/controller/internal/web/straycopy_restore_refused_test.go b/controller/internal/web/straycopy_restore_refused_test.go new file mode 100644 index 0000000..244e660 --- /dev/null +++ b/controller/internal/web/straycopy_restore_refused_test.go @@ -0,0 +1,27 @@ +package web + +import ( + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/backup" +) + +// The stray-copy fix: a restore is refused while a second-drive copy (rsync --delete) runs — every restore entry +// point asks restoreOpBlocked first. RED-PROOF (2026-10-22 night): with the Tier2CopyBlocksRestore check removed from +// restoreOpBlocked, this fails. +func TestStrayCopy_RestoreRefusedWhileACopyRuns(t *testing.T) { + s, _, _ := newOffboxWebServer(t) + if _, blocked := s.restoreOpBlocked(); blocked { + t.Fatal("fixture: nothing runs, a restore must be allowed") + } + done := backup.MarkTier2CopyForTest("immich") + msg, blocked := s.restoreOpBlocked() + done() + if !blocked || !strings.Contains(msg, "második meghajtóra") { + t.Fatalf("during a second-drive copy a restore must be refused with the copy named; got blocked=%v %q", blocked, msg) + } + if _, blocked := s.restoreOpBlocked(); blocked { + t.Fatal("after the copy a restore must be allowed again") + } +}