From d8c26344722bf8800315d01f28446b793a82b57c Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Tue, 6 Oct 2026 14:59:48 +0200 Subject: [PATCH] =?UTF-8?q?R-518=20option=20A:=20one=20stop=20per=20backup?= =?UTF-8?q?=20tier=20(09=20=C2=A73=20decision=20156)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each quiesce window now backs up ONLY the first due tier that starts and resumes the apps at its snapshotted; the upload finishes with the apps running. Any other due tier stays due and a later cycle (the next poll) takes it in its own short window - never straight after the first. This reverses R-82's 'ONE quiesce window for both due tiers'. A manual press (TriggerNow) backs up the primary (local) tier only, picked by the agent's Primary flag; the off-site tier follows at the next scheduled night run. If no available tier is flagged primary, the first available tier is backed up rather than nothing. Unchanged: marker written before any stop, one unquiesce per window, the max-quiesce bound, BUSY/start-error handling (the next tier is still tried in the same window when a tier does not start), breaker and contention. Tests: TestBothTiersDue_ExactlyOneQuiesceWindow -> TestBothTiersDue_FirstCycleRunsOnlyFirstTier; TestNonLastTierSnapshot_ DoesNotResumeApp -> TestFirstTierSnapshot_ResumesAppWhileUploadContinues (red on old code); new TestManualPress_RunsOnlyLocalTier (red on old code), TestLeftoverTier_RunsInNextCycleInItsOwnWindow, TestManualRunTiers_NoPrimaryAvailable_BacksUpFirstAvailable; TestNotify_BothFailingTiersAreReported now runs two cycles. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS --- controller/internal/quiesce/notify_test.go | 8 +- controller/internal/quiesce/quiesce.go | 119 ++++++---- controller/internal/quiesce/tier_skip_test.go | 12 + controller/internal/quiesce/tiers.go | 26 ++- controller/internal/quiesce/tiers_test.go | 206 +++++++++++++----- 5 files changed, 269 insertions(+), 102 deletions(-) diff --git a/controller/internal/quiesce/notify_test.go b/controller/internal/quiesce/notify_test.go index b66bcc2..1f11117 100644 --- a/controller/internal/quiesce/notify_test.go +++ b/controller/internal/quiesce/notify_test.go @@ -145,8 +145,12 @@ func TestNotify_BothFailingTiersAreReported(t *testing.T) { rec := &recNotifier{} l.SetTierNotifier(rec) - if err := l.runOnce(context.Background()); err != nil { - t.Fatal(err) + // `09` §3 decision 156: one tier per window, so the second tier is reached by the NEXT cycle — + // where the failed first tier is now in backoff and must not stand in its way. + for cycle := 1; cycle <= 2; cycle++ { + if err := l.runOnce(context.Background()); err != nil { + t.Fatalf("cycle %d: %v", cycle, err) + } } gotLocal, gotPBS := rec.count("failed", "local"), rec.count("failed", "felhom-pbs") if gotLocal != 1 || gotPBS != 1 { diff --git a/controller/internal/quiesce/quiesce.go b/controller/internal/quiesce/quiesce.go index 7ab0504..8e57eac 100644 --- a/controller/internal/quiesce/quiesce.go +++ b/controller/internal/quiesce/quiesce.go @@ -219,8 +219,9 @@ func (l *Loop) runOnce(ctx context.Context) error { return nil } - // R-82: resolve EVERY due tier up front. This is the dedup rule (tiers.go): both tiers due on - // the weekly night yields ONE window with two backups, never two stop/start cycles. + // R-82: resolve EVERY due tier up front (the breaker, the catch-up wait and the window gate below + // judge them all). Since `09` §3 decision 156 the window then backs up only the first one; the + // others stay due and the next poll takes them, each in its own short window (tiers.go table). dueTiers, _, err := l.resolveDueTiers(ctx) if err != nil { return fmt.Errorf("check due: %w", err) @@ -425,10 +426,13 @@ func (l *Loop) TriggerNow() error { ctx, cancel := context.WithTimeout(context.Background(), l.maxQuiesce+5*time.Minute) defer cancel() l.logger.Printf("[INFO] [quiesce] manual backup requested — quiescing now") - // Manual runs bypass due-ness (that is the point) but must still cover EVERY tier, in one - // window. A manual "Mentés most" that silently skipped the DR tier would be the same - // applied-and-empty fault in a different costume. - if err := l.quiesceAndPollTiers(ctx, l.allTiersForManualRun(ctx)); err != nil { + // Manual runs bypass due-ness (that is the point). Since `09` §3 decision 156 (R-518 option A) + // the press makes the LOCAL (primary) copy only, with one short stop; the off-site tier is not + // requested here — it stays due and the next scheduled night run takes it in its own window. + // The page says so in both languages (templates/backups.html, keys + // backups.a_mentes_alatt_az_alkalmazasok + backups.elinditod_a_teljes_rendszermentest_most, + // pinned by TestR518_BackupButtonStatesTheShortLocalOnlyStop). + if err := l.quiesceAndPollTiers(ctx, l.manualRunTiers(ctx)); err != nil { l.logger.Printf("[ERROR] [quiesce] manual backup cycle error: %v", err) } }() @@ -443,8 +447,33 @@ func (l *Loop) quiesceAndPoll(ctx context.Context) error { return l.quiesceAndPollTiers(ctx, []dueTier{{target: ""}}) } -// allTiersForManualRun lists every tier a manual run should cover: all advertised tiers on an R-82 -// agent, or the single untargeted tier otherwise. Due-ness is deliberately NOT consulted. +// manualRunTiers lists the tier a manual press backs up: on an R-82 agent ONLY the primary (local) +// tier, or the single untargeted tier otherwise. Due-ness is deliberately NOT consulted. +// +// `09` §3 decision 156 (R-518 option A): the press makes the local copy with one short stop; the +// off-site tier follows at the next scheduled night run. Pinned by TestManualPress_RunsOnlyLocalTier. +// +// The primary is the tier the agent FLAGS Primary — not the first in the list. If no kept tier carries +// the flag (the primary's storage reported absent, or an agent that flags none), the first kept tier is +// backed up rather than nothing: a press that silently backs up nothing would be the applied-and-empty +// fault in a different costume. +func (l *Loop) manualRunTiers(ctx context.Context) []dueTier { + all := l.allTiersForManualRun(ctx) + if len(all) <= 1 { + return all + } + for _, t := range all { + if t.primary { + return []dueTier{t} + } + } + l.logger.Printf("[WARN] [quiesce] manual run: no available tier is flagged primary — backing up tier %s (the first available)", tierLabel(all[0].target)) + return all[:1] +} + +// allTiersForManualRun lists every tier the agent can back up now: all advertised tiers whose storage +// is not absent on an R-82 agent, or the single untargeted tier otherwise. manualRunTiers picks the +// primary from it. func (l *Loop) allTiersForManualRun(ctx context.Context) []dueTier { tb, ok := l.backend.(TieredBackend) if !ok { @@ -462,20 +491,26 @@ func (l *Loop) allTiersForManualRun(ctx context.Context) []dueTier { kept := l.skipAbsentTiers(tiers) out := make([]dueTier, 0, len(kept)) for _, t := range kept { - out = append(out, dueTier{target: t.Target}) + out = append(out, dueTier{target: t.Target, primary: t.Primary}) } return out } -// quiesceAndPollTiers is the R-82 multi-tier cycle: ONE marker, ONE stop, N backups run -// SEQUENTIALLY inside the window, ONE resume, then the tail polled to completion. +// quiesceAndPollTiers is ONE quiesce window: ONE marker, ONE stop, ONE backup, ONE resume at that +// backup's `snapshotted`, then the upload polled to completion with the apps running. // -// Why sequential: vzdump takes a guest lock, so a second backup cannot start until the first -// finishes. Why the app stays down until the LAST tier snapshots: the whole point of quiescing is -// app-consistency, and resuming after tier 1's snapshot would leave tier 2 capturing a RUNNING app. -// Consequence, stated plainly because it is user-visible: on the both-due night downtime is -// (first tier's full backup) + (last tier's snapshot), not one snapshot. Tier ORDER therefore -// matters — see resolveDueTiers. +// ONE STOP PER BACKUP TIER (`09` §3 decision 156, R-518 option A, 2026-10-06). This REVERSES R-82's +// „ONE quiesce window for both due tiers (never two app outages for one night)": that window kept every +// app down for the first tier's whole upload (5 min 47 s measured on demo-hp, 9 apps). Now the window +// backs up only the FIRST tier in `tiers` that starts; the apps resume at its snapshot; any other due +// tier stays due and a LATER cycle (the next poll, after this one has finished) takes it in its own +// short window — never straight after this one. Every copy is still taken with the apps stopped, so +// each tier stays app-consistent. Pinned by TestBothTiersDue_FirstCycleRunsOnlyFirstTier, +// TestFirstTierSnapshot_ResumesAppWhileUploadContinues and TestLeftoverTier_RunsInNextCycleInItsOwnWindow. +// +// A tier that does not START (agent BUSY, or a start error) took no copy, so the next tier in `tiers` +// is tried inside the same window — exactly as before; the apps are already down and no copy has been +// made yet, so this still costs one stop for at most one backup. // // Crash-safety is unchanged and non-negotiable: the marker is written BEFORE anything stops, // unquiesce fires exactly once no matter which tier fails, and the GUARANTEE is the crash MARKER plus @@ -528,7 +563,7 @@ func (l *Loop) quiesceAndPollTiers(ctx context.Context, tiers []dueTier) error { deadline := l.now().Add(l.maxQuiesce) var firstErr error - // ONE window, N tiers, sequential. The app resumes only after the LAST tier snapshots. + // ONE window, ONE backup (decision 156): the first tier that starts is the window's only backup. for i, t := range tiers { label := tierLabel(t.target) last := i == len(tiers)-1 @@ -565,7 +600,9 @@ func (l *Loop) quiesceAndPollTiers(ctx context.Context, tiers []dueTier) error { _ = l.writeMarker(marker) // best-effort: record the CURRENT tier's job id for diagnosis l.logger.Printf("[INFO] [quiesce] tier %s: backup job %s started — polling", label, jobID) - phase, stillRunning, perr := l.pollTier(ctx, t.target, jobID, label, deadline, last, &unquiesced, unquiesce) + // Decision 156: this is the window's only backup, so the apps resume at ITS snapshot + // (pollTier's early resume) and its upload finishes with them running. + phase, stillRunning, perr := l.pollTier(ctx, t.target, jobID, label, deadline, &unquiesced, unquiesce) if perr != nil && firstErr == nil { firstErr = perr } @@ -596,6 +633,14 @@ func (l *Loop) quiesceAndPollTiers(ctx context.Context, tiers []dueTier) error { label, len(tiers)-i-1) break } + // Decision 156: one backup per window. The remaining due tiers stay due; a later cycle (the + // next poll — this cycle holds the single-flight lock until the upload above has finished) + // takes each in its own short window. Never straight after this one. + if rest := len(tiers) - i - 1; rest > 0 { + l.logger.Printf("[INFO] [quiesce] tier %s finished its window — %d other due tier(s) wait for a later cycle, each with its own short stop (`09` §3 decision 156)", + label, rest) + } + break } // Belt: if every tier failed to start, nothing above unquiesced. The deferred call covers it, @@ -604,16 +649,17 @@ func (l *Loop) quiesceAndPollTiers(ctx context.Context, tiers []dueTier) error { return firstErr } -// pollTier polls ONE tier's job. It returns the terminal (or snapshotted-at-deadline) phase. +// pollTier polls ONE tier's job — the window's only backup — to a TERMINAL phase (done/failed). // -// The app is resumed ONLY when this is the LAST tier — that is what keeps every tier -// app-consistent while still costing exactly one stop/start pair. For a non-last tier the loop -// waits for a TERMINAL phase (done/failed), because vzdump holds the guest lock and the next tier -// cannot start until this one truly finishes. +// The app resumes at `snapshotted` (8B.2 early resume) and the poll continues to the terminal phase +// with the app running, so the outcome is still recorded (breaker, hub notice) and the cycle holds the +// single-flight lock until the agent's job — and its guest lock — has finished. Since `09` §3 +// decision 156 every window has exactly one backup, so there is no "non-last tier" that must keep the +// app down any more. // Returns (phase, stillRunning, err). stillRunning=true means the quiesce bound elapsed while the // backup is STILL going — the caller must not start another tier (one backup at a time per guest). func (l *Loop) pollTier(ctx context.Context, target, jobID, label string, deadline time.Time, - last bool, unquiesced *bool, unquiesce func(string)) (string, bool, error) { + unquiesced *bool, unquiesce func(string)) (string, bool, error) { for { if !l.now().Before(deadline) { l.logger.Printf("[WARN] [quiesce] max-quiesce-duration (%s) exceeded on tier %s (job %s) — unquiescing while the backup continues on the agent", @@ -628,25 +674,18 @@ func (l *Loop) pollTier(ctx context.Context, target, jobID, label string, deadli } switch phase { case phaseSnapshotted: - // 8B.2 early resume — but ONLY on the last tier. Resuming here on a non-last tier would - // leave the following tier capturing a running app, losing app-consistency for exactly - // the DR tier we most want it on. - if last && !*unquiesced { - l.logger.Printf("[INFO] [quiesce] tier %s: job %s snapshotted — resuming app early (8B.2)", label, jobID) - unquiesce("snapshotted (early resume, last tier)") + // 8B.2 early resume. Safe for every tier since decision 156: no other tier is started in + // this window, so no later copy can capture a running app. + if !*unquiesced { + l.logger.Printf("[INFO] [quiesce] tier %s: job %s snapshotted — resuming app early (8B.2); the upload continues with the apps running", label, jobID) + unquiesce("snapshotted (early resume)") } case phaseDone: - if last { - l.logger.Printf("[INFO] [quiesce] tier %s: backup job %s done", label, jobID) - unquiesce("backup done") - } else { - l.logger.Printf("[INFO] [quiesce] tier %s: backup job %s done — next tier may start (app still quiesced)", label, jobID) - } + l.logger.Printf("[INFO] [quiesce] tier %s: backup job %s done", label, jobID) + unquiesce("backup done") return phaseDone, false, nil case phaseFailed: - if last { - unquiesce("backup failed") - } + unquiesce("backup failed") return phaseFailed, false, nil } select { diff --git a/controller/internal/quiesce/tier_skip_test.go b/controller/internal/quiesce/tier_skip_test.go index 345e27a..2dbb76b 100644 --- a/controller/internal/quiesce/tier_skip_test.go +++ b/controller/internal/quiesce/tier_skip_test.go @@ -79,3 +79,15 @@ func TestManualRun_UnknownStorageNotSkipped(t *testing.T) { t.Fatalf("a tier without an absent verdict was dropped: %v", got) } } + +// `09` §3 decision 156: a press backs up ONE tier. When no available tier is flagged primary (the +// primary's storage is absent), the first available tier is backed up — never nothing. +func TestManualRunTiers_NoPrimaryAvailable_BacksUpFirstAvailable(t *testing.T) { + be := newTierBackend() + be.tiers = []BackupTier{{Target: "local", Primary: true, StorageAbsent: true}, {Target: "felhom-pbs"}, {Target: "other"}} + l := newTierLoop(t, be, &fakeStacks{}, nil) + got := l.manualRunTiers(context.Background()) + if len(got) != 1 || got[0].target != "felhom-pbs" { + t.Fatalf("want exactly the first available tier, got %+v", got) + } +} diff --git a/controller/internal/quiesce/tiers.go b/controller/internal/quiesce/tiers.go index 79e7c0c..be7f2b2 100644 --- a/controller/internal/quiesce/tiers.go +++ b/controller/internal/quiesce/tiers.go @@ -5,22 +5,25 @@ import ( "errors" ) -// R-82 Slice B — one quiesce window, two tiers. +// R-82 Slice B — two tiers; since `09` §3 decision 156 (R-518 option A), one stop per tier. // // The agent gained per-target backup tiers in v0.97.0 ("local daily + PBS weekly"). The controller -// owns quiescing, so the multi-tier schedule has to be reconciled HERE: on the weekly night both -// tiers come due at once, and running two quiesce cycles would mean **two app outages for one -// night's work** — which would undo the entire argument for weekly-over-daily. +// owns quiescing, so the multi-tier schedule has to be reconciled HERE. R-82 put both tiers into ONE +// quiesce window ("never two app outages for one night"); that kept every app down for the whole +// local upload (5 min 47 s measured on demo-hp, 9 apps, 2026-10-05). Decision 156 reverses it: each +// window backs up one tier and resumes the apps at its snapshot. // -// THE DEDUP RULE (specified, not emergent): +// THE RULE (specified, not emergent): // // local due | PBS due | result // ----------+---------+--------------------------------------------------------------- // yes | no | one quiesce, local backup // no | yes | one quiesce, PBS backup -// yes | yes | ONE quiesce window, BOTH backups inside it — never two cycles +// yes | yes | one quiesce, local backup; PBS stays due → a LATER cycle, its own quiesce // no | no | no quiesce // +// A manual press backs up the primary (local) tier only (manualRunTiers). +// // ErrTiersUnsupported is returned by TieredBackend.Tiers when the agent does not serve // GET /backup/tiers — i.e. it predates R-82 (the endpoint 404s). It is the DESIGNED capability // probe, not an error condition: the loop degrades to the single untargeted tier and logs it once. @@ -131,15 +134,16 @@ type dueTier struct { ageSecs *int64 // state (R-88 Part 2) disambiguates a nil ageSecs. Empty = legacy agent. state AgeState + // primary is the agent's Primary flag (the local tier). Set on the manual path, which backs up + // only the primary (`09` §3 decision 156). + primary bool } // resolveDueTiers answers "what must this cycle back up?" — the dedup rule above, in one place. // -// Returns the due tiers IN AGENT ORDER (primary first). That order is deliberate and it is a -// downtime decision, not cosmetics: tiers run SEQUENTIALLY because vzdump holds a guest lock, and -// the app stays stopped until the LAST tier has snapshotted. Running the fast local tier first and -// the slow WAN/PBS tier last makes downtime ≈ (local backup) + (PBS snapshot); the reverse order -// would make it ≈ (PBS backup) + (local snapshot), which is far worse. +// Returns the due tiers IN AGENT ORDER (primary first). Since `09` §3 decision 156 a window backs up +// only the FIRST of them, so the order decides which tier goes first when both are due: the local +// copy (the one a restore most often reads) is taken first, the off-site copy at the next cycle. // // degraded is true when the agent is pre-R-82 and the caller must use the untargeted path. func (l *Loop) resolveDueTiers(ctx context.Context) (due []dueTier, degraded bool, err error) { diff --git a/controller/internal/quiesce/tiers_test.go b/controller/internal/quiesce/tiers_test.go index 602e4ad..f30a1ec 100644 --- a/controller/internal/quiesce/tiers_test.go +++ b/controller/internal/quiesce/tiers_test.go @@ -12,11 +12,11 @@ import ( "time" ) -// R-82 Slice B — one quiesce window, two tiers. +// R-82 Slice B — two tiers; since `09` §3 decision 156 (R-518 option A), one stop per tier. // // Two properties are load-bearing and neither is provable by "no error was returned": -// 1. On the both-due night there is EXACTLY ONE stop/start pair. Two would mean two app outages -// for one night's work, undoing the whole argument for weekly-over-daily. +// 1. Each window backs up ONE tier and resumes the apps at its snapshot; another due tier gets its +// own window in a later cycle (decision 156 reverses R-82's one window for both tiers). // 2. A new controller against an OLD agent still TAKES A BACKUP. The hollow version of that test // asserts "no error" while silently skipping the backup — the exact failure it exists to catch. @@ -41,11 +41,18 @@ type tierBackend struct { // way to assert "the app had not resumed when this tier started". stacks *fakeStacks startsAtStart []int + // stopsAtStart samples the stop count when each tier starts — a new window shows as a higher + // stop count than the previous tier saw (R-518, decision 156). + stopsAtStart []int + // statusRestarts[target] samples the restart count at EACH status poll of that tier, so a test can + // say "the apps were already started when the Nth poll answered" (R-518, decision 156). + statusRestarts map[string][]int } func newTierBackend() *tierBackend { return &tierBackend{ dueSet: map[string]bool{}, phases: map[string][]string{}, phaseIdx: map[string]int{}, + statusRestarts: map[string][]int{}, } } @@ -72,12 +79,16 @@ func (b *tierBackend) StartBackupFor(_ context.Context, target string) (string, b.started = append(b.started, target) if b.stacks != nil { b.startsAtStart = append(b.startsAtStart, len(b.stacks.startedNames())) + b.stopsAtStart = append(b.stopsAtStart, len(b.stacks.stoppedNames())) } return "job-" + target, nil } func (b *tierBackend) BackupStatusFor(_ context.Context, target string) (string, error) { b.mu.Lock() defer b.mu.Unlock() + if b.stacks != nil { + b.statusRestarts[target] = append(b.statusRestarts[target], len(b.stacks.startedNames())) + } seq := b.phases[target] i := b.phaseIdx[target] if i < len(seq) { @@ -108,6 +119,24 @@ func (b *tierBackend) restartsWhenEachTierStarted() []int { return append([]int(nil), b.startsAtStart...) } +func (b *tierBackend) stopsWhenEachTierStarted() []int { + b.mu.Lock() + defer b.mu.Unlock() + return append([]int(nil), b.stopsAtStart...) +} + +func (b *tierBackend) restartsAtEachPoll(target string) []int { + b.mu.Lock() + defer b.mu.Unlock() + return append([]int(nil), b.statusRestarts[target]...) +} + +func (b *tierBackend) setDue(target string, due bool) { + b.mu.Lock() + defer b.mu.Unlock() + b.dueSet[target] = due +} + func (b *tierBackend) startedTargets() []string { b.mu.Lock() defer b.mu.Unlock() @@ -129,20 +158,25 @@ func newTierLoop(t *testing.T, be Backend, st *fakeStacks, logTo io.Writer) *Loo }) } -// ---- RED-PROOF 3 — the both-due night ---------------------------------------------------- - -// EXACTLY ONE stop/start pair, with BOTH backups inside it. Asserting only "both backups ran" -// would pass against an implementation that quiesces twice — the count is the assertion. +// ---- R-518 option A — one stop per backup tier (`09` §3 decision 156) -------------------- // -// COMPANION RED-PROOF (observed): make runOnce call quiesceAndPollTiers once per due tier -// (a per-tier cycle instead of one window) → stops/starts become 2/2 and this fails with -// "want EXACTLY 1 stop and 1 start ... got stops=2 starts=2". Restored. -func TestBothTiersDue_ExactlyOneQuiesceWindow(t *testing.T) { +// Decision 156 REVERSES R-82's „ONE quiesce window for both due tiers (never two app outages for one +// night)". The one long window kept every app down for the whole local upload (5 min 47 s measured on +// demo-hp, 9 apps). Now each window runs ONLY the first due tier and resumes the apps at its +// `snapshotted`; another due tier waits for a LATER cycle and gets its own short window. Every copy is +// still taken with the apps stopped, so each stays app-consistent. + +// Both tiers due → the first cycle stops once, backs up ONLY the first (primary) tier, starts once. +// The other tier is not started in this cycle — never straight after tier 1 (the next poll picks it +// up, pinned by TestLeftoverTier_RunsInNextCycleInItsOwnWindow). +// +// Was TestBothTiersDue_ExactlyOneQuiesceWindow (R-82), which pinned both tiers inside one window. +func TestBothTiersDue_FirstCycleRunsOnlyFirstTier(t *testing.T) { be := newTierBackend() be.tiers = []BackupTier{{Target: "local", Primary: true}, {Target: "felhom-pbs"}} be.dueSet["local"] = true be.dueSet["felhom-pbs"] = true - be.phases["local"] = []string{phaseDone} + be.phases["local"] = []string{phaseSnapshotted, phaseDone} be.phases["felhom-pbs"] = []string{phaseSnapshotted, phaseDone} st := &fakeStacks{running: []string{"immich"}} @@ -151,14 +185,120 @@ func TestBothTiersDue_ExactlyOneQuiesceWindow(t *testing.T) { if err := l.runOnce(context.Background()); err != nil { t.Fatalf("runOnce: %v", err) } - stops, starts := len(st.stoppedNames()), len(st.startedNames()) - if stops != 1 || starts != 1 { - t.Fatalf("both-due night must be ONE quiesce window: want EXACTLY 1 stop and 1 start, got stops=%d starts=%d (stopped=%v started=%v)", + if stops, starts := len(st.stoppedNames()), len(st.startedNames()); stops != 1 || starts != 1 { + t.Fatalf("one window: want EXACTLY 1 stop and 1 start, got stops=%d starts=%d (stopped=%v started=%v)", stops, starts, st.stoppedNames(), st.startedNames()) } + if got := be.startedTargets(); len(got) != 1 || got[0] != "local" { + t.Fatalf("decision 156: a window backs up ONLY the first due tier (primary); got %v", got) + } +} + +// THE red test of R-518: the apps must run while the local copy still uploads. Local reports +// `snapshotted` three times, then `done`; the stacks must already have been STARTED when the SECOND +// `snapshotted` poll answered. Under R-82 the app stayed down until the LAST tier snapshotted, so the +// whole local upload ran with every app stopped. +// +// Was TestNonLastTierSnapshot_DoesNotResumeApp (R-82), which pinned the opposite (decision 156 +// reverses R-82's one window for both tiers). +func TestFirstTierSnapshot_ResumesAppWhileUploadContinues(t *testing.T) { + st := &fakeStacks{running: []string{"immich"}} + be := newTierBackend() + be.stacks = st + be.tiers = []BackupTier{{Target: "local", Primary: true}, {Target: "felhom-pbs"}} + be.dueSet["local"] = true + be.dueSet["felhom-pbs"] = true + be.phases["local"] = []string{phaseSnapshotted, phaseSnapshotted, phaseSnapshotted, phaseDone} + be.phases["felhom-pbs"] = []string{phaseSnapshotted, phaseDone} + + l := newTierLoop(t, be, st, nil) + if err := l.runOnce(context.Background()); err != nil { + t.Fatal(err) + } + polls := be.restartsAtEachPoll("local") + if len(polls) < 2 { + t.Fatalf("the local tier must be polled past its first snapshot; restarts per poll = %v", polls) + } + if polls[1] < 1 { + t.Fatalf("the apps were still STOPPED when the second `snapshotted` poll answered (restarts per poll = %v) — they must resume at the first tier's snapshot (decision 156)", polls) + } + if stops, starts := len(st.stoppedNames()), len(st.startedNames()); stops != 1 || starts != 1 { + t.Fatalf("want exactly one stop/start pair; got %d/%d", stops, starts) + } +} + +// The tier left over by a cycle IS run by the NEXT cycle, in its OWN window (a second stop that comes +// after the first window's start). Without this, decision 156 would silently drop the off-site copy. +func TestLeftoverTier_RunsInNextCycleInItsOwnWindow(t *testing.T) { + st := &fakeStacks{running: []string{"immich"}} + be := newTierBackend() + be.stacks = st + be.tiers = []BackupTier{{Target: "local", Primary: true}, {Target: "felhom-pbs"}} + be.dueSet["local"] = true + be.dueSet["felhom-pbs"] = true + be.phases["local"] = []string{phaseSnapshotted, phaseDone} + be.phases["felhom-pbs"] = []string{phaseSnapshotted, phaseDone} + l := newTierLoop(t, be, st, nil) + + if err := l.runOnce(context.Background()); err != nil { + t.Fatalf("cycle 1: %v", err) + } + if got := be.startedTargets(); len(got) != 1 || got[0] != "local" { + t.Fatalf("cycle 1 must run only the local tier; got %v", got) + } + // The agent now reports local as backed up; the off-site tier is still due. + be.setDue("local", false) + if err := l.runOnce(context.Background()); err != nil { + t.Fatalf("cycle 2: %v", err) + } got := be.startedTargets() - if len(got) != 2 || got[0] != "local" || got[1] != "felhom-pbs" { - t.Fatalf("both tiers must back up, primary first: got %v", got) + if len(got) != 2 || got[1] != "felhom-pbs" { + t.Fatalf("cycle 2 must run the left-over off-site tier; got %v", got) + } + // Its own window: when it started, the first window had already resumed (1 start) and a NEW stop + // had happened (2 stops). + stopsAt, startsAt := be.stopsWhenEachTierStarted(), be.restartsWhenEachTierStarted() + if stopsAt[1] != 2 || startsAt[1] != 1 { + t.Fatalf("the off-site tier must start in its OWN window; at its start stops=%d starts=%d (want 2/1)", stopsAt[1], startsAt[1]) + } + if stops, starts := len(st.stoppedNames()), len(st.startedNames()); stops != 2 || starts != 2 { + t.Fatalf("two cycles, two windows: want 2 stops / 2 starts, got %d/%d", stops, starts) + } + // A third cycle with nothing due touches nothing. + be.setDue("felhom-pbs", false) + if err := l.runOnce(context.Background()); err != nil { + t.Fatal(err) + } + if stops := len(st.stoppedNames()); stops != 2 { + t.Fatalf("nothing due must not stop the apps again; stops=%d", stops) + } +} + +// A manual press („Mentés most") makes the LOCAL copy only, in one short stop; the off-site tier is +// NOT requested — it follows at the next scheduled night run (decision 156). +func TestManualPress_RunsOnlyLocalTier(t *testing.T) { + st := &fakeStacks{running: []string{"immich"}} + be := newTierBackend() + be.stacks = st + // Off-site listed FIRST on purpose: the press must pick the tier the agent flags Primary, not + // whichever comes first. + be.tiers = []BackupTier{{Target: "felhom-pbs"}, {Target: "local", Primary: true}} + be.phases["local"] = []string{phaseSnapshotted, phaseDone} + be.phases["felhom-pbs"] = []string{phaseSnapshotted, phaseDone} + l := newTierLoop(t, be, st, nil) + + if err := l.TriggerNow(); err != nil { + t.Fatalf("TriggerNow: %v", err) + } + // TriggerNow holds l.mu for the whole async cycle; taking it waits for the cycle to finish. + l.mu.Lock() + l.mu.Unlock() //nolint:staticcheck // deliberate: wait for the async cycle + + if got := be.startedTargets(); len(got) != 1 || got[0] != "local" { + t.Fatalf("a manual press must back up ONLY the local (primary) tier; started=%v", got) + } + if stops, starts := len(st.stoppedNames()), len(st.startedNames()); stops != 1 || starts != 1 { + t.Fatalf("a manual press is one short stop; got %d/%d", stops, starts) } } @@ -200,38 +340,6 @@ func TestNoTierDue_NoQuiesce(t *testing.T) { } } -// The app must stay DOWN until the LAST tier snapshots. Resuming after tier 1 would leave the DR -// tier capturing a running app — losing app-consistency on exactly the tier we most want it on. -func TestNonLastTierSnapshot_DoesNotResumeApp(t *testing.T) { - st := &fakeStacks{running: []string{"immich"}} - be := newTierBackend() - be.stacks = st - be.tiers = []BackupTier{{Target: "local", Primary: true}, {Target: "felhom-pbs"}} - be.dueSet["local"] = true - be.dueSet["felhom-pbs"] = true - // The local tier snapshots first, then finishes. The app must NOT come back at its snapshot. - be.phases["local"] = []string{phaseSnapshotted, phaseSnapshotted, phaseDone} - be.phases["felhom-pbs"] = []string{phaseSnapshotted, phaseDone} - - l := newTierLoop(t, be, st, nil) - if err := l.runOnce(context.Background()); err != nil { - t.Fatal(err) - } - // THE assertion: when the SECOND (last) tier started, zero restarts had happened — i.e. the app - // was still quiesced. Resuming at tier 1's snapshot would leave the DR tier capturing a RUNNING - // app, losing app-consistency on exactly the tier we most want it on. - at := be.restartsWhenEachTierStarted() - if len(at) != 2 { - t.Fatalf("both tiers must start; sampled %v", at) - } - if at[1] != 0 { - t.Fatalf("the app had ALREADY resumed (%d restarts) when the last tier started — non-last snapshot must not resume", at[1]) - } - if stops, starts := len(st.stoppedNames()), len(st.startedNames()); stops != 1 || starts != 1 { - t.Fatalf("want exactly one stop/start pair; got %d/%d", stops, starts) - } -} - // ---- RED-PROOF 2 — new controller ↔ OLD agent -------------------------------------------- // The agent 404s /backup/tiers. The controller MUST degrade to the untargeted tier, LOG it, and