Files
felhom-agent/internal/localapi/stalelock.go
T
admin 3f37c5fc23 v0.62.0 — A1: pool-membership ownership check for the stale-lock reaper
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
2026-07-03 13:25:05 +02:00

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 }