diff --git a/CHANGELOG.md b/CHANGELOG.md index ef130fd..23f5c0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,28 @@ +## v0.51.0 — local vzdump retention default (`--prune-backups keep-last=3`) (2026-06-30) + +The PREVENTIVE counterpart to the hub's host_disk + storage_fill detectors: the agent's periodic local +whole-guest vzdump now prunes its own old archives, so a box can't refill its own root via its own backups +(the felhom-pve incident's root cause — that vzdump carried no retention, ~18 dumps piled under +`/var/lib/vz/dump`). + +- **`internal/proxmox/mutate.go`:** `VzdumpOptions.PruneBackups` → passed as PVE's `--prune-backups` on + the vzdump POST (vmid+storage scoped, so PVE prunes only THIS guest's archives on THIS storage). +- **`internal/backup/runner.go`:** `NewBackupRunner` gains a `retention` arg; the backup applies it via + `localPruneSpec` ONLY when the target is a **non-PBS** storage (resolved via `ListStorage`) — PBS offsite + retention is a separate lifecycle and is never pruned by the per-run flag. **Fail-safe:** if the target + type can't be confirmed (lookup error / not found) the run SKIPS pruning rather than risk pruning PBS + (the detectors remain the safety net). Only the periodic local-API runner sets retention; the + restore-test / selftest runners pass "". +- **`internal/config/config.go`:** `backup.local_backup_retention` (keep-last N) with `KeepLast()` clamped + to **≥1** (0/unset/negative → default 3) — a mis-config can NEVER prune the just-made backup — + + `PruneBackupsSpec()` → `keep-last=N`. Wired into the local-API backup runner (`main.go`). +- **Seeding:** `felhom.eu scripts/felhom-host-install.sh` seeds `local_backup_retention: 3` in the agent + config; the code default also protects any box where it is unset (KeepLast → 3) from day 0. +- F2-b stale-vzdump-lock recovery untouched. +- Tests: the local vzdump carries `--prune-backups keep-last=3` (+ companion: no-retention runner emits no + prune); **PBS is never pruned** (+ companion: same retention on a local target IS applied); + fail-safe-on-unknown-target; the **keep-last≥1 clamp** companion. `go build/vet/test ./...` green. + ## v0.50.0 — NAS network storage Part A1: NFS/SMB automount foundation (2026-06-30) Agent foundation of the validated `SPIKE-nas-storage-2026-06-29.md` (verdict READY): a customer NAS can diff --git a/cmd/felhom-agent/main.go b/cmd/felhom-agent/main.go index 6a46cd2..a884428 100644 --- a/cmd/felhom-agent/main.go +++ b/cmd/felhom-agent/main.go @@ -45,7 +45,7 @@ import ( // version is the agent version. Overridable at build time with // -ldflags "-X main.version="; defaults to the in-repo CHANGELOG version. -var version = "0.50.0" +var version = "0.51.0" // runGuestHook is the PVE pre-start hook body (`felhom-agent guest-hook `). On the // pre-start phase it creates placeholder dirs for any absent bind-mount source so the guest always boots @@ -681,7 +681,7 @@ func buildRestoreTestScheduler(cfg config.Config, px *proxmox.Client, engine *re } min, max := cfg.Backup.ScratchBand() target := cfg.Backup.BackupTarget() - runner := backup.NewBackupRunner(px, target, "", "felhom restore-test", logger) + runner := backup.NewBackupRunner(px, target, "", "felhom restore-test", "", logger) return backup.NewScheduler(backup.SchedulerOptions{ Runner: engine, Pick: runner.PickRestoreCandidate, @@ -732,7 +732,7 @@ func buildLocalAPIServer(cfg config.Config, px *proxmox.Client, store *backup.St } // v0.48.0: ride the served leaf fp on every host report so the hub can detect a re-key fleet-wide. collector.SetLeafFingerprint(fp) - runner := backup.NewBackupRunner(px, cfg.Backup.BackupTarget(), "", "felhom local-api", logger) + runner := backup.NewBackupRunner(px, cfg.Backup.BackupTarget(), "", "felhom local-api", cfg.Backup.PruneBackupsSpec(), logger) // Guest data-drive passthrough (slice 10 P2): a root-CLI runner for the `pct set` bind + chown // (same fenced ExecRunner the host-storage + provision back-half use). gaMode := proxmox.RunnerMode(cfg.Privileged.Mode) @@ -1052,7 +1052,7 @@ func runSelftestBackup(ctx context.Context, cfg config.Config, logger *slog.Logg defer cancel() fmt.Printf("=== felhom-agent %s selftest=backup (vmid %d → %s) ===\n", version, vmid, target) - runner := backup.NewBackupRunner(px, target, "", "felhom selftest", logger) + runner := backup.NewBackupRunner(px, target, "", "felhom selftest", "", logger) rec, err := runner.Backup(ctx, vmid) printJSON("backup record", rec) if err != nil { @@ -1108,7 +1108,7 @@ func runSelftestRestoreTest(ctx context.Context, cfg config.Config, logger *slog target := cfg.Backup.BackupTarget() if archive == "" { - runner := backup.NewBackupRunner(px, target, "", "", logger) + runner := backup.NewBackupRunner(px, target, "", "", "", logger) archive, err = runner.PickRestoreCandidate(ctx) if err != nil { fmt.Fprintln(os.Stderr, " [FAIL] pick backup:", err) diff --git a/internal/backup/backup_test.go b/internal/backup/backup_test.go index 9924e50..6fe78e5 100644 --- a/internal/backup/backup_test.go +++ b/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) diff --git a/internal/backup/prune_test.go b/internal/backup/prune_test.go new file mode 100644 index 0000000..19aa819 --- /dev/null +++ b/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) + } +} diff --git a/internal/backup/runner.go b/internal/backup/runner.go index 0c64a4f..553debb 100644 --- a/internal/backup/runner.go +++ b/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() diff --git a/internal/backup/runner_snapshot_test.go b/internal/backup/runner_snapshot_test.go index fd15bd3..f1d5ba9 100644 --- a/internal/backup/runner_snapshot_test.go +++ b/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{}) diff --git a/internal/config/config.go b/internal/config/config.go index e269032..67590cd 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -181,6 +181,31 @@ type BackupConfig struct { // its newest successful backup is older than this (or none exists). 0 → default (24h). The // hub-served per-guest policy is slice 10; this is the agent-local cadence. BackupCadenceSeconds int `json:"backup_cadence_seconds"` + + // LocalBackupRetention is keep-last=N for the per-run `--prune-backups` on a LOCAL vzdump target — + // so the agent's own local whole-guest backups can't pile up and refill root (the felhom-pve incident; + // the host_disk + storage_fill checkers are the detectors, this is the preventive default). 0/unset → + // default 3; ALWAYS clamped to ≥1 by KeepLast() so a mis-config can never prune the fresh backup. + // NEVER applied to a PBS target (offsite retention is a separate lifecycle). + LocalBackupRetention int `json:"local_backup_retention"` +} + +// defaultLocalBackupKeepLast is the local vzdump retention default (newest N restore points kept). +const defaultLocalBackupKeepLast = 3 + +// KeepLast returns the effective local-backup keep-last, clamped to ≥1 (0/unset → default 3, negative → +// default). The clamp is load-bearing: keep-last=0 would tell PVE to prune EVERY archive, including the +// one just made — a mis-config must never self-destruct the fresh backup. +func (b BackupConfig) KeepLast() int { + if b.LocalBackupRetention < 1 { + return defaultLocalBackupKeepLast + } + return b.LocalBackupRetention +} + +// PruneBackupsSpec returns the PVE `--prune-backups` value for the local vzdump (e.g. "keep-last=3"). +func (b BackupConfig) PruneBackupsSpec() string { + return fmt.Sprintf("keep-last=%d", b.KeepLast()) } // BackupCadence returns the per-guest /backup/due window: positive as-is, else 24h default. diff --git a/internal/proxmox/mutate.go b/internal/proxmox/mutate.go index b874040..501b0ac 100644 --- a/internal/proxmox/mutate.go +++ b/internal/proxmox/mutate.go @@ -74,6 +74,10 @@ type VzdumpOptions struct { // Notes is the PVE `notes-template` for the backup (a template string PVE expands, // e.g. with {{guestname}}/{{node}}). Optional. Notes string + // PruneBackups is the PVE `--prune-backups` retention spec applied AFTER this backup, e.g. + // "keep-last=3". PVE prunes only THIS vmid's archives on THIS storage (the vzdump is vmid+storage + // scoped), so it never touches other guests/storages or PBS. Empty → no prune (legacy behaviour). + PruneBackups string } // Vzdump starts a backup via POST /nodes/{node}/vzdump. Returns the UPID. An @@ -94,6 +98,10 @@ func (c *Client) Vzdump(ctx context.Context, opts VzdumpOptions) (string, error) if opts.Notes != "" { v.Set("notes-template", opts.Notes) // PVE 9.x param name (verified on demo) } + if opts.PruneBackups != "" { + // Per-run retention: PVE prunes older archives of THIS vmid on THIS storage after the backup. + v.Set("prune-backups", opts.PruneBackups) + } return c.dataString(ctx, http.MethodPost, "/nodes/"+c.node+"/vzdump", v) }