Files
felhom-agent/internal/reconcile/restoretest_test.go
T
admin 6e86483185 restore-test: verdict is liveness, not start-task exitstatus (v0.7.0)
Fixes the crying-wolf false-fail surfaced by the live hub-enrollment runbook:
PVE's guest-start task exits "WARNINGS: 1" for the benign systemd-nesting
advisory, and WaitTask treated any non-OK exitstatus as failure, so the verdict
was decided by an advisory exit code before the real boot check ran. Every
modern-distro restore-test reported pass:false.

- proxmox.WaitOptions.AllowWarnings (opt-in; default keeps all callers strict)
- restore-test start step accepts warnings, surfaces them, verdict stays waitRunning
- RestoreTestResult.StartWarnings/.WarningsRecognized + version-free "enable
  nesting" recognizer (can't rot back at systemd 258+); GuestAPI.TaskLogTail
- hub.RestoreTest.warnings/.warnings_recognized wire fields (consumed by hub v0.7.5)
- scheduler logs clean / passed-with-recognized / passed-with-unrecognized warnings
- tests: WaitTask warnings matrix; restore-test pass/fail-on-liveness; version-free
  regression guard (systemd 256-300)

Single agent bump 0.6.0 -> 0.7.0 covering the agent half of both task phases.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-09 19:30:03 +02:00

359 lines
13 KiB
Go

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())
}
}
// 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())
}
}
func TestRunRestoreTest_RestoreFailureStillTearsDown(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)
}
// Even though restore failed, the scratch entry was journaled BEFORE the restore, so
// teardown runs (idempotent — destroys the maybe-partial guest).
if len(api.destroys) != 1 {
t.Fatalf("teardown must run after a restore failure: %+v", api.destroys)
}
if len(j.InFlight()) != 0 {
t.Errorf("scratch entry must be terminal: %+v", j.InFlight())
}
}
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)
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); ok {
t.Error("full 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) → 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, 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, 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, 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")
}
}
// 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
}