admin
367a503a0f
F-REBOOT — a guest rebooted mid-backup never came back (fault 11: 9m47s of total appliance outage, no lock, nothing retrying). The existing stale-lock recovery is correct but missed it two ways: its predicate needs a stale vzdump lock and that guest was unlocked, and it runs only at agent startup. New periodic guest-power watchdog acts on 'should be running, is not, is not locked'. onboot is the should-be-running signal, not invented here: stalelock.go already uses it for this same decision, it is 0 on scratch/golden, and pve-guests uses it at host boot. Guards: onboot:0 never touched (Scenario B), a locked guest is left to the stale-lock path, a guest with a vzdump in flight is left stopped, unprovable ownership acts on nothing, unconfirmable backup state fails safe. Bounded retry 3x at 1/2/4m then ERROR (Scenario C) — a healthy start takes ~25s. F-LEAK — a failed restore-test could not destroy its scratch (403 VM.Allocate). It is pool membership, not privsep: VM.Allocate is granted at /pool/felhom only, and a failed restore never completes the --pool association. Fix needs NO new grant — Pool.Allocate is already held, so the teardown adopts the stranded scratch into the pool and retries the destroy. Guarded by scratchAdoptAllowed: scratch provenance AND the numeric band, both required (Scenario E). Six red-proofs across both fixes, all observed failing.
240 lines
8.7 KiB
Go
240 lines
8.7 KiB
Go
package localapi
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
"log/slog"
|
|
"testing"
|
|
"time"
|
|
|
|
"gitea.dooplex.hu/admin/felhom-agent/internal/proxmox"
|
|
)
|
|
|
|
// F-REBOOT (Campaign 8 fault 11): a guest rebooted mid-backup never came back — stopped, unlocked,
|
|
// nothing retrying, 9m47s of total appliance outage.
|
|
//
|
|
// Scenario A (a crashed guest is restarted), B (a deliberately stopped guest is left alone) and
|
|
// C (bounded retry, then escalate). B and C are what make A safe.
|
|
|
|
type fakeGuestPowerCtl struct {
|
|
guests []proxmox.Guest
|
|
guestsErr error
|
|
locks map[int]string // vmid -> lock ("" = unlocked)
|
|
onboot map[int]bool
|
|
backupRun map[int]bool
|
|
backupErr error
|
|
started []int
|
|
startErr error
|
|
}
|
|
|
|
func (f *fakeGuestPowerCtl) Guests(context.Context) ([]proxmox.Guest, error) {
|
|
return f.guests, f.guestsErr
|
|
}
|
|
func (f *fakeGuestPowerCtl) Lock(_ context.Context, vmid int) (string, bool, error) {
|
|
return f.locks[vmid], f.onboot[vmid], nil
|
|
}
|
|
func (f *fakeGuestPowerCtl) BackupRunning(_ context.Context, vmid int) (bool, error) {
|
|
return f.backupRun[vmid], f.backupErr
|
|
}
|
|
func (f *fakeGuestPowerCtl) HasVzdumpSnapshot(context.Context, int) (bool, error) { return false, nil }
|
|
func (f *fakeGuestPowerCtl) Unlock(context.Context, int) error { return nil }
|
|
func (f *fakeGuestPowerCtl) DeleteVzdumpSnapshot(context.Context, int) error { return nil }
|
|
func (f *fakeGuestPowerCtl) Start(_ context.Context, vmid int) error {
|
|
f.started = append(f.started, vmid)
|
|
return f.startErr
|
|
}
|
|
|
|
func gpServer(t *testing.T, ctl StaleLockController, now func() time.Time) *Server {
|
|
t.Helper()
|
|
s := &Server{staleLock: ctl, logger: slog.New(slog.NewTextHandler(discardW{}, nil))}
|
|
if now != nil {
|
|
s.now = now
|
|
} else {
|
|
s.now = func() time.Time { return time.Now().UTC() }
|
|
}
|
|
return s
|
|
}
|
|
|
|
type discardW struct{}
|
|
|
|
func (discardW) Write(p []byte) (int, error) { return len(p), nil }
|
|
|
|
// Scenario A — a guest that should be running (onboot) and is stopped-and-unlocked IS started.
|
|
//
|
|
// RED-PROOF: delete the `s.staleLock.Start(...)` call in recoverOneStoppedGuest (or make the whole
|
|
// function return before it) → started is empty and this fails with
|
|
// "guest 9201 was NOT started — this is F-REBOOT".
|
|
func TestGuestPower_StoppedOnbootGuestIsStarted(t *testing.T) {
|
|
ctl := &fakeGuestPowerCtl{
|
|
guests: []proxmox.Guest{{VMID: 9201, Status: "stopped"}},
|
|
locks: map[int]string{9201: ""},
|
|
onboot: map[int]bool{9201: true},
|
|
}
|
|
s := gpServer(t, ctl, nil)
|
|
|
|
s.GuestPowerTick(context.Background())
|
|
|
|
if len(ctl.started) != 1 || ctl.started[0] != 9201 {
|
|
t.Fatalf("guest 9201 was NOT started — this is F-REBOOT (started=%v)", ctl.started)
|
|
}
|
|
}
|
|
|
|
// Scenario B — a DELIBERATELY stopped guest (onboot:0) is never started. This is the trap: fighting
|
|
// the operator makes maintenance impossible and is worse than the outage being fixed.
|
|
//
|
|
// RED-PROOF: remove the `if !onboot { return }` guard → the golden/scratch guest is started and this
|
|
// fails with "a deliberately stopped guest (onboot:0) was started".
|
|
func TestGuestPower_DeliberatelyStoppedGuestIsLeftAlone(t *testing.T) {
|
|
ctl := &fakeGuestPowerCtl{
|
|
guests: []proxmox.Guest{{VMID: 9100, Status: "stopped"}, {VMID: 990000, Status: "stopped"}},
|
|
locks: map[int]string{9100: "", 990000: ""},
|
|
onboot: map[int]bool{9100: false, 990000: false}, // golden + scratch
|
|
}
|
|
s := gpServer(t, ctl, nil)
|
|
|
|
s.GuestPowerTick(context.Background())
|
|
|
|
if len(ctl.started) != 0 {
|
|
t.Errorf("a deliberately stopped guest (onboot:0) was started: %v", ctl.started)
|
|
}
|
|
}
|
|
|
|
// A LOCKED stopped guest belongs to the stale-lock path, which knows how to prove a lock is stale.
|
|
// This watchdog must not race it.
|
|
func TestGuestPower_LockedGuestIsLeftToTheStaleLockPath(t *testing.T) {
|
|
ctl := &fakeGuestPowerCtl{
|
|
guests: []proxmox.Guest{{VMID: 9201, Status: "stopped"}},
|
|
locks: map[int]string{9201: "snapshot-delete"},
|
|
onboot: map[int]bool{9201: true},
|
|
}
|
|
s := gpServer(t, ctl, nil)
|
|
s.GuestPowerTick(context.Background())
|
|
if len(ctl.started) != 0 {
|
|
t.Errorf("started a LOCKED guest: %v — that races the stale-lock recovery", ctl.started)
|
|
}
|
|
}
|
|
|
|
// A guest whose vzdump is genuinely in flight must be left stopped — a stop-mode backup stops the
|
|
// guest ON PURPOSE, and starting it underneath would corrupt the backup.
|
|
func TestGuestPower_InFlightBackupIsNotDisturbed(t *testing.T) {
|
|
ctl := &fakeGuestPowerCtl{
|
|
guests: []proxmox.Guest{{VMID: 9201, Status: "stopped"}},
|
|
locks: map[int]string{9201: ""},
|
|
onboot: map[int]bool{9201: true},
|
|
backupRun: map[int]bool{9201: true},
|
|
}
|
|
s := gpServer(t, ctl, nil)
|
|
s.GuestPowerTick(context.Background())
|
|
if len(ctl.started) != 0 {
|
|
t.Errorf("started a guest with a vzdump in flight: %v", ctl.started)
|
|
}
|
|
}
|
|
|
|
// Fail-safe: if we cannot confirm no backup is running, do NOT start.
|
|
func TestGuestPower_UnconfirmableBackupFailsSafe(t *testing.T) {
|
|
ctl := &fakeGuestPowerCtl{
|
|
guests: []proxmox.Guest{{VMID: 9201, Status: "stopped"}},
|
|
locks: map[int]string{9201: ""},
|
|
onboot: map[int]bool{9201: true},
|
|
backupErr: errors.New("task list unavailable"),
|
|
}
|
|
s := gpServer(t, ctl, nil)
|
|
s.GuestPowerTick(context.Background())
|
|
if len(ctl.started) != 0 {
|
|
t.Errorf("started despite being unable to confirm no backup is running: %v", ctl.started)
|
|
}
|
|
}
|
|
|
|
// Unknown ownership ⇒ act on nothing. Starting a co-tenant's guest is worse than leaving ours down.
|
|
func TestGuestPower_UnprovenOwnershipActsOnNothing(t *testing.T) {
|
|
ctl := &fakeGuestPowerCtl{guestsErr: errors.New("pool read failed")}
|
|
s := gpServer(t, ctl, nil)
|
|
s.GuestPowerTick(context.Background())
|
|
if len(ctl.started) != 0 {
|
|
t.Errorf("acted with unproven ownership: %v", ctl.started)
|
|
}
|
|
}
|
|
|
|
// Scenario C — bounded retry, then escalate. A guest that will not start must NOT be retried forever.
|
|
//
|
|
// RED-PROOF: remove the `if st.attempts >= guestPowerMaxAttempts` branch → the sweep keeps starting
|
|
// on every tick and this fails with "start attempted N times, want at most 3 — infinite retry loop".
|
|
func TestGuestPower_RetryIsBoundedThenEscalates(t *testing.T) {
|
|
ctl := &fakeGuestPowerCtl{
|
|
guests: []proxmox.Guest{{VMID: 9201, Status: "stopped"}},
|
|
locks: map[int]string{9201: ""},
|
|
onboot: map[int]bool{9201: true},
|
|
startErr: errors.New("cannot start: storage offline"),
|
|
}
|
|
base := time.Now().UTC()
|
|
now := base
|
|
s := gpServer(t, ctl, func() time.Time { return now })
|
|
|
|
// Drive many ticks, advancing well past every backoff each time.
|
|
for i := 0; i < 12; i++ {
|
|
s.GuestPowerTick(context.Background())
|
|
now = now.Add(10 * time.Minute)
|
|
}
|
|
|
|
if len(ctl.started) > guestPowerMaxAttempts {
|
|
t.Errorf("start attempted %d times, want at most %d — this is an infinite retry loop",
|
|
len(ctl.started), guestPowerMaxAttempts)
|
|
}
|
|
if len(ctl.started) != guestPowerMaxAttempts {
|
|
t.Errorf("start attempted %d times, want exactly %d before giving up", len(ctl.started), guestPowerMaxAttempts)
|
|
}
|
|
}
|
|
|
|
// The backoff must actually hold a retry back — otherwise "bounded" is only bounded by luck.
|
|
func TestGuestPower_BackoffDefersTheNextAttempt(t *testing.T) {
|
|
ctl := &fakeGuestPowerCtl{
|
|
guests: []proxmox.Guest{{VMID: 9201, Status: "stopped"}},
|
|
locks: map[int]string{9201: ""},
|
|
onboot: map[int]bool{9201: true},
|
|
startErr: errors.New("boom"),
|
|
}
|
|
base := time.Now().UTC()
|
|
now := base
|
|
s := gpServer(t, ctl, func() time.Time { return now })
|
|
|
|
s.GuestPowerTick(context.Background()) // attempt 1, arms a 1m backoff
|
|
if len(ctl.started) != 1 {
|
|
t.Fatalf("precondition: want 1 attempt, got %d", len(ctl.started))
|
|
}
|
|
now = base.Add(30 * time.Second) // still inside the 1m backoff
|
|
s.GuestPowerTick(context.Background())
|
|
if len(ctl.started) != 1 {
|
|
t.Errorf("retried inside the backoff window (%d attempts) — the bound is not being honoured", len(ctl.started))
|
|
}
|
|
now = base.Add(90 * time.Second) // past it
|
|
s.GuestPowerTick(context.Background())
|
|
if len(ctl.started) != 2 {
|
|
t.Errorf("did not retry after the backoff lapsed (%d attempts)", len(ctl.started))
|
|
}
|
|
}
|
|
|
|
// A guest that comes back healthy must lose its attempt history, so it does not carry old failures
|
|
// into the next incident.
|
|
func TestGuestPower_RunningGuestClearsAttemptHistory(t *testing.T) {
|
|
ctl := &fakeGuestPowerCtl{
|
|
guests: []proxmox.Guest{{VMID: 9201, Status: "stopped"}},
|
|
locks: map[int]string{9201: ""},
|
|
onboot: map[int]bool{9201: true},
|
|
startErr: errors.New("boom"),
|
|
}
|
|
base := time.Now().UTC()
|
|
now := base
|
|
s := gpServer(t, ctl, func() time.Time { return now })
|
|
s.GuestPowerTick(context.Background())
|
|
|
|
if _, due := s.guestPowerDue(9201); due {
|
|
t.Error("precondition: a backoff should be armed after a failed start")
|
|
}
|
|
// it comes back up
|
|
ctl.guests = []proxmox.Guest{{VMID: 9201, Status: "running"}}
|
|
s.GuestPowerTick(context.Background())
|
|
if _, due := s.guestPowerDue(9201); !due {
|
|
t.Error("attempt history survived the guest coming back healthy")
|
|
}
|
|
}
|