diff --git a/internal/backup/backup_test.go b/internal/backup/backup_test.go index cd1afc0..86f25fc 100644 --- a/internal/backup/backup_test.go +++ b/internal/backup/backup_test.go @@ -150,13 +150,14 @@ func TestBackup_VzdumpFailureReturnsFailedRecord(t *testing.T) { func TestPickRestoreCandidate_NewestOrEmpty(t *testing.T) { const big = 4 << 30 // a plausible whole-guest archive api := &fakeBackupAPI{content: []proxmox.StorageContent{ - {VolID: "a", Content: "backup", CTime: 10, Size: big}, - {VolID: "b", Content: "backup", CTime: 99, Size: big}, + // R-689: real vzdump names with their vmid — only a backup OF A GUEST is a candidate. + {VolID: "local:backup/vzdump-lxc-9001-a.tar.zst", VMID: 9001, Content: "backup", CTime: 10, Size: big}, + {VolID: "local:backup/vzdump-lxc-9001-b.tar.zst", VMID: 9001, Content: "backup", CTime: 99, Size: big}, {VolID: "iso", Content: "iso", CTime: 999, Size: big}, // not a backup → ignored }} r := NewBackupRunner(api, "local", "", "", "", quiet()) vol, err := r.PickRestoreCandidate(context.Background()) - if err != nil || vol != "b" { + if err != nil || vol != "local:backup/vzdump-lxc-9001-b.tar.zst" { t.Fatalf("pick = %q,%v want newest 'b'", vol, err) } // no backups → "". @@ -176,12 +177,12 @@ func TestPickRestoreCandidate_NewestOrEmpty(t *testing.T) { // `pick = "phantom" want the newest COMPLETE archive 'real'`. func TestPickRestoreCandidate_SkipsImplausibleArchives(t *testing.T) { api := &fakeBackupAPI{content: []proxmox.StorageContent{ - {VolID: "real", Content: "backup", CTime: 10, Size: 4 << 30}, - {VolID: "phantom", Content: "backup", CTime: 99, Size: 1}, // newest, and impossible + {VolID: "felhom-pbs:backup/ct/9001/real", VMID: 9001, Content: "backup", CTime: 10, Size: 4 << 30}, + {VolID: "felhom-pbs:backup/ct/9001/phantom", VMID: 9001, Content: "backup", CTime: 99, Size: 1}, // newest, and impossible }} r := NewBackupRunner(api, "local", "", "", "", quiet()) vol, err := r.PickRestoreCandidate(context.Background()) - if err != nil || vol != "real" { + if err != nil || vol != "felhom-pbs:backup/ct/9001/real" { t.Fatalf("pick = %q,%v want the newest COMPLETE archive 'real'", vol, err) } } diff --git a/internal/backup/r689_golden_test.go b/internal/backup/r689_golden_test.go new file mode 100644 index 0000000..867a1d6 --- /dev/null +++ b/internal/backup/r689_golden_test.go @@ -0,0 +1,56 @@ +package backup + +import ( + "context" + "testing" + "time" + + "gitea.dooplex.hu/admin/felhom-agent/internal/proxmox" +) + +// R-689 (v0.135.0) — demo-hp keeps its golden template in `local:backup/`. It is content "backup", +// 654 MB and so "plausibly complete", and it was the newest SETTLED entry: the restore test picked it +// every 6 h and failed extractconfig with a 403 (measured 2026-09-24 10:36, 09-25 04:57 and 10:57), +// while the guest's own archive — younger than the 24 h settle — went untested and nothing was proven. +// +// COMPANION RED-PROOF (REPORT.md): drop the guestBackupArchive call from PickSettledRestoreCandidateOn — +// this test then picks `local:backup/felhom-golden-0.236.0.tar.zst`. +func TestR689_TheRestoreTestNeverPicksTheGolden(t *testing.T) { + const day = int64(86400) + now := int64(1790370000) // 2026-09-25 ~19:00Z + api := &fakeBackupAPI{content: []proxmox.StorageContent{ + // the guest's real archive, settled (older than the cutoff below) + {VolID: "local:backup/vzdump-lxc-9201-2026_09_22-21_59_25.tar.zst", Content: "backup", VMID: 9201, Size: 8 << 30, CTime: now - 3*day}, + // the golden: newer, settled, big, and NOT a backup of a guest + {VolID: "local:backup/felhom-golden-0.236.0.tar.zst", Content: "backup", Size: 654115664, CTime: now - 2*day}, + // a hand-copied tarball that PVE happens to attribute to a vmid — the name is not a vzdump's + {VolID: "local:backup/copy-of-9201.tar.zst", Content: "backup", VMID: 9201, Size: 8 << 30, CTime: now - 2*day}, + }} + r := NewBackupRunner(api, "local", proxmox.ModeSnapshot, "", "keep-last=1", quiet()) + got, _, err := r.PickSettledRestoreCandidateOn(context.Background(), "local", time.Unix(now-day, 0).UTC()) + if err != nil { + t.Fatal(err) + } + if got != "local:backup/vzdump-lxc-9201-2026_09_22-21_59_25.tar.zst" { + t.Fatalf("picked %q — the restore test must prove a backup OF A GUEST", got) + } +} + +func TestR689_GuestBackupArchiveShapes(t *testing.T) { + for _, c := range []struct { + e proxmox.StorageContent + ok bool + }{ + {proxmox.StorageContent{VolID: "local:backup/vzdump-lxc-9201-2026_09_24-21_59_25.tar.zst", VMID: 9201}, true}, + {proxmox.StorageContent{VolID: "local:backup/vzdump-qemu-300-2026_09_24-21_59_25.vma.zst", VMID: 300}, true}, + {proxmox.StorageContent{VolID: "felhom-pbs:backup/ct/9201/2026-07-28T05:31:14Z", VMID: 9201}, true}, + {proxmox.StorageContent{VolID: "felhom-pbs:backup/vm/300/2026-07-28T05:31:14Z", VMID: 300}, true}, + {proxmox.StorageContent{VolID: "local:backup/felhom-golden-0.236.0.tar.zst"}, false}, + {proxmox.StorageContent{VolID: "local:backup/vzdump-lxc-9201-x.tar.zst", VMID: 9202}, false}, // vmid disagrees with the name + {proxmox.StorageContent{VolID: "felhom-pbs:backup/ct/9201/2026-07-28T05:31:14Z"}, false}, // no vmid reported + } { + if ok, why := guestBackupArchive(c.e); ok != c.ok { + t.Errorf("%s vmid=%d: ok=%v (%s), want %v", c.e.VolID, c.e.VMID, ok, why, c.ok) + } + } +} diff --git a/internal/backup/runner.go b/internal/backup/runner.go index b45b537..248fa3f 100644 --- a/internal/backup/runner.go +++ b/internal/backup/runner.go @@ -5,6 +5,7 @@ import ( "fmt" "log/slog" "sort" + "strconv" "strings" "sync" "time" @@ -362,6 +363,14 @@ func (r *BackupRunner) PickSettledRestoreCandidateOn(ctx context.Context, target if e.Content != "backup" { continue } + // R-689 (v0.135.0): only a backup OF A GUEST is a restore-test candidate. demo-hp keeps its golden + // template in `local:backup/` — content "backup", 654 MB, plausibly complete — and it was picked as + // the newest settled archive every 6 h and failed extractconfig (403) each time, while the guest's + // real archive went untested. + if ok, why := guestBackupArchive(e); !ok { + r.noteNotAGuestBackupOnce(e, why) + continue + } if !notAfter.IsZero() && e.CTime > notAfter.Unix() { continue // not settled yet — a newer archive is not a reason to re-prove an older one } @@ -591,3 +600,46 @@ func ToHubRestoreTest(res reconcile.RestoreTestResult, testedAt time.Time) hub.R } return rt } + +// guestBackupArchive reports whether a storage entry is a whole-guest backup of a known guest — a +// `vzdump---…` file on a dir storage, or a `backup/{ct,vm}//