R-88 Part 1: a failing backup stops re-quiescing (v0.176.0)

internal/quiesce had no failure counter, no backoff and no breaker, and the driver
is a plain 5-minute ticker — so a tier that was due and kept failing stopped and
restarted every customer app stack every 5 minutes indefinitely. Live on
demo-felhom 2026-07-27: three cycles in eleven minutes against an unreachable PBS
tier; it ended only because PBS recovered.

The breaker gates the QUIESCE, not the backup — the harm was the outage taken to
attempt it, so backed-off tiers are dropped before any stack is stopped. Per
target (a broken offsite tier must not suppress a healthy local one), 15m→30m→
1h→2h→4h capped, reset on success, never permanent, never applied to TriggerNow,
and stillRunning is not a failure. State is in-memory on purpose: forgetting a
backoff costs one attempt; persisting one could outlive the fix.

Part 3 invariant recorded on scheduledRunAllowed — a missing value means UNKNOWN,
and only a positive 'never' may fire the safety valve. Fourth instance of the
class (hub v0.12.0, v0.73.0, R-81, R-88).

Part 2 (unknown != never) is NOT in this commit: the agent returns byte-identical
responses for 'read errored' and 'never backed up', so the controller cannot tell
them apart. That needs an agent wire change and is tracked separately.
This commit is contained in:
2026-07-27 16:21:58 +02:00
parent 3f0420ff9c
commit 32200c7b5f
6 changed files with 677 additions and 1 deletions
@@ -0,0 +1,146 @@
package quiesce
import (
"context"
"testing"
"time"
)
// R-88 Part 3 — the invariant, pinned by name so a refactor has to delete an obviously-named
// contract to reintroduce the bug. See the comment block on scheduledRunAllowed.
//
// A missing value means UNKNOWN. Only a POSITIVE determination of "never backed up" may fire the
// safety valve. Prior instances of the opposite mistake: hub v0.12.0, v0.73.0, R-81, R-88.
//
// ── WHY ONLY HALF THE CONTRACT IS PINNED HERE ────────────────────────────────────────────────
//
// Scenario C ("unknown must NOT bypass the window gate") is NOT testable in this package today, and
// writing a test that pretends otherwise would be worse than leaving it out. The agent returns
// byte-identical responses for "the storage read errored" and "there has never been a backup" —
// same Due, same Reason, same nil AgeSecs — so the controller has nothing to discriminate on. Giving
// "unknown" its own representation is an agent wire change, tracked as R-88 Part 2.
//
// What IS pinned here is Scenario D, and it is the half that a careless Part 2 would break. A fix
// that makes nil stop firing the valve — the obvious way to "fix C" — turns a loud bug into a silent
// one: a box powered on only outside its backup window would never back up at all, and nobody would
// notice for weeks. This test fails against that implementation.
// agedBackend wraps tierBackend so DueFor can report a REAL age. The shared fake always returns nil
// ("never"), which is exactly the case under test here — so the control case needs its own shape
// rather than a field added to a fake three other suites depend on.
type agedBackend struct {
*tierBackend
ages map[string]*int64
}
func (a *agedBackend) DueFor(ctx context.Context, target string) (bool, *int64, error) {
due, _, err := a.tierBackend.DueFor(ctx, target)
return due, a.ages[target], err
}
// SCENARIO D — a genuinely never-backed-up box still gets its first backup, outside the window.
//
// COMPANION RED-PROOF (observed): flip the nil branch of scheduledRunAllowed from `return true` to
// `return false` — i.e. apply "unknown must not bypass the gate" too broadly, which is precisely the
// over-correction Scenario C invites — and this fails with:
//
// "CONTRACT VIOLATED: a never-backed-up box outside its window did NOT back up (0 stack stop(s)) —
// the safety valve was removed; a box only ever powered on outside its window would starve"
//
// (DELETING the branch outright instead panics on the nil deref two lines down — the test still
// catches it, but `return false` is the mutation a real over-correction would produce.)
//
// Restored.
func TestContract_NeverBackedUp_RunsOutsideTheWindow(t *testing.T) {
// 12:00 Budapest, window 02:30 → gate [04:30, 08:30). Firmly outside.
outside := atBudapest(12, 0)
be := newTierBackend()
be.tiers = []BackupTier{{Target: "local"}}
be.dueSet["local"] = true
be.phases["local"] = []string{phaseDone}
// DueFor returns a nil age — "never backed up" (see tierBackend.DueFor).
st := &fakeStacks{running: []string{"bookstack"}}
l := windowLoop(t, be, st, "02:30", outside)
if err := l.runOnce(context.Background()); err != nil {
t.Fatalf("runOnce: %v", err)
}
if got := len(st.stoppedNames()); got == 0 {
t.Fatalf("CONTRACT VIOLATED: a never-backed-up box outside its window did NOT back up (%d stack stop(s)) — "+
"the safety valve was removed; a box only ever powered on outside its window would starve", got)
}
}
// The control for the test above: with a RECENT backup, the same cycle at the same hour defers.
// Without this, TestContract_NeverBackedUp_RunsOutsideTheWindow would also pass against a gate that
// was simply disabled.
func TestContract_RecentBackup_DefersOutsideTheWindow(t *testing.T) {
outside := atBudapest(12, 0)
be := newTierBackend()
be.tiers = []BackupTier{{Target: "local"}}
be.dueSet["local"] = true
be.phases["local"] = []string{phaseDone}
aged := &agedBackend{tierBackend: be, ages: map[string]*int64{"local": i64(20 * 3600)}} // 20h old
st := &fakeStacks{running: []string{"bookstack"}}
l := windowLoop(t, aged, st, "02:30", outside)
if err := l.runOnce(context.Background()); err != nil {
t.Fatalf("runOnce: %v", err)
}
if got := len(st.stoppedNames()); got != 0 {
t.Fatalf("a recently-backed-up box outside its window must DEFER, got %d stack stop(s) — "+
"if this passes the gate is not actually gating anything", got)
}
}
// The valve boundary as a truth table on the real predicate, named so it cannot be quietly dropped.
func TestContract_SafetyValveBoundary(t *testing.T) {
const window = "02:30"
outside := atBudapest(12, 0)
h := func(hours int64) *int64 { return i64(hours * 3600) }
cases := []struct {
name string
age *int64
want bool
}{
{"never backed up (nil) → the valve FIRES", nil, true},
{"just inside cadence+24h → defer", h(47), false},
{"exactly at cadence+24h → defer (strictly greater)", h(48), false},
{"past cadence+24h → the valve fires", h(49), true},
}
for _, c := range cases {
if got := scheduledRunAllowed(outside, window, c.age, cadence24); got != c.want {
t.Errorf("CONTRACT VIOLATED: %s → scheduledRunAllowed = %v, want %v", c.name, got, c.want)
}
}
}
// The breaker must not be able to starve a first-ever backup either: a never-backed-up tier that
// fails still backs off (it must — that is the whole point), but the backoff EXPIRES and it retries.
func TestContract_BreakerNeverStarvesAFirstBackup(t *testing.T) {
now := time.Date(2026, 7, 27, 12, 0, 0, 0, time.UTC)
be := tierBackendFailing("local", "local")
st := &fakeStacks{running: []string{"bookstack"}}
l := breakerLoop(t, be, st, &now)
attempts := 0
for i := 0; i < 4; i++ {
before := len(st.stoppedNames())
if err := l.runOnce(context.Background()); err != nil {
t.Fatal(err)
}
if len(st.stoppedNames()) > before {
attempts++
}
now = now.Add(backoffFor(i+1) + time.Minute) // step past each successive backoff
}
if attempts < 4 {
t.Fatalf("CONTRACT VIOLATED: a never-backed-up tier attempted only %d time(s) across 4 expired backoffs; "+
"the breaker must bound the RETRY INTERVAL, never stop retrying", attempts)
}
}