Skip to content

Commit bf5d32f

Browse files
author
Алекс
committed
alerts: a flapping crash reason no longer wipes the app's diagnosis
A crashlooping pod alternates between Waiting/CrashLoopBackOff and terminated-with-nonzero-exit, so detectPodAlertAt returns 'CrashLoopBackOff' or 'Error' for one unchanged failure. Every reason comparison in the watcher was a raw string equality, so each flap read as a new incident: the ELSE branch of touchAppHealthAlertSeen wrote NULLIF('','') over cause, cause_line and cause_kind even on a tick that read no fresh evidence, and first_detected_at was reset in both touchAppHealthAlertSeen and claimAppHealthAlertSlot, so the escalating cooldown ladder could never advance and 'crashing since' was wrong. Live proof 2026-09-27 [psql + kubectl]: wow83168-gmail-com-prod/nodejs-argo (683 restarts, log says Cannot find module '/tmp/index.js') and lifecoachrussia-yandex-ru-prod/gulyaev-ai-core (1512 restarts, log says Full Knowledge Vault could not be opened) both sat in CrashLoopBackOff with empty cause_kind while the cause was readable from their logs; nodejs-argo still carried last_sent_cause_kind='app_code', proving a verdict had been known and then erased. Across the table 2/82 rows show that emailed-then-erased signature, 21/82 have first_detected_at newer than last_sent_at (an email sent before the incident supposedly began) and 32/64 successfully sent alert emails carried no diagnosis. Reasons are now compared by incident class: CrashLoopBackOff and Error are one class, ImagePullBackOff and ErrImagePull are one class, OOMKilled and anything unrecognized map to themselves. The class mapping lives once in reasonIncidentClass and once as the SQL fragment appHealthReasonClassSQLCase, reused by both statements so they cannot drift. maybeCauseRefresh's cheap-skip compares classes too, so a pure flap no longer forces a live kube GetLogs call every tick for every crashlooping app. A genuine class change still clears a stale verdict, which is the regression this guard exists for: an old ErrImagePull verdict must not survive the flip to CrashLoopBackOff and keep blaming the registry for the user's own crash. Tests RAN against a real postgres (17 RUN, 17 PASS, 0 SKIP), including the six pre-existing app-health guards. Mutation check: collapsing reasonIncidentClass to the raw reason fails three class tests, restoring passes.
1 parent e472797 commit bf5d32f

4 files changed

Lines changed: 460 additions & 46 deletions

File tree

‎automator/state/backlog/items/0521-upload-без-манифеста-framework-даёт-success-билд.md‎

Lines changed: 0 additions & 32 deletions
This file was deleted.
Lines changed: 262 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,262 @@
1+
package api
2+
3+
import (
4+
"context"
5+
"testing"
6+
"time"
7+
8+
"github.com/google/uuid"
9+
)
10+
11+
// TestTouchAppHealthAlertSeenSurvivesSameClassFlapWhenNotRefreshed is
12+
// RED-proof for the live bug (2026-09-27): wow83168-gmail-com-prod/
13+
// nodejs-argo and lifecoachrussia-yandex-ru-prod/gulyaev-ai-core both have a
14+
// crash whose exact log line (see the seeded cause below) has not changed,
15+
// yet their live app_health_alerts row has cause/cause_line/cause_kind all
16+
// empty because detectPodAlertAt's raw reason flapped between
17+
// reasonCrashLoopBackOff and reasonError on the same underlying crash, and
18+
// the old code compared raw reason strings: the moment the reason flipped,
19+
// touchAppHealthAlertSeen's cause CASE fell into its ELSE branch and wrote
20+
// NULLIF passed empty strings resulting in a NULL write unconditionally, even though refreshed=false (no
21+
// new evidence was read this tick). This proves a same-CLASS flap
22+
// (CrashLoopBackOff -> Error) with refreshed=false must keep the stored
23+
// triplet intact.
24+
func TestTouchAppHealthAlertSeenSurvivesSameClassFlapWhenNotRefreshed(t *testing.T) {
25+
pool := testAdvisoryPool(t)
26+
ctx := context.Background()
27+
ns := "test-ns-flap-keep-" + uuid.NewString()[:8]
28+
t.Cleanup(func() {
29+
_, _ = pool.Exec(context.Background(), `DELETE FROM app_health_alerts WHERE namespace = $1`, ns)
30+
})
31+
32+
touchAppHealthAlertSeen(ctx, pool, ns, "nodejs-argo", reasonCrashLoopBackOff, "pod-1/nodejs-argo",
33+
"Судя по логам, это ошибка в коде приложения (Node.js).", "Error: Cannot find module '/tmp/index.js'", "app_code", true)
34+
35+
touchAppHealthAlertSeen(ctx, pool, ns, "nodejs-argo", reasonError, "pod-1/nodejs-argo exit=1", "", "", "", false)
36+
37+
var cause, causeLine, causeKind string
38+
if err := pool.QueryRow(ctx,
39+
`SELECT COALESCE(cause, ''), COALESCE(cause_line, ''), COALESCE(cause_kind, '') FROM app_health_alerts WHERE namespace = $1 AND app_name = 'nodejs-argo'`,
40+
ns).Scan(&cause, &causeLine, &causeKind); err != nil {
41+
t.Fatalf("read back row after flap: %v", err)
42+
}
43+
if cause == "" || causeLine == "" || causeKind == "" {
44+
t.Fatalf("expected the stored diagnosis to survive a same-class flap (CrashLoopBackOff -> Error) with refreshed=false, got cause=%q cause_line=%q cause_kind=%q", cause, causeLine, causeKind)
45+
}
46+
if causeLine != "Error: Cannot find module '/tmp/index.js'" {
47+
t.Fatalf("expected the original cause_line to be unchanged, got %q", causeLine)
48+
}
49+
50+
touchAppHealthAlertSeen(ctx, pool, ns, "nodejs-argo", reasonCrashLoopBackOff, "pod-1/nodejs-argo", "", "", "", false)
51+
if err := pool.QueryRow(ctx,
52+
`SELECT COALESCE(cause, ''), COALESCE(cause_line, ''), COALESCE(cause_kind, '') FROM app_health_alerts WHERE namespace = $1 AND app_name = 'nodejs-argo'`,
53+
ns).Scan(&cause, &causeLine, &causeKind); err != nil {
54+
t.Fatalf("read back row after flap back: %v", err)
55+
}
56+
if cause == "" || causeLine == "" || causeKind == "" {
57+
t.Fatalf("expected the stored diagnosis to survive flapping back to CrashLoopBackOff, got cause=%q cause_line=%q cause_kind=%q", cause, causeLine, causeKind)
58+
}
59+
}
60+
61+
// TestTouchAppHealthAlertSeenClearsOnGenuineClassChange is the CRITICAL
62+
// correctness guard this fix must preserve (see task diagnosis): an old
63+
// ErrImagePull verdict (cause_kind=platform_registry, the live shape from
64+
// the 2026-08-30 gateway/internal-prod incident) must NOT survive a genuine
65+
// class change to CrashLoopBackOff, even though both reasons individually
66+
// flap against a sibling (ErrImagePull<->ImagePullBackOff,
67+
// CrashLoopBackOff<->Error). ImagePullBackOff and CrashLoopBackOff are
68+
// different classes, so this transition with no new verdict (refreshed=true,
69+
// causeKind empty -- the fresh log matched no signature) must clear the
70+
// stale triplet, exactly as the pre-existing
71+
// TestTouchAppHealthAlertSeenClearsStaleCauseWhenRefreshed test already
72+
// pins for the raw-reason case; this test pins the same contract survives
73+
// the class-based comparison.
74+
func TestTouchAppHealthAlertSeenClearsOnGenuineClassChange(t *testing.T) {
75+
pool := testAdvisoryPool(t)
76+
ctx := context.Background()
77+
ns := "test-ns-class-change-clear-" + uuid.NewString()[:8]
78+
t.Cleanup(func() {
79+
_, _ = pool.Exec(context.Background(), `DELETE FROM app_health_alerts WHERE namespace = $1`, ns)
80+
})
81+
82+
touchAppHealthAlertSeen(ctx, pool, ns, "gateway", reasonErrImagePull, "pod-1/gateway",
83+
"Похоже, проблема на стороне реестра образов.", "", "platform_registry", true)
84+
85+
var cause, causeLine, causeKind string
86+
if err := pool.QueryRow(ctx,
87+
`SELECT COALESCE(cause, ''), COALESCE(cause_line, ''), COALESCE(cause_kind, '') FROM app_health_alerts WHERE namespace = $1 AND app_name = 'gateway'`,
88+
ns).Scan(&cause, &causeLine, &causeKind); err != nil {
89+
t.Fatalf("read back seeded row: %v", err)
90+
}
91+
if causeKind != "platform_registry" {
92+
t.Fatalf("expected the seeded platform_registry cause to be stored, got cause_kind=%q", causeKind)
93+
}
94+
95+
touchAppHealthAlertSeen(ctx, pool, ns, "gateway", reasonCrashLoopBackOff, "pod-2/gateway", "", "", "", true)
96+
97+
if err := pool.QueryRow(ctx,
98+
`SELECT COALESCE(cause, ''), COALESCE(cause_line, ''), COALESCE(cause_kind, '') FROM app_health_alerts WHERE namespace = $1 AND app_name = 'gateway'`,
99+
ns).Scan(&cause, &causeLine, &causeKind); err != nil {
100+
t.Fatalf("read back row after class change: %v", err)
101+
}
102+
if cause != "" || causeLine != "" || causeKind != "" {
103+
t.Fatalf("expected the stale platform_registry verdict to be cleared on a genuine class change (ImagePull -> CrashLoop), got cause=%q cause_line=%q cause_kind=%q", cause, causeLine, causeKind)
104+
}
105+
}
106+
107+
// TestTouchAppHealthAlertSeenOverwritesOnPositiveNewVerdictAcrossFlap proves
108+
// the fourth required behavior: a positive new classification (OOMKilled,
109+
// refreshed=true) always overwrites, even when it arrives on top of an
110+
// incident that was itself mid-flap between CrashLoopBackOff and Error. The
111+
// class guard must never block a genuine new diagnosis from landing.
112+
func TestTouchAppHealthAlertSeenOverwritesOnPositiveNewVerdictAcrossFlap(t *testing.T) {
113+
pool := testAdvisoryPool(t)
114+
ctx := context.Background()
115+
ns := "test-ns-oom-overwrite-" + uuid.NewString()[:8]
116+
t.Cleanup(func() {
117+
_, _ = pool.Exec(context.Background(), `DELETE FROM app_health_alerts WHERE namespace = $1`, ns)
118+
})
119+
120+
touchAppHealthAlertSeen(ctx, pool, ns, "worker", reasonCrashLoopBackOff, "pod-1/worker",
121+
"Судя по логам, это ошибка в коде приложения.", "panic: boom", "app_code", true)
122+
touchAppHealthAlertSeen(ctx, pool, ns, "worker", reasonError, "pod-1/worker exit=2", "", "", "", false)
123+
124+
touchAppHealthAlertSeen(ctx, pool, ns, "worker", reasonOOMKilled, "pod-1/worker",
125+
"Приложению не хватает памяти.", "", "resource_limit", true)
126+
127+
var cause, causeLine, causeKind string
128+
if err := pool.QueryRow(ctx,
129+
`SELECT COALESCE(cause, ''), COALESCE(cause_line, ''), COALESCE(cause_kind, '') FROM app_health_alerts WHERE namespace = $1 AND app_name = 'worker'`,
130+
ns).Scan(&cause, &causeLine, &causeKind); err != nil {
131+
t.Fatalf("read back row after OOM overwrite: %v", err)
132+
}
133+
if causeKind != "resource_limit" {
134+
t.Fatalf("expected the OOMKilled verdict to overwrite the previous app_code verdict, got cause_kind=%q", causeKind)
135+
}
136+
if causeLine != "" {
137+
t.Fatalf("resource_limit verdicts carry no cause_line by design, got %q", causeLine)
138+
}
139+
}
140+
141+
// TestFirstDetectedAtSurvivesSameClassFlap proves the incident-age fix
142+
// directly: first_detected_at must not reset to now() on a same-class flap.
143+
// Live proof this matters: gulyaev-ai-core has 1512 restarts but
144+
// first_detected_at was observed at 2026-09-27 05:56, i.e. minutes before
145+
// the row was read, because every flap between CrashLoopBackOff and Error
146+
// was resetting the incident's age and so the escalating cooldown ladder
147+
// (appHealthAlertEscalationSteps: 24h -> 72h after 3d -> weekly after 14d)
148+
// could never advance for a genuinely long-running, constantly-flapping
149+
// incident.
150+
func TestFirstDetectedAtSurvivesSameClassFlap(t *testing.T) {
151+
pool := testAdvisoryPool(t)
152+
ctx := context.Background()
153+
ns := "test-ns-first-detected-flap-" + uuid.NewString()[:8]
154+
t.Cleanup(func() {
155+
_, _ = pool.Exec(context.Background(), `DELETE FROM app_health_alerts WHERE namespace = $1`, ns)
156+
})
157+
158+
touchAppHealthAlertSeen(ctx, pool, ns, "gulyaev-ai-core", reasonCrashLoopBackOff, "pod-1/gulyaev-ai-core", "", "", "", false)
159+
160+
if _, err := pool.Exec(ctx,
161+
`UPDATE app_health_alerts SET first_detected_at = now() - interval '5 days' WHERE namespace = $1 AND app_name = 'gulyaev-ai-core'`,
162+
ns); err != nil {
163+
t.Fatalf("backdate first_detected_at: %v", err)
164+
}
165+
166+
var firstDetected time.Time
167+
if err := pool.QueryRow(ctx,
168+
`SELECT first_detected_at FROM app_health_alerts WHERE namespace = $1 AND app_name = 'gulyaev-ai-core'`,
169+
ns).Scan(&firstDetected); err != nil {
170+
t.Fatalf("read back backdated first_detected_at: %v", err)
171+
}
172+
173+
touchAppHealthAlertSeen(ctx, pool, ns, "gulyaev-ai-core", reasonError, "pod-1/gulyaev-ai-core exit=1", "", "", "", false)
174+
touchAppHealthAlertSeen(ctx, pool, ns, "gulyaev-ai-core", reasonCrashLoopBackOff, "pod-1/gulyaev-ai-core", "", "", "", false)
175+
176+
var firstDetectedAfterFlap time.Time
177+
if err := pool.QueryRow(ctx,
178+
`SELECT first_detected_at FROM app_health_alerts WHERE namespace = $1 AND app_name = 'gulyaev-ai-core'`,
179+
ns).Scan(&firstDetectedAfterFlap); err != nil {
180+
t.Fatalf("read back first_detected_at after flapping: %v", err)
181+
}
182+
if !firstDetectedAfterFlap.Equal(firstDetected) {
183+
t.Fatalf("expected first_detected_at to survive a same-class flap, got %v (was %v)", firstDetectedAfterFlap, firstDetected)
184+
}
185+
}
186+
187+
// TestFirstDetectedAtResetsOnGenuineClassChange is the paired guard: a real
188+
// class change (a new failure kind starting) must still reset the incident
189+
// age, or a stale first_detected_at would keep an unrelated NEW failure on
190+
// the tail end of the OLD incident's escalated (slower) cooldown cadence.
191+
func TestFirstDetectedAtResetsOnGenuineClassChange(t *testing.T) {
192+
pool := testAdvisoryPool(t)
193+
ctx := context.Background()
194+
ns := "test-ns-first-detected-reset-" + uuid.NewString()[:8]
195+
t.Cleanup(func() {
196+
_, _ = pool.Exec(context.Background(), `DELETE FROM app_health_alerts WHERE namespace = $1`, ns)
197+
})
198+
199+
touchAppHealthAlertSeen(ctx, pool, ns, "web", reasonImagePullBackOff, "pod-1/web", "", "", "", false)
200+
if _, err := pool.Exec(ctx,
201+
`UPDATE app_health_alerts SET first_detected_at = now() - interval '10 days' WHERE namespace = $1 AND app_name = 'web'`,
202+
ns); err != nil {
203+
t.Fatalf("backdate first_detected_at: %v", err)
204+
}
205+
206+
touchAppHealthAlertSeen(ctx, pool, ns, "web", reasonCrashLoopBackOff, "pod-2/web", "", "", "", false)
207+
208+
var firstDetected time.Time
209+
if err := pool.QueryRow(ctx,
210+
`SELECT first_detected_at FROM app_health_alerts WHERE namespace = $1 AND app_name = 'web'`,
211+
ns).Scan(&firstDetected); err != nil {
212+
t.Fatalf("read back first_detected_at after class change: %v", err)
213+
}
214+
if time.Since(firstDetected) > time.Minute {
215+
t.Fatalf("expected first_detected_at to reset to roughly now() on a genuine class change, got %v (age %v)", firstDetected, time.Since(firstDetected))
216+
}
217+
}
218+
219+
// TestClaimAppHealthAlertSlotFirstDetectedAtSurvivesSameClassFlap proves the
220+
// same first_detected_at protection applies to claimAppHealthAlertSlot, the
221+
// second write path the diagnosis identifies (line ~555): the cooldown
222+
// claim's own first_detected_at CASE must also compare by class, not by raw
223+
// reason, or the escalation ladder still resets every time an email claim
224+
// lands mid-flap even though touchAppHealthAlertSeen's copy is fixed.
225+
func TestClaimAppHealthAlertSlotFirstDetectedAtSurvivesSameClassFlap(t *testing.T) {
226+
pool := testAdvisoryPool(t)
227+
ctx := context.Background()
228+
ns := "test-ns-claim-flap-" + uuid.NewString()[:8]
229+
t.Cleanup(func() {
230+
_, _ = pool.Exec(context.Background(), `DELETE FROM app_health_alerts WHERE namespace = $1`, ns)
231+
})
232+
233+
if !claimAppHealthAlertSlot(ctx, pool, ns, "web", reasonCrashLoopBackOff, "pod/web", 24*time.Hour) {
234+
t.Fatalf("first claim must succeed")
235+
}
236+
if _, err := pool.Exec(ctx,
237+
`UPDATE app_health_alerts SET first_detected_at = now() - interval '20 days', last_sent_at = now() - interval '8 days' WHERE namespace = $1 AND app_name = 'web'`,
238+
ns); err != nil {
239+
t.Fatalf("backdate row: %v", err)
240+
}
241+
242+
var firstDetected time.Time
243+
if err := pool.QueryRow(ctx,
244+
`SELECT first_detected_at FROM app_health_alerts WHERE namespace = $1 AND app_name = 'web'`,
245+
ns).Scan(&firstDetected); err != nil {
246+
t.Fatalf("read back backdated first_detected_at: %v", err)
247+
}
248+
249+
if !claimAppHealthAlertSlot(ctx, pool, ns, "web", reasonError, "pod/web exit=1", 24*time.Hour) {
250+
t.Fatalf("claim on a same-class flap past the escalated cooldown must still succeed")
251+
}
252+
253+
var firstDetectedAfter time.Time
254+
if err := pool.QueryRow(ctx,
255+
`SELECT first_detected_at FROM app_health_alerts WHERE namespace = $1 AND app_name = 'web'`,
256+
ns).Scan(&firstDetectedAfter); err != nil {
257+
t.Fatalf("read back first_detected_at after claim: %v", err)
258+
}
259+
if !firstDetectedAfter.Equal(firstDetected) {
260+
t.Fatalf("expected claimAppHealthAlertSlot to preserve first_detected_at across a same-class flap, got %v (was %v)", firstDetectedAfter, firstDetected)
261+
}
262+
}

0 commit comments

Comments
 (0)