diff --git a/CONTEXT.md b/CONTEXT.md index b9dc31b..cf06626 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -5,6 +5,23 @@ ## Current +- **2026-07-28 — v0.106.0: F-CRIT-2 fixed — a failed backup no longer looks like a fresh one.** + `NewestArchiveTime` counted an aborted PBS upload (1 byte, manifest-less, NEWEST) as a successful + backup, so the tier reported fresh, went **not due**, and was never retried — 7 days of silence on + the real 168h cadence, invisible to both the R-88 breaker (defers only DUE tiers) and the hub + deadline monitor (reads the same freshness). Now only *plausibly complete* entries count, via a + measured floor `minPlausibleArchiveBytes` = 1 MiB; undecidable ⇒ not counted. + **Size is the only tier-agnostic discriminator** — `verification` and `encrypted` are absent on + every local (dir) archive and on a good PBS snapshot until verify-new catches up, so gating on + either would reject 100% of local backups and cause fleet-wide backup THRASH. Floor measured: + smallest real backup on the fleet is 612,397,450 B, so 1 MiB leaves 584x headroom (asserted by a + test). Rejections logged at WARN once per volid. Re-tested live by replaying campaign fault 2 on + demo-hp — both directions, incl. a no-thrash window with 91 scheduler ticks as the positive + observable. Deployed on both boxes. Detail: `REPORT.md`. + **Also established:** server-side prune does NOT count phantoms toward `keep-last` (dry-run kept + 2 real + the phantom) ⇒ **no retention/data-loss bug** — but it never removes them either, so they + accumulate. Filed as R-99 (LOW). + - **2026-07-25 — v0.95.0 (additive): SMART coverage fixes (spike B+A) + device model.** Union-path drives (USB/registry) now get SMART via `storage.SmartReader.SMARTForBacking` wired into the localapi `/disks` union (localapi `Smart` seam); `smartDeviceFor` resolves dm/LVM to the whole disk via diff --git a/REPORT.md b/REPORT.md index ec5028f..dec0eb9 100644 --- a/REPORT.md +++ b/REPORT.md @@ -1,70 +1,140 @@ -# REPORT — R-88 Part 2: the agent can say "unknown" (v0.105.0) (2026-07-27) +# REPORT — F-CRIT-2: a failed backup must not look like a fresh one (v0.106.0) (2026-07-28) -**Overwritten** per the standing rule. Agent **v0.104.0 → v0.105.0**. Producer half of a wire -contract; the controller consumer is v0.178.0. +**Overwritten** per the standing rule. Agent **v0.105.0 → v0.106.0**. Campaign 8 finding F-CRIT-2, +fixed and **re-tested live by replaying the original injection**. Scope: `felhom-agent` only. No +wire/contract change; the controller needed no change. -## What was broken +## The defect -`newestArchiveOn` promised in its own doc comment that *"errors and unsupported services degrade to -'unknown', never to 'no backup'"*. Its `(time.Time, bool)` signature made that **impossible**: an -error and a genuine not-found both returned `(zero, false)`, so `/backup/due` emitted a POSITIVE -`"no successful backup recorded yet"` with a nil age. The controller read that as *never backed up*, -fired its window-gate safety valve, and quiesced customer app stacks **outside** the backup window — -what happened during the 2026-07-27 PBS outage. The comment described an intent the type forbade. +`NewestArchiveTime` (`internal/backup/runner.go`) selected the newest storage entry with +`e.Content == "backup" && e.VMID == vmid` and **no completeness test**. A PBS daemon killed +mid-upload leaves an aborted snapshot that PBS publishes into the same listing: manifest-less, +**1 byte, and NEWEST**. Counting it made the tier report freshly backed up, so it went **not due** +and was never retried — seven days of silence on the real 168h cadence, with the R-88 breaker +(defers only *due* tiers) and the hub deadline monitor (reads the same freshness) both blind to it. -## The wire encoding, and how the zero value is handled +R-84's *"ask the storage, it is ground truth"* was right. The bug is that **presence was taken for +validity** — and a phantom is a more convincing lie than an empty array, because absence at least +reads as absence. -**A string enum**, matching the repo's existing convention (`phase` on `/backup/status`): +## Phase 0 — the discriminator survey + +The campaign's own phantom was gone (Phase D deleted it deliberately to stop the suppression), so +one was recreated on demo-hp by replaying fault 2. Raw PVE API, captured live: ``` -age_state: "known" | "absent" | "unknown" (omitted entirely by a pre-v0.105.0 agent) +PHANTOM: {"content":"backup","ctime":1785216674,"format":"pbs-ct","size":1,"subtype":"lxc", + "vmid":9201,"volid":"felhom-pbs:backup/ct/9201/2026-07-28T05:31:14Z"} +GOOD: {... "encrypted":"3f:4f:65:...","notes":"felhom local-api","size":4353457559, + "verification":{"state":"ok","upid":"..."} ...} ``` -The zero value is `""`, and it means **"legacy agent, no information"** — never "unknown". A `bool` -would have made the legacy case indistinguishable from a real `false`, which is the trap the task -named. The controller maps `""` (and any unrecognised future value) to `AgeStateLegacy` explicitly. +| candidate | phantom | good PBS | good **local** (dir) | verdict | +|---|---|---|---|---| +| `verification` | **absent** | present | **absent** | UNUSABLE — would reject every local backup | +| `encrypted` | absent | present | **absent** | UNUSABLE — same | +| `notes` | absent | present | present | too fragile (agent-set only) | +| `size` | **1 B** | 4.35 GB | 1.59 GB | **robust, tier-agnostic** | -Internally `archiveLookup` (`found`/`absent`/`unknown`) replaces the old bool, so the doc comment's -promise is now something the type can actually express. +`verification` is absent on the phantom **and** on every local archive **and** on a good PBS +snapshot until `verify-new` catches up — gating on it would have caused fleet-wide backup thrash. +A PBS-level manifest query is not reachable from `BackupRunner` (PVE-only interface, tier-agnostic). -## Two decisions worth stating +## The fix -**The fail-safe direction is unchanged: unknown is still DUE.** An agent that cannot read the storage -must never suppress a backup. Only the *window-gate bypass* narrows. +`minPlausibleArchiveBytes = 1 MiB`, measured not chosen — fleet survey 2026-07-28: -**A service with NO lister deliberately stays `absent`, not `unknown`.** This broke a pre-existing -test (`TestBackupDue_ServiceWithoutLister_UnchangedBehaviour`) and the test was right: calling it -"unknown" would stop the controller firing its first-backup valve on a pre-R-84 build, so a genuinely -new box would never back up outside its window and nobody would notice for weeks. On that path the -in-memory record is the only registry that exists, so its absence means "no backup recorded" in the -only terms available. `unknown` is reserved for a lister that was asked and could not answer. - -An **unparseable** in-memory timestamp is now `unknown` too — a backup did happen, we simply cannot -date it. It previously fell through to the same positive "never" claim. - -## Tests — 29 packages ok, 0 failed; `build`/`vet`/`test` each rc=0, run separately, vet unpiped - -+5. Red-proofs observed and restored, in both directions: - -| Mutation | Observed | +| | bytes | |---|---| -| error path → `archiveAbsent` (the pre-fix collapse) | `an unreadable storage must report age_state="unknown", got "absent" — that is a POSITIVE claim of 'never backed up' built out of two absences` | -| `!found` → `archiveUnknown` (the over-correction) | `a genuine never-backed-up tier must report age_state="absent", got "unknown" — the controller only licenses a first backup outside the window on ABSENT` | +| smallest **real** backup anywhere | **612,397,450** (~584 MiB) | +| demo-hp local / PBS | 1.59 GB / 4.35–4.37 GB | +| demo-felhom local / PBS | 5.82–5.84 GB / 14.47–14.51 GB | +| phantom | **1** | -## Deployed — producer BEFORE consumer +1 MiB sits 584× below the smallest real backup and 1,048,576× above the phantom. +`TestMinPlausibleArchiveBytes_LeavesHeadroomBelowTheSmallestRealBackup` asserts that headroom so +the floor cannot be quietly raised into the thrash zone. -demo-hp then demo-felhom, both `felhom-agent 0.105.0`, service active, previous binary kept as -`.bak-0.104.0`. **Verified on the live wire** from inside guest 9201, over the exact route the -controller calls: +**Fail-safe direction:** undecidable ⇒ **not** counted. Erring toward "less fresh" costs one extra +backup; counting an undecidable entry is the defect. +**Rejections are never silent:** WARN, once per distinct volid (not per 5-minute poll — ~288 +lines/day would bury it), naming the snapshot, its size and the reason. + +## Red-proofs — all four observed failing + +| red-proof | observed failure | +|---|---| +| remove the completeness test | `got ctime 1785216674, want 1785210042` · `the 1-byte manifest-less phantom set tier freshness — this is F-CRIT-2` | +| reject everything (Scenario D) | `a REAL local dir vzdump ... backup was rejected — this is the backup-thrash path, not extra safety` | +| flip the fail-safe (unknown ⇒ complete) | `size 0 counted as a complete backup — undecidable must fail safe` | +| remove the once-per-volid dedupe | `rejection logged 5 times across 5 polls, want exactly 1` · `got 6 rejection lines for 2 distinct phantoms` | + +`go build`, `go vet ./...`, `go test ./...` — all green (run separately from the commit). + +## §5 — LIVE re-test of the campaign injection (demo-hp) + +Fault 2 replayed against the **fixed** agent: PBS daemon killed mid-upload, phantom created +(`2026-07-28T05:31:14Z`, 1 byte, no `index.json.blob`). The fix does not prevent the phantom — PBS +creates it — it stops it *counting*. + +**Rejection logged, naming the snapshot:** ``` -tier=local due=True age_state=known age_seconds=91097 reason=older than cadence -tier=felhom-pbs due=False age_state=known age_seconds=100172 reason=within cadence window +level=WARN msg="backup: ignoring an INCOMPLETE archive when computing tier freshness — it is not a successful backup" + target=felhom-pbs vmid=9201 volid=felhom-pbs:backup/ct/9201/2026-07-28T05:31:14Z size_bytes=1 + reason="size 1 B is below the 1048576 B plausibility floor — an aborted/incomplete archive, not a successful backup" ``` +Logged **once across 8 due-checks**. (It appears twice in the day's journal only because the agent +was restarted for the cadence change below — a process restart clears the in-memory dedupe, which +is the documented behaviour.) -## NOT yet live-validated +**The discriminating test.** PBS cadence set to 1h so the two readings diverge: the phantom (24 min +old) would read *not due*; the genuine backup (2h15m old) reads *due*. Pre-fix the tier stays +silent; post-fix it must back up. +``` +05:53:17 [quiesce] backup due on 1 tier(s) — quiescing 1 stack(s): [paperless-ngx] +05:53:24 [quiesce] tier felhom-pbs: backup job backup-9201-felhom-pbs-1785218004218916301 started — polling +05:54:35 [quiesce] tier felhom-pbs: backup job ... done +agent: backup: completed target=felhom-pbs archive=felhom-pbs:backup/ct/9201/2026-07-28T05:53:24Z size_bytes=4359968099 +``` +That single observation proves all three: the tier is **not** reported fresh, its age comes from +the **genuine** backup, and it **is** due and attempted. -- **No `unknown` state has ever been observed on real hardware.** Both boxes report `known`; the - unknown path is unit-proven only, and reproducing it live means making PBS unreadable, which must - not be done to ep0. -- The `absent` path is likewise unobserved live — it needs a genuinely fresh tier. +**The inverse — no thrash.** After the successful backup, `backup job started` held at **3 → 3** +across the following poll cycles, with **91 controller scheduler ticks** in the window proving the +loop was alive (not silence from a dead loop). demo-felhom, which has no phantom, logged **0** +rejections and **0** errors. + +## Deployed + +`felhom-agent 0.106.0` on **demo-hp** and **demo-felhom**; both `active`, real cadences restored +(`local 0` ⇒24h, `restore_test 302400`, `felhom-pbs 604800`). demo-felhom 15/15 containers healthy, +demo-hp 8/8, zero unhealthy. Previous binaries kept as `/usr/local/bin/felhom-agent.bak-0.105.0`. + +## Established while investigating — prune does NOT count phantoms + +A `keep-last 2` dry-run against three real snapshots plus the phantom retained **two real ones plus +the phantom**: +``` +2026-07-27T21:00:03Z remove 2026-07-27T23:09:55Z keep +2026-07-28T03:40:42Z keep 2026-07-28T05:31:14Z keep ← PHANTOM +``` +So the feared "one phantom ⇒ one real backup / two phantoms ⇒ zero" **does not occur — there is no +retention or data-loss bug.** But prune never removes phantoms either, so they accumulate one per +aborted upload, forever. Filed separately as **R-99** (hygiene, LOW), not as a retention bug. + +## Left in place, deliberately + +The demo-hp phantom (`2026-07-28T05:31:14Z`) is **not** deleted. It is now demonstrably harmless +(ignored, and announced once) and serves as a live regression fixture. No automatic phantom cleanup +was added — deletion on a customer datastore is a separate ruling. + +## NOT yet live-validated (carried forward from Campaign 8, minus what this closes) + +- **F-CRIT-1** — an app that fails to restart after a quiesce still never alarms. Untouched here. +- Fault 4 — restic transport interruption; four injection approaches defeated by guest-bridged + networking. **Still the most valuable follow-up**, because this fix answers the phantom question + for PBS and leaves the analogous restic question open: does an interrupted restic run leave a + partial pack that later reads as success? +- Fault 12 (host reboot mid-backup); the three-way backup+restore-test+GC overlap; the agent's own + DR bring-up path (`ModeDRGuestLoss`), still read-only-verified.