f27f7a2659
The v0.107.0 watchdog was silent on a healthy box, so its health could only be inferred from absence — F-OBS's shape, shipped in the same session F-OBS was fixed. INFO summary every 10th sweep with what it saw; aborted sweeps are not counted. Red-proofs 7 and 8.
271 lines
11 KiB
Go
271 lines
11 KiB
Go
package localapi
|
|
|
|
import (
|
|
"context"
|
|
"log/slog"
|
|
"sync"
|
|
"time"
|
|
|
|
"gitea.dooplex.hu/admin/felhom-agent/internal/proxmox"
|
|
)
|
|
|
|
// F-REBOOT (Campaign 8 fault 11) — a guest that should be running and is not.
|
|
//
|
|
// THE OUTAGE THIS EXISTS TO KILL. A `pct reboot` issued while a vzdump was in flight completed its
|
|
// SHUTDOWN half and never issued the start. The guest was found `stopped` with 0 containers, no
|
|
// lock, and nothing retrying; it stayed down 9m47s until a human ran `pct start`. The backup itself
|
|
// SUCCEEDED — so every alarm the appliance has was silent, because nothing was broken except that
|
|
// the customer's entire appliance was off.
|
|
//
|
|
// WHY THE EXISTING RECOVERY MISSED IT. `RecoverStaleLockedGuests` (stalelock.go) already does
|
|
// unlock → delete dangling snapshot → start iff onboot, and it is CORRECT. It missed this by two
|
|
// gaps, both narrow:
|
|
// - its predicate acts only on a guest holding a stale vzdump lock (`backup`/`snapshot-delete`);
|
|
// fault 11's guest was stopped and UNLOCKED, so it returned early;
|
|
// - it runs ONCE at agent startup, on the load-bearing invariant that a backup lock present then
|
|
// is stale by definition. A guest that goes down while the agent is already up is never
|
|
// re-examined.
|
|
//
|
|
// This watchdog closes exactly those two gaps and nothing more: it is periodic, and it acts on
|
|
// "should be running, is not, and is not locked".
|
|
//
|
|
// ── THE TRAP, WHICH IS THE SAME SHAPE AS F-CRIT-1's ──────────────────────────────────────────
|
|
//
|
|
// A guest the operator deliberately stopped must NOT be auto-started. Fighting the operator makes
|
|
// maintenance impossible and is worse than the outage — the same over-correction that F-CRIT-1's fix
|
|
// had to avoid when it stopped whitelisting StateStopped.
|
|
//
|
|
// The distinction used is `onboot`, and it is deliberately NOT invented here:
|
|
// - it is ALREADY the distinction stalelock.go uses for exactly this decision
|
|
// (`if onboot && g.Status != "running"`), so the two paths cannot disagree;
|
|
// - it is 1 on customer guests and 0 on scratch/golden guests (agent v0.101.0 sets scratch to 0);
|
|
// - it is the same flag `pve-guests` itself consults at host boot, so the agent AGREES WITH THE
|
|
// PLATFORM rather than maintaining a second, private definition of "should be running".
|
|
//
|
|
// The hub's desired-state `Run` (internal/desired) is a stronger signal and is wired, but it is
|
|
// hub-dependent. `onboot` keeps working on a box that has lost hub contact — which is precisely when
|
|
// an unattended appliance most needs to come back up.
|
|
|
|
const (
|
|
// guestPowerInterval is how often the watchdog looks. Matches the guestnet watchdog's cadence so
|
|
// the two guest-facing sweeps stay in step, and is far below the 9m47s outage the finding recorded.
|
|
guestPowerInterval = 60 * time.Second
|
|
|
|
// guestPowerMaxAttempts bounds the retry. A guest that will not start must not be started in a
|
|
// loop forever (Scenario C) — after this many failures the watchdog stops trying and raises it.
|
|
guestPowerMaxAttempts = 3
|
|
|
|
// guestPowerHeartbeatEvery emits a summary line every Nth sweep. 10 x 60s = 10 minutes, matching
|
|
// the controller's deadapp heartbeat.
|
|
//
|
|
// WHY THIS EXISTS, and it is a correction to this file's OWN first version (v0.107.0): the
|
|
// watchdog logged at startup and when it ACTED, and was otherwise silent. A silent watchdog is
|
|
// indistinguishable from a dead one — which is F-OBS, the very finding fixed in the same session
|
|
// this file shipped in, and it is what standing rule 3 exists to prevent. An operator needs a
|
|
// POSITIVE observable that the sweep is running; "no start lines" must not be the only evidence.
|
|
guestPowerHeartbeatEvery = 10
|
|
)
|
|
|
|
// guestPowerBackoff is the delay before each retry: 1m, 2m, 4m.
|
|
//
|
|
// Measured, not picked round: a healthy `pct start` of guest 9201 completed in ~25 s (observed twice
|
|
// on 2026-07-28), so even the first 1-minute wait carries 2.4x headroom over a normal start. Three
|
|
// attempts bound the disruption at roughly 7 minutes — inside the 9m47s outage this fixes — while
|
|
// never becoming an unbounded loop.
|
|
var guestPowerBackoff = []time.Duration{time.Minute, 2 * time.Minute, 4 * time.Minute}
|
|
|
|
// guestPowerState is one guest's recovery attempt record. In-memory on purpose, like the R-88
|
|
// breaker: an agent restart re-attempts immediately, which is the cheap direction to fail — a
|
|
// forgotten backoff costs one extra start attempt, whereas persisting it could carry a stale
|
|
// "this guest won't start" verdict across the restart that fixed it.
|
|
type guestPowerState struct {
|
|
attempts int
|
|
nextAt time.Time
|
|
raised bool // the give-up fault has already been raised for this run
|
|
}
|
|
|
|
// WatchGuestPower runs the guest-power sweep every guestPowerInterval until ctx is done. No-op when
|
|
// the stale-lock controller is not wired (it supplies the ownership-proven guest list).
|
|
func (s *Server) WatchGuestPower(ctx context.Context) {
|
|
if s.staleLock == nil {
|
|
return
|
|
}
|
|
s.logger.Info("guest-power: watchdog started", "interval", guestPowerInterval.String(),
|
|
"max_attempts", guestPowerMaxAttempts)
|
|
t := time.NewTicker(guestPowerInterval)
|
|
defer t.Stop()
|
|
for {
|
|
select {
|
|
case <-ctx.Done():
|
|
return
|
|
case <-t.C:
|
|
s.GuestPowerTick(ctx)
|
|
}
|
|
}
|
|
}
|
|
|
|
// GuestPowerTick performs one sweep. Exported so a test (and a live check) can drive exactly one
|
|
// cycle instead of waiting on the ticker.
|
|
func (s *Server) GuestPowerTick(ctx context.Context) {
|
|
if s.staleLock == nil {
|
|
return
|
|
}
|
|
guests, err := s.staleLock.Guests(ctx)
|
|
if err != nil {
|
|
// Unknown ownership ⇒ do nothing. Never fall back to an unfiltered list: starting a
|
|
// co-tenant's guest would be worse than leaving ours down.
|
|
s.logger.Warn("guest-power: guest list unavailable — skipping sweep (ownership unproven)", "err", err)
|
|
return
|
|
}
|
|
var stopped int
|
|
for _, g := range guests {
|
|
if ctx.Err() != nil {
|
|
return
|
|
}
|
|
if g.Status != "running" {
|
|
stopped++
|
|
}
|
|
s.recoverOneStoppedGuest(ctx, g)
|
|
}
|
|
|
|
s.guestPowerSweeps++
|
|
noteGuestPowerSweep(s.logger, s.guestPowerSweeps, len(guests), stopped)
|
|
}
|
|
|
|
// noteGuestPowerSweep emits the liveness observable every guestPowerHeartbeatEvery sweeps.
|
|
//
|
|
// It carries WHAT THE SWEEP SAW, not merely that it ran: a line saying "I am alive" cannot
|
|
// distinguish "alive, all guests up" from "alive, one guest down and being left alone on purpose",
|
|
// and the second is the state an operator needs to see. Pure and separately testable — the mistake
|
|
// being corrected here was untestable precisely because it lived inline.
|
|
func noteGuestPowerSweep(logger *slog.Logger, sweeps, evaluated, stopped int) {
|
|
if logger == nil || sweeps <= 0 || sweeps%guestPowerHeartbeatEvery != 0 {
|
|
return
|
|
}
|
|
logger.Info("guest-power: watchdog alive",
|
|
"sweeps_since_boot", sweeps, "guests_evaluated", evaluated, "currently_stopped", stopped)
|
|
}
|
|
|
|
// recoverOneStoppedGuest starts a single guest that should be running and is not.
|
|
func (s *Server) recoverOneStoppedGuest(ctx context.Context, g proxmox.Guest) {
|
|
if g.Status == "running" {
|
|
s.forgetGuestPower(g.VMID) // healthy again: clear any attempt history
|
|
return
|
|
}
|
|
|
|
lock, onboot, err := s.staleLock.Lock(ctx, g.VMID)
|
|
if err != nil {
|
|
s.logger.Warn("guest-power: read guest config failed — skipping", "vmid", g.VMID, "err", err)
|
|
return
|
|
}
|
|
|
|
// SCENARIO B — a deliberately stopped guest is left alone, forever. onboot:0 means the operator
|
|
// (or the golden-image provisioning) does not want this guest running.
|
|
if !onboot {
|
|
return
|
|
}
|
|
|
|
// A locked guest belongs to another operation, mid-flight or stale. The stale-lock recovery owns
|
|
// that case and knows how to prove a lock is stale; this watchdog must not race it or start a
|
|
// guest whose lock means "a restore is writing my disks right now".
|
|
if lock != "" {
|
|
s.logger.Info("guest-power: guest is stopped but LOCKED — leaving it to the stale-lock path",
|
|
"vmid", g.VMID, "lock", lock)
|
|
return
|
|
}
|
|
|
|
// Never start a guest while a vzdump is genuinely in flight for it — a stop-mode backup stops the
|
|
// guest ON PURPOSE and starting it underneath would corrupt the backup. Fail safe on doubt.
|
|
running, err := s.staleLock.BackupRunning(ctx, g.VMID)
|
|
if err != nil {
|
|
s.logger.Warn("guest-power: could not confirm no backup is running — NOT starting (fail-safe)",
|
|
"vmid", g.VMID, "err", err)
|
|
return
|
|
}
|
|
if running {
|
|
s.logger.Info("guest-power: a vzdump is in flight — leaving the guest stopped until it finishes",
|
|
"vmid", g.VMID)
|
|
return
|
|
}
|
|
|
|
st, due := s.guestPowerDue(g.VMID)
|
|
if !due {
|
|
return
|
|
}
|
|
if st.attempts >= guestPowerMaxAttempts {
|
|
// SCENARIO C — bounded. Raise it ONCE and stop retrying; an infinite silent retry loop is the
|
|
// over-correction here, and a guest that has refused three starts needs a human, not a fourth.
|
|
if !st.raised {
|
|
s.markGuestPowerRaised(g.VMID)
|
|
s.logger.Error("guest-power: GIVING UP — guest should be running (onboot) but failed to start after repeated attempts; it needs operator attention",
|
|
"vmid", g.VMID, "attempts", st.attempts, "status", g.Status)
|
|
}
|
|
return
|
|
}
|
|
|
|
s.logger.Warn("guest-power: guest should be running (onboot) but is stopped and unlocked — starting it",
|
|
"vmid", g.VMID, "status", g.Status, "attempt", st.attempts+1, "of", guestPowerMaxAttempts)
|
|
if err := s.staleLock.Start(ctx, g.VMID); err != nil {
|
|
s.noteGuestPowerFailure(g.VMID)
|
|
s.logger.Error("guest-power: start failed", "vmid", g.VMID, "attempt", st.attempts+1, "err", err)
|
|
return
|
|
}
|
|
s.forgetGuestPower(g.VMID)
|
|
s.logger.Warn("guest-power: STARTED a guest that should have been running", "vmid", g.VMID)
|
|
}
|
|
|
|
// ---- attempt bookkeeping (guarded by its own mutex; independent of the jobs lock) ----------
|
|
|
|
var guestPowerMu sync.Mutex
|
|
|
|
// guestPowerDue reports the guest's attempt state and whether a new attempt is due now.
|
|
func (s *Server) guestPowerDue(vmid int) (guestPowerState, bool) {
|
|
guestPowerMu.Lock()
|
|
defer guestPowerMu.Unlock()
|
|
if s.guestPower == nil {
|
|
s.guestPower = map[int]guestPowerState{}
|
|
}
|
|
st := s.guestPower[vmid]
|
|
if st.nextAt.IsZero() || !s.now().Before(st.nextAt) {
|
|
return st, true
|
|
}
|
|
return st, false
|
|
}
|
|
|
|
// noteGuestPowerFailure records a failed start and arms the next backoff.
|
|
func (s *Server) noteGuestPowerFailure(vmid int) {
|
|
guestPowerMu.Lock()
|
|
defer guestPowerMu.Unlock()
|
|
if s.guestPower == nil {
|
|
s.guestPower = map[int]guestPowerState{}
|
|
}
|
|
st := s.guestPower[vmid]
|
|
st.attempts++
|
|
i := st.attempts - 1
|
|
if i >= len(guestPowerBackoff) {
|
|
i = len(guestPowerBackoff) - 1
|
|
}
|
|
st.nextAt = s.now().Add(guestPowerBackoff[i])
|
|
s.guestPower[vmid] = st
|
|
}
|
|
|
|
// markGuestPowerRaised records that the give-up fault has been raised, so it is logged once.
|
|
func (s *Server) markGuestPowerRaised(vmid int) {
|
|
guestPowerMu.Lock()
|
|
defer guestPowerMu.Unlock()
|
|
st := s.guestPower[vmid]
|
|
st.raised = true
|
|
s.guestPower[vmid] = st
|
|
}
|
|
|
|
// forgetGuestPower clears a guest's attempt history — called when it is running again, so a guest
|
|
// that recovers does not carry its old failures into the next incident.
|
|
func (s *Server) forgetGuestPower(vmid int) {
|
|
guestPowerMu.Lock()
|
|
defer guestPowerMu.Unlock()
|
|
if s.guestPower == nil {
|
|
return
|
|
}
|
|
delete(s.guestPower, vmid)
|
|
}
|