C9-F1 + C9-F2: a restore that restored nothing, and a crash loop nobody saw (v0.183.0)
Both are the system reporting healthy while the customer is not, and both live in the same status-derivation code. Neither is fixed by making the system quieter. C9-F1 (HIGH) — Tier-2 writes recovery-unit/ on EVERY run and RestoreTier2Files has never read it (tier2_restore.go:101-104 reads hdd/ + userdata/ only). Phase 0 enumerated all 53 catalog templates against both demo boxes: 43 apps have NO readable subtree, so the button stopped the app, restored 0 files, restarted it and said "Nincs hiányzó fájl — minden fájl megvan a helyén." — at the moment the customer pressed it because files were missing, with 156 MB of BookStack's data unread in the same copy. 9 apps have file legs but never their DB or volumes, so the same sentence was also a clean bill of health over data never opened (immich: 1.3 GB Postgres unit). Honesty half shipped: a pre-flight coverage check refuses UP FRONT without stopping the app and NAMES the action that works; a run that proceeds claims only what it EXAMINED and discloses that the database and volumes are not covered. Completeness is filed as C9-F1b — routing to the Tier-1 unit restore puts a destructive operation behind a non-destructive button, so its confirm copy has to carry that difference. C9-F4 filed: nothing reads the Tier-2 recovery-unit/ mirror, so the second local copy that exists for drive loss is unreachable by any customer action. C9-F2 (HIGH) — a crash loop was counted as working. StateRestarting is deliberately NOT added to IsDownState (that alarms on every deploy fleet-wide, the over-correction F-A1 nearly cost us); a sustained run becomes down after crashLoopAfter = 5m, set above the 120s deploy timeout, Mealie's 60s start_period and R-97b's 180s grace. The dashboard counter uses the same predicate, so it no longer contradicts the alarm on the same screen. README's claim that faults "still surface as restarting" was a wish with no test — corrected in place; it is the seventh such instance. Six red-proofs observed, including the one that matters most: adding StateRestarting to IsDownState fails the brief-restart test with "every deploy and update would page the operator". go test ./... rc=0, 27 packages, run and read separately from this commit.
This commit is contained in:
@@ -1,6 +1,8 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"time"
|
||||
|
||||
"testing"
|
||||
|
||||
"gitea.dooplex.hu/admin/felhom-controller/internal/notify"
|
||||
@@ -56,7 +58,7 @@ func TestClassifyRunStates_StoppedIsSuppressed(t *testing.T) {
|
||||
stack("nextcloud", stacks.StateDegraded, true, false),
|
||||
}
|
||||
|
||||
dead, states := classifyRunStates(sts, nil, nil)
|
||||
dead, states := classifyRunStates(sts, nil, nil, time.Now())
|
||||
|
||||
gotDead := deadNames(dead)
|
||||
if len(gotDead) != 2 || !gotDead["immich"] || !gotDead["nextcloud"] {
|
||||
@@ -90,7 +92,7 @@ func TestClassifyRunStates_FaultParity(t *testing.T) {
|
||||
stack("nextcloud", stacks.StateDegraded, true, false),
|
||||
}
|
||||
|
||||
dead, states := classifyRunStates(sts, nil, nil)
|
||||
dead, states := classifyRunStates(sts, nil, nil, time.Now())
|
||||
|
||||
gotDead := deadNames(dead)
|
||||
if len(gotDead) != 2 || !gotDead["immich"] || !gotDead["nextcloud"] {
|
||||
@@ -116,7 +118,7 @@ func TestClassifyRunStates_SkipsDeployingAndUndeployed(t *testing.T) {
|
||||
stack("mid", stacks.StateDeploying, true, true), // mid-deploy → skipped
|
||||
stack("gone", stacks.StateExited, false, false), // not deployed → skipped
|
||||
}
|
||||
dead, states := classifyRunStates(sts, nil, nil)
|
||||
dead, states := classifyRunStates(sts, nil, nil, time.Now())
|
||||
if len(dead) != 0 || len(states) != 0 {
|
||||
t.Fatalf("deploying and undeployed stacks must be skipped, got dead=%+v states=%+v", dead, states)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,139 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"gitea.dooplex.hu/admin/felhom-controller/internal/stacks"
|
||||
)
|
||||
|
||||
// C9-F2 — a SUSTAINED `restarting` is a crash loop and must alarm; a BRIEF one must not.
|
||||
//
|
||||
// The defect: `IsDownState` excludes `restarting` as "self-recovering", but for the catalog's
|
||||
// standard `restart: unless-stopped` Docker retries forever, so a crash loop sat in `restarting`
|
||||
// indefinitely and was counted as working. Campaign 9 watched docmost loop for nine minutes
|
||||
// (restartcount 18) while the F-OBS heartbeat printed "4 deployed app(s) evaluated, 0 currently down".
|
||||
//
|
||||
// The whole design tension is that B must keep passing while A does: an alarm that fires on every
|
||||
// deploy is one the operator learns to ignore.
|
||||
|
||||
// restartingSince builds a deployed stack that has been restarting since `since`.
|
||||
func restartingSince(name string, since time.Time) stacks.Stack {
|
||||
s := stack(name, stacks.StateRestarting, true, false)
|
||||
s.RestartingSince = since
|
||||
return s
|
||||
}
|
||||
|
||||
// SCENARIO A — a crash loop alarms. A stack restarting for longer than the threshold enters BOTH the
|
||||
// banner dead-list and the notifier Down-set, so app_start_failed can fire.
|
||||
//
|
||||
// RED-PROOF (observed): drop `|| crashLooping` from the `down` expression in classifyRunStates →
|
||||
//
|
||||
// crashloop_classify_test.go:52: docmost is NOT in the Down-set — a crash loop is silent (this is C9-F2)
|
||||
// crashloop_classify_test.go:55: docmost is NOT in the banner dead-list
|
||||
func TestClassifyRunStates_SustainedRestartingAlarms(t *testing.T) {
|
||||
now := time.Now()
|
||||
sts := []stacks.Stack{
|
||||
stack("paperless-ngx", stacks.StateRunning, true, false),
|
||||
restartingSince("docmost", now.Add(-9*time.Minute)), // the Campaign 9 observation, exactly
|
||||
}
|
||||
|
||||
dead, states := classifyRunStates(sts, nil, nil, now)
|
||||
|
||||
if !downByName(states)["docmost"] {
|
||||
t.Errorf("docmost is NOT in the Down-set — a crash loop is silent (this is C9-F2)")
|
||||
}
|
||||
if !deadNames(dead)["docmost"] {
|
||||
t.Errorf("docmost is NOT in the banner dead-list")
|
||||
}
|
||||
if downByName(states)["paperless-ngx"] {
|
||||
t.Errorf("a healthy app was dragged down with it")
|
||||
}
|
||||
}
|
||||
|
||||
// SCENARIO B — a normal deploy or update does NOT alarm. `docker compose up -d` passes through
|
||||
// restarting; alarming there would page the operator on every routine operation, fleet-wide.
|
||||
//
|
||||
// This is the test that must fail against the naive fix. RED-PROOF (observed): add StateRestarting
|
||||
// to IsDownState instead of using the threshold →
|
||||
//
|
||||
// crashloop_classify_test.go:78: a BRIEFLY restarting app alarms — every deploy and update would page the operator
|
||||
func TestClassifyRunStates_BriefRestartingIsSilent(t *testing.T) {
|
||||
now := time.Now()
|
||||
sts := []stacks.Stack{
|
||||
restartingSince("mealie", now.Add(-30*time.Second)), // mid-deploy
|
||||
restartingSince("ghost", now.Add(-2*time.Minute)), // slow image pull, still normal
|
||||
}
|
||||
|
||||
dead, states := classifyRunStates(sts, nil, nil, now)
|
||||
|
||||
for _, name := range []string{"mealie", "ghost"} {
|
||||
if downByName(states)[name] {
|
||||
t.Errorf("a BRIEFLY restarting app alarms (%s) — every deploy and update would page the operator", name)
|
||||
}
|
||||
}
|
||||
if len(dead) != 0 {
|
||||
t.Errorf("banner dead-list should be empty during normal restarts, got %v", deadNames(dead))
|
||||
}
|
||||
}
|
||||
|
||||
// The boundary itself, asserted from both sides so the threshold cannot drift silently.
|
||||
func TestCrashLooping_ThresholdBoundary(t *testing.T) {
|
||||
now := time.Now()
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
age time.Duration
|
||||
want bool
|
||||
}{
|
||||
{"just under the threshold", 4*time.Minute + 59*time.Second, false},
|
||||
{"exactly at the threshold", 5 * time.Minute, true},
|
||||
{"well past it", 30 * time.Minute, true},
|
||||
} {
|
||||
s := restartingSince("app", now.Add(-tc.age))
|
||||
if got := s.CrashLooping(now); got != tc.want {
|
||||
t.Errorf("%s: CrashLooping(age=%s) = %v, want %v", tc.name, tc.age, got, tc.want)
|
||||
}
|
||||
}
|
||||
|
||||
// A stack that is not restarting is never a crash loop, however old the stamp.
|
||||
s := stack("app", stacks.StateRunning, true, false)
|
||||
s.RestartingSince = now.Add(-time.Hour)
|
||||
if s.CrashLooping(now) {
|
||||
t.Error("a RUNNING stack reported as crash-looping — the state test is missing")
|
||||
}
|
||||
|
||||
// A zero stamp is "not yet observed restarting", never a crash loop — this is what makes the
|
||||
// first scan after a controller restart silent instead of alarming on everything at once.
|
||||
z := stack("app", stacks.StateRestarting, true, false)
|
||||
if z.CrashLooping(now) {
|
||||
t.Error("a zero RestartingSince reported as crash-looping — a controller restart would alarm fleet-wide")
|
||||
}
|
||||
}
|
||||
|
||||
// SCENARIO C — R-97b's quiesce suppression still wins inside its window. A stack the backup stopped
|
||||
// and is restarting must stay silent while suppressed, even if its restarting run is old enough to
|
||||
// qualify. The window EXPIRES, so a genuinely dead app still alarms afterwards — proven by the
|
||||
// second half of this test.
|
||||
//
|
||||
// RED-PROOF (observed): drop `&& !quiesced[st.Name]` from the `down` expression →
|
||||
//
|
||||
// crashloop_classify_test.go:129: a quiesced stack alarms — every backup would page the customer
|
||||
func TestClassifyRunStates_QuiesceSuppressionBeatsCrashLoop(t *testing.T) {
|
||||
now := time.Now()
|
||||
sts := []stacks.Stack{restartingSince("docmost", now.Add(-9*time.Minute))}
|
||||
|
||||
// Inside the R-97b window.
|
||||
_, states := classifyRunStates(sts, map[string]bool{"docmost": true}, nil, now)
|
||||
if downByName(states)["docmost"] {
|
||||
t.Errorf("a quiesced stack alarms — every backup would page the customer")
|
||||
}
|
||||
|
||||
// Window expired (the stack is no longer reported as suppressed): the same stack must now alarm.
|
||||
dead, states := classifyRunStates(sts, nil, nil, now)
|
||||
if !downByName(states)["docmost"] {
|
||||
t.Errorf("suppression outlived its window — a genuinely dead app stayed silent (R-97b's own warning)")
|
||||
}
|
||||
if !deadNames(dead)["docmost"] {
|
||||
t.Errorf("suppression outlived its window for the banner too")
|
||||
}
|
||||
}
|
||||
@@ -1,6 +1,8 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"time"
|
||||
|
||||
"testing"
|
||||
|
||||
"gitea.dooplex.hu/admin/felhom-controller/internal/stacks"
|
||||
@@ -27,7 +29,7 @@ func TestClassifyRunStates_FailedRestartAlarmsDespiteStateStopped(t *testing.T)
|
||||
}
|
||||
failed := map[string]bool{"immich": true}
|
||||
|
||||
dead, states := classifyRunStates(sts, nil, failed)
|
||||
dead, states := classifyRunStates(sts, nil, failed, time.Now())
|
||||
|
||||
if !downByName(states)["immich"] {
|
||||
t.Error("a stack that FAILED to restart is silent (Down=false) — this is F-CRIT-1")
|
||||
@@ -54,7 +56,7 @@ func TestClassifyRunStates_UserStopStillSilent(t *testing.T) {
|
||||
// only immich failed to restart; cwa was never touched by a quiesce
|
||||
failed := map[string]bool{"immich": true}
|
||||
|
||||
dead, states := classifyRunStates(sts, nil, failed)
|
||||
dead, states := classifyRunStates(sts, nil, failed, time.Now())
|
||||
down := downByName(states)
|
||||
|
||||
if down["cwa"] || deadNames(dead)["cwa"] {
|
||||
@@ -75,7 +77,7 @@ func TestClassifyRunStates_NoFailedRestartsIsV0164Behaviour(t *testing.T) {
|
||||
stack("nextcloud", stacks.StateDegraded, true, false),
|
||||
}
|
||||
|
||||
dead, states := classifyRunStates(sts, nil, nil)
|
||||
dead, states := classifyRunStates(sts, nil, nil, time.Now())
|
||||
down := downByName(states)
|
||||
|
||||
if down["cwa"] {
|
||||
@@ -99,7 +101,7 @@ func TestClassifyRunStates_GraceWindowStillSuppresses(t *testing.T) {
|
||||
quiesced := map[string]bool{"immich": true} // still inside quiesceAlarmGrace
|
||||
failed := map[string]bool{"immich": true} // and we already know the restart failed
|
||||
|
||||
dead, states := classifyRunStates(sts, quiesced, failed)
|
||||
dead, states := classifyRunStates(sts, quiesced, failed, time.Now())
|
||||
|
||||
if downByName(states)["immich"] {
|
||||
t.Error("alarmed while still inside the grace window — R-97b Scenario E broken")
|
||||
@@ -115,7 +117,7 @@ func TestClassifyRunStates_UndeployedIgnored(t *testing.T) {
|
||||
stack("ghost", stacks.StateStopped, false, false),
|
||||
stack("deploying", stacks.StateStopped, true, true),
|
||||
}
|
||||
dead, states := classifyRunStates(sts, nil, map[string]bool{"ghost": true, "deploying": true})
|
||||
dead, states := classifyRunStates(sts, nil, map[string]bool{"ghost": true, "deploying": true}, time.Now())
|
||||
if len(dead) != 0 || len(states) != 0 {
|
||||
t.Errorf("undeployed/deploying stacks were classified: dead=%v states=%v", deadNames(dead), states)
|
||||
}
|
||||
|
||||
@@ -1242,7 +1242,7 @@ func runBootReconcile(ctx context.Context, mgr bootrecon.StackProvider, logger *
|
||||
func scanDeployedAppRunStates(mgr *stacks.Manager, q *quiesce.Loop) ([]web.DeadApp, []notify.AppRunState) {
|
||||
// R-97b: a stack THIS controller stopped for a backup is not a fault. q may be nil (unprovisioned
|
||||
// guest) — SuppressedStacks is nil-safe and returns nothing, i.e. suppress nothing.
|
||||
return classifyRunStates(mgr.GetStacks(), q.SuppressedStacks(), q.FailedRestarts())
|
||||
return classifyRunStates(mgr.GetStacks(), q.SuppressedStacks(), q.FailedRestarts(), time.Now())
|
||||
}
|
||||
|
||||
// classifyRunStates is the pure fix-3 derivation over a plain stack slice. It splits the deployed
|
||||
@@ -1274,7 +1274,9 @@ func scanDeployedAppRunStates(mgr *stacks.Manager, q *quiesce.Loop) ([]web.DeadA
|
||||
// which is correct: out-of-band tampering IS reportable.) IsDownState is intentionally left unchanged
|
||||
// — other callers rely on stopped counting as down; the suppression is a filter at this single
|
||||
// derivation point only.
|
||||
func classifyRunStates(sts []stacks.Stack, quiesced map[string]bool, failedRestart map[string]bool) ([]web.DeadApp, []notify.AppRunState) {
|
||||
// `now` is injected (C9-F2) so the crash-loop threshold is a unit-testable contract rather than a
|
||||
// property of the wall clock.
|
||||
func classifyRunStates(sts []stacks.Stack, quiesced map[string]bool, failedRestart map[string]bool, now time.Time) ([]web.DeadApp, []notify.AppRunState) {
|
||||
var dead []web.DeadApp
|
||||
var states []notify.AppRunState
|
||||
for _, st := range sts {
|
||||
@@ -1289,8 +1291,15 @@ func classifyRunStates(sts []stacks.Stack, quiesced map[string]bool, failedResta
|
||||
// F-CRIT-1: StateStopped is whitelisted as a deliberate user stop UNLESS the quiesce loop
|
||||
// reports that it stopped this stack and could not restart it. That single term is what turns
|
||||
// an indefinitely-silent dead app back into an alarm, without re-alarming genuine user stops.
|
||||
// C9-F2: a SUSTAINED restarting is a crash loop, and a crash loop is a dead app. Deliberately
|
||||
// NOT folded into IsDownState — that would alarm on every deploy and update fleet-wide, which
|
||||
// is the over-correction F-A1 nearly cost us. The threshold (stacks.crashLoopAfter, 5 min) sits
|
||||
// above the deploy health timeout, the slowest catalog start_period AND R-97b's grace, so a
|
||||
// brief restart never reaches it and the two suppression windows compose into one bounded
|
||||
// delay. Quiesce suppression below still wins inside its own window.
|
||||
crashLooping := st.CrashLooping(now)
|
||||
userStopped := st.State == stacks.StateStopped && !failedRestart[st.Name]
|
||||
down := stacks.IsDownState(st.State) && !userStopped && !quiesced[st.Name]
|
||||
down := (stacks.IsDownState(st.State) || crashLooping) && !userStopped && !quiesced[st.Name]
|
||||
states = append(states, notify.AppRunState{Name: st.Name, DisplayName: st.Meta.DisplayName, Down: down})
|
||||
if down {
|
||||
dead = append(dead, web.DeadApp{Name: st.Name, DisplayName: st.Meta.DisplayName, State: string(st.State)})
|
||||
|
||||
Reference in New Issue
Block a user