079265ad8e
F-CRIT-1 — an app that failed to restart after a quiesce never alarmed, for two
independent reasons, either of which alone kept it dead:
1. restartAll returned nothing, so the failure was logged and dropped and no
caller could learn a customer's app had not come back. It now returns the
stacks that failed; both call sites record the outcome.
2. classifyRunStates whitelists StateStopped on invariant I1 ('StateStopped
means the user stopped it'). The quiesce loop stops stacks by the same
compose-down path, so a failed restart is also StateStopped and was
whitelisted into silence. Loop.FailedRestarts() is now the only thing that
lifts the whitelist, so genuine user stops stay silent (v0.164.0 pinned).
F-A1 — HTTP 409 is the agent's single-flight gate refusing while a restore-test
holds it, not a failure. agentapi now returns a typed *StatusError on POST, the
adapter maps 409 -> quiesce.ErrTierBusy, and the loop defers: no breaker, no
event, no operator email, tier stays DUE.
Two traps avoided. Silence: contention outliving contentionAlarmAfter (3h, set
by the agent's own 120m PBS restore-test ceiling) raises its own BLOCKED signal.
App thrash: removing the failure treatment also removes the breaker's deferral,
so a contended tier is dropped BEFORE anything stops (contentionRetryAfter 15m,
against a 12m01s longest observed restore-test).
Three comments corrected; the invariant rule added to both CLAUDE.md copies.
Six red-proofs, all observed failing.
202 lines
7.3 KiB
Go
202 lines
7.3 KiB
Go
package quiesce
|
|
|
|
import (
|
|
"context"
|
|
"fmt"
|
|
"testing"
|
|
"time"
|
|
)
|
|
|
|
// F-A1 (Campaign 8): the agent's HTTP 409 is its R-85 single-flight gate refusing while a
|
|
// restore-test holds it — contention, not failure. The controller armed the R-88 breaker and
|
|
// emailed the operator anyway, on both boxes.
|
|
//
|
|
// Scenarios E (contention is not failure), F (the tier stays due), G (a REAL error still alarms)
|
|
// and H (unending contention IS a fault). G and H are what make E safe: a suite containing only E
|
|
// passes against a controller that swallows every failure as contention and never alarms again.
|
|
|
|
// recordingNotifier captures what would reach the operator.
|
|
type recordingNotifier struct {
|
|
failed []string
|
|
recovered []string
|
|
}
|
|
|
|
func (r *recordingNotifier) BackupFailed(tier, message, errMsg string) {
|
|
r.failed = append(r.failed, tier+"|"+message)
|
|
}
|
|
func (r *recordingNotifier) BackupRecovered(tier, message string) {
|
|
r.recovered = append(r.recovered, tier)
|
|
}
|
|
|
|
func busyErr() error { return fmt.Errorf("%w: agentapi: POST /backup: HTTP 409", ErrTierBusy) }
|
|
|
|
// Scenario E — a 409 must NOT arm the breaker and must NOT notify.
|
|
//
|
|
// RED-PROOF: delete the `errors.Is(err, ErrTierBusy)` branch in the start path → the 409 falls into
|
|
// noteTierFailure and this fails on both counts ("breaker armed after a 409" and "operator was
|
|
// notified").
|
|
func TestContention_409IsNotAFailure(t *testing.T) {
|
|
be := &fakeBackend{due: true, startErr: busyErr()}
|
|
st := &fakeStacks{running: []string{"paperless-ngx"}}
|
|
l := testLoop(t, be, st)
|
|
n := &recordingNotifier{}
|
|
l.SetTierNotifier(n)
|
|
|
|
if err := l.runOnce(context.Background()); err != nil {
|
|
t.Fatalf("runOnce: %v", err)
|
|
}
|
|
|
|
if got := l.breaker.failuresFor(""); got != 0 {
|
|
t.Errorf("breaker armed after a 409 (%d consecutive failures) — contention is not failure", got)
|
|
}
|
|
if len(n.failed) != 0 {
|
|
t.Errorf("operator was notified for a 409: %v", n.failed)
|
|
}
|
|
// the apps must still come back — a refusal must never hold the customer's stacks down
|
|
if len(st.startedNames()) != 1 {
|
|
t.Errorf("stacks not restarted after a 409: %v", st.startedNames())
|
|
}
|
|
}
|
|
|
|
// Scenario F — the tier stays DUE and the next cycle retries once the gate frees.
|
|
//
|
|
// RED-PROOF: mark the tier satisfied on a 409 (e.g. call noteTierSuccess) → the retry never
|
|
// happens and startCalls stays at 1.
|
|
func TestContention_TierStaysDueAndRetries(t *testing.T) {
|
|
be := &fakeBackend{due: true, startErr: busyErr()}
|
|
st := &fakeStacks{running: []string{"paperless-ngx"}}
|
|
l := testLoop(t, be, st)
|
|
|
|
if err := l.runOnce(context.Background()); err != nil {
|
|
t.Fatalf("cycle 1: %v", err)
|
|
}
|
|
// the restore-test finishes; the contention skip window must not outlive it artificially
|
|
be.startErr = nil
|
|
be.phases = []string{"done"}
|
|
l.contention.clear("")
|
|
|
|
if err := l.runOnce(context.Background()); err != nil {
|
|
t.Fatalf("cycle 2: %v", err)
|
|
}
|
|
if be.startCalls != 2 {
|
|
t.Errorf("backup attempted %d times, want 2 — the tier must stay due and retry after contention", be.startCalls)
|
|
}
|
|
}
|
|
|
|
// Scenario G — a REAL failure (not 409) must still arm the breaker and notify, exactly as before.
|
|
//
|
|
// RED-PROOF: treat every start error as contention → this fails with "a real 500 was swallowed as
|
|
// contention", which is the over-correction that would make E worthless.
|
|
func TestContention_RealFailureStillAlarms(t *testing.T) {
|
|
be := &fakeBackend{due: true, startErr: fmt.Errorf("agentapi: POST /backup: HTTP 500")}
|
|
st := &fakeStacks{running: []string{"paperless-ngx"}}
|
|
l := testLoop(t, be, st)
|
|
n := &recordingNotifier{}
|
|
l.SetTierNotifier(n)
|
|
|
|
_ = l.runOnce(context.Background())
|
|
|
|
if got := l.breaker.failuresFor(""); got != 1 {
|
|
t.Errorf("breaker did not arm on a real 500 (failures=%d) — a real 500 was swallowed as contention", got)
|
|
}
|
|
if len(n.failed) != 1 {
|
|
t.Fatalf("operator was NOT notified of a real failure: %v", n.failed)
|
|
}
|
|
}
|
|
|
|
// Scenario H — unending contention IS a fault, and it alarms ONCE.
|
|
//
|
|
// RED-PROOF: remove the contentionAlarmAfter check in noteTierContention → contention is silent
|
|
// forever and this fails with "no alarm after ...".
|
|
func TestContention_UnendingContentionAlarmsOnce(t *testing.T) {
|
|
l := testLoop(t, &fakeBackend{}, &fakeStacks{})
|
|
n := &recordingNotifier{}
|
|
l.SetTierNotifier(n)
|
|
|
|
base := time.Now()
|
|
now := base
|
|
l.now = func() time.Time { return now }
|
|
|
|
// contention starts, then persists past the bound
|
|
l.noteTierContention("felhom-pbs", "felhom-pbs")
|
|
for _, step := range []time.Duration{30 * time.Minute, 2 * time.Hour, contentionAlarmAfter + time.Minute, contentionAlarmAfter + 2*time.Hour} {
|
|
now = base.Add(step)
|
|
l.noteTierContention("felhom-pbs", "felhom-pbs")
|
|
}
|
|
|
|
if len(n.failed) == 0 {
|
|
t.Fatalf("no alarm after %s of unbroken contention — that is a silence path, the exact thing this fix must not create", contentionAlarmAfter)
|
|
}
|
|
if len(n.failed) != 1 {
|
|
t.Errorf("alarmed %d times, want exactly 1 (edge-triggered like R-97a): %v", len(n.failed), n.failed)
|
|
}
|
|
if got := n.failed[0]; !contains(got, "BLOCKED") {
|
|
t.Errorf("the alarm must name CONTENTION, not a backup failure; got %q", got)
|
|
}
|
|
}
|
|
|
|
// Below the bound, contention must stay quiet — otherwise every normal restore-test pages someone.
|
|
func TestContention_BelowTheBoundIsQuiet(t *testing.T) {
|
|
l := testLoop(t, &fakeBackend{}, &fakeStacks{})
|
|
n := &recordingNotifier{}
|
|
l.SetTierNotifier(n)
|
|
base := time.Now()
|
|
now := base
|
|
l.now = func() time.Time { return now }
|
|
|
|
// the longest restore-test actually observed on the fleet was 12m01s
|
|
for _, step := range []time.Duration{0, 5 * time.Minute, 12 * time.Minute, 90 * time.Minute} {
|
|
now = base.Add(step)
|
|
l.noteTierContention("felhom-pbs", "felhom-pbs")
|
|
}
|
|
if len(n.failed) != 0 {
|
|
t.Errorf("contention alarmed before %s: %v — a normal restore-test must never page anyone", contentionAlarmAfter, n.failed)
|
|
}
|
|
}
|
|
|
|
// A contended tier is dropped BEFORE any stack is stopped. Without this, removing the (wrong)
|
|
// failure treatment would leave the loop re-quiescing every poll for the whole restore-test —
|
|
// stopping the customer's apps each time, which is worse than the bug being fixed.
|
|
func TestContention_ContendedTierIsDroppedBeforeStopping(t *testing.T) {
|
|
be := &fakeBackend{due: true, startErr: busyErr()}
|
|
st := &fakeStacks{running: []string{"paperless-ngx"}}
|
|
l := testLoop(t, be, st)
|
|
|
|
if err := l.runOnce(context.Background()); err != nil {
|
|
t.Fatalf("cycle 1: %v", err)
|
|
}
|
|
stoppedAfterFirst := len(st.stoppedNames())
|
|
|
|
// immediately after, still inside contentionRetryAfter
|
|
if err := l.runOnce(context.Background()); err != nil {
|
|
t.Fatalf("cycle 2: %v", err)
|
|
}
|
|
if got := len(st.stoppedNames()); got != stoppedAfterFirst {
|
|
t.Errorf("stacks were stopped again while the tier was still contended (%d -> %d) — that is app thrash", stoppedAfterFirst, got)
|
|
}
|
|
if be.startCalls != 1 {
|
|
t.Errorf("backup re-attempted while contended (%d calls) — the tier should have been dropped before quiescing", be.startCalls)
|
|
}
|
|
}
|
|
|
|
// The tracker's own contract: clear() reports whether there was a run to end.
|
|
func TestContentionTracker_ClearReportsTheEdge(t *testing.T) {
|
|
c := newContentionTracker()
|
|
if c.clear("x") {
|
|
t.Error("clear() on an unknown target reported an edge")
|
|
}
|
|
c.note("x", time.Now())
|
|
if !c.clear("x") {
|
|
t.Error("clear() did not report the edge after a noted contention")
|
|
}
|
|
}
|
|
|
|
func contains(hay, needle string) bool {
|
|
for i := 0; i+len(needle) <= len(hay); i++ {
|
|
if hay[i:i+len(needle)] == needle {
|
|
return true
|
|
}
|
|
}
|
|
return false
|
|
}
|