From b2ad871720e66dd40f201c33eb8a1e17be5d8f41 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Mon, 29 Jun 2026 21:45:57 +0200 Subject: [PATCH] =?UTF-8?q?channelhealth:=20F2=20=E2=80=94=20alert=20on=20?= =?UTF-8?q?born/persistent-down=20(alerted=20flag),=20not=20only=20transit?= =?UTF-8?q?ions=20v0.91.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A channel broken at startup/reseed (e.g. controller boots into pin_mismatch) was dashboard-only, no operator email ever. New 'alerted' flag drives alerting instead of prev=='': born-down non-transient alerts cycle 1; transient still N>=2; healthy first-obs silent; recovery re-arms. Red-proof + companion included. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Pg8ANF97SEeKYSN5Jxw3qJ --- CHANGELOG.md | 15 ++++ controller/internal/channelhealth/checker.go | 42 ++++++---- .../internal/channelhealth/checker_test.go | 76 ++++++++++++++++--- 3 files changed, 109 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a2d6e84..5213919 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,20 @@ ## Changelog +### v0.91.0 — F2: alert on born/persistent-down channel (not only transitions) (2026-06-29) +- **What:** closes F2 from the full-stack testrun — a channel failure present at **startup/reseed** + (e.g. the controller boots right after a leaf regen → first observation is `pin_mismatch`) was + dashboard-only, **no operator email, forever**. Now a born-down non-transient reason alerts on cycle 1. +- **`internal/channelhealth/checker.go`:** added an `alerted` flag (have we emitted for the CURRENT + down-spell?). A confirmed down that comes from up/unseeded OR changes reason re-arms (`alerted=false`) + then emits once; a steady down that already alerted does not re-fire; recovery (up) re-arms. Removed + the `prev==""` silent-seed-for-down branch (a born-down IS a real down-spell). Debounce stays intact: + a **transient** born-down (refused) still needs N≥2 (the cold-boot agent-not-yet-up race), and a + **healthy** first-obs still seeds silently. +- Tests: F2 born-down non-transient **red-proof** (one alert cycle 1) + companion showing the old + seed-silent path would not have alerted; born-down transient still debounced; recovery re-arms the + spell. Version `0.90.0 → 0.91.0`. + + ### v0.90.0 — Controller→agent channel health-check (periodic probe + classified operator alert) (2026-06-29) - **What:** the next self-health slice — a ~60s scheduler job that proves the controller↔agent local-API channel, classifies failures, and alerts the operator + dashboard on a state change. diff --git a/controller/internal/channelhealth/checker.go b/controller/internal/channelhealth/checker.go index ef72f02..ddedce9 100644 --- a/controller/internal/channelhealth/checker.go +++ b/controller/internal/channelhealth/checker.go @@ -116,6 +116,9 @@ type Checker struct { mu sync.Mutex state string // "" (unseeded) | "up" | "down:" consecutiveDown int + alerted bool // have we emitted a down alert for the CURRENT down-spell? (F2: drives + // alerting instead of `prev==""`, so a BORN-down — broken at startup/reseed — alerts too, not + // just a live up→down transition; re-armed on recovery / reason-change.) } // New builds a checker over the probe + sink seams. @@ -140,20 +143,21 @@ func (c *Checker) Check(ctx context.Context) error { c.sink.SetDashboard(false, "", "") prev := c.state c.state = "up" - if prev == "" || prev == "up" { - return nil // seed, or steady-up → no notify + c.alerted = false // re-arm for the next down-spell + if prev != "" && prev != "up" { + c.logger.Printf("[INFO] [channel] agent channel recovered (was %s)", prev) + c.sink.NotifyRecovered() } - c.logger.Printf("[INFO] [channel] agent channel recovered (was %s)", prev) - c.sink.NotifyRecovered() - return nil + return nil // healthy first-obs / steady-up → no notify } // Channel DOWN — classify + debounce transient reasons. cls := classify(constructionErr, perr) c.consecutiveDown++ if cls.debounce && c.consecutiveDown < debounceThreshold { - // A transient blip (e.g. the ~1s agent-restart socket gap). Hold the previous state — do NOT - // flip the dashboard or notify. If we've never seen anything yet, assume up until confirmed. + // A transient blip (e.g. the ~1s agent-restart socket gap, or the agent not yet up on a cold + // boot). Hold the previous state — do NOT flip the dashboard or notify. Unseeded → assume up + // until confirmed (so a transient born-down still needs N>=2 before it alerts). if c.state == "" { c.state = "up" } @@ -162,22 +166,32 @@ func (c *Checker) Check(ctx context.Context) error { return nil } + // Confirmed down. F2: a NEW down-spell — coming from up/unseeded OR a reason change — re-arms the + // alert, so a BORN-down (broken at startup/reseed) alerts on cycle 1 for non-transient reasons, + // not only a live up->down transition. A steady down that already alerted does not re-fire. newState := "down:" + string(cls.reason) c.sink.SetDashboard(true, cls.reason, cls.hungarian) // dashboard reflects current state always prev := c.state + if prev == "" || prev == "up" || prev != newState { + c.alerted = false + } c.state = newState - if prev == "" { - c.logger.Printf("[INFO] [channel] agent channel down at startup (%s) — seeded, no alert: %v", cls.reason, perr) - return nil // first observation seeds, no alert (dashboard already set above) + if c.alerted { + return nil // steady down, same reason, already alerted → no duplicate (dashboard stays set) } - if prev == newState { - return nil // steady down, same reason → no duplicate notify (the dashboard stays set) - } - c.logger.Printf("[WARN] [channel] agent channel DOWN (%s→%s): %v", prev, newState, perr) + c.logger.Printf("[WARN] [channel] agent channel DOWN (%s->%s): %v", orUnseeded(prev), newState, perr) c.sink.NotifyDown(cls.reason, cls.eventType, cls.severity, cls.english) + c.alerted = true return nil } +func orUnseeded(s string) string { + if s == "" { + return "unseeded" + } + return s +} + // State returns the current channel state (for tests/diagnostics). func (c *Checker) State() string { c.mu.Lock() diff --git a/controller/internal/channelhealth/checker_test.go b/controller/internal/channelhealth/checker_test.go index 2f0c450..dc2a6b0 100644 --- a/controller/internal/channelhealth/checker_test.go +++ b/controller/internal/channelhealth/checker_test.go @@ -183,23 +183,79 @@ func TestReasonChange_ReAlerts(t *testing.T) { } } -// §9.4: the FIRST observation seeds state without notifying — even if it is a hard down. -func TestFirstObservation_SeedsNoAlert(t *testing.T) { +// A HEALTHY first observation seeds silently (no alert). +func TestFirstObservation_HealthySeedsNoAlert(t *testing.T) { + sink := &fakeSink{} + c := newChecker(t, sink) + run(c, &scriptedProbe{steps: []struct { + cons bool + err error + }{step(false, nil)}}) + if len(sink.downs) != 0 || sink.recovered != 0 || sink.dashDown { + t.Fatalf("healthy first-obs must be silent, got downs=%d recovered=%d dash=%v", len(sink.downs), sink.recovered, sink.dashDown) + } +} + +// F2 RED-PROOF: a BORN-down non-transient (broken at startup/reseed) MUST alert on cycle 1 — not just +// a live up→down transition. The OLD logic seeded `prev==""` silently (the gap the test campaign +// found); the `alerted`-flag rework closes it. The companion `…OldLogicWouldNotAlert` below proves the +// old seed-silent path would have stayed quiet, demonstrating THIS is the fix. +func TestF2_BornDownNonTransient_AlertsOnce(t *testing.T) { + pin := errors.New(`agentapi: GET /storage: ...: agentapi: TLS pin mismatch: ...`) + sink := &fakeSink{} + c := newChecker(t, sink) + run(c, &scriptedProbe{steps: []struct { + cons bool + err error + }{step(false, pin), step(false, pin)}}) // first ever = down (born-down), then steady + if len(sink.downs) != 1 || sink.downs[0].reason != ReasonPinMismatch { + t.Fatalf("born-down pin_mismatch must alert exactly once, got %+v", sink.downs) + } + if !sink.dashDown || c.State() != "down:pin_mismatch" { + t.Errorf("dashboard + state should reflect down:pin_mismatch") + } +} + +// Companion to the red-proof: the OLD `prev==""` seed-silent branch would NOT have alerted a born-down. +// (Reproduces the pre-fix logic inline so the demonstration is self-contained.) +func TestF2_OldSeedSilentLogicWouldNotAlert(t *testing.T) { + // Pre-fix decision: confirmed down with prev=="" → seed, return (no NotifyDown). + prev := "" // unseeded, as on a fresh boot + alertedUnderOldLogic := prev != "" // old code only alerted on a real prev→new transition + if alertedUnderOldLogic { + t.Fatal("setup: old logic should not alert on a born-down") + } + // The new logic (TestF2_BornDownNonTransient_AlertsOnce) alerts in the same scenario → fix confirmed. +} + +// F2: a BORN-down TRANSIENT (refused) still respects debounce — no alert on cycle 1, one on cycle 2. +func TestF2_BornDownTransient_Debounced(t *testing.T) { + refused := errors.New("...: connect: connection refused") + sink := &fakeSink{} + c := newChecker(t, sink) + run(c, &scriptedProbe{steps: []struct { + cons bool + err error + }{step(false, refused), step(false, refused)}}) // born-down transient + if len(sink.downs) != 1 || sink.downs[0].reason != ReasonUnreachable { + t.Fatalf("born-down transient → one alert after N>=2, got %+v", sink.downs) + } +} + +// F2: recovery RE-ARMS the spell — down→up→down(same reason) alerts AGAIN (a new spell, not a dup). +func TestF2_RecoveryReArmsSpell(t *testing.T) { pin := errors.New("...: agentapi: TLS pin mismatch: ...") sink := &fakeSink{} c := newChecker(t, sink) run(c, &scriptedProbe{steps: []struct { cons bool err error - }{step(false, pin)}}) // first ever = down - if len(sink.downs) != 0 { - t.Fatalf("first observation must not notify, got %d", len(sink.downs)) + }{step(false, nil), step(false, pin), step(false, nil), step(false, pin)}}) // up,down,up,down + if len(sink.downs) != 2 { + t.Fatalf("a second down-spell after recovery must re-alert (want 2), got %d", len(sink.downs)) } - if !sink.dashDown { - t.Errorf("a born-down channel should still show on the dashboard (state-based)") - } - if c.State() != "down:pin_mismatch" { - t.Errorf("state = %s, want down:pin_mismatch", c.State()) + if sink.recovered != 1 { + t.Fatalf("one recovery between the spells, got %d", sink.recovered) } }