Files
felhom-agent/internal/localapi/stalelock_test.go
T
admin 6e38e2f921 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
2026-06-30 09:00:32 +02:00

225 lines
7.9 KiB
Go

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
}