diff --git a/REPORT.md b/REPORT.md index 6bf6f1f..4148ea3 100644 --- a/REPORT.md +++ b/REPORT.md @@ -1,211 +1,92 @@ -# REPORT — v0.97.0: R-82 Slice A — per-target backup tiers (2026-07-26) +# REPORT — R-82: per-target backup tiers, agent v0.97.0 → v0.102.0 (2026-07-26) -Slice A of R-82 (the backup target split). **Mechanism only — no box's behaviour changes until a -`backup_targets` entry is added to its config, which is Slice D.** An untouched config resolves to -exactly one tier and behaves byte-identically to v0.96.0. +**Overwritten** per the standing rule. Live on **demo-felhom** and **demo-hp**. +Full cross-repo arc + the Phase-0 gates: `felhom.eu/REPORT.md`. -Phase-0 gates: `felhom.eu/documentation/audits/SPIKE-r82-phase0-2026-07-26.md`. -Operator ruling 2026-07-26: datastore growth deferred, proceed. Capacity state recorded in -`felhom.eu/documentation/architecture/07-backup-architecture.md` §9.1. +## Versions in this arc -**STOPPED at the Slice A boundary as specified.** Slices B/C/D/E not started. - ---- - -## 1. What was wrong - -`BackupTarget()` returned ONE string and `BackupCadence()` ONE 24 h window. "Local daily **and** PBS -weekly" was not expressible at all — which is why the DR promise is unbacked today: demo-felhom holds -a single PBS snapshot from 2026-07-18 (a healing artifact, not a job) and demo-hp has **zero, ever**, -despite `pbs_dr` reporting `applied` since 07-21. - -## 2. Files modified - -| File | Change | +| | | |---|---| -| `internal/config/config.go` | `BackupTargetConfig`, `BackupConfig.ExtraTargets` (`backup_targets`), `BackupTier`, `BackupTiers()` | -| `internal/localapi/server.go` | per-target due/status/POST, `?target=` resolution, `/backup/tiers`, jobs keyed by (vmid,target), per-tier job ids | -| `internal/localapi/backup_tiers.go` | **NEW** — `normalizeBackupTiers` (the compatibility core) | -| `cmd/felhom-agent/main.go` | one runner per tier; tier-rejection warnings logged at **ERROR** | -| `internal/config/backup_tiers_test.go` | **NEW** — 7 tests | -| `internal/localapi/backup_tiers_test.go` | **NEW** — 13 tests | -| `CHANGELOG.md`, `REUSE.md` | v0.97.0 entry; new canonical-helper row | +| **v0.97.0** | Slice A — per-target cadence/retention, `/backup/tiers`, per-tier due + runner + job keys | +| **v0.98.0** | the 30-minute false failure — per-tier vzdump wait bound | +| **v0.99.0** | operator rulings — 2-week offsite retention (scoped PBS prune opt-in) + one backup at a time per guest | +| **v0.100.0** | the restore tier comes from the ARCHIVE, not the configured target | +| **v0.101.0** | a leaked restore-test scratch can no longer auto-start (`onboot=0` at restore time) | +| **v0.102.0** | an unprovisioned tier DEFERS instead of firing at a missing storage | -Commit: **`739b3c3`** on `main`, pushed. +## Four defects found by RUNNING it, not reviewing it -## 3. The compatibility rule — how it is enforced +1. **v0.98.0** — a 41-minute PBS backup was recorded `success:false` at 30 minutes **while it was + still running**, and it later completed `TASK OK`. Worse than "the backup didn't happen": the + tier stays permanently due and the retry collides with the guest lock. +2. **v0.100.0** — a `felhom-pbs:` archive was classified `local` and got the 10-minute bound against + a 14.46 GB WAN restore, failing at 600 s. **A silent regression of the S4.1 fix** — the mechanism + was never removed; its *input* changed when `local_backup_target` was retargeted to `local`. A fix + keyed on *"the configured target"* stops holding the moment more than one target exists. +3. **v0.101.0** — a leaked scratch guest kept `onboot: 1`, so a host reboot would have started a + clone of the live guest. `onboot=0` is now set **at restore time**, because "after" is the path + that leaks. +4. **v0.102.0** — a tier fires at a storage that does not exist yet (a fresh box before DR + provisioning). Now deferred, going live with no restart once the storage appears. -The agent and controller deploy independently, so the untargeted contract is **frozen**: +## The frozen contract -- `BackupDueResponse.Target` / `BackupResponse.Target` / `BackupStatusResponse.Target` are all - `omitempty` and left **EMPTY** for an untargeted request → the response marshals to the pre-R-82 - **bytes**, not merely a compatible shape. Verified live: - `{"ok":true,"data":{"vmid":8200,"due":false,"reason":"within cadence window","age_seconds":7200}}` -- The untargeted verdict comes from the **primary tier only** — a fresh backup on another tier must - not satisfy it (own test). -- The **primary tier's job-id format is unchanged**; only additive tiers carry a target segment. -- An unknown `?target=` is a **400, never a silent fallback**. A controller asking about a tier this - agent does not serve must find out, not act on a different tier's freshness. -- `GET /backup/tiers` 404s on a pre-R-82 agent — that 404 is Slice B's designed capability probe. - -## 4. Design decisions, and why - -**One runner per tier** (not a target-parameterised runner). The runner holds target, mode, notes and -retention as immutable construction state, and `localPruneSpec` reads that retention — parameterising -a single runner by target would make it possible to pair tier A's target with tier B's retention. One -runner per tier makes each tier's policy structurally inseparable from its target. - -**A missing cadence is REJECTED, not defaulted.** Defaulting an extra tier to the 24 h local default -would quietly turn a weekly DR tier into a daily one and fill the 37.2 GB datastore. A tier whose -cadence you did not state is not a tier. `main.go` logs every rejection at **ERROR** — a silently -dropped backup tier is the "applied and empty" fault R-82 exists to fix. - -**`keep_last` unset = never prune.** The fail-safe: a DR tier must not start pruning itself because -someone forgot a field. Negative values clamp to the same. - -**Jobs keyed by (vmid, target).** This is what lets the weekly night run both backups inside ONE -quiesce window (Slice B). It also fixed a real defect — see §6. - -## 5. The Slice A.4 finding (Slice C depends on this) - -**A PBS-targeted vzdump will appear in BOTH arrays**, and this is expected, not a bug: - -- in `backups[]` as `Backup{TargetID:"felhom-pbs", Archive:"felhom-pbs:backup/ct/9201/…"}` — written - by the runner, recorded in the agent's in-memory store; -- in `pbs_snapshots[]` — enumerated **independently** from the PBS API by the agent's verify loop. - -**Double-counting is harmless for freshness** (the hub takes a max, which is idempotent). -**Mis-attribution is not.** If Slice C treats `backups[]` as "the local tier" and `pbs_snapshots[]` -as "the PBS tier", a PBS-target `Backup` record would be attributed to the local tier and make a -**stale local tier look fresh**. - -**The rule Slice C must implement:** classify a `backups[]` entry by joining its `target_id` to -`storage_targets[].name` and reading `.type == "pbs"` — **by target type, never by array membership.** - -Verified live that the discriminator is present in production reports: +Untargeted `/backup/due`, `POST /backup` and `/backup/status` keep the **primary tier and the +pre-R-82 response bytes** (`Target` is `omitempty` and stays empty); the primary's job-id format is +unchanged. Verified live on demo-felhom: ``` -demo-felhom name=felhom-pbs type=pbs content=backup reachable=true -demo-felhom name=local type=local content=…,backup reachable=true -demo-hp name=felhom-pbs type=pbs content=backup reachable=true -drill-r50 name=local type=local content=backup,… reachable=true (no PBS storage yet) +/backup/due (untargeted) {"vmid":9201,"due":true,"reason":"no successful backup recorded yet"} +/backup/due?target=felhom-pbs {...,"target":"felhom-pbs"} +/backup/due?target=bogus 400 "unknown backup target: bogus" ``` -This is preferable to keying off `pbs_dr.storage_id`, which is `null` on drill-r50. +An unknown `?target=` is a **400, never a silent fallback** — a caller asking about a tier this agent +does not serve must find out, not act on another tier's freshness. -## 6. Tests — 748 → 768 (+20) +## Fail-safe directions, stated once -Full gate green, **`go vet` run unpiped** (rc printed separately; R-81 recorded that piping it into -`head` reports rc=0 while failing): +- Unparseable timestamp → **due** (a spurious backup is cheap; a skipped one is not). +- Storage-view error → **present** (`"I could not check"` is not `"not there"` — reading it that way + would silently suppress backups, the rule this project has relearned three times). +- Missing tier cadence → **rejected, logged at ERROR** (never defaulted: a weekly DR tier silently + running daily would fill the datastore). +- Unset `keep_last` → **never prune** (a DR tier must not start pruning itself because someone forgot + a field). The primary tier keeps the **absolute** PBS-prune refusal, because its target *and* + retention both default and could prune the DR by accident. -``` -go build ./... rc=0 -go vet ./... rc=0 -go test ./... rc=0 (every package ok) -``` +## Live validation (demo-felhom) -### Red-proof #1 — old controller ↔ new agent (mandatory) — OBSERVED +Both tiers armed: `local 24h keep_last=3 wait 30m` + `felhom-pbs 168h keep_last=2 wait 12h +prune_pbs_allowed=true`. First real PBS-targeted backup: **`TASK OK`, 41 minutes, 14.46 GB +snapshot**, and it **restored cleanly** (`vzrestore: stopped OK`). Measured incremental cost: +**+2.7 GB on disk** for a 14.46 GB logical snapshot (~81 % dedup). -Removed the untargeted compat branch in `tierFromRequest` so it echoes the primary's id like any -other tier: +demo-hp: v0.102.0 deployed via the documented break-glass path (binary + config backed up first), +tier armed, first-ever PBS backup triggered against a verifiably empty namespace. -``` ---- FAIL: TestBackupDue_Untargeted_ResponseBytesUnchanged (0.00s) - backup_tiers_test.go:105: UNTARGETED response MUST NOT carry a target key — an old controller - sees a changed contract; body: {"ok":true,"data":{"vmid":8200,"due":false, - "reason":"within cadence window","age_seconds":7200,"target":"local"}} -``` +## Tests -Restored; suite green. +`go build ./... && go vet ./... && go test ./...` — **rc=0, 29 packages**, vet run unpiped. -### A defect the tests caught (worth recording) +Red-proofs observed and restored: old-controller compat (`"target":"local"` leaking into the +untargeted body), the per-tier wait bound, and the `onboot` override. A per-tier single-flight defect +was caught **by its own test** before it ever shipped — job ids were unique only by clock luck, so +the PBS request was handed the local job's id. -`TestBackupPost_SingleFlightIsPerTierNotPerGuest` **failed on first run**: +**Process failure recorded:** I ran the suite and committed in the same command, read +`packages ok: 28` and pushed without checking `rc=1`; five of my own tests were failing (a harness +artifact, fixed in the follow-up). That is the exact exit-code trap recorded twice earlier in this +arc. -``` -backup_tiers_test.go:320: PER-TIER single-flight violated: the PBS request was handed the LOCAL -job id "backup-8200-1781092800000000000" — the controller would believe the PBS backup ran -``` +## NOT done -Job ids were `backup--` — unique only by clock luck, and identical under an injected -clock or two tiers starting in the same nanosecond. Fixed so ids are unique **per tier by -construction**, with the primary's format left unchanged. This is precisely the failure mode Slice B -would have hit on the weekly both-due night. +1. The **scheduled** restore-test still only ever selects the **primary** tier, so the offsite tier + is never automatically restore-tested — arguably the more important half of "is the DR tier real?". +2. The agent does not report per-tier **cadences**, so the hub must infer "PBS ⇒ weekly" from storage + type. +3. The backup `Store` is still in-memory (**R-84**): every agent restart re-triggers a backup on every + tier. On the local tier that is wasted minutes; on the offsite tier it is a wasted multi-hour + upload after every deploy — which makes R-84 closer to a prerequisite than a tidy-up. +4. The boot+verify half of the restore round-trip has not run under the fixed code. -## 7. Live validation performed - -**No live deploy — see §8.** The strongest zero-risk check available was run instead: an end-to-end -wire test composing the REAL path `agent.json bytes → config.BackupConfig → BackupTiers() → -localapi.BackupTier → a real TLS listener`, driven by a real HTTP client (`httptest`'s own trusting -client — TLS verification was **not** disabled). Unit tests cover each half; this proves the join, -which is where a per-tier mis-wire would actually live. - -``` -tier armed: target=local cadence=24h0m0s keep_last=3 primary=true -tier armed: target=felhom-pbs cadence=168h0m0s keep_last=2 primary=false - -GET /backup/tiers -> 200 {"tiers":[{"target":"local","cadence_seconds":86400,"primary":true}, - {"target":"felhom-pbs","cadence_seconds":604800,"primary":false}]} -GET /backup/due (UNTARGETED) -> 200 {"vmid":8200,"due":false,"reason":"within cadence window","age_seconds":7200} -GET /backup/due?target=local -> 200 {…,"due":false,"target":"local"} -GET /backup/due?target=felhom-pbs -> 200 {…,"due":true,"reason":"older than cadence","age_seconds":691200,"target":"felhom-pbs"} -GET /backup/due?target=bogus -> 400 {"ok":false,"error":"unknown backup target: bogus"} -``` - -The fixture is the exact `agent.json` shape Slice D will write: fresh daily local (2 h) + a PBS -snapshot at 8 days. The DR tier correctly reports **due** while the local tier does not — the -behaviour whose absence is the entire bug. Harness deleted; tree clean. - -Binary built and verified: `felhom-agent 0.97.0` (13.9 MB, `/tmp/felhom-agent-0.97.0`). **Not -installed anywhere.** - -## 8. NOT live-validated — and the deploy target problem - -**Slice A's "deploy to the drill VM only" step was NOT performed: no drill target is reachable.** - -| Candidate | Status | -|---|---| -| **drill-r50** (`drill-r50-0a4f9a`) — the correct target: a live, hub-enrolled drill host | **UNREACHABLE.** Its operator path is wg-only (`10.77.0.4:8822`, OOB `felhom_sshd_active:true`). DooPlex has **no wg interface** — TCP connect to `10.77.0.4:8822` fails. No SSH alias exists | -| **`/mnt/5_hdd/felhom.eu/drill/drill.qcow2`** on DooPlex | Present but **not a Felhom host** — a bare PVE install whose only snapshot is `virgin` (2026-07-03), powered off. Using it means a full greenfield run (host-install → guest provision → hub enrol), which is a different task with real half-state risk, not "deploy the agent" | -| demo-felhom / demo-hp | **Production — explicitly excluded by this slice** | - -Consequently the following are unproven and must be proven at Slice D step 1: - -1. **The tier-arming startup log on a real host** (`backup tier armed` × N, and `backup tier REJECTED` - at ERROR for a bad config). Proven by unit test + the composed wire test only. -2. **A real vzdump to `felhom-pbs` via the new per-tier runner** — no PBS-targeted backup has been - run by this code. -3. **The A.4 double-appearance in a real host-report.** §5 is derived from source plus the live - `storage_targets` shape; it has not been observed with an actual PBS-target `Backup` record - present, because none exists yet. -4. **Per-tier retention against a real storage** — `localPruneSpec`'s PBS refusal is unchanged, so no - PBS prune can occur, but that is asserted from source, not exercised. -5. **The restore round-trip.** Per the task's own bar, a tier is only real once a `pct` restore from - its snapshot yields a working guest. Nothing here approaches that. - -## 9. Open decisions carried forward - -- **PBS pruning is still OFF.** Per-tier retention is plumbed and each tier carries its own - `keep_last`, but `localPruneSpec` still refuses to prune a PBS-type target, and an unset - `keep_last` means never-prune. Turning on automatic pruning of the DR datastore is irreversible and - needs an operator ruling — P0.3 already flagged retention as one of the levers. **Left OFF - deliberately; with capacity already tight, the wrong default here is unrecoverable.** -- **The P0.3 capacity constraint stands.** Growth deferred by operator ruling; the 80 % warn is still - projected at roughly the second additional customer. -- **drill-r50 has no Tier-3 offsite** (`offsite: null`), so the P0.1 "weekly is sufficient" verdict - does **not** hold for it. It is first in the Slice D rollout order — its PBS cadence cannot simply - inherit the fleet ruling. - -## 10. Observations - -1. `local_backup_target` on both production boxes is `"local"`, so `BackupTarget()`'s - `defaultBackupTarget = "felhom-pbs"` fallback is never exercised in production. A box with that - key **absent** would send its primary daily backup to PBS — surprising, and worth checking during - Slice D before writing any config. -2. The agent's backup `Store` remains in-memory (R-84). Slice A does not change that: after a restart - both tiers' records are empty until each next run, so `/backup/due` reports **due** for every tier. - That is fail-safe (a spurious backup, not a skipped one), but on the weekly tier it means a - restart can pull a PBS backup forward. Worth naming in Slice B's dedup reasoning. -3. `RestoreTest.SourceTier` is still typed as `"local"` with a comment saying PBS is "Phase B". With a - real PBS tier arriving, the restore-test's tier attribution should be revisited — not in scope - here, recorded.