agent v0.51.0: local vzdump retention default (--prune-backups keep-last=3)

The preventive counterpart to host_disk + storage_fill detectors: the periodic local
whole-guest vzdump now prunes its own old archives (keep-last=3, clamped >=1) so a box
can't refill its own root via its own backups. Local target only — PBS never pruned
(resolved via ListStorage; fail-safe skip on unknown). Seeded in host-install.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HxLA1mZurFq9kt8hneFeCs

internal/backup/backup_test.go

@@ -24,6 +24,8 @@ type fakeBackupAPI struct {
 	cfgErr     error
 	content    []proxmox.StorageContent
 	contentErr error
+	storages   []proxmox.Storage // returned by ListStorage (the local-prune scope gate)
+	storageErr error
 	vzdumps    []proxmox.VzdumpOptions
 	logLines   []string      // returned by TaskLogTail (e.g. "INFO: backup mode: stop")
 	waitGate   chan struct{} // if non-nil, WaitTask blocks until closed (8B.2 watcher timing)
@@ -45,6 +47,9 @@ func (f *fakeBackupAPI) GuestConfig(_ context.Context, _ int) (proxmox.GuestConf
 func (f *fakeBackupAPI) StorageContent(_ context.Context, _ string) ([]proxmox.StorageContent, error) {
 	return f.content, f.contentErr
 }
+func (f *fakeBackupAPI) ListStorage(_ context.Context) ([]proxmox.Storage, error) {
+	return f.storages, f.storageErr
+}
 func (f *fakeBackupAPI) TaskLogTail(_ context.Context, _ string, _ int) ([]string, error) {
 	return f.logLines, nil
 }
@@ -73,7 +78,7 @@ func TestBackup_SuccessResolvesArchiveAndBulkGap(t *testing.T) {
 			{VolID: "local:backup/other-9002.tar.zst", Content: "backup", VMID: 9002, Size: 7, CTime: 999},
 		},
 	}
-	r := NewBackupRunner(api, "local", "", "felhom test", quiet())
+	r := NewBackupRunner(api, "local", "", "felhom test", "", quiet())
 	rec, err := r.Backup(context.Background(), 9001)
 	if err != nil {
 		t.Fatalf("Backup: %v", err)
@@ -107,7 +112,7 @@ func TestBackup_ReportsActualModeFromTaskLog(t *testing.T) {
 		content:    []proxmox.StorageContent{{VolID: "v", Content: "backup", VMID: 9001, Size: 10, CTime: 1}},
 		logLines:   []string{"INFO: CT Name: spike", "INFO: backup mode: stop", "INFO: Finished"},
 	}
-	r := NewBackupRunner(api, "local", proxmox.ModeSnapshot, "", quiet())
+	r := NewBackupRunner(api, "local", proxmox.ModeSnapshot, "", "", quiet())
 	rec, err := r.Backup(context.Background(), 9001)
 	if err != nil {
 		t.Fatal(err)
@@ -119,7 +124,7 @@ func TestBackup_ReportsActualModeFromTaskLog(t *testing.T) {
 
 func TestBackup_VzdumpFailureReturnsFailedRecord(t *testing.T) {
 	api := &fakeBackupAPI{vzdumpErr: errors.New("vzdump boom")}
-	r := NewBackupRunner(api, "local", "", "", quiet())
+	r := NewBackupRunner(api, "local", "", "", "", quiet())
 	rec, err := r.Backup(context.Background(), 9001)
 	if err == nil {
 		t.Fatal("expected error")
@@ -135,7 +140,7 @@ func TestPickRestoreCandidate_NewestOrEmpty(t *testing.T) {
 		{VolID: "b", Content: "backup", CTime: 99},
 		{VolID: "iso", Content: "iso", CTime: 999}, // not a backup → ignored
 	}}
-	r := NewBackupRunner(api, "local", "", "", quiet())
+	r := NewBackupRunner(api, "local", "", "", "", quiet())
 	vol, err := r.PickRestoreCandidate(context.Background())
 	if err != nil || vol != "b" {
 		t.Fatalf("pick = %q,%v want newest 'b'", vol, err)

internal/backup/prune_test.go

@@ -0,0 +1,99 @@
+package backup
+
+import (
+	"context"
+	"testing"
+
+	"gitea.dooplex.hu/admin/felhom-agent/internal/config"
+	"gitea.dooplex.hu/admin/felhom-agent/internal/proxmox"
+)
+
+// localTarget is a non-PBS dir storage; pbsTarget is a PBS storage — for the scope gate.
+var (
+	localTargetStores = []proxmox.Storage{{Storage: "local", Type: "dir", Content: "backup"}}
+	pbsTargetStores   = []proxmox.Storage{{Storage: "felhom-pbs", Type: "pbs", Content: "backup"}}
+)
+
+func okAPI(stores []proxmox.Storage) *fakeBackupAPI {
+	return &fakeBackupAPI{vzdumpUPID: "UPID:vzdump:1", storages: stores}
+}
+
+// TestPrune_LocalCarriesKeepLast: a LOCAL-target backup with retention carries `--prune-backups
+// keep-last=3` on the vzdump. Companion: the SAME runner built with no retention ("") emits NO prune
+// option — proving the flag only rides when retention is set (a no-prune build accumulates).
+func TestPrune_LocalCarriesKeepLast(t *testing.T) {
+	api := okAPI(localTargetStores)
+	r := NewBackupRunner(api, "local", proxmox.ModeStop, "", "keep-last=3", quiet())
+	_, _ = r.Backup(context.Background(), 9201) // archive-resolution may fail in the fake; we assert the captured vzdump opts
+	if len(api.vzdumps) != 1 || api.vzdumps[0].PruneBackups != "keep-last=3" {
+		t.Fatalf("local backup must carry prune-backups keep-last=3, got %q", api.vzdumps[0].PruneBackups)
+	}
+
+	// COMPANION: no retention → no prune option (dumps would accumulate).
+	api2 := okAPI(localTargetStores)
+	r2 := NewBackupRunner(api2, "local", proxmox.ModeStop, "", "", quiet())
+	_, _ = r2.Backup(context.Background(), 9201)
+	if api2.vzdumps[0].PruneBackups != "" {
+		t.Fatalf("a no-retention runner must NOT prune, got %q", api2.vzdumps[0].PruneBackups)
+	}
+}
+
+// TestPrune_NeverPrunesPBS is the scope rule (§9): retention is NOT applied when the target is a PBS
+// storage (offsite retention is a separate lifecycle). Companion: the identical retention on a LOCAL
+// target IS applied — proving the gate keys on storage type, not luck.
+func TestPrune_NeverPrunesPBS(t *testing.T) {
+	api := okAPI(pbsTargetStores)
+	r := NewBackupRunner(api, "felhom-pbs", proxmox.ModeStop, "", "keep-last=3", quiet())
+	_, _ = r.Backup(context.Background(), 9201)
+	if api.vzdumps[0].PruneBackups != "" {
+		t.Fatalf("a PBS target must NEVER be pruned by the per-run flag, got %q", api.vzdumps[0].PruneBackups)
+	}
+
+	// COMPANION: same retention, local target → applied.
+	api2 := okAPI(localTargetStores)
+	r2 := NewBackupRunner(api2, "local", proxmox.ModeStop, "", "keep-last=3", quiet())
+	_, _ = r2.Backup(context.Background(), 9201)
+	if api2.vzdumps[0].PruneBackups != "keep-last=3" {
+		t.Fatalf("control: a local target with the same retention MUST be pruned, got %q", api2.vzdumps[0].PruneBackups)
+	}
+}
+
+// TestPrune_FailSafeOnUnknownTarget: if the target's type can't be confirmed (lookup error / not in the
+// list), the run SKIPS pruning rather than risk pruning a PBS/unknown storage.
+func TestPrune_FailSafeOnUnknownTarget(t *testing.T) {
+	// target not present in the list → skip.
+	api := &fakeBackupAPI{vzdumpUPID: "UPID:vzdump:1", storages: localTargetStores}
+	r := NewBackupRunner(api, "some-other-store", proxmox.ModeStop, "", "keep-last=3", quiet())
+	_, _ = r.Backup(context.Background(), 9201)
+	if api.vzdumps[0].PruneBackups != "" {
+		t.Fatalf("an unknown target must skip pruning (fail-safe), got %q", api.vzdumps[0].PruneBackups)
+	}
+}
+
+// TestPrune_KeepLastClamp (§7-B): a 0/negative/unset LocalBackupRetention clamps to ≥1 (default 3) so the
+// vzdump NEVER prunes the archive it just made. Companion: a no-clamp impl that returns 0 would emit
+// keep-last=0 → PVE prunes everything → FAILS the "≥1" assertion.
+func TestPrune_KeepLastClamp(t *testing.T) {
+	cases := []struct {
+		set  int
+		want int
+	}{
+		{0, 3},   // unset → default
+		{-5, 3},  // negative → default
+		{1, 1},   // honored
+		{3, 3},   // honored
+		{10, 10}, // honored
+	}
+	for _, c := range cases {
+		b := config.BackupConfig{LocalBackupRetention: c.set}
+		if got := b.KeepLast(); got != c.want {
+			t.Errorf("KeepLast(%d) = %d, want %d", c.set, got, c.want)
+		}
+		if b.KeepLast() < 1 {
+			t.Fatalf("keep-last must NEVER be < 1 (would prune the fresh backup), got %d for %d", b.KeepLast(), c.set)
+		}
+	}
+	if spec := (config.BackupConfig{}).PruneBackupsSpec(); spec != "keep-last=3" {
+		t.Fatalf("default PruneBackupsSpec = %q, want keep-last=3", spec)
+	}
+}

internal/backup/runner.go

@@ -19,6 +19,8 @@ type BackupAPI interface {
 	WaitTask(ctx context.Context, upid string, opts proxmox.WaitOptions) (proxmox.TaskStatus, error)
 	GuestConfig(ctx context.Context, vmid int) (proxmox.GuestConfig, error)
 	StorageContent(ctx context.Context, store string) ([]proxmox.StorageContent, error)
+	// ListStorage enumerates storages (name+type) — used to scope local-only retention (never prune PBS).
+	ListStorage(ctx context.Context) ([]proxmox.Storage, error)
 	// TaskLogTail reads trailing task-log lines — used to read the ACTUAL vzdump mode
 	// (PVE may downgrade a requested snapshot to stop for a stopped guest — spike B1).
 	TaskLogTail(ctx context.Context, upid string, limit int) ([]string, error)
@@ -32,20 +34,51 @@ type BackupRunner struct {
 	target string             // backup storage (content=backup)
 	mode   proxmox.BackupMode // default ModeSnapshot
 	notes  string             // optional notes-template
-	logger *slog.Logger
-	now    func() time.Time
+	// retention is the per-run `--prune-backups` spec (e.g. "keep-last=3") applied to a LOCAL target after
+	// each successful backup, so the agent's own backups can't pile up and refill root. Empty → no prune
+	// (the legacy behaviour; restore-test/selftest runners pass ""). NEVER applied to a PBS target.
+	retention string
+	logger    *slog.Logger
+	now       func() time.Time
 }
 
 // NewBackupRunner builds a runner. mode defaults to snapshot (works for a stopped guest and
-// for lvm-thin); the caller may pass ModeStop for storages without snapshot support.
-func NewBackupRunner(api BackupAPI, target string, mode proxmox.BackupMode, notes string, logger *slog.Logger) *BackupRunner {
+// for lvm-thin); the caller may pass ModeStop for storages without snapshot support. retention is the
+// per-run prune spec ("keep-last=N", or "" to never prune) — only the periodic local backup sets it.
+func NewBackupRunner(api BackupAPI, target string, mode proxmox.BackupMode, notes, retention string, logger *slog.Logger) *BackupRunner {
 	if mode == "" {
 		mode = proxmox.ModeSnapshot
 	}
 	if logger == nil {
 		logger = slog.Default()
 	}
-	return &BackupRunner{api: api, target: target, mode: mode, notes: notes, logger: logger, now: func() time.Time { return time.Now().UTC() }}
+	return &BackupRunner{api: api, target: target, mode: mode, notes: notes, retention: retention, logger: logger, now: func() time.Time { return time.Now().UTC() }}
+}
+
+// localPruneSpec returns the `--prune-backups` spec to apply to THIS backup, or "" to skip pruning. It
+// applies the configured retention ONLY when the target is a non-PBS storage — PBS offsite retention is a
+// separate lifecycle and must never be pruned by the per-run flag (§9). Fail-safe: if the target's type
+// can't be confirmed (lookup error / not found), it SKIPS pruning rather than risk pruning PBS — the
+// host_disk + storage_fill detectors remain the safety net.
+func (r *BackupRunner) localPruneSpec(ctx context.Context) string {
+	if r.retention == "" {
+		return ""
+	}
+	stores, err := r.api.ListStorage(ctx)
+	if err != nil {
+		r.logger.Warn("backup: could not resolve target storage type — skipping local prune this run", "target", r.target, "err", err)
+		return ""
+	}
+	for _, s := range stores {
+		if s.Storage == r.target {
+			if s.Type == "pbs" {
+				return "" // PBS retention is out of scope — never prune the offsite DR
+			}
+			return r.retention
+		}
+	}
+	r.logger.Warn("backup: target storage not found in list — skipping local prune this run (fail-safe)", "target", r.target)
+	return ""
 }
 
 // snapshotMarker is the vzdump task-log line that signals the storage snapshot has been created
@@ -99,6 +132,7 @@ func (r *BackupRunner) backup(ctx context.Context, vmid int, onSnapshot func())
 
 	upid, err := r.api.Vzdump(ctx, proxmox.VzdumpOptions{
 		VMID: vmid, Storage: r.target, Mode: r.mode, Notes: r.notes,
+		PruneBackups: r.localPruneSpec(ctx), // local target → keep-last=N; PBS/unknown → "" (no prune)
 	})
 	if err != nil {
 		rec.Error = err.Error()

internal/backup/runner_snapshot_test.go

@@ -22,7 +22,7 @@ func TestBackupWithSnapshotHook_FiresOnMarker(t *testing.T) {
 		logLines:   []string{"INFO: backup mode: snapshot", "INFO: create storage snapshot 'vzdump'"},
 		content:    []proxmox.StorageContent{{VolID: "local:backup/vzdump-lxc-9001-x", Content: "backup", VMID: 9001, Size: 100, CTime: 1}},
 	}
-	r := NewBackupRunner(api, "local", "", "", quiet())
+	r := NewBackupRunner(api, "local", "", "", "", quiet())
 
 	var fired int32
 	done := make(chan struct{})
@@ -55,7 +55,7 @@ func TestBackupWithSnapshotHook_StopMode_NeverFires(t *testing.T) {
 		logLines:   []string{"INFO: backup mode: stop"}, // downgraded; no snapshot marker
 		content:    []proxmox.StorageContent{{VolID: "local:backup/vzdump-lxc-9001-x", Content: "backup", VMID: 9001, Size: 100, CTime: 1}},
 	}
-	r := NewBackupRunner(api, "local", "", "", quiet())
+	r := NewBackupRunner(api, "local", "", "", "", quiet())
 
 	var fired int32
 	done := make(chan struct{})