From 68bcebe493f6721bc7be32936696276967c0cd59 Mon Sep 17 00:00:00 2001 From: Claude Code Date: Sun, 26 Jul 2026 12:36:47 +0200 Subject: [PATCH] =?UTF-8?q?REPORT:=20R-82=20Slice=20A=20(agent=20v0.97.0)?= =?UTF-8?q?=20=E2=80=94=20per-target=20tiers=20built,=20NOT=20deployed=20(?= =?UTF-8?q?no=20drill=20target=20reachable)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- REPORT.md | 232 ++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 206 insertions(+), 26 deletions(-) diff --git a/REPORT.md b/REPORT.md index 29e9224..6bf6f1f 100644 --- a/REPORT.md +++ b/REPORT.md @@ -1,31 +1,211 @@ -# REPORT — v0.96.0: R-50 island NIC (provisioning half) (2026-07-25) +# REPORT — v0.97.0: R-82 Slice A — per-target backup tiers (2026-07-26) -Implements the agent half of the R-50 island control plane (spike GO, -`felhom.eu/documentation/audits/SPIKE-island-bridge-2026-07-25.md`). A fresh install is born immune to -F1 (a LAN/DHCP/site move can no longer take the control plane down). +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. -## What changed -- `internal/config/config.go` — `LocalAPIConfig.IslandBridge` + `IslandGuestAddr`; `IslandEnabled()`; - `Validate()` all-or-nothing + CIDR guard. -- `internal/reconcile/bringup.go` — `BringUpSpec` island fields; `buildBringUpConfig` attaches a static - `net1` (island NIC) on provision AND DR when configured; absent otherwise. -- `cmd/felhom-agent/main.go` — both `RunBringUp` call sites plumb `cfg.LocalAPI.Island*`. -- `configs/agent.example.json` — island shape (island bind + island_bridge/guest_addr + lan_resolver.host_ip). -- `configs/felhom-localapi-firewall.example` — rewritten for the island (the portless bind IS the LAN close). -- `REUSE.md` — island-NIC provisioning pattern + the "endpoint follows listen_addr" note. +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. -## A0/A3 determination -- **A0 = config-only for the endpoint:** `main.go` sets the bootstrap `Endpoint: cfg.LocalAPI.ListenAddr`; - moving the bind to the island moves the guest dial with no template change. The version bump is for the - net1-attach (the operator chose agent-bringup over golden-bake to eliminate the golden-coupling risk). -- **A3 = healer verify-only:** the guestnet healer is eth0-only (`parseMode` dev-scoped); the static island - eth1 is outside its scope. Locked with a red-proof test — no healer change. +**STOPPED at the Slice A boundary as specified.** Slices B/C/D/E not started. -## Tests (green, non-hollow) -`TestBuildBringUpConfig_IslandNIC`, `TestLocalAPIConfig_IslandValidation`, -`TestParseMode_IslandStaticNICDoesNotConfuseEth0`. Full `go build/vet/test ./...` green. +--- -## Coupling / deploy order -A host-install that writes the island config REQUIRES agent ≥ 0.96.0 (to read `island_guest_addr` and -attach net1). **Vouch 0.96.0 before island installs go live.** MinAgent unchanged (no controller coupling). -Fleet migration of existing boxes = `felhom.eu/documentation/runbooks/RUNBOOK-island-migration.md` (Phase B). +## 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 | + +Commit: **`739b3c3`** on `main`, pushed. + +## 3. The compatibility rule — how it is enforced + +The agent and controller deploy independently, so the untargeted contract is **frozen**: + +- `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: + +``` +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) +``` + +This is preferable to keying off `pbs_dr.storage_id`, which is `null` on drill-r50. + +## 6. Tests — 748 → 768 (+20) + +Full gate green, **`go vet` run unpiped** (rc printed separately; R-81 recorded that piping it into +`head` reports rc=0 while failing): + +``` +go build ./... rc=0 +go vet ./... rc=0 +go test ./... rc=0 (every package ok) +``` + +### Red-proof #1 — old controller ↔ new agent (mandatory) — OBSERVED + +Removed the untargeted compat branch in `tierFromRequest` so it echoes the primary's id like any +other tier: + +``` +--- 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"}} +``` + +Restored; suite green. + +### A defect the tests caught (worth recording) + +`TestBackupPost_SingleFlightIsPerTierNotPerGuest` **failed on first run**: + +``` +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 +``` + +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. + +## 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.