0fabc15896
CORRECTION: I earlier reported that the restore-test would boot a scratch guest with the live guest's MAC/static island IP/hostname and break the control plane. That was WRONG — RunRestoreTest step 2 link-downs EVERY interface (withLinkDown, unit-tested) before the guest is ever started. The design already handled it. The real, narrower hazard: a restore that fails BEFORE step 2 (what the v0.100.0 wait bug caused) leaves a scratch holding the SOURCE guest's config verbatim, including onboot:1. If teardown also fails (403 missing VM.Allocate — PVE associates the pool only at restore completion), a host reboot would start that leaked clone alongside the original with NICs up. - proxmox.RestoreLXCOptions.ConfigOverrides: guest-config params applied AT RESTORE TIME. - The restore-test passes onboot=0 — at restore time, not after, because 'after' is exactly the path that leaks. NOT changed: the link-down step (already correct, the primary defence); the agent's Proxmox privileges (widening VM.Allocate to /vms would remove the accidental guard that stopped a destructive mid-restore teardown). restore_test_cadence_seconds was set to -1 on demo-felhom under the mistaken reading; re-enabled. Red-proof observed; full suite green (29 packages).
694 lines
29 KiB
Go
694 lines
29 KiB
Go
package reconcile
|
|
|
|
import (
|
|
"context"
|
|
"encoding/json"
|
|
"errors"
|
|
"strconv"
|
|
"strings"
|
|
"testing"
|
|
"time"
|
|
|
|
"gitea.dooplex.hu/admin/felhom-agent/internal/proxmox"
|
|
)
|
|
|
|
// scratchCfg builds a fake GuestConfig with one net interface (so the link-down SetConfig
|
|
// step runs).
|
|
func scratchCfg() proxmox.GuestConfig {
|
|
return proxmox.GuestConfig{Extra: map[string]json.RawMessage{
|
|
"net0": json.RawMessage(`"name=eth0,bridge=vmbr0,hwaddr=AA:BB:CC:DD:EE:FF,ip=dhcp"`),
|
|
}}
|
|
}
|
|
|
|
func TestRunRestoreTest_PassAndTeardown(t *testing.T) {
|
|
api := &fakeAPI{cfg: map[int]proxmox.GuestConfig{990000: scratchCfg()}} // empty lxc → 990000 free; running default
|
|
e, j, q := newEngine(t, api, EmptyProvider{})
|
|
defer q.Close()
|
|
|
|
res := e.RunRestoreTest(context.Background(), RestoreTestSpec{
|
|
Archive: "local:backup/x.tar.zst", RestoreStorage: "local-lvm",
|
|
ScratchMin: 990000, ScratchMax: 990009, SourceTier: "local",
|
|
})
|
|
if res.Skipped || !res.Pass || res.Err != nil {
|
|
t.Fatalf("expected pass, got %+v", res)
|
|
}
|
|
if res.ScratchVMID != 990000 || res.Verified != "boot+running" {
|
|
t.Fatalf("result = %+v", res)
|
|
}
|
|
if len(api.restores) != 1 || api.restores[0].VMID != 990000 || api.restores[0].Archive != "local:backup/x.tar.zst" {
|
|
t.Fatalf("restore not issued correctly: %+v", api.restores)
|
|
}
|
|
// net link-down applied before boot.
|
|
foundLinkDown := false
|
|
for _, s := range api.sets {
|
|
if s.vmid == 990000 && s.params["net0"] != "" && contains2(s.params["net0"], "link_down=1") {
|
|
foundLinkDown = true
|
|
}
|
|
}
|
|
if !foundLinkDown {
|
|
t.Errorf("expected a net link-down SetConfig, got %+v", api.sets)
|
|
}
|
|
// teardown destroyed the scratch guest, and the journal entry is terminal (not in-flight).
|
|
if len(api.destroys) != 1 || api.destroys[0] != 990000 {
|
|
t.Fatalf("scratch not torn down: %+v", api.destroys)
|
|
}
|
|
if len(j.InFlight()) != 0 {
|
|
t.Errorf("scratch entry must be terminal after teardown: %+v", j.InFlight())
|
|
}
|
|
}
|
|
|
|
// TestRunRestoreTest_TierAwareRestoreTimeout (S4.1) pins that the restore-task wait carries the
|
|
// tier-derived timeout: the configured (generous) value for a pbs/WAN restore, and 0 (→ the 10m
|
|
// WaitOptions default, UNCHANGED) for a local restore. Red-proof: revert the L246 wait to
|
|
// WaitOptions{} → the pbs assertion (120m) fails.
|
|
func TestRunRestoreTest_TierAwareRestoreTimeout(t *testing.T) {
|
|
const restoreUPID = "UPID:node:1:2:3:4:vzrestore:990000:tok:" // async restore → the wait fires
|
|
|
|
restoreWaitTimeout := func(api *fakeAPI) (time.Duration, bool) {
|
|
for i, u := range api.waits {
|
|
if u == restoreUPID {
|
|
return api.waitOpts[i].Timeout, true
|
|
}
|
|
}
|
|
return 0, false
|
|
}
|
|
|
|
// pbs tier → the configured generous timeout is passed to WaitTask.
|
|
pbsAPI := &fakeAPI{cfg: map[int]proxmox.GuestConfig{990000: scratchCfg()}, restoreUPID: restoreUPID}
|
|
e, _, q := newEngine(t, pbsAPI, EmptyProvider{})
|
|
defer q.Close()
|
|
e.RunRestoreTest(context.Background(), RestoreTestSpec{
|
|
Archive: "felhom-offsite:backup/ct/9201/x", RestoreStorage: "local-lvm",
|
|
ScratchMin: 990000, ScratchMax: 990009, SourceTier: "pbs",
|
|
RestoreTaskTimeout: 120 * time.Minute,
|
|
})
|
|
if to, ok := restoreWaitTimeout(pbsAPI); !ok || to != 120*time.Minute {
|
|
t.Errorf("pbs restore wait Timeout = %v (found=%v), want 120m", to, ok)
|
|
}
|
|
|
|
// local tier → 0 (→ WaitOptions' 10m default preserved, UNCHANGED).
|
|
localAPI := &fakeAPI{cfg: map[int]proxmox.GuestConfig{990000: scratchCfg()}, restoreUPID: restoreUPID}
|
|
e2, _, q2 := newEngine(t, localAPI, EmptyProvider{})
|
|
defer q2.Close()
|
|
e2.RunRestoreTest(context.Background(), RestoreTestSpec{
|
|
Archive: "local:backup/x.tar.zst", RestoreStorage: "local-lvm",
|
|
ScratchMin: 990000, ScratchMax: 990009, SourceTier: "local",
|
|
RestoreTaskTimeout: 0,
|
|
})
|
|
if to, ok := restoreWaitTimeout(localAPI); !ok || to != 0 {
|
|
t.Errorf("local restore wait Timeout = %v (found=%v), want 0 (→10m default)", to, ok)
|
|
}
|
|
}
|
|
|
|
// startWarnAPI builds a fakeAPI whose guest-start task exits "WARNINGS: 1" and whose start
|
|
// task log contains the given warning lines. The guest reaches running (status default).
|
|
func startWarnAPI(startUPID string, logLines []string) *fakeAPI {
|
|
return &fakeAPI{
|
|
cfg: map[int]proxmox.GuestConfig{990000: scratchCfg()},
|
|
startUPID: startUPID,
|
|
waitFunc: func(upid string) (proxmox.TaskStatus, error) {
|
|
if upid == startUPID {
|
|
return proxmox.TaskStatus{Status: "stopped", ExitStatus: "WARNINGS: 1"}, nil
|
|
}
|
|
return proxmox.TaskStatus{Status: "stopped", ExitStatus: "OK"}, nil
|
|
},
|
|
logTailFunc: func(string) ([]string, error) { return logLines, nil },
|
|
}
|
|
}
|
|
|
|
func TestRunRestoreTest_PassWithRecognizedWarnings(t *testing.T) {
|
|
// The crux of the fix: start exits WARNINGS (systemd-nesting advisory) AND the guest
|
|
// reaches running → PASS. Warnings surfaced, recognized; verdict is liveness, not exit code.
|
|
const startUPID = "UPID:demo:start:990000:"
|
|
api := startWarnAPI(startUPID, []string{
|
|
"run_buffer: starting CT",
|
|
"WARN: Systemd 257 detected. You may need to enable nesting.",
|
|
"CT started",
|
|
})
|
|
e, _, q := newEngine(t, api, EmptyProvider{})
|
|
defer q.Close()
|
|
|
|
res := e.RunRestoreTest(context.Background(), RestoreTestSpec{
|
|
Archive: "vol", RestoreStorage: "local-lvm", ScratchMin: 990000, ScratchMax: 990009,
|
|
})
|
|
if !res.Pass || res.Err != nil {
|
|
t.Fatalf("start-with-warnings + running must PASS, got %+v", res)
|
|
}
|
|
if res.Verified != "boot+running" {
|
|
t.Errorf("verified = %q", res.Verified)
|
|
}
|
|
if len(res.StartWarnings) != 1 || !contains2(res.StartWarnings[0], "enable nesting") {
|
|
t.Fatalf("the nesting warning must be surfaced, got %+v", res.StartWarnings)
|
|
}
|
|
if !res.WarningsRecognized {
|
|
t.Errorf("the nesting warning must be recognized (benign)")
|
|
}
|
|
}
|
|
|
|
func TestRunRestoreTest_PassWithUnrecognizedWarning(t *testing.T) {
|
|
// An UNRECOGNIZED warning + running still PASSES (verdict is liveness), but is flagged
|
|
// not-recognized so the operator looks. Visibility-only, never a false-fail.
|
|
const startUPID = "UPID:demo:start:990000:"
|
|
api := startWarnAPI(startUPID, []string{"WARN: something unexpected during start"})
|
|
e, _, q := newEngine(t, api, EmptyProvider{})
|
|
defer q.Close()
|
|
|
|
res := e.RunRestoreTest(context.Background(), RestoreTestSpec{
|
|
Archive: "vol", RestoreStorage: "local-lvm", ScratchMin: 990000, ScratchMax: 990009,
|
|
})
|
|
if !res.Pass || res.Err != nil {
|
|
t.Fatalf("unrecognized warning + running must still PASS, got %+v", res)
|
|
}
|
|
if len(res.StartWarnings) != 1 {
|
|
t.Fatalf("warning must still be surfaced, got %+v", res.StartWarnings)
|
|
}
|
|
if res.WarningsRecognized {
|
|
t.Errorf("an unrecognized warning must NOT be recognized")
|
|
}
|
|
}
|
|
|
|
func TestRunRestoreTest_LivenessIsTheVerdict(t *testing.T) {
|
|
// Start exits WARNINGS but the guest NEVER reaches running → FAIL. The verdict is
|
|
// liveness; warnings can never turn a non-running guest into a pass.
|
|
const startUPID = "UPID:demo:start:990000:"
|
|
api := startWarnAPI(startUPID, []string{"WARN: Systemd 257 detected. You may need to enable nesting."})
|
|
api.status = map[int]proxmox.Guest{990000: {VMID: 990000, Status: "stopped"}}
|
|
e, _, q := newEngine(t, api, EmptyProvider{})
|
|
defer q.Close()
|
|
|
|
res := e.RunRestoreTest(context.Background(), RestoreTestSpec{
|
|
Archive: "vol", RestoreStorage: "local-lvm", ScratchMin: 990000, ScratchMax: 990009,
|
|
BootTimeout: 40 * time.Millisecond,
|
|
})
|
|
if res.Pass || res.Err == nil {
|
|
t.Fatalf("not-running must FAIL regardless of warnings, got %+v", res)
|
|
}
|
|
if len(api.destroys) != 1 {
|
|
t.Errorf("teardown must still run: %+v", api.destroys)
|
|
}
|
|
}
|
|
|
|
// TestWarningsRecognized_VersionFree is the regression guard: the recognizer must match the
|
|
// nesting advisory for the CURRENT systemd version AND future ones (258/259…), proving the
|
|
// "enable nesting" anchor is version-independent and can't silently rot back into the bug.
|
|
func TestWarningsRecognized_VersionFree(t *testing.T) {
|
|
for _, v := range []int{256, 257, 258, 259, 300} {
|
|
line := []string{"WARN: Systemd " + strconv.Itoa(v) + " detected. You may need to enable nesting."}
|
|
if !warningsRecognized(line) {
|
|
t.Errorf("systemd %d nesting advisory must be recognized (version-free anchor): %q", v, line[0])
|
|
}
|
|
}
|
|
// Empty ⇒ trivially recognized (N/A).
|
|
if !warningsRecognized(nil) {
|
|
t.Error("empty warnings must be trivially recognized")
|
|
}
|
|
// An unrelated warning is NOT recognized.
|
|
if warningsRecognized([]string{"WARN: disk nearly full"}) {
|
|
t.Error("an unrelated warning must not be recognized")
|
|
}
|
|
// Mixed: one benign + one unrelated ⇒ NOT recognized (every line must match).
|
|
if warningsRecognized([]string{
|
|
"WARN: Systemd 257 detected. You may need to enable nesting.",
|
|
"WARN: disk nearly full",
|
|
}) {
|
|
t.Error("a mix with any unrecognized line must not be recognized")
|
|
}
|
|
}
|
|
|
|
func TestExtractWarningLines(t *testing.T) {
|
|
got := extractWarningLines([]string{
|
|
"run_buffer: starting",
|
|
"WARN: Systemd 257 detected. You may need to enable nesting.",
|
|
" WARN: indented warning ",
|
|
"INFO: not a warning",
|
|
})
|
|
if len(got) != 2 {
|
|
t.Fatalf("want 2 warning lines, got %d: %+v", len(got), got)
|
|
}
|
|
if !contains2(got[0], "enable nesting") || got[1] != "WARN: indented warning" {
|
|
t.Errorf("warning extraction/trim wrong: %+v", got)
|
|
}
|
|
}
|
|
|
|
func TestRunRestoreTest_TeardownOnFailedVerify(t *testing.T) {
|
|
// Guest never reaches running → verify fails, but teardown MUST still run.
|
|
api := &fakeAPI{
|
|
cfg: map[int]proxmox.GuestConfig{990000: scratchCfg()},
|
|
status: map[int]proxmox.Guest{990000: {VMID: 990000, Status: "stopped"}},
|
|
}
|
|
e, j, q := newEngine(t, api, EmptyProvider{})
|
|
defer q.Close()
|
|
|
|
res := e.RunRestoreTest(context.Background(), RestoreTestSpec{
|
|
Archive: "vol", RestoreStorage: "local-lvm",
|
|
ScratchMin: 990000, ScratchMax: 990009, BootTimeout: 40 * time.Millisecond,
|
|
})
|
|
if res.Pass || res.Err == nil {
|
|
t.Fatalf("expected a failed verify, got %+v", res)
|
|
}
|
|
if len(api.destroys) != 1 || api.destroys[0] != 990000 {
|
|
t.Fatalf("teardown MUST run even on a failed verify: destroys=%+v", api.destroys)
|
|
}
|
|
if len(j.InFlight()) != 0 {
|
|
t.Errorf("scratch entry must be terminal after teardown: %+v", j.InFlight())
|
|
}
|
|
}
|
|
|
|
// TestRunRestoreTest_RestoreNoLaunchNoTeardown is the F1b red-proof (campaign pool-effects): a
|
|
// restore that fails SYNCHRONOUSLY (no UPID — nothing created) must NOT run teardown. The old
|
|
// behavior destroyed the picked vmid anyway — the exact destroy-innocent-guest bug when a
|
|
// pool-invisible squatter occupied the band vmid. Red-proof: revert the `launched` gate in
|
|
// runScratchTest and this test fails with destroys=[990000].
|
|
func TestRunRestoreTest_RestoreNoLaunchNoTeardown(t *testing.T) {
|
|
api := &fakeAPI{restoreErr: errors.New("restore boom")}
|
|
e, j, q := newEngine(t, api, EmptyProvider{})
|
|
defer q.Close()
|
|
|
|
res := e.RunRestoreTest(context.Background(), RestoreTestSpec{
|
|
Archive: "vol", RestoreStorage: "local-lvm", ScratchMin: 990000, ScratchMax: 990009,
|
|
})
|
|
if res.Pass || res.Err == nil {
|
|
t.Fatalf("expected restore failure, got %+v", res)
|
|
}
|
|
// THE point: no teardown — the txn created nothing at that vmid.
|
|
if len(api.destroys) != 0 {
|
|
t.Fatalf("a no-launch restore failure must NOT destroy the vmid: %+v", api.destroys)
|
|
}
|
|
// The entry is closed terminal in-process (not left for Recover).
|
|
if len(j.InFlight()) != 0 {
|
|
t.Errorf("scratch entry must be terminal: %+v", j.InFlight())
|
|
}
|
|
}
|
|
|
|
// TestRunRestoreTest_LaunchedTaskFailureStillTearsDown (no-regression, Scenario B): the restore
|
|
// POST was accepted (UPID) but the task then fails → we own the maybe-partial guest → teardown
|
|
// MUST still run.
|
|
func TestRunRestoreTest_LaunchedTaskFailureStillTearsDown(t *testing.T) {
|
|
const restoreUPID = "UPID:demo:restore:990000:"
|
|
api := &fakeAPI{
|
|
restoreUPID: restoreUPID,
|
|
waitFunc: func(upid string) (proxmox.TaskStatus, error) {
|
|
if upid == restoreUPID {
|
|
return proxmox.TaskStatus{}, errors.New("restore task failed")
|
|
}
|
|
return proxmox.TaskStatus{Status: "stopped", ExitStatus: "OK"}, nil
|
|
},
|
|
}
|
|
e, j, q := newEngine(t, api, EmptyProvider{})
|
|
defer q.Close()
|
|
|
|
res := e.RunRestoreTest(context.Background(), RestoreTestSpec{
|
|
Archive: "vol", RestoreStorage: "local-lvm", ScratchMin: 990000, ScratchMax: 990009,
|
|
})
|
|
if res.Pass || res.Err == nil {
|
|
t.Fatalf("expected restore-task failure, got %+v", res)
|
|
}
|
|
if len(api.destroys) != 1 || api.destroys[0] != 990000 {
|
|
t.Fatalf("teardown must run after a LAUNCHED restore fails: %+v", api.destroys)
|
|
}
|
|
if len(j.InFlight()) != 0 {
|
|
t.Errorf("scratch entry must be terminal: %+v", j.InFlight())
|
|
}
|
|
}
|
|
|
|
// TestRunRestoreTest_AdvancesPastOccupiedBandVMID is the F2 test (campaign pool-effects): the
|
|
// first band vmid is refused "already exists" (an invisible squatter) → the test ADVANCES to the
|
|
// next band vmid and PASSES there; the squatter is never destroyed; no FAIL, no false alert.
|
|
func TestRunRestoreTest_AdvancesPastOccupiedBandVMID(t *testing.T) {
|
|
squatterRefusal := &proxmox.APIError{
|
|
StatusCode: 500, Method: "POST", Path: "/nodes/x/lxc",
|
|
Body: `{"message":"CT 990000 already exists on node 'x'\n","data":null}`,
|
|
}
|
|
api := &fakeAPI{cfg: map[int]proxmox.GuestConfig{990001: scratchCfg()}}
|
|
api.restoreFunc = func(opts proxmox.RestoreLXCOptions) (string, error) {
|
|
if opts.VMID == 990000 {
|
|
return "", squatterRefusal
|
|
}
|
|
return "", nil // 990001 restores fine (synchronous OK path)
|
|
}
|
|
e, j, q := newEngine(t, api, EmptyProvider{})
|
|
defer q.Close()
|
|
|
|
res := e.RunRestoreTest(context.Background(), RestoreTestSpec{
|
|
Archive: "vol", RestoreStorage: "local-lvm", ScratchMin: 990000, ScratchMax: 990009,
|
|
})
|
|
if res.Skipped || !res.Pass || res.Err != nil {
|
|
t.Fatalf("must advance past the squatter and PASS, got %+v", res)
|
|
}
|
|
if res.ScratchVMID != 990001 {
|
|
t.Fatalf("must have advanced to 990001, got %d", res.ScratchVMID)
|
|
}
|
|
// The squatter at 990000 must NEVER be destroyed; only the own scratch at 990001 is.
|
|
for _, d := range api.destroys {
|
|
if d == 990000 {
|
|
t.Fatalf("the squatter at 990000 must never be destroyed: destroys=%+v", api.destroys)
|
|
}
|
|
}
|
|
if len(api.destroys) != 1 || api.destroys[0] != 990001 {
|
|
t.Fatalf("own scratch teardown expected at 990001: %+v", api.destroys)
|
|
}
|
|
if len(j.InFlight()) != 0 {
|
|
t.Errorf("all entries must be terminal: %+v", j.InFlight())
|
|
}
|
|
}
|
|
|
|
// TestRunRestoreTest_BandFullOfSquattersSkips (F2): every band vmid is refused "already exists"
|
|
// → Skipped (NOT a FAIL — no false "backup unrestorable" alert), nothing destroyed, bounded.
|
|
func TestRunRestoreTest_BandFullOfSquattersSkips(t *testing.T) {
|
|
api := &fakeAPI{}
|
|
api.restoreFunc = func(opts proxmox.RestoreLXCOptions) (string, error) {
|
|
return "", &proxmox.APIError{StatusCode: 500, Body: "CT already exists on node 'x'"}
|
|
}
|
|
e, _, q := newEngine(t, api, EmptyProvider{})
|
|
defer q.Close()
|
|
|
|
res := e.RunRestoreTest(context.Background(), RestoreTestSpec{
|
|
Archive: "vol", RestoreStorage: "local-lvm", ScratchMin: 990000, ScratchMax: 990002,
|
|
})
|
|
if !res.Skipped || res.Err != nil || res.Pass {
|
|
t.Fatalf("a squatter-full band must SKIP (not fail), got %+v", res)
|
|
}
|
|
if len(api.destroys) != 0 {
|
|
t.Fatalf("nothing may be destroyed: %+v", api.destroys)
|
|
}
|
|
if len(api.restores) != 3 {
|
|
t.Errorf("must have tried each band vmid exactly once (bounded), got %d", len(api.restores))
|
|
}
|
|
}
|
|
|
|
func TestRunRestoreTest_FullBandSkips(t *testing.T) {
|
|
// Whole band occupied → skipped, never run / out-of-band.
|
|
var guests []proxmox.Guest
|
|
for id := 990000; id <= 990001; id++ {
|
|
guests = append(guests, proxmox.Guest{VMID: id})
|
|
}
|
|
api := &fakeAPI{lxc: guests}
|
|
e, _, q := newEngine(t, api, EmptyProvider{})
|
|
defer q.Close()
|
|
|
|
res := e.RunRestoreTest(context.Background(), RestoreTestSpec{
|
|
Archive: "vol", RestoreStorage: "local-lvm", ScratchMin: 990000, ScratchMax: 990001,
|
|
})
|
|
if !res.Skipped {
|
|
t.Fatalf("full band must skip, got %+v", res)
|
|
}
|
|
if len(api.restores) != 0 || len(api.destroys) != 0 {
|
|
t.Errorf("a skipped test must not restore or destroy anything")
|
|
}
|
|
}
|
|
|
|
func TestRunRestoreTest_InvalidBandErrors(t *testing.T) {
|
|
e, _, q := newEngine(t, &fakeAPI{}, EmptyProvider{})
|
|
defer q.Close()
|
|
res := e.RunRestoreTest(context.Background(), RestoreTestSpec{Archive: "v", RestoreStorage: "s", ScratchMin: 0})
|
|
if res.Err == nil {
|
|
t.Fatal("an invalid scratch band must error")
|
|
}
|
|
}
|
|
|
|
func TestPickScratchVMID(t *testing.T) {
|
|
// excludes 9999 and in-use; lowest free.
|
|
got, ok := pickScratchVMID([]proxmox.Guest{{VMID: 990000}}, 990000, 990009, nil)
|
|
if !ok || got != 990001 {
|
|
t.Errorf("pick = %d,%v want 990001,true", got, ok)
|
|
}
|
|
// full band.
|
|
full := []proxmox.Guest{{VMID: 990000}, {VMID: 990001}}
|
|
if _, ok := pickScratchVMID(full, 990000, 990001, nil); ok {
|
|
t.Error("full band must return ok=false")
|
|
}
|
|
// the exclude set (F2 band-advance: vmids PVE reported occupied) is honored.
|
|
got, ok = pickScratchVMID(nil, 990000, 990009, map[int]bool{990000: true, 990001: true})
|
|
if !ok || got != 990002 {
|
|
t.Errorf("exclude-set pick = %d,%v want 990002,true", got, ok)
|
|
}
|
|
if _, ok := pickScratchVMID(nil, 990000, 990001, map[int]bool{990000: true, 990001: true}); ok {
|
|
t.Error("a fully-excluded band must return ok=false")
|
|
}
|
|
}
|
|
|
|
func TestWithLinkDown(t *testing.T) {
|
|
got := withLinkDown("name=eth0,bridge=vmbr0,ip=dhcp")
|
|
if !contains2(got, "link_down=1") || !contains2(got, "name=eth0") {
|
|
t.Errorf("withLinkDown lost fields or didn't set link_down: %q", got)
|
|
}
|
|
// idempotent: an existing link_down is replaced, not duplicated.
|
|
got = withLinkDown("name=eth0,link_down=0,bridge=vmbr0")
|
|
if count(got, "link_down=") != 1 || !contains2(got, "link_down=1") {
|
|
t.Errorf("withLinkDown must replace an existing link_down (got %q)", got)
|
|
}
|
|
}
|
|
|
|
// --- recover the leaked scratch guest (the headline crash-safety test) ---
|
|
|
|
func TestRecover_LeakedScratchDestroyed(t *testing.T) {
|
|
// The scratch guest still exists at startup (agent crashed mid-test AFTER the restore
|
|
// launched — the entry carries the UPID = the launch proof) → Recover destroys it.
|
|
api := &fakeAPI{lxc: []proxmox.Guest{{VMID: 990000, Status: "running"}}}
|
|
e, j, _ := newEngine(t, api, EmptyProvider{})
|
|
if err := j.Append(JournalEntry{OpID: "scratch-990000-1", VMID: 990000, Kind: scratchKind, Scratch: true, UPID: "UPID:demo:restore:990000:", State: OpTaskRunning, At: time.Now().UTC()}); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
res := e.Recover(context.Background())
|
|
if res.ScratchDestroyed != 1 {
|
|
t.Fatalf("leaked scratch must be destroyed, got %+v", res)
|
|
}
|
|
if len(api.destroys) != 1 || api.destroys[0] != 990000 {
|
|
t.Fatalf("DestroyLXC not called for the leaked scratch: %+v", api.destroys)
|
|
}
|
|
if len(j.InFlight()) != 0 {
|
|
t.Errorf("resolved scratch entry must not be in-flight: %+v", j.InFlight())
|
|
}
|
|
}
|
|
|
|
func TestRecover_LeakedScratchAlreadyGone(t *testing.T) {
|
|
// Crash AFTER the destroy task but BEFORE the terminal record → guest already gone →
|
|
// idempotent clean (no destroy issued).
|
|
api := &fakeAPI{lxc: []proxmox.Guest{{VMID: 9001, Status: "stopped"}}} // 990000 absent
|
|
e, j, _ := newEngine(t, api, EmptyProvider{})
|
|
j.Append(JournalEntry{OpID: "scratch-990000-1", VMID: 990000, Kind: scratchKind, Scratch: true, UPID: "UPID:demo:restore:990000:", State: OpTaskRunning, At: time.Now().UTC()})
|
|
res := e.Recover(context.Background())
|
|
if res.ScratchClean != 1 || len(api.destroys) != 0 {
|
|
t.Fatalf("already-gone scratch must be clean with no destroy, got res=%+v destroys=%+v", res, api.destroys)
|
|
}
|
|
if len(j.InFlight()) != 0 {
|
|
t.Errorf("entry must be resolved: %+v", j.InFlight())
|
|
}
|
|
}
|
|
|
|
func TestRecover_LeakedScratchListUnreadable(t *testing.T) {
|
|
api := &fakeAPI{listErr: errors.New("api down")}
|
|
e, j, _ := newEngine(t, api, EmptyProvider{})
|
|
j.Append(JournalEntry{OpID: "scratch-990000-1", VMID: 990000, Kind: scratchKind, Scratch: true, UPID: "UPID:demo:restore:990000:", State: OpTaskRunning, At: time.Now().UTC()})
|
|
res := e.Recover(context.Background())
|
|
if res.Unresolved != 1 || len(j.InFlight()) != 1 {
|
|
t.Fatalf("unreadable list must leave the scratch in-flight for a later Recover, got res=%+v inflight=%d", res, len(j.InFlight()))
|
|
}
|
|
if len(api.destroys) != 0 {
|
|
t.Error("must not destroy when it can't confirm the guest exists")
|
|
}
|
|
}
|
|
|
|
// TestRecover_ScratchNoUPIDAbandoned (F1c, scratch twin of the bring-up test): a crash left a
|
|
// Scratch entry with NO UPID while a guest sits at the band vmid (an invisible squatter, or a
|
|
// pre-existing guest under a broad token). Recover must ABANDON, never destroy — no journaled
|
|
// UPID ⇒ the restore-test created nothing there.
|
|
func TestRecover_ScratchNoUPIDAbandoned(t *testing.T) {
|
|
api := &fakeAPI{lxc: []proxmox.Guest{{VMID: 990000, Status: "stopped"}}} // a guest IS at the vmid
|
|
e, j, _ := newEngine(t, api, EmptyProvider{})
|
|
j.Append(JournalEntry{OpID: "scratch-990000-1", VMID: 990000, Kind: scratchKind, Scratch: true, State: OpStarted, At: time.Now().UTC()})
|
|
res := e.Recover(context.Background())
|
|
if len(api.destroys) != 0 {
|
|
t.Fatalf("a no-UPID Scratch entry must NEVER destroy the vmid: destroys=%+v", api.destroys)
|
|
}
|
|
if res.RolledBack != 1 || res.ScratchDestroyed != 0 {
|
|
t.Fatalf("entry must be abandoned via the no-UPID fail-safe path, got %+v", res)
|
|
}
|
|
if len(j.InFlight()) != 0 {
|
|
t.Errorf("abandoned entry must be terminal: %+v", j.InFlight())
|
|
}
|
|
}
|
|
|
|
// small string helpers (avoid importing strings in the test for one call).
|
|
func contains2(s, sub string) bool { return indexOf(s, sub) >= 0 }
|
|
func count(s, sub string) int {
|
|
n, i := 0, 0
|
|
for {
|
|
j := indexOf(s[i:], sub)
|
|
if j < 0 {
|
|
return n
|
|
}
|
|
n++
|
|
i += j + len(sub)
|
|
}
|
|
}
|
|
func indexOf(s, sub string) int {
|
|
for i := 0; i+len(sub) <= len(s); i++ {
|
|
if s[i:i+len(sub)] == sub {
|
|
return i
|
|
}
|
|
}
|
|
return -1
|
|
}
|
|
|
|
// ── GL-5b: full-fidelity restore-test (archive-derived params + mount parity) ────────────────────
|
|
|
|
// gl5bArchiveCfg is a 9201-shaped archive config: rootfs + two storage mpN + the two structural binds.
|
|
const gl5bArchiveCfg = `hostname: demo
|
|
rootfs: local-lvm:vm-9201-disk-0,size=32G
|
|
mp0: local-lvm:vm-9201-disk-1,mp=/var/lib/docker,backup=1,size=200G
|
|
mp1: local-lvm:vm-9201-disk-2,mp=/mnt/sys_drive,backup=1,size=50G
|
|
mp8: /mnt/felhom-drives,mp=/mnt/felhom-drives
|
|
mp9: /var/lib/felhom-agent/guests/9201/bootstrap,mp=/etc/felhom-bootstrap,ro=1
|
|
`
|
|
|
|
// gl5bRestoredCfg builds the scratch guest's restored config as PVE would report it after a
|
|
// FULL-fidelity restore (storage mpN at archived path+size, binds as 1G throwaways) + a net0 so
|
|
// the link-down step runs.
|
|
func gl5bRestoredCfg() proxmox.GuestConfig {
|
|
c := scratchCfg()
|
|
c.Extra["mp0"] = json.RawMessage(`"local-lvm:vm-990000-disk-1,mp=/var/lib/docker,backup=1,size=200G"`)
|
|
c.Extra["mp1"] = json.RawMessage(`"local-lvm:vm-990000-disk-2,mp=/mnt/sys_drive,backup=1,size=50G"`)
|
|
c.Extra["mp8"] = json.RawMessage(`"local-lvm:vm-990000-disk-3,mp=/mnt/felhom-drives,backup=0,size=1G"`)
|
|
c.Extra["mp9"] = json.RawMessage(`"local-lvm:vm-990000-disk-4,mp=/etc/felhom-bootstrap,backup=0,size=1G"`)
|
|
return c
|
|
}
|
|
|
|
// GL-5b Scenario A: the restore-test passes the FULL drRestoreOverrides param set (derived from
|
|
// the ARCHIVE config, not any live guest) and the parity assert verifies the restored mpN set.
|
|
func TestRunRestoreTest_FullFidelityParams(t *testing.T) {
|
|
api := &fakeAPI{cfg: map[int]proxmox.GuestConfig{990000: gl5bRestoredCfg()}, extractCfg: gl5bArchiveCfg}
|
|
e, _, q := newEngine(t, api, EmptyProvider{})
|
|
defer q.Close()
|
|
|
|
res := e.RunRestoreTest(context.Background(), RestoreTestSpec{
|
|
Archive: "local:backup/vzdump-lxc-9201-x.tar.zst", RestoreStorage: "local-lvm",
|
|
ScratchMin: 990000, ScratchMax: 990009, SourceTier: "local",
|
|
})
|
|
if res.Err != nil || !res.Pass {
|
|
t.Fatalf("expected pass, got %+v", res)
|
|
}
|
|
if len(api.extracts) != 1 || api.extracts[0] != "local:backup/vzdump-lxc-9201-x.tar.zst" {
|
|
t.Fatalf("params must derive from the ARCHIVE's extracted config: %+v", api.extracts)
|
|
}
|
|
ov := api.restores[0].MountOverrides
|
|
want := map[string]string{
|
|
"rootfs": "local-lvm:32",
|
|
"mp0": "local-lvm:200,mp=/var/lib/docker,backup=1",
|
|
"mp1": "local-lvm:50,mp=/mnt/sys_drive,backup=1",
|
|
"mp8": "local-lvm:1,mp=/mnt/felhom-drives,backup=0",
|
|
"mp9": "local-lvm:1,mp=/etc/felhom-bootstrap,backup=0",
|
|
}
|
|
if len(ov) != len(want) {
|
|
t.Fatalf("MountOverrides = %+v, want %+v", ov, want)
|
|
}
|
|
for k, v := range want {
|
|
if ov[k] != v {
|
|
t.Errorf("override[%s] = %q, want %q", k, ov[k], v)
|
|
}
|
|
}
|
|
if res.MountParity != "ok" {
|
|
t.Errorf("MountParity = %q, want ok", res.MountParity)
|
|
}
|
|
if len(res.MountInventory) != 4 {
|
|
t.Errorf("MountInventory must carry the 4 verified mpN, got %v", res.MountInventory)
|
|
}
|
|
// scratch still torn down (full-fidelity changes verification, not lifecycle)
|
|
if len(api.destroys) != 1 || api.destroys[0] != 990000 {
|
|
t.Errorf("scratch must be torn down: %+v", api.destroys)
|
|
}
|
|
}
|
|
|
|
// GL-5b Scenario B (the non-hollow core): a restored guest MISSING a storage mpN — exactly PVE's
|
|
// drop-unlisted-mountpoints shape — boots green but must FAIL on parity, naming the mpN.
|
|
// COMPANION RED-PROOF: with the 2b parity assert removed, this run passes silently (mutation
|
|
// run→fail→revert recorded in the REPORT).
|
|
func TestRunRestoreTest_ParityCatchesDroppedMount(t *testing.T) {
|
|
c := gl5bRestoredCfg()
|
|
delete(c.Extra, "mp0") // constraint-(b): the docker-data volume silently dropped
|
|
api := &fakeAPI{cfg: map[int]proxmox.GuestConfig{990000: c}, extractCfg: gl5bArchiveCfg}
|
|
e, _, q := newEngine(t, api, EmptyProvider{})
|
|
defer q.Close()
|
|
|
|
res := e.RunRestoreTest(context.Background(), RestoreTestSpec{
|
|
Archive: "local:backup/vzdump-lxc-9201-x.tar.zst", RestoreStorage: "local-lvm",
|
|
ScratchMin: 990000, ScratchMax: 990009, SourceTier: "local",
|
|
})
|
|
if res.Pass || res.Err == nil {
|
|
t.Fatalf("a dropped mpN must FAIL the test, got %+v", res)
|
|
}
|
|
if !strings.Contains(res.Err.Error(), "mount parity FAILED") || !strings.Contains(res.Err.Error(), "mp0 MISSING") {
|
|
t.Fatalf("the verdict must name the missing mpN, got: %v", res.Err)
|
|
}
|
|
if res.MountParity != "mismatch" {
|
|
t.Errorf("MountParity = %q, want mismatch", res.MountParity)
|
|
}
|
|
// the guest never boots (parity fails pre-start) and the scratch is still torn down
|
|
if len(api.starts) != 0 {
|
|
t.Errorf("a parity-failed scratch must not be started: %+v", api.starts)
|
|
}
|
|
if len(api.destroys) != 1 {
|
|
t.Errorf("teardown must still fire (launch-proven): %+v", api.destroys)
|
|
}
|
|
}
|
|
|
|
// GL-5b Scenario C: refusals propagate — an unreadable archive config or an unknown-topology
|
|
// archive refuses UP FRONT (no restore, no scratch, nothing to tear down).
|
|
func TestRunRestoreTest_RefusalsPropagate(t *testing.T) {
|
|
// unreadable archive config
|
|
apiErr := &fakeAPI{cfg: map[int]proxmox.GuestConfig{990000: scratchCfg()}}
|
|
apiErr.extractErr = errors.New("proxmox: GET extractconfig -> HTTP 500: volume not found")
|
|
e1, _, q1 := newEngine(t, apiErr, EmptyProvider{})
|
|
defer q1.Close()
|
|
res := e1.RunRestoreTest(context.Background(), RestoreTestSpec{
|
|
Archive: "local:backup/gone.tar.zst", RestoreStorage: "local-lvm",
|
|
ScratchMin: 990000, ScratchMax: 990009, SourceTier: "local",
|
|
})
|
|
if res.Err == nil || !strings.Contains(res.Err.Error(), "extract archive config") {
|
|
t.Fatalf("unreadable archive config must refuse naming the step, got %+v", res)
|
|
}
|
|
if len(apiErr.restores) != 0 || len(apiErr.destroys) != 0 {
|
|
t.Fatalf("the refusal must fire BEFORE any restore/teardown: %+v %+v", apiErr.restores, apiErr.destroys)
|
|
}
|
|
|
|
// unknown bind topology
|
|
apiBind := &fakeAPI{cfg: map[int]proxmox.GuestConfig{990000: scratchCfg()},
|
|
extractCfg: "rootfs: l:d,size=8G\nmp3: /srv/other,mp=/data\n"}
|
|
e2, _, q2 := newEngine(t, apiBind, EmptyProvider{})
|
|
defer q2.Close()
|
|
res = e2.RunRestoreTest(context.Background(), RestoreTestSpec{
|
|
Archive: "local:backup/x.tar.zst", RestoreStorage: "local-lvm",
|
|
ScratchMin: 990000, ScratchMax: 990009, SourceTier: "local",
|
|
})
|
|
if res.Err == nil || !strings.Contains(res.Err.Error(), "unknown bind mountpoint") {
|
|
t.Fatalf("unknown topology must refuse via drRestoreOverrides' error, got %+v", res)
|
|
}
|
|
if len(apiBind.restores) != 0 {
|
|
t.Fatalf("never restore a partial guest to verify it: %+v", apiBind.restores)
|
|
}
|
|
}
|
|
|
|
// A leaked scratch guest must never AUTO-START. The normal path link-downs every NIC before boot
|
|
// (TestRestoreTest… above), so the source can never be conflicted with on the happy path. This
|
|
// covers the abnormal one: a restore that fails BEFORE the link-down step leaves a scratch carrying
|
|
// the SOURCE guest's config verbatim — including `onboot: 1`, its MAC, its static island IP and its
|
|
// hostname. Observed live 2026-07-26, when a wait-timeout left exactly such a guest on demo-felhom.
|
|
// onboot=0 is therefore set AT RESTORE TIME, not after: after is too late for the path that leaks.
|
|
func TestRestoreTest_RestoreSetsOnbootZero(t *testing.T) {
|
|
api := &fakeAPI{cfg: map[int]proxmox.GuestConfig{990000: scratchCfg()}}
|
|
e, _, q := newEngine(t, api, EmptyProvider{})
|
|
defer q.Close()
|
|
|
|
_ = e.RunRestoreTest(context.Background(), RestoreTestSpec{
|
|
Archive: "local:backup/x.tar.zst", RestoreStorage: "local-lvm",
|
|
ScratchMin: 990000, ScratchMax: 990009, SourceTier: "local",
|
|
})
|
|
if len(api.restores) != 1 {
|
|
t.Fatalf("want one restore, got %+v", api.restores)
|
|
}
|
|
if got := api.restores[0].ConfigOverrides["onboot"]; got != "0" {
|
|
t.Fatalf("the restore MUST set onboot=0 so a leaked scratch cannot auto-start; got %q (%#v)",
|
|
got, api.restores[0].ConfigOverrides)
|
|
}
|
|
}
|