From 18d03bd437c3e2c8bbbd882fc92afa90524d7c4d Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Thu, 17 Sep 2026 10:26:42 +0200 Subject: [PATCH] v0.132.0: the slow crash-loop counter (R-539, operator ruling 3 of 2026-09-16) Beside the unchanged 3-in-15-minutes brake, a second counter: restarts the supervisor performed in the last 24 hours. At the fifth the heartbeat stanza sets slow_crashloop_since (moving at most once per 24 h), slow_crashloop and restarts_24h; hub v0.117.0 mints controller_slow_crashloop (warning, operator-only) when the timestamp moves. It never stops restarting. Persisted per guest (tmp+rename, 0600) so an agent restart or reboot does not reset it - unlike the fast record, whose reason for staying in memory (a persisted give-up outliving the fix) does not apply to a counter that only warns. Deliberate kills count. The startup line prints the new limits. Red-proofs seen failing: no counter; the once-per-24h guard removed ('the operator would be mailed per restart'); the save removed ('Restarts24h:1' after an agent restart). Negative control: restarts 7 h apart never raise it. go build/vet/test ./... green, 30 packages. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS --- CHANGELOG.md | 26 +++++ internal/hub/report.go | 7 ++ internal/localapi/controllersupervisor.go | 98 ++++++++++++++++- .../localapi/controllersupervisor_test.go | 101 +++++++++++++++++- 4 files changed, 229 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 196614c..83bbc33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,29 @@ +## Unreleased (to become v0.132.0) — a controller that dies slowly is reported, not just restarted (2026-09-17, R-539) + +**MinAgent impact:** none required by any controller. Hub **v0.117.0** turns the new fields into +`controller_slow_crashloop`; an older hub ignores them. + +- **R-539 (operator ruling 3 of 2026-09-16) — the slow crash-loop counter.** The 3-restarts-in-15-minutes + brake cannot see a controller that dies every 20 minutes (measured 2026-09-16, R-531: four restarts, + none accumulating, only an `info` event that mails nobody). Beside it, unchanged, a second counter: + restarts the supervisor performed in the last **24 hours**; at the **fifth**, the heartbeat's + `controller_supervisor` stanza sets `slow_crashloop_since` (and `slow_crashloop: true`, + `restarts_24h`). The hub mails on that timestamp MOVING; it moves **at most once per 24 hours**. It + does **not** stop restarting — the fast brake remains the only brake. +- **Persisted per guest** at `/var/lib/felhom-agent/guests//controller-restarts-24h.json` + (tmp + rename, 0600), so an agent restart or a host reboot does not reset it. The fast record stays + in memory; the reason it does (a persisted "give up" could outlive the fix) does not apply to a + counter that only warns. Unreadable or corrupt → WARN and a clean start, never a blocked supervisor. +- **Deliberate kills count.** The supervisor cannot tell an operator's `docker kill` from a crash + (measured 2026-09-15); a controller killed five times a day is worth a line either way. +- The supervisor's startup line now prints `slow_crashloop_max=5 slow_crashloop_window=24h0m0s`. + +**Red-proofs, each seen failing:** five restarts 20 minutes apart raise it +(`TestControllerSupervisor_SlowCrashloop` — fails with no counter; fails again with the once-per-24-hours +guard removed, "the operator would be mailed per restart"); the counter survives an agent restart +(`…SlowCounterSurvivesAgentRestart` — fails with the save removed, `Restarts24h:1`); restarts seven hours +apart never raise it (the negative control). Wire shape extended with `restarts_24h` and `slow_crashloop`. + ## v0.131.0 — a dead controller comes back by itself; the backup status speaks per tier (2026-09-15, R-523 / R-517 / R-518) > **RELEASED 2026-09-15** by `scripts/release-agent.sh` — tag `v0.131.0`, sha256 `1118b552f7e775fbde9544c7764ede7e6046e0a7db16ae8494d07a18e3c2ac9c`. Delivered to boxes by the controller v0.243.0 floor (declared MinAgent), not by hand. diff --git a/internal/hub/report.go b/internal/hub/report.go index 06f37b9..6320c92 100644 --- a/internal/hub/report.go +++ b/internal/hub/report.go @@ -150,6 +150,13 @@ type ControllerSupervisorGuest struct { Crashloop bool `json:"crashloop"` CrashloopSince string `json:"crashloop_since,omitempty"` // RFC3339; the last crash-loop, kept after it ends Parked bool `json:"parked"` + // R-539 (v0.132.0) — the SLOW crash loop. Restarts24h counts restarts the supervisor performed in + // the last 24 hours (persisted, so an agent restart does not reset it); SlowCrashloop is true while + // the last raise is under 24 hours old; SlowCrashloopSince is the raise itself, which the hub keys on + // MOVING (hub v0.117.0 controller_slow_crashloop). It moves at most once per 24 hours. + Restarts24h int `json:"restarts_24h"` + SlowCrashloop bool `json:"slow_crashloop"` + SlowCrashloopSince string `json:"slow_crashloop_since,omitempty"` // RFC3339 } // PBSDRStatus is the per-heartbeat PBS-DR-tier bridge state (slice 2). States: diff --git a/internal/localapi/controllersupervisor.go b/internal/localapi/controllersupervisor.go index f5b3dad..6649137 100644 --- a/internal/localapi/controllersupervisor.go +++ b/internal/localapi/controllersupervisor.go @@ -2,6 +2,7 @@ package localapi import ( "context" + "encoding/json" "os" "path/filepath" "sort" @@ -68,6 +69,20 @@ const ( ControllerParkedMarker = "controller-parked" defaultGuestsStateDir = "/var/lib/felhom-agent/guests" + + // R-539 (operator ruling 3 of 2026-09-16) — the SLOW crash loop. The 3-in-15-minutes brake above + // cannot see a controller that dies every 20 minutes: no two restarts share its window, so it is + // restarted for ever and the only trace is an info event that mails nobody (measured 2026-09-16, + // R-531). A second counter over 24 hours raises a WARNING at the fifth restart. It does NOT stop + // restarting — the fast brake stays the only brake, unchanged. Every restart the supervisor + // performs counts, including one that follows a deliberate operator `docker kill` (measured + // 2026-09-15: the supervisor cannot tell a kill from a crash, and a controller that is killed five + // times a day is worth a line to the operator either way). + controllerSlowCrashloopWindow = 24 * time.Hour + controllerSlowCrashloopMax = 5 + // controllerSlowCounterFile holds the 24-hour restart times and the last raise, per guest, beside + // the parked marker. + controllerSlowCounterFile = "controller-restarts-24h.json" ) // controllerSupState is one guest's supervisor record. In-memory on purpose (the guest-power @@ -81,6 +96,13 @@ type controllerSupState struct { lastReason string crashloopSince time.Time // zero = not in a crash-loop pause parked bool + + // R-539 — the slow counter. PERSISTED, unlike everything above, and the precedent's reason does not + // apply to it: persisting the fast record could carry a stale "give up" across the restart that + // fixed it, but this record never gives anything up — it only warns. Losing it on an agent restart, + // on the other hand, would hide exactly the box it exists for (one whose agent restarts too). + restarts24h []time.Time + slowCrashloopSince time.Time // the last raise; kept after it ages out, the hub keys on it MOVING } type controllerSupervisor struct { @@ -98,7 +120,9 @@ func (s *Server) WatchControllers(ctx context.Context) { } s.logger.Info("controller-supervisor: started", "interval", controllerSupervisorInterval.String(), "confirm_sweeps", controllerSupervisorConfirm, "crashloop_max", controllerCrashloopMax, - "crashloop_window", controllerCrashloopWindow.String(), "guests_dir", s.guestsStateDir()) + "crashloop_window", controllerCrashloopWindow.String(), + "slow_crashloop_max", controllerSlowCrashloopMax, "slow_crashloop_window", controllerSlowCrashloopWindow.String(), + "guests_dir", s.guestsStateDir()) t := time.NewTicker(controllerSupervisorInterval) defer t.Stop() for { @@ -139,11 +163,61 @@ func (s *Server) supState(vmid int) *controllerSupState { st := s.ctrlSup.guests[vmid] if st == nil { st = &controllerSupState{} + s.loadSlowCounter(vmid, st) s.ctrlSup.guests[vmid] = st } return st } +// slowCounterRecord is the on-disk shape of the R-539 counter. +type slowCounterRecord struct { + Restarts []time.Time `json:"restarts"` + SlowCrashloopSince time.Time `json:"slow_crashloop_since,omitempty"` +} + +func (s *Server) slowCounterPath(vmid int) string { + return filepath.Join(s.guestsStateDir(), strconv.Itoa(vmid), controllerSlowCounterFile) +} + +// loadSlowCounter restores the persisted counter into a fresh state. Absent = a clean start; unreadable +// or corrupt = a clean start with a WARN (a warning counter must never block supervision). +func (s *Server) loadSlowCounter(vmid int, st *controllerSupState) { + b, err := os.ReadFile(s.slowCounterPath(vmid)) + if err != nil { + if !os.IsNotExist(err) { + s.logger.Warn("controller-supervisor: slow counter unreadable — starting it from zero", "vmid", vmid, "err", err) + } + return + } + var rec slowCounterRecord + if err := json.Unmarshal(b, &rec); err != nil { + s.logger.Warn("controller-supervisor: slow counter corrupt — starting it from zero", "vmid", vmid, "err", err) + return + } + st.restarts24h = pruneBefore(rec.Restarts, s.clock().Add(-controllerSlowCrashloopWindow)) + st.slowCrashloopSince = rec.SlowCrashloopSince + if len(st.restarts24h) > 0 || !st.slowCrashloopSince.IsZero() { + s.logger.Info("controller-supervisor: slow counter restored from disk", "vmid", vmid, + "restarts_24h", len(st.restarts24h), "slow_crashloop_since", st.slowCrashloopSince.Format(time.RFC3339)) + } +} + +// saveSlowCounter writes the counter atomically (tmp + rename, 0600). A failure is logged and the +// in-memory counter carries on — the next restart retries the write. +func (s *Server) saveSlowCounter(vmid int, rec slowCounterRecord) { + path := s.slowCounterPath(vmid) + b, err := json.Marshal(rec) + if err == nil { + tmp := path + ".tmp" + if err = os.WriteFile(tmp, b, 0o600); err == nil { + err = os.Rename(tmp, path) + } + } + if err != nil { + s.logger.Warn("controller-supervisor: could not persist the slow counter (kept in memory)", "vmid", vmid, "path", path, "err", err) + } +} + // ControllerSupervisorTick performs one sweep. Exported so a test (and a live check) can drive one // cycle without waiting on the ticker. func (s *Server) ControllerSupervisorTick(ctx context.Context) { @@ -299,8 +373,22 @@ func (s *Server) superviseOneController(ctx context.Context, vmid int, guestStat st.lastRestartAt = now st.lastReason = reason st.notRunningSeen = 0 + // R-539: the slow counter. Raise at most once per 24 hours — the hub mails on the raise MOVING. + st.restarts24h = append(pruneBefore(st.restarts24h, now.Add(-controllerSlowCrashloopWindow)), now) + n24 := len(st.restarts24h) + raised := false + if n24 >= controllerSlowCrashloopMax && (st.slowCrashloopSince.IsZero() || now.Sub(st.slowCrashloopSince) >= controllerSlowCrashloopWindow) { + st.slowCrashloopSince = now + raised = true + } + rec := slowCounterRecord{Restarts: append([]time.Time(nil), st.restarts24h...), SlowCrashloopSince: st.slowCrashloopSince} s.ctrlSup.mu.Unlock() - s.logger.Warn("controller-supervisor: RESTARTED the controller", "vmid", vmid, "reason", reason) + s.saveSlowCounter(vmid, rec) + s.logger.Warn("controller-supervisor: RESTARTED the controller", "vmid", vmid, "reason", reason, "restarts_24h", n24) + if raised { + s.logger.Warn("controller-supervisor: SLOW CRASH-LOOP — the controller keeps dying; still restarting it, raising controller_slow_crashloop", + "vmid", vmid, "restarts_24h", n24, "window", controllerSlowCrashloopWindow.String(), "threshold", controllerSlowCrashloopMax) + } return false } @@ -352,6 +440,12 @@ func (s *Server) ControllerSupervisorStatus(_ context.Context) *hub.ControllerSu if !st.crashloopSince.IsZero() { g.CrashloopSince = st.crashloopSince.UTC().Format(time.RFC3339) } + now := s.clock() + g.Restarts24h = len(pruneBefore(append([]time.Time(nil), st.restarts24h...), now.Add(-controllerSlowCrashloopWindow))) + if !st.slowCrashloopSince.IsZero() { + g.SlowCrashloopSince = st.slowCrashloopSince.UTC().Format(time.RFC3339) + g.SlowCrashloop = now.Sub(st.slowCrashloopSince) < controllerSlowCrashloopWindow + } out.Guests = append(out.Guests, g) } sort.Slice(out.Guests, func(i, j int) bool { return out.Guests[i].VMID < out.Guests[j].VMID }) diff --git a/internal/localapi/controllersupervisor_test.go b/internal/localapi/controllersupervisor_test.go index 5c8ce06..9b857e7 100644 --- a/internal/localapi/controllersupervisor_test.go +++ b/internal/localapi/controllersupervisor_test.go @@ -243,9 +243,108 @@ func TestControllerSupervisorStanza_WireShape(t *testing.T) { t.Fatal(err) } g := m["guests"][0] - for _, k := range []string{"vmid", "restarts_total", "last_restart_at", "last_reason", "crashloop", "parked"} { + for _, k := range []string{"vmid", "restarts_total", "last_restart_at", "last_reason", "crashloop", "parked", "restarts_24h", "slow_crashloop"} { if _, ok := g[k]; !ok { t.Fatalf("stanza lacks %q — the hub keys on it: %s", k, b) } } } + + +// ---- R-539 (operator ruling 3 of 2026-09-16): the SLOW crash loop ---------------------------------- + +// supKillOnce kills the controller and lets the supervisor restart it (two confirming sweeps), then +// moves the clock on by gap. The container comes back "running", so each restart is a separate act. +func supKillOnce(t *testing.T, s *Server, ex *supExec, clk *supClock, gap time.Duration) { + t.Helper() + before := ex.count(9201) + ex.mu.Lock() + ex.status[9201] = "exited" + ex.mu.Unlock() + s.ControllerSupervisorTick(context.Background()) + clk.t = clk.t.Add(controllerSupervisorInterval) + s.ControllerSupervisorTick(context.Background()) + if ex.count(9201) != before+1 { + t.Fatalf("kill was not followed by exactly one restart (restarts %d → %d)", before, ex.count(9201)) + } + clk.t = clk.t.Add(gap) +} + +// The consequence: a controller that dies every 20 minutes — never three times inside the 15-minute +// brake — raises slow_crashloop on the FIFTH restart in 24 hours, and the raise does not move again on +// the sixth (the hub mails on movement; once per 24 hours is the ruling). +// +// RED-PROOF: without the slow counter the stanza never sets slow_crashloop → "five restarts 20 minutes +// apart did not raise slow_crashloop — this is R-539". +func TestControllerSupervisor_SlowCrashloop(t *testing.T) { + ex := &supExec{status: map[int]string{9201: "running"}, onRestart: "running"} + s, clk, _ := supServer(t, ex, runningGuest(9201), 9201) + ctx := context.Background() + + for i := 1; i <= 4; i++ { + supKillOnce(t, s, ex, clk, 20*time.Minute) + } + g := s.ControllerSupervisorStatus(ctx).Guests[0] + if g.Crashloop { + t.Fatalf("the 15-minute brake fired on restarts 20 minutes apart — the fixture is wrong: %+v", g) + } + if g.SlowCrashloop || g.SlowCrashloopSince != "" { + t.Fatalf("slow_crashloop raised after only 4 restarts: %+v", g) + } + supKillOnce(t, s, ex, clk, 20*time.Minute) + g = s.ControllerSupervisorStatus(ctx).Guests[0] + if !g.SlowCrashloop || g.SlowCrashloopSince == "" || g.Restarts24h != 5 { + t.Fatalf("five restarts 20 minutes apart did not raise slow_crashloop — this is R-539: %+v", g) + } + first := g.SlowCrashloopSince + supKillOnce(t, s, ex, clk, 20*time.Minute) + g = s.ControllerSupervisorStatus(ctx).Guests[0] + if g.SlowCrashloopSince != first { + t.Fatalf("the raise moved again on the 6th restart (%q → %q) — the operator would be mailed per restart", first, g.SlowCrashloopSince) + } + if !g.SlowCrashloop { + t.Fatalf("slow_crashloop cleared while the loop continues: %+v", g) + } +} + +// The negative control: restarts that never reach five inside any 24 hours never raise it. +func TestControllerSupervisor_SpreadRestartsNeverSlowCrashloop(t *testing.T) { + ex := &supExec{status: map[int]string{9201: "running"}, onRestart: "running"} + s, clk, _ := supServer(t, ex, runningGuest(9201), 9201) + for i := 0; i < 8; i++ { // eight restarts, 7 hours apart: at most 4 inside any 24 hours + supKillOnce(t, s, ex, clk, 7*time.Hour) + } + g := s.ControllerSupervisorStatus(context.Background()).Guests[0] + if g.SlowCrashloop || g.SlowCrashloopSince != "" { + t.Fatalf("restarts 7 hours apart raised slow_crashloop: %+v", g) + } + if g.Restarts24h > 4 { + t.Fatalf("restarts_24h=%d — the 24-hour window is not pruning", g.Restarts24h) + } +} + +// An agent restart must not reset the slow counter (the ruling; a box whose AGENT also restarts would +// otherwise never reach five). The same state directory, a fresh Server. +// +// RED-PROOF: keep the counter in memory only → the second Server starts at 0 → "the agent restart +// reset the slow counter". +func TestControllerSupervisor_SlowCounterSurvivesAgentRestart(t *testing.T) { + ex := &supExec{status: map[int]string{9201: "running"}, onRestart: "running"} + s, clk, dir := supServer(t, ex, runningGuest(9201), 9201) + for i := 0; i < 4; i++ { + supKillOnce(t, s, ex, clk, 20*time.Minute) + } + s2 := &Server{ + staleLock: s.staleLock, + guestExec: ex, + guestsDir: dir, + swapInFlight: map[int]bool{}, + logger: s.logger, + now: clk.now, + } + supKillOnce(t, s2, ex, clk, 20*time.Minute) + g := s2.ControllerSupervisorStatus(context.Background()).Guests[0] + if g.Restarts24h != 5 || !g.SlowCrashloop { + t.Fatalf("the agent restart reset the slow counter: %+v", g) + } +}