R-271: the channel recovery after a controller restart is reported
The agent_channel_unauthorized alert's remedy is a re-bootstrap (a restart), and an unseeded->up first observation was silent, so following the instruction guaranteed no recovery event. A down alert now leaves a marker in the data dir; the first UP after a restart sends the recovery and clears it. A restart with no alert outstanding stays silent. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS
This commit is contained in:
@@ -2002,7 +2002,8 @@ func main() {
|
|||||||
// launches the job immediately since the scheduler is already started.
|
// launches the job immediately since the scheduler is already started.
|
||||||
if cfg.LocalAPI.Endpoint != "" {
|
if cfg.LocalAPI.Endpoint != "" {
|
||||||
chSink := channelSink{notifier: notifier, alertMgr: alertMgr}
|
chSink := channelSink{notifier: notifier, alertMgr: alertMgr}
|
||||||
chChecker := channelhealth.New(webServer.ProbeAgentChannel, chSink, logger)
|
chChecker := channelhealth.New(webServer.ProbeAgentChannel, chSink, logger).
|
||||||
|
WithAlertMarker(filepath.Join(cfg.Paths.DataDir, "channelhealth-down-alerted")) // R-271
|
||||||
sched.Every("agent-channel-health", 60*time.Second, chChecker.Check)
|
sched.Every("agent-channel-health", 60*time.Second, chChecker.Check)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -18,6 +18,7 @@ package channelhealth
|
|||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"log"
|
"log"
|
||||||
|
"os"
|
||||||
"strings"
|
"strings"
|
||||||
"sync"
|
"sync"
|
||||||
"time"
|
"time"
|
||||||
@@ -121,7 +122,8 @@ type Checker struct {
|
|||||||
mu sync.Mutex
|
mu sync.Mutex
|
||||||
state string // "" (unseeded) | stateUnconfirmed (debounce placeholder) | "up" | "down:<reason>"
|
state string // "" (unseeded) | stateUnconfirmed (debounce placeholder) | "up" | "down:<reason>"
|
||||||
consecutiveDown int
|
consecutiveDown int
|
||||||
alerted bool // have we emitted a down alert for the CURRENT down-spell? (F2: drives
|
markerPath string // R-271: see WithAlertMarker ("" = no persistence)
|
||||||
|
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
|
// 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.)
|
// just a live up→down transition; re-armed on recovery / reason-change.)
|
||||||
}
|
}
|
||||||
@@ -131,6 +133,41 @@ func New(probe Probe, sink Sink, logger *log.Logger) *Checker {
|
|||||||
return &Checker{probe: probe, sink: sink, logger: logger}
|
return &Checker{probe: probe, sink: sink, logger: logger}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// WithAlertMarker makes ONE fact survive a controller restart: "a down alert went out and no recovery
|
||||||
|
// has been sent since" (R-271). Without it the operator's trail could never close — the 401 alert's
|
||||||
|
// own remedy is a re-bootstrap, i.e. a restart, which reset the state to unseeded, and an unseeded→up
|
||||||
|
// first observation is silent by design. With the marker, that first UP sends the recovery the
|
||||||
|
// operator is waiting for. The file holds the down state's name only (no secret). "" = disabled.
|
||||||
|
// Pinned by TestR271_RecoveryAfterRestartIsNotified.
|
||||||
|
func (c *Checker) WithAlertMarker(path string) *Checker {
|
||||||
|
c.markerPath = path
|
||||||
|
return c
|
||||||
|
}
|
||||||
|
|
||||||
|
func (c *Checker) markAlerted(state string) {
|
||||||
|
if c.markerPath == "" {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if err := os.WriteFile(c.markerPath, []byte(state+"\n"), 0o600); err != nil {
|
||||||
|
c.logger.Printf("[WARN] [channel] could not record the down alert for after a restart (%s): %v — a recovery after a restart will not be reported", c.markerPath, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// takeAlertMarker reports (and clears) a down alert recorded before this process started.
|
||||||
|
func (c *Checker) takeAlertMarker() (string, bool) {
|
||||||
|
if c.markerPath == "" {
|
||||||
|
return "", false
|
||||||
|
}
|
||||||
|
b, err := os.ReadFile(c.markerPath)
|
||||||
|
if err != nil {
|
||||||
|
return "", false
|
||||||
|
}
|
||||||
|
if rerr := os.Remove(c.markerPath); rerr != nil && !os.IsNotExist(rerr) {
|
||||||
|
c.logger.Printf("[WARN] [channel] could not clear the down-alert marker %s: %v", c.markerPath, rerr)
|
||||||
|
}
|
||||||
|
return strings.TrimSpace(string(b)), true
|
||||||
|
}
|
||||||
|
|
||||||
// Check runs one probe cycle: classify, debounce, reflect on the dashboard, and notify on a real
|
// Check runs one probe cycle: classify, debounce, reflect on the dashboard, and notify on a real
|
||||||
// transition. Best-effort + idempotent — safe to call on a timer. Never returns an error to the
|
// transition. Best-effort + idempotent — safe to call on a timer. Never returns an error to the
|
||||||
// scheduler (a probe failure IS the signal, not a job failure).
|
// scheduler (a probe failure IS the signal, not a job failure).
|
||||||
@@ -149,9 +186,16 @@ func (c *Checker) Check(ctx context.Context) error {
|
|||||||
prev := c.state
|
prev := c.state
|
||||||
c.state = "up"
|
c.state = "up"
|
||||||
c.alerted = false // re-arm for the next down-spell
|
c.alerted = false // re-arm for the next down-spell
|
||||||
|
was, marked := "", false
|
||||||
|
if prev != "up" {
|
||||||
|
was, marked = c.takeAlertMarker() // R-271: a down alert from before a restart
|
||||||
|
}
|
||||||
if prev != "" && prev != "up" {
|
if prev != "" && prev != "up" {
|
||||||
c.logger.Printf("[INFO] [channel] agent channel recovered (was %s)", prev)
|
c.logger.Printf("[INFO] [channel] agent channel recovered (was %s)", prev)
|
||||||
c.sink.NotifyRecovered()
|
c.sink.NotifyRecovered()
|
||||||
|
} else if marked {
|
||||||
|
c.logger.Printf("[INFO] [channel] agent channel recovered (was %s before the controller restarted — the down alert sent then is now closed)", was)
|
||||||
|
c.sink.NotifyRecovered()
|
||||||
}
|
}
|
||||||
return nil // healthy first-obs / steady-up → no notify
|
return nil // healthy first-obs / steady-up → no notify
|
||||||
}
|
}
|
||||||
@@ -197,6 +241,7 @@ func (c *Checker) Check(ctx context.Context) error {
|
|||||||
c.logger.Printf("[WARN] [channel] agent channel DOWN (%s->%s): %v", orUnseeded(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.sink.NotifyDown(cls.reason, cls.eventType, cls.severity, cls.english)
|
||||||
c.alerted = true
|
c.alerted = true
|
||||||
|
c.markAlerted(newState)
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,69 @@
|
|||||||
|
package channelhealth
|
||||||
|
|
||||||
|
import (
|
||||||
|
"errors"
|
||||||
|
"io"
|
||||||
|
"log"
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
// R-271: the 401 alert's remedy is a re-bootstrap (a controller restart), and an unseeded→up first
|
||||||
|
// observation is silent — so following the instruction guaranteed no recovery event. The consequence
|
||||||
|
// asserted across a simulated restart: a NEW checker over the same marker file notifies recovery on
|
||||||
|
// its first UP; a restart with no alert outstanding stays silent (the seeding rule is unchanged).
|
||||||
|
func TestR271_RecoveryAfterRestartIsNotified(t *testing.T) {
|
||||||
|
marker := filepath.Join(t.TempDir(), "channelhealth-down-alerted")
|
||||||
|
lg := log.New(io.Discard, "", 0)
|
||||||
|
|
||||||
|
// Process 1: the channel goes down with a 401 and the alert is sent.
|
||||||
|
s1 := &fakeSink{}
|
||||||
|
c1 := New(nil, s1, lg).WithAlertMarker(marker)
|
||||||
|
run(c1, &scriptedProbe{steps: []struct {
|
||||||
|
cons bool
|
||||||
|
err error
|
||||||
|
}{step(false, nil), step(false, errors.New("agentapi: GET /storage: HTTP 401"))}})
|
||||||
|
if len(s1.downs) != 1 {
|
||||||
|
t.Fatalf("setup: want one down alert, got %d", len(s1.downs))
|
||||||
|
}
|
||||||
|
|
||||||
|
// Restart (the remedy). Process 2 sees the channel UP on its first probe.
|
||||||
|
s2 := &fakeSink{}
|
||||||
|
c2 := New(nil, s2, lg).WithAlertMarker(marker)
|
||||||
|
run(c2, &scriptedProbe{steps: []struct {
|
||||||
|
cons bool
|
||||||
|
err error
|
||||||
|
}{step(false, nil), step(false, nil)}})
|
||||||
|
if s2.recovered != 1 {
|
||||||
|
t.Fatalf("R-271: the recovery after a restart must be notified exactly once, got %d", s2.recovered)
|
||||||
|
}
|
||||||
|
if _, err := os.Stat(marker); !os.IsNotExist(err) {
|
||||||
|
t.Errorf("the marker must be cleared once the recovery is sent (stat err=%v)", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Process 3: a restart with nothing outstanding stays silent (first observation seeds).
|
||||||
|
s3 := &fakeSink{}
|
||||||
|
c3 := New(nil, s3, lg).WithAlertMarker(marker)
|
||||||
|
run(c3, &scriptedProbe{steps: []struct {
|
||||||
|
cons bool
|
||||||
|
err error
|
||||||
|
}{step(false, nil)}})
|
||||||
|
if s3.recovered != 0 {
|
||||||
|
t.Errorf("a restart with no alert outstanding must not send a recovery, got %d", s3.recovered)
|
||||||
|
}
|
||||||
|
|
||||||
|
// A live recovery (no restart) clears the marker too, so a later restart is silent.
|
||||||
|
s4 := &fakeSink{}
|
||||||
|
c4 := New(nil, s4, lg).WithAlertMarker(marker)
|
||||||
|
run(c4, &scriptedProbe{steps: []struct {
|
||||||
|
cons bool
|
||||||
|
err error
|
||||||
|
}{step(false, nil), step(false, errors.New("agentapi: GET /storage: HTTP 401")), step(false, nil)}})
|
||||||
|
if s4.recovered != 1 {
|
||||||
|
t.Fatalf("live recovery: want 1, got %d", s4.recovered)
|
||||||
|
}
|
||||||
|
if _, err := os.Stat(marker); !os.IsNotExist(err) {
|
||||||
|
t.Errorf("a live recovery must clear the marker (stat err=%v)", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user