package reconcile import ( "context" "encoding/json" "errors" "strconv" "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 }