Files
felhom-agent/REPORT.md
T

212 lines
12 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# REPORT — v0.97.0: R-82 Slice A — per-target backup tiers (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.
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.
**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 |
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-<vmid>-<unixnano>` — 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.