diff --git a/CHANGELOG.md b/CHANGELOG.md index 7167222..01e3bbd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,16 @@ +## Unreleased — the orphan thin check runs only after a failed backup (R-1012 follow-up) (2026-10-17) + +**Not released, not vouched; the next vouched agent release carries it.** v0.159.0/v0.159.1 ran the R-1012 orphan check +before EVERY snapshot-mode whole-guest backup, and that check takes a short metadata snapshot of the live thin pool. +Now `BackupRunner` runs it only when the snapshot step has failed (the vzdump start or its task) — the moment the +answer is needed. The finding is still appended in plain words to the failed backup's error, exactly as before; a +successful backup never runs the check and keeps a clean record; stop mode and an unwired runner are unchanged. The +`thin-orphan-check` capability text says „after a failed whole-guest backup". Tests +`TestBackup_SnapshotFailure_NamesTheOrphan` (now: exactly 1 call, after the failure) and +`TestBackup_ThinCheck_SilentWhenNotApplicable` (now: 0 calls on a successful backup), red-proved against the v0.159.1 +`runner.go`: failure legs „thin check ran 2 times, want 1", success leg `calls=1`. No root file, sudoers or bundle +change. (Installer side, felhom.eu, same night: `felhom-host-install.sh --rotate-recovery-only`, R-1141.) + ## v0.159.1 — the signed decommission prunes the guest-bind record (found by v0.159.0's own live proof) (2026-10-17) **v0.159.0 is superseded — do not vouch it.** Everything of v0.159.0 below, plus: `signedjobs.DecommissionExecutor` diff --git a/REPORT.md b/REPORT.md index 89f3f29..24bc18f 100644 --- a/REPORT.md +++ b/REPORT.md @@ -1,22 +1,41 @@ -# REPORT — agent 0.159.1: forget a gone decommissioned drive (R-1124), name an orphan thin device (R-1012), Go 1.27.2 (2026-10-17) +# REPORT — the orphan thin check runs only after a failed backup (unreleased) (2026-10-17, night) -Written in ASD-STE100 Simplified Technical English. Session report: felhom.eu `REPORT-agent-allowlist-rotation-2026-10-17.md`. -Evidence: felhom.eu `documentation/audits/agent-allowlist-rotation-2026-10-17/P1/`. +Written in ASD-STE100 Simplified Technical English. Architecture read: felhom.eu `documentation/architecture/03-host-agent.md` +(the `FELHOM_THINCHECK` row), `runbooks/whole-guest-backup-orphan-thin.md`, R-1012's closing note. -- **Released:** v0.159.0 (superseded — do not vouch) and **v0.159.1** (sha256 `de8f6ad9…aff9`, bundle `2af3d27d…7cd6`). - Step bundle `0.159.0-step1` (`1d65edb3…d6cf`) for a box on the v0.158.1 bundle (the new bundle adds `felhom-thin-check`). -- **R-1124:** `ForgetRetiredDrives` (start + 20-s tick) disables and removes, through the new root verb - `felhom-priv-apply unit-forget`, the unit of a DECOMMISSIONED drive that is absent and unmounted. A present one keeps its - unit (one-click re-enroll). v0.159.1: the signed decommission also prunes the guest-bind record. -- **R-1012:** read-only root helper `felhom-thin-check `; `internal/thincheck` (two reads, fails open); - `BackupRunner.SetThinCheck` appends the plain-word finding to a failed snapshot backup's error. Never removes. -- **Proof on demo-hp:** loop-backed test drive forgotten 8 s after the unplug; on 0.159.1 the bind-record warning stopped - (0 lines, control 3); scratch thin pool: a made orphan listed, a foreign metadata snapshot refused; `pve/data` read and - released; a 9201 whole-guest backup ran the check first and finished green. All red-proofs in P1. -- **Delivery:** demo-hp done (0.159.1 + bundle, 75/75). Tester 1 and demo-felhom: by signed jobs on the operator's yes. - **The vouch is the operator's.** +## In plain words -## Observations +- Before tonight the agent looked for an orphan thin device before **every** whole-guest backup. To look, it takes a + short metadata snapshot of the live disk pool. Now it looks **only when the backup's snapshot step has failed** — the + only moment somebody needs the answer. The operator's failure mail still names the orphan, in the same words. +- **Not released. Not vouched.** The next agent release that the operator vouches carries it. Nothing was copied to a + box. -1. The operator-signed decommission did not prune the guest-bind record (the self-serve one did). NOT-A-FINDING: fixed in this session in v0.159.1 (the size rule), test red-proved. -2. A loop device reads as role „system", so the self-serve decommission refuses it. NOT-A-FINDING: correct (fail safe to protected); the live proof used the operator-signed path. +## What changed + +- `internal/backup/runner.go`: the pre-run check (and its WARN line) is gone; `explainFailure` (after a vzdump start + failure or a task failure, snapshot mode only) is now the only caller. Comments say so. +- `internal/capability/manifest.go`: the `thin-orphan-check` text says „after a failed whole-guest backup". +- `internal/backup/r1012_thincheck_test.go`: failure → exactly ONE call and the finding in the error; success → ZERO + calls and a clean record; stop mode → zero calls; unwired → error unchanged. + +## Red-proof + +The new tests, run against the v0.159.1 `runner.go` (`git show HEAD:…`), then the fix restored: + +| test | failed with | +|---|---| +| `TestBackup_SnapshotFailure_NamesTheOrphan/{task_fails,start_fails}` | `thin check ran 2 times, want 1 (after the failure only)` | +| `TestBackup_ThinCheck_SilentWhenNotApplicable` | `success: … Success:true … calls=1 (want no check on a successful backup, clean record)` | + +Restored → both PASS; `git diff` on `runner.go` = the fix only. + +## Gates + +`go build ./... && go vet ./... && go test ./...` and `python3 scripts/agent_gates.py` — results in the session report. + +## Not live-validated + +No agent was built or delivered (no release tonight). The changed path is the failure path; the next release's live +proof should make one snapshot backup succeed (helper NOT called: no `felhom-thin-check` line in the agent journal +and none in the sudo log for that run) and one fail (the finding in the failure text). diff --git a/internal/backup/r1012_thincheck_test.go b/internal/backup/r1012_thincheck_test.go index 571d6bb..20a5798 100644 --- a/internal/backup/r1012_thincheck_test.go +++ b/internal/backup/r1012_thincheck_test.go @@ -10,8 +10,9 @@ import ( ) // R-1012: a snapshot backup that fails carries the orphan finding in its error — the text the controller mails to -// the operator (`whole_guest_backup_failed`). The check runs once before the run and once, FRESH, after the failure. -// RED-PROOF (P1/redproof-thincheck.txt): with explainFailure's append removed this test fails. +// the operator (`whole_guest_backup_failed`). The check runs ONCE, after the failure — never before the run. +// RED-PROOF (P1/redproof-thincheck.txt): with explainFailure's append removed this test fails. RED-PROOF (2026-10-17 +// night, felhom-agent REPORT.md): with the v0.159.0 pre-run check restored, calls = 2 and this test fails. func TestBackup_SnapshotFailure_NamesTheOrphan(t *testing.T) { const why = "probable cause: thin pool pve/data holds device 4 that LVM does not know" for name, api := range map[string]*fakeBackupAPI{ @@ -29,15 +30,17 @@ func TestBackup_SnapshotFailure_NamesTheOrphan(t *testing.T) { if !strings.Contains(rec.Error, why) { t.Fatalf("the failure does not name the orphan: %q", rec.Error) } - if calls != 2 { - t.Fatalf("thin check ran %d times, want 2 (before the run and after the failure)", calls) + if calls != 1 { + t.Fatalf("thin check ran %d times, want 1 (after the failure only)", calls) } }) } } -// A successful backup keeps a clean record (the pre-check only logs); a stop-mode backup takes no snapshot, so the -// check never runs; an unwired runner behaves exactly as before. +// A successful backup never runs the check (it takes a metadata snapshot of the live pool, and nobody needs the answer) +// and keeps a clean record; a stop-mode backup takes no snapshot, so the check never runs; an unwired runner behaves +// exactly as before. RED-PROOF (2026-10-17 night): with the v0.159.0 pre-run check restored, the success leg fails +// with calls=1. func TestBackup_ThinCheck_SilentWhenNotApplicable(t *testing.T) { ok := &fakeBackupAPI{vzdumpUPID: "UPID:x", content: []proxmox.StorageContent{{VolID: "local:backup/vzdump-lxc-9201-x", Content: "backup", VMID: 9201, Size: 100, CTime: 1}}} @@ -45,8 +48,8 @@ func TestBackup_ThinCheck_SilentWhenNotApplicable(t *testing.T) { calls := 0 r.SetThinCheck(func(context.Context, int) string { calls++; return "probable cause: x" }) rec, err := r.Backup(context.Background(), 9201) - if err != nil || !rec.Success || rec.Error != "" || calls != 1 { - t.Fatalf("success: err=%v rec=%+v calls=%d (want one pre-check, clean record)", err, rec, calls) + if err != nil || !rec.Success || rec.Error != "" || calls != 0 { + t.Fatalf("success: err=%v rec=%+v calls=%d (want no check on a successful backup, clean record)", err, rec, calls) } stop := NewBackupRunner(&fakeBackupAPI{vzdumpUPID: "UPID:x", waitErr: errors.New("boom")}, "local", proxmox.ModeStop, "", "", quiet()) diff --git a/internal/backup/runner.go b/internal/backup/runner.go index 9bc8f1e..db4f962 100644 --- a/internal/backup/runner.go +++ b/internal/backup/runner.go @@ -62,8 +62,9 @@ type BackupRunner struct { // this target, so a leftover from a power cut cannot hold the space this backup needs. nil = nothing. beforeBackup func(ctx context.Context, storage string, vmid int) // thinCheck (R-1012) names an orphan thin device in the guest's pools in plain words ("" = none / not checked). It - // runs before a snapshot-mode backup (a WARN in the log) and again when the vzdump fails, so the failure the - // controller mails to the operator says why. It never removes anything. nil = not wired. + // runs ONLY when a snapshot-mode vzdump has failed (start or task), so the failure the controller mails to the + // operator says why; a successful backup never runs it (it takes a metadata snapshot of the live pool). It never + // removes anything. nil = not wired. thinCheck func(ctx context.Context, vmid int) string logger *slog.Logger now func() time.Time @@ -117,8 +118,8 @@ func (r *BackupRunner) SetBeforeBackup(fn func(ctx context.Context, storage stri // SetThinCheck wires the R-1012 orphan check (see BackupRunner.thinCheck). func (r *BackupRunner) SetThinCheck(fn func(ctx context.Context, vmid int) string) { r.thinCheck = fn } -// explainFailure appends the orphan finding to a failed snapshot-mode backup's error. A FRESH check (not the one from -// before the run): the failure itself may be what made the orphan. Bounded, so a stuck helper cannot hold the job. +// explainFailure appends the orphan finding to a failed snapshot-mode backup's error — the only place the check runs, so +// it sees the pool as the failure left it. Bounded, so a stuck helper cannot hold the job. func (r *BackupRunner) explainFailure(ctx context.Context, vmid int, rec *hub.Backup) { if r.thinCheck == nil || r.mode != proxmox.ModeSnapshot { return @@ -221,11 +222,8 @@ func (r *BackupRunner) backup(ctx context.Context, vmid int, onSnapshot func()) return rec, fmt.Errorf("backup: %s", rec.Error) } - if r.thinCheck != nil && r.mode == proxmox.ModeSnapshot { - if why := r.thinCheck(ctx, vmid); why != "" { - r.logger.Warn("backup: an orphan thin device may make this snapshot backup fail (R-1012)", "vmid", vmid, "finding", why) - } - } + // R-1012: no orphan check here. It takes a metadata snapshot of the live pool, and its answer is only needed when the + // snapshot step has failed — explainFailure runs it then (pinned by TestBackup_ThinCheck_SilentWhenNotApplicable). 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) diff --git a/internal/capability/manifest.go b/internal/capability/manifest.go index 3f1db5a..06035cf 100644 --- a/internal/capability/manifest.go +++ b/internal/capability/manifest.go @@ -91,7 +91,7 @@ var manifest = []Capability{ {"disk-mkfs-xfs", "guarded format (xfs)", "/usr/local/sbin/felhom-mkfs-guarded", []string{"/dev/sda", "xfs"}, false, ""}, {"disk-smart", "disk SMART health read", "/usr/sbin/smartctl", []string{"-a", "-j", "/dev/sda"}, false, ""}, {"disk-lvs", "thin-pool usage read", "/usr/sbin/lvs", []string{"--reportformat", "json", "--units", "b", "-o", "lv_name,data_percent,metadata_percent", "--", "pve/data"}, false, ""}, - {"thin-orphan-check", "orphan thin device check around a whole-guest backup (read-only, R-1012)", "/usr/local/sbin/felhom-thin-check", []string{"pve", "data"}, false, ""}, + {"thin-orphan-check", "orphan thin device check after a failed whole-guest backup (read-only, R-1012)", "/usr/local/sbin/felhom-thin-check", []string{"pve", "data"}, false, ""}, // ---- Storage mount units (watchdog re-mount) ---- {"mount-unit-install", "fs-UUID mount unit install (root content check, R-861)", "/usr/local/sbin/felhom-priv-apply", []string{"unit", "mnt-felhom\\x2dx.mount"}, false, ""},