From 16dbc8322196ab5c3cd2939283dec13a04eb7dbe Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Sun, 27 Sep 2026 13:45:56 +0200 Subject: [PATCH] agent: the restore test takes only archives of a guest that still exists (R-689, second half) Measured on demo-hp right after v0.135.0: with the golden skipped the pick fell to a leftover archive of guest 9100, deleted in August. "does not exist" skips it; any other lookup error makes the tier unknown. Red-proofed. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS --- internal/backup/backup_test.go | 7 +++++- internal/backup/r689_golden_test.go | 37 +++++++++++++++++++++++++++++ internal/backup/runner.go | 20 ++++++++++++++++ 3 files changed, 63 insertions(+), 1 deletion(-) diff --git a/internal/backup/backup_test.go b/internal/backup/backup_test.go index 86f25fc..503f569 100644 --- a/internal/backup/backup_test.go +++ b/internal/backup/backup_test.go @@ -2,6 +2,7 @@ package backup import ( "context" + "fmt" "encoding/json" "errors" "io" @@ -21,6 +22,7 @@ type fakeBackupAPI struct { vzdumpErr error waitErr error cfg proxmox.GuestConfig + goneGuests map[int]bool // R-689: vmids whose config lookup answers "does not exist" cfgErr error content []proxmox.StorageContent contentErr error @@ -43,7 +45,10 @@ func (f *fakeBackupAPI) WaitTask(_ context.Context, _ string, _ proxmox.WaitOpti } return proxmox.TaskStatus{Status: "stopped", ExitStatus: "OK"}, f.waitErr } -func (f *fakeBackupAPI) GuestConfig(_ context.Context, _ int) (proxmox.GuestConfig, error) { +func (f *fakeBackupAPI) GuestConfig(_ context.Context, vmid int) (proxmox.GuestConfig, error) { + if f.goneGuests[vmid] { // R-689: PVE's answer for a deleted guest + return proxmox.GuestConfig{}, fmt.Errorf("proxmox: GET /nodes/n/lxc/%d/config -> HTTP 500: Configuration file 'nodes/n/lxc/%d.conf' does not exist", vmid, vmid) + } return f.cfg, f.cfgErr } func (f *fakeBackupAPI) StorageContent(_ context.Context, _ string) ([]proxmox.StorageContent, error) { diff --git a/internal/backup/r689_golden_test.go b/internal/backup/r689_golden_test.go index 867a1d6..7a15aa4 100644 --- a/internal/backup/r689_golden_test.go +++ b/internal/backup/r689_golden_test.go @@ -2,6 +2,7 @@ package backup import ( "context" + "fmt" "testing" "time" @@ -54,3 +55,39 @@ func TestR689_GuestBackupArchiveShapes(t *testing.T) { } } } + +// R-689 (v0.136.0) — the measured demo-hp shape right after v0.135.0: the golden (skipped), a leftover archive +// of guest 9100 deleted in August (settled), and today's archive of 9201 (not settled yet). The pick must be +// NOTHING — never the deleted guest's archive. With 9201's archive settled, that one. +// +// COMPANION RED-PROOF (REPORT.md): drop the known-guest check — the pick is the 9100 leftover. +func TestR689_AnArchiveOfADeletedGuestIsNeverPicked(t *testing.T) { + const day = int64(86400) + now := int64(1790476000) + api := &fakeBackupAPI{goneGuests: map[int]bool{9100: true}, content: []proxmox.StorageContent{ + {VolID: "local:backup/felhom-golden-0.236.0.tar.zst", Content: "backup", Size: 654115664, CTime: now - 14*day}, + {VolID: "local:backup/vzdump-lxc-9100-2026_08_21-17_59_15.tar.zst", Content: "backup", VMID: 9100, Size: 656970239, CTime: now - 37*day}, + {VolID: "local:backup/vzdump-lxc-9201-2026_09_27-04_35_47.tar.zst", Content: "backup", VMID: 9201, Size: 8 << 30, CTime: now - 7*3600}, + }} + 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 || got != "" { + t.Fatalf("picked %q err=%v — a deleted guest's archive proves nothing about this box", got, err) + } + got, _, _ = r.PickSettledRestoreCandidateOn(context.Background(), "local", time.Unix(now, 0).UTC()) + if got != "local:backup/vzdump-lxc-9201-2026_09_27-04_35_47.tar.zst" { + t.Fatalf("with 9201's archive settled the pick is %q", got) + } +} + +// Any OTHER lookup failure is not "the guest is gone": the tier must read UNKNOWN (an error), never +// "nothing to prove". +func TestR689_AGuestLookupFailureIsUnknownNotEmpty(t *testing.T) { + api := &fakeBackupAPI{cfgErr: fmt.Errorf("proxmox: connection refused"), content: []proxmox.StorageContent{ + {VolID: "local:backup/vzdump-lxc-9201-x.tar.zst", Content: "backup", VMID: 9201, Size: 8 << 30, CTime: 10}, + }} + r := NewBackupRunner(api, "local", proxmox.ModeSnapshot, "", "keep-last=1", quiet()) + if _, _, err := r.PickSettledRestoreCandidateOn(context.Background(), "local", time.Time{}); err == nil { + t.Fatal("a failed guest lookup read as a clean answer") + } +} diff --git a/internal/backup/runner.go b/internal/backup/runner.go index 248fa3f..7210487 100644 --- a/internal/backup/runner.go +++ b/internal/backup/runner.go @@ -359,6 +359,7 @@ func (r *BackupRunner) PickSettledRestoreCandidateOn(ctx context.Context, target } var best string var bestCTime int64 = -1 + known := map[int]bool{} // vmid → the guest exists on this node (asked once per vmid per pick) for _, e := range contents { if e.Content != "backup" { continue @@ -371,6 +372,25 @@ func (r *BackupRunner) PickSettledRestoreCandidateOn(ctx context.Context, target r.noteNotAGuestBackupOnce(e, why) continue } + // R-689 (v0.136.0): … OF A GUEST THAT STILL EXISTS here. Measured on demo-hp 2026-09-27 right after + // v0.135.0: with the golden skipped, the pick fell to `vzdump-lxc-9100-2026_08_21…`, a leftover of a + // guest deleted in August — proving nothing about any guest this box runs. "Does not exist" skips the + // archive; any OTHER lookup failure is returned, so the tier reads UNKNOWN, never "nothing to prove". + if _, seen := known[e.VMID]; !seen { + _, err := r.api.GuestConfig(ctx, e.VMID) + switch { + case err == nil: + known[e.VMID] = true + case strings.Contains(err.Error(), "does not exist"): + known[e.VMID] = false + default: + return "", time.Time{}, fmt.Errorf("checking whether guest %d still exists: %w", e.VMID, err) + } + } + if !known[e.VMID] { + r.noteNotAGuestBackupOnce(e, fmt.Sprintf("guest %d no longer exists on this node", e.VMID)) + 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 }