3f37c5fc23
staleLockController.Guests() = ListLXC ∩ GET /pools/felhom members (ownership PROVEN via the pool registry, never assumed from enumeration scope); pool-read failure fail-safes the whole recovery through the existing guest-list guard. New Client.Pool read (needs Pool.Audit — host-install v1.9.0; Pool.Allocate does NOT satisfy it, spike T2). Composed pve:pool-read capability (non-critical) + --selftest pool-read line. Red-proofed negative tests drive the REAL controller over a broad-token-shaped fake. Per SPIKE-a1-pool-membership-read-2026-07-03.md; audit A1 (AUDIT-blast-radius-hostroot-localapi-2026-07-02). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PSK5g6qYLknKj8u3QAFEr6
290 lines
12 KiB
Go
290 lines
12 KiB
Go
package localapi
|
|
|
|
import (
|
|
"context"
|
|
"fmt"
|
|
"log/slog"
|
|
"strconv"
|
|
|
|
"gitea.dooplex.hu/admin/felhom-agent/internal/proxmox"
|
|
)
|
|
|
|
// Stale-lock recovery (TESTRUN finding F2-b). A host reboot DURING a vzdump backup leaves the guest
|
|
// with a backup lock (`lock: snapshot-delete` after an interrupted snapshot-mode backup, or `lock:
|
|
// backup`) and possibly a dangling `vzdump` snapshot. With `onboot: 1`, pve-guests then FAILS to start
|
|
// the locked CT ("CT is locked (snapshot-delete)") — the customer box stays DOWN until a human runs
|
|
// `pct unlock`. This makes the agent self-heal it at startup.
|
|
//
|
|
// The load-bearing invariant (B.0): at AGENT STARTUP the agent's own backup loop has not run yet, so a
|
|
// backup lock present then is STALE BY DEFINITION — UNLESS a vzdump is genuinely in-flight (an external
|
|
// backup, or one that outlived a bare `systemctl restart felhom-agent`). The recovery therefore clears a
|
|
// lock ONLY after confirming no vzdump task is running for that guest. If that confirmation can't be
|
|
// made, it FAILS SAFE (leaves the lock) — a wrongly-cleared live backup would corrupt state.
|
|
//
|
|
// Scope is deliberately narrow: only the two vzdump-related locks are ever touched. migrate/disk/create/
|
|
// rollback/mounted/snapshot locks are left alone (they belong to a different operation, mid-flight or
|
|
// needing different handling). The recovery is idempotent and never touches a guest without a stale lock.
|
|
//
|
|
// Ownership (audit A1, v0.62.0): the scan covers ONLY felhom-pool members — the production controller
|
|
// intersects ListLXC with GET /pools/{pool} membership, so even under a broad token the reaper can
|
|
// never unlock/start a co-tenant's guest. A failed pool read fails safe (whole recovery skipped).
|
|
|
|
// staleBackupLocks are the lock values an interrupted vzdump can leave. ONLY these are cleared.
|
|
var staleBackupLocks = map[string]bool{
|
|
"backup": true, // interrupted suspend/stop-mode backup
|
|
"snapshot-delete": true, // interrupted snapshot-mode backup's cleanup (the F2-b repro)
|
|
}
|
|
|
|
// vzdumpSnapshotName is the snapshot a vzdump snapshot-mode backup creates then deletes; an interrupted
|
|
// one leaves it dangling under exactly this name.
|
|
const vzdumpSnapshotName = "vzdump"
|
|
|
|
// StaleLockController is the seam the recovery composes over. Reads go through the Proxmox API; the
|
|
// unlock is the one op with no API equivalent (`pct unlock`, the fenced root-CLI runner). Satisfied in
|
|
// production by *staleLockController (over *proxmox.Client + a proxmox.Runner).
|
|
type StaleLockController interface {
|
|
// Guests lists the node's guests (vmid + running status).
|
|
Guests(ctx context.Context) ([]proxmox.Guest, error)
|
|
// Lock returns the guest's current lock ("" if unlocked) and its onboot flag.
|
|
Lock(ctx context.Context, vmid int) (lock string, onboot bool, err error)
|
|
// BackupRunning reports whether a vzdump task is genuinely in-flight for vmid (the invariant guard).
|
|
BackupRunning(ctx context.Context, vmid int) (bool, error)
|
|
// HasVzdumpSnapshot reports whether a dangling `vzdump` snapshot exists for vmid.
|
|
HasVzdumpSnapshot(ctx context.Context, vmid int) (bool, error)
|
|
// Unlock clears the guest's lock (`pct unlock` — no API equivalent).
|
|
Unlock(ctx context.Context, vmid int) error
|
|
// DeleteVzdumpSnapshot removes the dangling `vzdump` snapshot (API + WaitTask).
|
|
DeleteVzdumpSnapshot(ctx context.Context, vmid int) error
|
|
// Start starts the guest (API + WaitTask).
|
|
Start(ctx context.Context, vmid int) error
|
|
}
|
|
|
|
// RecoverStaleLockedGuests scans every guest at startup and clears a stale vzdump lock (unlock →
|
|
// delete the dangling snapshot → start iff onboot). No-op when the controller is not wired.
|
|
func (s *Server) RecoverStaleLockedGuests(ctx context.Context) {
|
|
if s.staleLock == nil {
|
|
return
|
|
}
|
|
guests, err := s.staleLock.Guests(ctx)
|
|
if err != nil {
|
|
s.logger.Warn("stale-lock: guest list unavailable — skipping recovery", "err", err)
|
|
return
|
|
}
|
|
for _, g := range guests {
|
|
s.recoverOneStaleLock(ctx, g)
|
|
}
|
|
}
|
|
|
|
// recoverOneStaleLock applies the recovery to a single guest. It acts ONLY on a stale backup lock and
|
|
// only when no backup is in-flight; every branch is logged so an operator can see what was (or wasn't)
|
|
// cleared.
|
|
func (s *Server) recoverOneStaleLock(ctx context.Context, g proxmox.Guest) {
|
|
lock, onboot, err := s.staleLock.Lock(ctx, g.VMID)
|
|
if err != nil {
|
|
s.logger.Warn("stale-lock: read guest config failed — skipping", "vmid", g.VMID, "err", err)
|
|
return
|
|
}
|
|
if !staleBackupLocks[lock] {
|
|
return // unlocked, or a non-backup lock we deliberately leave alone (the overwhelming common case)
|
|
}
|
|
|
|
// INVARIANT GUARD: a backup lock is only STALE when no vzdump is genuinely running. Confirm before
|
|
// clearing; on any doubt, FAIL SAFE and leave the lock (clearing a live backup's lock corrupts it).
|
|
running, err := s.staleLock.BackupRunning(ctx, g.VMID)
|
|
if err != nil {
|
|
s.logger.Warn("stale-lock: could not confirm no backup is running — NOT clearing (fail-safe)",
|
|
"vmid", g.VMID, "lock", lock, "err", err)
|
|
return
|
|
}
|
|
if running {
|
|
s.logger.Warn("stale-lock: a vzdump backup is genuinely in-flight — leaving the lock (NOT stale)",
|
|
"vmid", g.VMID, "lock", lock)
|
|
return
|
|
}
|
|
|
|
s.logger.Warn("stale-lock: clearing a stale backup lock left by an interrupted backup",
|
|
"vmid", g.VMID, "lock", lock, "onboot", onboot, "status", g.Status)
|
|
if err := s.staleLock.Unlock(ctx, g.VMID); err != nil {
|
|
s.logger.Error("stale-lock: pct unlock failed", "vmid", g.VMID, "err", err)
|
|
return
|
|
}
|
|
|
|
// Remove the dangling vzdump snapshot, if any (an interrupted snapshot-mode backup leaves it). Done
|
|
// only when one actually exists, so a stop/suspend-mode interruption (no snapshot) doesn't no-op-fail.
|
|
has, err := s.staleLock.HasVzdumpSnapshot(ctx, g.VMID)
|
|
if err != nil {
|
|
s.logger.Warn("stale-lock: snapshot list failed — skipping snapshot cleanup", "vmid", g.VMID, "err", err)
|
|
} else if has {
|
|
if err := s.staleLock.DeleteVzdumpSnapshot(ctx, g.VMID); err != nil {
|
|
s.logger.Error("stale-lock: delete dangling vzdump snapshot failed", "vmid", g.VMID, "err", err)
|
|
} else {
|
|
s.logger.Info("stale-lock: removed dangling vzdump snapshot", "vmid", g.VMID)
|
|
}
|
|
}
|
|
|
|
// Start ONLY a guest that is configured to auto-start AND is not already running. A deliberately-
|
|
// stopped guest (onboot:0, e.g. the golden) is unlocked but never started; an already-running guest
|
|
// (a bare agent restart found it up) is left as-is.
|
|
if onboot && g.Status != "running" {
|
|
if err := s.staleLock.Start(ctx, g.VMID); err != nil {
|
|
s.logger.Error("stale-lock: start after unlock failed", "vmid", g.VMID, "err", err)
|
|
return
|
|
}
|
|
s.logger.Warn("stale-lock: started CT after clearing the stale lock (onboot)", "vmid", g.VMID)
|
|
}
|
|
}
|
|
|
|
// staleLockController is the production StaleLockController over the Proxmox API client + the fenced
|
|
// root-CLI runner. Reads (guest list, config, snapshots, running tasks), snapshot-delete and start go
|
|
// through the API (token-authed, WaitTask-asserted); only `pct unlock` shells out (no API equivalent).
|
|
type staleLockController struct {
|
|
px staleLockAPI
|
|
runner proxmox.Runner
|
|
pool string // ownership registry: only members of this PVE pool are ever scanned (A1)
|
|
logger *slog.Logger // the one success-path scan-summary line; nil = silent
|
|
}
|
|
|
|
// staleLockAPI is the subset of *proxmox.Client the controller uses (kept narrow for clarity/testing).
|
|
type staleLockAPI interface {
|
|
ListLXC(ctx context.Context) ([]proxmox.Guest, error)
|
|
Pool(ctx context.Context, name string) (proxmox.PoolInfo, error)
|
|
GuestConfig(ctx context.Context, vmid int) (proxmox.GuestConfig, error)
|
|
ListSnapshots(ctx context.Context, vmid int) ([]proxmox.Snapshot, error)
|
|
ListRunningTasks(ctx context.Context) ([]proxmox.TaskStatus, error)
|
|
DeleteSnapshot(ctx context.Context, vmid int, snapname string) (string, error)
|
|
Start(ctx context.Context, vmid int) (string, error)
|
|
WaitTask(ctx context.Context, upid string, opts proxmox.WaitOptions) (proxmox.TaskStatus, error)
|
|
}
|
|
|
|
// NewStaleLockController builds the production controller. Returns nil if px or runner is nil (the
|
|
// feature then stays unwired and RecoverStaleLockedGuests is a no-op). pool names the PVE pool the
|
|
// scan is restricted to (reconcile.DefaultPool in production).
|
|
func NewStaleLockController(px staleLockAPI, runner proxmox.Runner, pool string, logger *slog.Logger) StaleLockController {
|
|
if px == nil || runner == nil {
|
|
return nil
|
|
}
|
|
return &staleLockController{px: px, runner: runner, pool: pool, logger: logger}
|
|
}
|
|
|
|
// Guests returns ListLXC ∩ the felhom pool's members (audit A1: ownership is PROVEN via the pool
|
|
// registry, never assumed from enumeration scope). Under the pool-scoped token the intersect is a
|
|
// no-op (ListLXC is already pool-filtered — spike T1); under a broad token it is the guard that
|
|
// keeps the reaper off co-tenant guests. A pool-read failure returns an error — the caller's
|
|
// existing "guest list unavailable — skipping recovery" guard then fail-safes the whole scan
|
|
// (unknown ownership ⇒ don't act, mirroring reconcile/recover.go's proof-of-launch gate). NEVER
|
|
// fall back to the unfiltered ListLXC list on error.
|
|
func (c *staleLockController) Guests(ctx context.Context) ([]proxmox.Guest, error) {
|
|
lxc, err := c.px.ListLXC(ctx)
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
pool, err := c.px.Pool(ctx, c.pool)
|
|
if err != nil {
|
|
return nil, fmt.Errorf("pool membership read (pool=%s): %w", c.pool, err)
|
|
}
|
|
// A pool can hold storages too (type "storage", no vmid) — membership is nonzero-vmid guests only.
|
|
members := make(map[int]bool, len(pool.Members))
|
|
for _, m := range pool.Members {
|
|
if m.VMID != 0 && m.Type != "storage" {
|
|
members[m.VMID] = true
|
|
}
|
|
}
|
|
owned := lxc[:0:0]
|
|
for _, g := range lxc {
|
|
if members[g.VMID] {
|
|
owned = append(owned, g)
|
|
}
|
|
}
|
|
if c.logger != nil {
|
|
c.logger.Info("stale-lock: scanning pool guests", "pool", c.pool, "listed", len(lxc), "scanned", len(owned))
|
|
}
|
|
return owned, nil
|
|
}
|
|
|
|
func (c *staleLockController) Lock(ctx context.Context, vmid int) (string, bool, error) {
|
|
cfg, err := c.px.GuestConfig(ctx, vmid)
|
|
if err != nil {
|
|
return "", false, err
|
|
}
|
|
return cfg.Lock(), cfg.OnBoot(), nil
|
|
}
|
|
|
|
func (c *staleLockController) BackupRunning(ctx context.Context, vmid int) (bool, error) {
|
|
tasks, err := c.px.ListRunningTasks(ctx)
|
|
if err != nil {
|
|
return false, err
|
|
}
|
|
id := strconv.Itoa(vmid)
|
|
for _, t := range tasks {
|
|
// A single-guest vzdump task carries the vmid in its ID field.
|
|
if t.Type == "vzdump" && t.ID == id {
|
|
return true, nil
|
|
}
|
|
}
|
|
return false, nil
|
|
}
|
|
|
|
func (c *staleLockController) HasVzdumpSnapshot(ctx context.Context, vmid int) (bool, error) {
|
|
snaps, err := c.px.ListSnapshots(ctx, vmid)
|
|
if err != nil {
|
|
return false, err
|
|
}
|
|
for _, sn := range snaps {
|
|
if sn.Name == vzdumpSnapshotName {
|
|
return true, nil
|
|
}
|
|
}
|
|
return false, nil
|
|
}
|
|
|
|
func (c *staleLockController) Unlock(ctx context.Context, vmid int) error {
|
|
_, stderr, err := c.runner.Run(ctx, "pct", "unlock", strconv.Itoa(vmid))
|
|
if err != nil {
|
|
return &runnerError{op: "pct unlock", stderr: string(stderr), err: err}
|
|
}
|
|
return nil
|
|
}
|
|
|
|
func (c *staleLockController) DeleteVzdumpSnapshot(ctx context.Context, vmid int) error {
|
|
upid, err := c.px.DeleteSnapshot(ctx, vmid, vzdumpSnapshotName)
|
|
if err != nil {
|
|
return err
|
|
}
|
|
if upid == "" {
|
|
return nil // synchronous completion (no task to wait on)
|
|
}
|
|
_, err = c.px.WaitTask(ctx, upid, proxmox.WaitOptions{})
|
|
return err
|
|
}
|
|
|
|
func (c *staleLockController) Start(ctx context.Context, vmid int) error {
|
|
upid, err := c.px.Start(ctx, vmid)
|
|
if err != nil {
|
|
return err
|
|
}
|
|
if upid == "" {
|
|
return nil
|
|
}
|
|
// AllowWarnings: an unprivileged LXC start emits a "WARNINGS: 1" advisory (the systemd-nesting
|
|
// notice) even when the guest boots fine — same as the restore-test's start step (task.go). Without
|
|
// this the recovery false-logs a start error for a guest that is actually up (seen live on 9999).
|
|
_, err = c.px.WaitTask(ctx, upid, proxmox.WaitOptions{AllowWarnings: true})
|
|
return err
|
|
}
|
|
|
|
// runnerError wraps a fenced-runner failure with its stderr (the runner returns the two separately).
|
|
type runnerError struct {
|
|
op string
|
|
stderr string
|
|
err error
|
|
}
|
|
|
|
func (e *runnerError) Error() string {
|
|
if e.stderr != "" {
|
|
return e.op + ": " + e.err.Error() + ": " + e.stderr
|
|
}
|
|
return e.op + ": " + e.err.Error()
|
|
}
|
|
|
|
func (e *runnerError) Unwrap() error { return e.err }
|