agent v0.49.0: reboot-during-backup stale-lock recovery (F2-b) + shared-parent script redeploy fix (F2-a)
F2-b: at startup, recover a guest left with a stale vzdump lock by a reboot-during-backup — pct unlock -> delete dangling vzdump snapshot -> start iff onboot, guarded by a no-vzdump-running invariant (fail-safe). New internal/localapi/stalelock.go; proxmox GuestConfig.Lock()/OnBoot(), ListSnapshots, ListRunningTasks, Snapshot type. New narrow sudoers grant FELHOM_STALELOCK (pct unlock) + Critical capability stalelock-unlock. F2-a: EnsureSharedParent only redeployed the boot script when the UNIT differed, so the v0.36.6 make-private fix never reached hosts whose unit was current -> /mnt/felhom-drives stayed in root's shared:1 and doubled every drive bind. New sharedParentInstallStale compares BOTH script and unit. Boot-time-only; never churns the live mount. Both root causes confirmed live on felhom-pve before fixing. Green gate (build/vet/test) all pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0162BnMpUXscPsUB1cU8Tr6K
This commit is contained in:
@@ -97,4 +97,8 @@ var manifest = []Capability{
|
||||
{"controllerswap-inspect", "controller-swap / managed auto-update", "/usr/sbin/pct", []string{"exec", "9201", "--", "docker", "inspect", "-f", "{{.State.Running}}", "felhom-controller"}, true},
|
||||
{"controllerswap-restart", "controller-swap / managed auto-update", "/usr/sbin/pct", []string{"exec", "9201", "--", "systemctl", "restart", "felhom-controller-bootstrap.service"}, true},
|
||||
{"controllerswap-write", "controller-swap / managed auto-update", "/usr/sbin/pct", []string{"exec", "9201", "--", "tee", "/etc/felhom-controller-image"}, true},
|
||||
|
||||
// ---- Stale-lock recovery (FELHOM_STALELOCK, v0.49.0; Critical: a guest stuck behind a stale
|
||||
// reboot-during-backup lock can't start → the customer box stays DOWN until this clears it) ----
|
||||
{"stalelock-unlock", "reboot-during-backup stale-lock recovery", "/usr/sbin/pct", []string{"unlock", "9201"}, true},
|
||||
}
|
||||
|
||||
@@ -114,17 +114,35 @@ func (b *GuestBinder) EnsureSharedParent(ctx context.Context) error {
|
||||
return fmt.Errorf("shared-parent: make-shared: %w", err)
|
||||
}
|
||||
}
|
||||
// Install the boot-persistence unit only when missing OR its content differs from what we ship (so a
|
||||
// unit-template fix deploys) — EnsureSharedParent runs on a periodic reconcile, and re-writing files +
|
||||
// daemon-reload every tick would be wasteful, so the common case (unchanged) is a cheap read.
|
||||
if cur, err := os.ReadFile(sharedParentUnitPath); err != nil || string(cur) != sharedParentUnit {
|
||||
// (Re)install the boot-persistence files only when the on-disk SCRIPT or UNIT differs from what we
|
||||
// ship (or is missing) — EnsureSharedParent runs on a periodic reconcile, so re-writing files +
|
||||
// daemon-reload every tick would be wasteful; the common case (both current) is two cheap reads.
|
||||
//
|
||||
// F2-a: this MUST compare the SCRIPT too, not just the unit. The v0.36.6 make-private fix changed
|
||||
// only the script (the unit was unchanged), so the earlier unit-only gate never redeployed it —
|
||||
// leaving hosts running the pre-fix script (no make-private), whose self-bind stays in root's shared
|
||||
// peer group and DOUBLES every drive bind. Comparing both files closes that deploy gap.
|
||||
if sharedParentInstallStale(sharedParentUnitPath, sharedParentScriptPath) {
|
||||
if ierr := b.installSharedParentUnit(ctx); ierr != nil {
|
||||
b.logger.Warn("shared-parent: boot-persistence unit install failed (live setup OK; survives until host reboot)", "err", ierr)
|
||||
b.logger.Warn("shared-parent: boot-persistence (re)install failed (live setup OK; survives until host reboot)", "err", ierr)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// sharedParentInstallStale reports whether the on-disk boot script OR unit is missing or differs from
|
||||
// what this build ships — the trigger to (re)install both. Comparing BOTH (not the unit alone) is the
|
||||
// F2-a fix: a script-only change must still redeploy. Pure (path args) so it is unit-testable.
|
||||
func sharedParentInstallStale(unitPath, scriptPath string) bool {
|
||||
if cur, err := os.ReadFile(unitPath); err != nil || string(cur) != sharedParentUnit {
|
||||
return true
|
||||
}
|
||||
if cur, err := os.ReadFile(scriptPath); err != nil || string(cur) != sharedParentScript {
|
||||
return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// installSharedParentUnit writes the script + unit (from agent-written temps) and enables the unit so the
|
||||
// shared parent is re-established on every host boot before pve-guests. Idempotent.
|
||||
func (b *GuestBinder) installSharedParentUnit(ctx context.Context) error {
|
||||
|
||||
@@ -0,0 +1,77 @@
|
||||
package localapi
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestSharedParentInstallStale_ScriptOnlyChangeRedeploys is the F2-a regression proof: when the on-disk
|
||||
// UNIT is current but the SCRIPT differs (exactly the v0.36.6 make-private situation), the gate MUST
|
||||
// report stale so the corrected script redeploys. The earlier unit-only gate returned false here and
|
||||
// left hosts running the pre-fix script that doubled drive binds.
|
||||
func TestSharedParentInstallStale_ScriptOnlyChangeRedeploys(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
unitPath := filepath.Join(dir, "felhom-shared-parent.service")
|
||||
scriptPath := filepath.Join(dir, "felhom-shared-parent.sh")
|
||||
|
||||
// Unit current, script STALE (a pre-make-private body).
|
||||
if err := os.WriteFile(unitPath, []byte(sharedParentUnit), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
staleScript := "#!/bin/sh\nset -e\nmkdir -p /mnt/felhom-drives\nmount --make-shared /mnt/felhom-drives\n"
|
||||
if err := os.WriteFile(scriptPath, []byte(staleScript), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
if !sharedParentInstallStale(unitPath, scriptPath) {
|
||||
t.Fatal("a stale SCRIPT with a current unit must trigger reinstall (F2-a) — gate returned not-stale")
|
||||
}
|
||||
}
|
||||
|
||||
// TestSharedParentInstallStale_BothCurrentIsNoop: when both files match what we ship, the gate reports
|
||||
// not-stale (the cheap common case — no churn on every reconcile).
|
||||
func TestSharedParentInstallStale_BothCurrentIsNoop(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
unitPath := filepath.Join(dir, "felhom-shared-parent.service")
|
||||
scriptPath := filepath.Join(dir, "felhom-shared-parent.sh")
|
||||
if err := os.WriteFile(unitPath, []byte(sharedParentUnit), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(scriptPath, []byte(sharedParentScript), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
if sharedParentInstallStale(unitPath, scriptPath) {
|
||||
t.Fatal("both files current must be a no-op; gate reported stale")
|
||||
}
|
||||
}
|
||||
|
||||
// TestSharedParentInstallStale_MissingFilesAreStale: a missing unit or script (fresh host) is stale.
|
||||
func TestSharedParentInstallStale_MissingFilesAreStale(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
unitPath := filepath.Join(dir, "felhom-shared-parent.service")
|
||||
scriptPath := filepath.Join(dir, "felhom-shared-parent.sh")
|
||||
|
||||
if !sharedParentInstallStale(unitPath, scriptPath) {
|
||||
t.Fatal("missing files must be stale (fresh install)")
|
||||
}
|
||||
|
||||
// Unit present + current, script missing → still stale.
|
||||
if err := os.WriteFile(unitPath, []byte(sharedParentUnit), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if !sharedParentInstallStale(unitPath, scriptPath) {
|
||||
t.Fatal("a missing script with a current unit must be stale (F2-a)")
|
||||
}
|
||||
}
|
||||
|
||||
// TestSharedParentScript_HasMakePrivate is a content guard: the shipped boot script MUST contain the
|
||||
// make-private step (the actual fix for the doubling). If a refactor drops it, the parent re-joins
|
||||
// root's shared group on boot and binds double again.
|
||||
func TestSharedParentScript_HasMakePrivate(t *testing.T) {
|
||||
if want := "mount --make-private " + StableParentDir; !strings.Contains(sharedParentScript, want) {
|
||||
t.Fatalf("shipped boot script missing %q — the doubling fix would regress", want)
|
||||
}
|
||||
}
|
||||
@@ -87,6 +87,9 @@ type Options struct {
|
||||
// ControllerSwap runs guest commands (pct exec) for the agentic controller-update swap (Phase 1).
|
||||
// OPTIONAL — when nil, POST /controller/swap reports "not configured". Satisfied by *GuestBinder.
|
||||
ControllerSwap GuestExecutor
|
||||
// StaleLock recovers a guest left with a stale vzdump lock by a reboot-during-backup (F2-b), run at
|
||||
// startup by RecoverStaleLockedGuests. OPTIONAL — when nil, the recovery is a no-op.
|
||||
StaleLock StaleLockController
|
||||
// ControllerSwapStateDir holds the per-guest swap state file (crash-safety). "" → /var/lib/felhom-agent.
|
||||
ControllerSwapStateDir string
|
||||
// Intent records drive enroll/eject intent for the self-heal watchdog (slice 10 P3). OPTIONAL —
|
||||
@@ -158,6 +161,7 @@ type Server struct {
|
||||
intent IntentRecorder // slice 10 P3 (optional)
|
||||
guestBinds *GuestBindStore // F9 startup bind re-assert record (optional)
|
||||
formatJobs *FormatJobStore // F20-BUG3 detached-format job record (optional)
|
||||
staleLock StaleLockController // F2-b startup stale-lock recovery (optional)
|
||||
host storage.HostReader // role classification source (optional; defaults to ProcHostReader)
|
||||
|
||||
hostMetrics HostMetricsProvider // slice 9 (optional)
|
||||
@@ -224,6 +228,7 @@ func NewServer(o Options) (*Server, error) {
|
||||
intent: o.Intent,
|
||||
guestBinds: o.GuestBinds,
|
||||
formatJobs: o.FormatJobs,
|
||||
staleLock: o.StaleLock,
|
||||
host: o.HostReader,
|
||||
hostMetrics: o.HostMetrics,
|
||||
hostID: o.HostID,
|
||||
|
||||
@@ -0,0 +1,245 @@
|
||||
package localapi
|
||||
|
||||
import (
|
||||
"context"
|
||||
"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.
|
||||
|
||||
// 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
|
||||
}
|
||||
|
||||
// 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)
|
||||
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).
|
||||
func NewStaleLockController(px staleLockAPI, runner proxmox.Runner) StaleLockController {
|
||||
if px == nil || runner == nil {
|
||||
return nil
|
||||
}
|
||||
return &staleLockController{px: px, runner: runner}
|
||||
}
|
||||
|
||||
func (c *staleLockController) Guests(ctx context.Context) ([]proxmox.Guest, error) {
|
||||
return c.px.ListLXC(ctx)
|
||||
}
|
||||
|
||||
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
|
||||
}
|
||||
_, err = c.px.WaitTask(ctx, upid, proxmox.WaitOptions{})
|
||||
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 }
|
||||
@@ -0,0 +1,224 @@
|
||||
package localapi
|
||||
|
||||
import (
|
||||
"context"
|
||||
"io"
|
||||
"log/slog"
|
||||
"testing"
|
||||
|
||||
"gitea.dooplex.hu/admin/felhom-agent/internal/proxmox"
|
||||
)
|
||||
|
||||
// fakeStaleLock is a scripted StaleLockController: it answers the reads from per-vmid maps and records
|
||||
// the mutating calls (unlock/delsnapshot/start) so a test can assert the exact recovery sequence.
|
||||
type fakeStaleLock struct {
|
||||
guests []proxmox.Guest
|
||||
lock map[int]string
|
||||
onboot map[int]bool
|
||||
snap map[int]bool // a dangling vzdump snapshot exists
|
||||
running map[int]bool // a vzdump task is genuinely in-flight
|
||||
|
||||
unlocked []int
|
||||
delsnap []int
|
||||
started []int
|
||||
|
||||
lockErr map[int]error
|
||||
runningErr map[int]error
|
||||
}
|
||||
|
||||
func (f *fakeStaleLock) Guests(context.Context) ([]proxmox.Guest, error) { return f.guests, nil }
|
||||
|
||||
func (f *fakeStaleLock) Lock(_ context.Context, vmid int) (string, bool, error) {
|
||||
if err := f.lockErr[vmid]; err != nil {
|
||||
return "", false, err
|
||||
}
|
||||
return f.lock[vmid], f.onboot[vmid], nil
|
||||
}
|
||||
|
||||
func (f *fakeStaleLock) BackupRunning(_ context.Context, vmid int) (bool, error) {
|
||||
if err := f.runningErr[vmid]; err != nil {
|
||||
return false, err
|
||||
}
|
||||
return f.running[vmid], nil
|
||||
}
|
||||
|
||||
func (f *fakeStaleLock) HasVzdumpSnapshot(_ context.Context, vmid int) (bool, error) {
|
||||
return f.snap[vmid], nil
|
||||
}
|
||||
|
||||
func (f *fakeStaleLock) Unlock(_ context.Context, vmid int) error {
|
||||
f.unlocked = append(f.unlocked, vmid)
|
||||
return nil
|
||||
}
|
||||
|
||||
func (f *fakeStaleLock) DeleteVzdumpSnapshot(_ context.Context, vmid int) error {
|
||||
f.delsnap = append(f.delsnap, vmid)
|
||||
return nil
|
||||
}
|
||||
|
||||
func (f *fakeStaleLock) Start(_ context.Context, vmid int) error {
|
||||
f.started = append(f.started, vmid)
|
||||
return nil
|
||||
}
|
||||
|
||||
func staleLockServer(f *fakeStaleLock) *Server {
|
||||
return &Server{staleLock: f, logger: slog.New(slog.NewTextHandler(io.Discard, nil))}
|
||||
}
|
||||
|
||||
func contains(xs []int, v int) bool {
|
||||
for _, x := range xs {
|
||||
if x == v {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// TestStaleLock_RecoversStoppedLockedGuest is the F2-b core: a stopped, onboot guest with a
|
||||
// snapshot-delete lock + a dangling vzdump snapshot is unlocked, its snapshot deleted, and it is
|
||||
// started — the exact reboot-during-backup recovery.
|
||||
func TestStaleLock_RecoversStoppedLockedGuest(t *testing.T) {
|
||||
f := &fakeStaleLock{
|
||||
guests: []proxmox.Guest{{VMID: 9201, Status: "stopped"}},
|
||||
lock: map[int]string{9201: "snapshot-delete"},
|
||||
onboot: map[int]bool{9201: true},
|
||||
snap: map[int]bool{9201: true},
|
||||
running: map[int]bool{9201: false},
|
||||
}
|
||||
staleLockServer(f).RecoverStaleLockedGuests(context.Background())
|
||||
|
||||
if !contains(f.unlocked, 9201) {
|
||||
t.Fatalf("expected pct unlock for 9201, unlocked=%v", f.unlocked)
|
||||
}
|
||||
if !contains(f.delsnap, 9201) {
|
||||
t.Fatalf("expected dangling vzdump snapshot delete for 9201, delsnap=%v", f.delsnap)
|
||||
}
|
||||
if !contains(f.started, 9201) {
|
||||
t.Fatalf("expected start (onboot, stopped) for 9201, started=%v", f.started)
|
||||
}
|
||||
}
|
||||
|
||||
// TestStaleLock_NoLockTouchesNothing is the COMPANION proof: a healthy guest with no lock has NONE of
|
||||
// the mutating ops invoked — recovery acts only on the stale state.
|
||||
func TestStaleLock_NoLockTouchesNothing(t *testing.T) {
|
||||
f := &fakeStaleLock{
|
||||
guests: []proxmox.Guest{{VMID: 9201, Status: "running"}},
|
||||
lock: map[int]string{9201: ""}, // unlocked
|
||||
onboot: map[int]bool{9201: true},
|
||||
}
|
||||
staleLockServer(f).RecoverStaleLockedGuests(context.Background())
|
||||
|
||||
if len(f.unlocked)+len(f.delsnap)+len(f.started) != 0 {
|
||||
t.Fatalf("no-lock guest must be untouched; got unlocked=%v delsnap=%v started=%v", f.unlocked, f.delsnap, f.started)
|
||||
}
|
||||
}
|
||||
|
||||
// TestStaleLock_NonBackupLockLeftAlone: a non-vzdump lock (e.g. migrate) is NEVER cleared — scope is
|
||||
// strictly the two backup locks.
|
||||
func TestStaleLock_NonBackupLockLeftAlone(t *testing.T) {
|
||||
f := &fakeStaleLock{
|
||||
guests: []proxmox.Guest{{VMID: 9201, Status: "running"}},
|
||||
lock: map[int]string{9201: "migrate"},
|
||||
onboot: map[int]bool{9201: true},
|
||||
}
|
||||
staleLockServer(f).RecoverStaleLockedGuests(context.Background())
|
||||
|
||||
if len(f.unlocked) != 0 {
|
||||
t.Fatalf("a migrate lock must not be cleared; unlocked=%v", f.unlocked)
|
||||
}
|
||||
}
|
||||
|
||||
// TestStaleLock_OnbootZeroNotStarted: a stale-locked guest with onboot=0 (e.g. the golden) is unlocked
|
||||
// + snapshot-cleaned but NOT started — a deliberately-stopped guest stays stopped.
|
||||
func TestStaleLock_OnbootZeroNotStarted(t *testing.T) {
|
||||
f := &fakeStaleLock{
|
||||
guests: []proxmox.Guest{{VMID: 9100, Status: "stopped"}},
|
||||
lock: map[int]string{9100: "snapshot-delete"},
|
||||
onboot: map[int]bool{9100: false},
|
||||
snap: map[int]bool{9100: true},
|
||||
}
|
||||
staleLockServer(f).RecoverStaleLockedGuests(context.Background())
|
||||
|
||||
if !contains(f.unlocked, 9100) || !contains(f.delsnap, 9100) {
|
||||
t.Fatalf("onboot=0 guest should still be unlocked + snapshot-cleaned; unlocked=%v delsnap=%v", f.unlocked, f.delsnap)
|
||||
}
|
||||
if contains(f.started, 9100) {
|
||||
t.Fatalf("onboot=0 guest must NOT be started; started=%v", f.started)
|
||||
}
|
||||
}
|
||||
|
||||
// TestStaleLock_DelsnapOnlyWhenSnapshotExists: a stop/suspend-mode interruption (lock but NO dangling
|
||||
// snapshot) is unlocked + started, but delsnapshot is NOT called.
|
||||
func TestStaleLock_DelsnapOnlyWhenSnapshotExists(t *testing.T) {
|
||||
f := &fakeStaleLock{
|
||||
guests: []proxmox.Guest{{VMID: 9201, Status: "stopped"}},
|
||||
lock: map[int]string{9201: "backup"},
|
||||
onboot: map[int]bool{9201: true},
|
||||
snap: map[int]bool{9201: false}, // no dangling snapshot
|
||||
}
|
||||
staleLockServer(f).RecoverStaleLockedGuests(context.Background())
|
||||
|
||||
if !contains(f.unlocked, 9201) || !contains(f.started, 9201) {
|
||||
t.Fatalf("expected unlock + start; unlocked=%v started=%v", f.unlocked, f.started)
|
||||
}
|
||||
if len(f.delsnap) != 0 {
|
||||
t.Fatalf("delsnapshot must not run without a dangling snapshot; delsnap=%v", f.delsnap)
|
||||
}
|
||||
}
|
||||
|
||||
// TestStaleLock_RunningBackupNotCleared is the INVARIANT guard: a backup lock present WITH a genuinely
|
||||
// in-flight vzdump must NOT be cleared (clearing a live backup's lock corrupts it).
|
||||
func TestStaleLock_RunningBackupNotCleared(t *testing.T) {
|
||||
f := &fakeStaleLock{
|
||||
guests: []proxmox.Guest{{VMID: 9201, Status: "running"}},
|
||||
lock: map[int]string{9201: "backup"},
|
||||
onboot: map[int]bool{9201: true},
|
||||
running: map[int]bool{9201: true}, // a real backup is running
|
||||
}
|
||||
staleLockServer(f).RecoverStaleLockedGuests(context.Background())
|
||||
|
||||
if len(f.unlocked)+len(f.delsnap)+len(f.started) != 0 {
|
||||
t.Fatalf("a live backup's lock must be left alone; unlocked=%v delsnap=%v started=%v", f.unlocked, f.delsnap, f.started)
|
||||
}
|
||||
}
|
||||
|
||||
// TestStaleLock_RunningProbeErrorFailsSafe: if the no-backup-running confirmation errors, the lock is
|
||||
// LEFT (fail-safe) rather than blind-cleared.
|
||||
func TestStaleLock_RunningProbeErrorFailsSafe(t *testing.T) {
|
||||
f := &fakeStaleLock{
|
||||
guests: []proxmox.Guest{{VMID: 9201, Status: "stopped"}},
|
||||
lock: map[int]string{9201: "snapshot-delete"},
|
||||
onboot: map[int]bool{9201: true},
|
||||
runningErr: map[int]error{9201: context.DeadlineExceeded},
|
||||
}
|
||||
staleLockServer(f).RecoverStaleLockedGuests(context.Background())
|
||||
|
||||
if len(f.unlocked) != 0 {
|
||||
t.Fatalf("an unconfirmable backup state must fail safe (no unlock); unlocked=%v", f.unlocked)
|
||||
}
|
||||
}
|
||||
|
||||
// TestStaleLock_AlreadyRunningNotRestarted: a bare agent restart can find the guest UP behind a stale
|
||||
// lock — it is unlocked but NOT (re)started.
|
||||
func TestStaleLock_AlreadyRunningNotRestarted(t *testing.T) {
|
||||
f := &fakeStaleLock{
|
||||
guests: []proxmox.Guest{{VMID: 9201, Status: "running"}},
|
||||
lock: map[int]string{9201: "snapshot-delete"},
|
||||
onboot: map[int]bool{9201: true},
|
||||
snap: map[int]bool{9201: true},
|
||||
}
|
||||
staleLockServer(f).RecoverStaleLockedGuests(context.Background())
|
||||
|
||||
if !contains(f.unlocked, 9201) {
|
||||
t.Fatalf("a running but stale-locked guest should still be unlocked; unlocked=%v", f.unlocked)
|
||||
}
|
||||
if contains(f.started, 9201) {
|
||||
t.Fatalf("an already-running guest must NOT be started; started=%v", f.started)
|
||||
}
|
||||
}
|
||||
|
||||
// TestStaleLock_NilControllerNoop: an unwired controller is a safe no-op (the optional-feature contract).
|
||||
func TestStaleLock_NilControllerNoop(t *testing.T) {
|
||||
s := &Server{logger: slog.New(slog.NewTextHandler(io.Discard, nil))}
|
||||
s.RecoverStaleLockedGuests(context.Background()) // must not panic
|
||||
}
|
||||
@@ -54,6 +54,24 @@ func (c *Client) GuestConfig(ctx context.Context, vmid int) (GuestConfig, error)
|
||||
return cfg, c.get(ctx, path, &cfg)
|
||||
}
|
||||
|
||||
// ListSnapshots returns GET /nodes/{node}/lxc/{vmid}/snapshot (the guest's snapshots, including the
|
||||
// synthetic "current"). The startup stale-lock recovery uses it to find a dangling "vzdump" snapshot
|
||||
// left by an interrupted snapshot-mode backup.
|
||||
func (c *Client) ListSnapshots(ctx context.Context, vmid int) ([]Snapshot, error) {
|
||||
var ss []Snapshot
|
||||
path := fmt.Sprintf("/nodes/%s/lxc/%d/snapshot", c.node, vmid)
|
||||
return ss, c.get(ctx, path, &ss)
|
||||
}
|
||||
|
||||
// ListRunningTasks returns the node's currently-running tasks (GET /nodes/{node}/tasks?running=1).
|
||||
// The startup stale-lock recovery uses it as the load-bearing safety guard: a backup lock is cleared
|
||||
// ONLY when no vzdump task is genuinely in-flight for the guest (an agent restart while a real backup
|
||||
// runs must never clear the live lock).
|
||||
func (c *Client) ListRunningTasks(ctx context.Context) ([]TaskStatus, error) {
|
||||
var ts []TaskStatus
|
||||
return ts, c.get(ctx, "/nodes/"+c.node+"/tasks?running=1", &ts)
|
||||
}
|
||||
|
||||
// ListStorage returns GET /storage (cluster-wide storage definitions).
|
||||
func (c *Client) ListStorage(ctx context.Context) ([]Storage, error) {
|
||||
var ss []Storage
|
||||
|
||||
@@ -118,6 +118,35 @@ func (g *GuestConfig) Nets() map[string]string {
|
||||
return g.prefixed("net")
|
||||
}
|
||||
|
||||
// Lock returns the guest's current lock ("backup", "snapshot-delete", "migrate", …) from the config,
|
||||
// or "" when unlocked. An interrupted vzdump leaves a "backup" or "snapshot-delete" lock — the signal
|
||||
// the startup stale-lock recovery (F2-b) keys on.
|
||||
func (g *GuestConfig) Lock() string {
|
||||
raw, ok := g.Extra["lock"]
|
||||
if !ok {
|
||||
return ""
|
||||
}
|
||||
var s string
|
||||
if json.Unmarshal(raw, &s) != nil {
|
||||
return ""
|
||||
}
|
||||
return s
|
||||
}
|
||||
|
||||
// OnBoot reports whether the guest is configured to auto-start at host boot (onboot:1). PVE omits the
|
||||
// key when 0, so an absent key reads as false.
|
||||
func (g *GuestConfig) OnBoot() bool {
|
||||
raw, ok := g.Extra["onboot"]
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
var n int
|
||||
if json.Unmarshal(raw, &n) != nil {
|
||||
return false
|
||||
}
|
||||
return n == 1
|
||||
}
|
||||
|
||||
func (g *GuestConfig) prefixed(prefix string) map[string]string {
|
||||
out := map[string]string{}
|
||||
for k, raw := range g.Extra {
|
||||
@@ -136,6 +165,16 @@ func (g *GuestConfig) prefixed(prefix string) map[string]string {
|
||||
return out
|
||||
}
|
||||
|
||||
// Snapshot is one entry of GET /nodes/{node}/lxc/{vmid}/snapshot. The list always includes the
|
||||
// synthetic "current" pseudo-snapshot; named entries are real snapshots. An interrupted vzdump
|
||||
// snapshot-mode backup leaves a dangling snapshot named exactly "vzdump".
|
||||
type Snapshot struct {
|
||||
Name string `json:"name"`
|
||||
Description string `json:"description,omitempty"`
|
||||
SnapTime int64 `json:"snaptime,omitempty"`
|
||||
Parent string `json:"parent,omitempty"`
|
||||
}
|
||||
|
||||
// Storage is one entry of GET /storage (cluster) and GET /nodes/{node}/storage
|
||||
// (the latter adds usage fields). Unused fields stay zero.
|
||||
//
|
||||
|
||||
Reference in New Issue
Block a user