admin
bde43f3a74
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PSK5g6qYLknKj8u3QAFEr6
80 lines
6.7 KiB
Markdown
80 lines
6.7 KiB
Markdown
# REPORT — v0.104.0: off-box unit discovery (durable, deployment-independent) + no-silent-success
|
||
|
||
**Date:** 2026-07-09 · **Class:** implementation (controller only). **Baseline:** `main` @ `59eb3be`
|
||
(v0.103.0) → `908e4b9` (v0.104.0). Fixes the off-box mis-resolution + silent-success landmine surfaced by
|
||
the Storage-Box spike and pinned by the DIAG report.
|
||
|
||
## The bug (DIAG root cause)
|
||
`runOffboxInternal` located each toggled app's recovery unit via `RecoveryUnitPath(AppNamespaceRoot(stack),
|
||
stack)`. `AppNamespaceRoot`→`GetAppDrivePath` reads the app's **live** `app.yaml` `HDD_PATH` and returns
|
||
`""` when the app isn't currently deployed/scanned — which **silently falls back to `systemDataPath`**. So a
|
||
toggled-but-undeployed app (DIAG: on the demo, *no* app is deployed and primary backups sit on the USB drive)
|
||
was looked for on the wrong drive → `os.Stat` failed → skipped → the run returned `nil` → status `ok` with
|
||
0 snapshots and no operator alert.
|
||
|
||
## The fix
|
||
- **Discovery over inference** (`internal/backup/offbox.go`): `offboxCandidateNSRoots` builds the durable
|
||
candidate set = every registered *schedulable, non-decommissioned* storage path (`GetSchedulableStoragePaths`)
|
||
∪ `systemDataPath`, deduped by resolved nsRoot. `discoverOffboxUnit(app)` scans each for
|
||
`backups/primary/<app>`, independent of live deploy state; if the same unit exists on >1 drive (drive
|
||
churn) it backs up the **newest by manifest `CreatedAt`** (`offboxUnitTime`, mtime fallback) and WARN-logs
|
||
the stale one. A disconnected drive's path is simply absent → `os.Stat` fails → "not here" (correct).
|
||
- **No silent success** (`RunOffboxBackup`): `runOffboxInternal` now returns `(backedUp, missing, err)`.
|
||
`len(apps)>0 && backedUp==0` → a hard error → `LastStatus="error"`, `LastError` names the missing apps,
|
||
and `offboxNotify` fires with a non-nil err (operator alert). A *partial* run stays `ok` but sets a
|
||
Hungarian **`OffboxTarget.LastWarning`** naming the skipped apps.
|
||
- **`AppNamespaceRoot` and the primary-WRITE paths (`CaptureRecoveryUnit`/dumps) are unchanged** — this only
|
||
changes offbox's read/discovery + status reporting.
|
||
|
||
## Files changed (commit `908e4b9`)
|
||
- `controller/internal/backup/offbox.go` — `offboxCandidateNSRoots`, `discoverOffboxUnit`, `offboxUnitTime`;
|
||
`runOffboxInternal` rewritten (discovery loop, new signature); `RunOffboxBackup` 0/N + `LastWarning` logic.
|
||
- `controller/internal/settings/settings.go` — `OffboxTarget.LastWarning string` (`last_warning,omitempty`, additive, no migration).
|
||
- `controller/internal/web/offbox_handlers.go` — preserve `LastWarning` across a config edit.
|
||
- `controller/internal/web/templates/backups.html` — render `LastWarning` on `/backups` in the `--warn` style.
|
||
- `controller/internal/backup/offbox_test.go` — 6 new non-hollow tests + helpers.
|
||
- `CHANGELOG.md`, `CONTEXT.md`, `controller/README.md` — docs.
|
||
|
||
## Tests + companion red-proofs
|
||
**Green gate:** `go build ./... && go vet ./... && go test ./...` — all green (module `controller/`).
|
||
**Offbox suite: 8 → 14 tests** (before → after). New (all PASS), asserting the **effect** (exact discovered
|
||
`src`, `LastStatus`/`LastError`/`LastWarning`, notify-err — never just "no error"):
|
||
- `TestOffbox_DiscoversUnitOnRegisteredDrive` (A) — unit on a registered drive (not systemDataPath) is
|
||
discovered; backup `src` = the USB nsRoot path; status ok, no warning.
|
||
- `TestOffbox_NoUnitAnywhereIsHardError` (B) — 0/N → error, 0 backups, operator alerted, `LastError` names the app.
|
||
- `TestOffbox_PartialRunWarnsNotErrors` (C) — present app backed up, notify-err nil, status ok, `LastWarning` names the missing one.
|
||
- `TestOffbox_MultipleUnitsPicksNewest` (D) — two copies → exactly one backup, the newer by manifest `CreatedAt`.
|
||
- `TestOffbox_AllPresentHappyPath` (E) — both backed up, ok, no warning, snapshot count reflects stats.
|
||
- `TestOffbox_NoAppsToggledIsCleanOK` (edge) — 0 toggled → clean ok no-op, no backup call.
|
||
|
||
**Companion red-proofs (run live, then reverted — `git diff` clean):**
|
||
- **A:** temporarily reverted the loop to the old `AppNamespaceRoot`-based resolution → `TestOffbox_DiscoversUnitOnRegisteredDrive`
|
||
**FAILED** with `run: off-box backup produced no snapshots: 1 app(s) toggled … (missing: audiobookshelf)`
|
||
(the old resolution looked at `systemDataPath`, missed the unit on the registered drive). Proves discovery is load-bearing.
|
||
- **B:** temporarily guarded the 0/N promotion with `if false` → `TestOffbox_NoUnitAnywhereIsHardError`
|
||
**FAILED** with `0-of-N toggled apps backed up must ERROR (no silent success)` (the run silently returned `ok`).
|
||
Proves the landmine fix is load-bearing.
|
||
|
||
## Deploy verification (guest 9201)
|
||
- Built + pushed `gitea.dooplex.hu/admin/felhom-controller:0.104.0` (digest `sha256:431f9f9f…d74`) on the build server.
|
||
- Deployed via the bootstrap mechanism (`docker pull` → write `/etc/felhom-controller-image` → restart `felhom-controller-bootstrap.service`).
|
||
- `docker ps`: `gitea.dooplex.hu/admin/felhom-controller:0.104.0 Up (healthy)`; `/etc/felhom-controller-image` = `:0.104.0`.
|
||
- Startup logs clean: settings loaded, scheduler registered (incl. daily `offbox-backup`), logscanner `errors=0 warnings=0 issues=0`, no panics.
|
||
|
||
## NOT yet live-validated against the Storage Box
|
||
**This task did NOT re-point offbox at the live Storage Box or run a real offbox→box round-trip** — the spike's
|
||
box repos + credentials were torn down. The endpoint-level re-validation against the box is a separate
|
||
**supervised** runbook once box access is re-provisioned. The unit suite fully covers the discovery + status logic.
|
||
|
||
## Observations (noticed, not acted on — out of scope)
|
||
- **Pre-existing flaky test:** `internal/web/async_restore_test.go::TestBackupTier2Restore_DoubleClickRefused`
|
||
flakes ~1–2/20 on the **clean baseline** too (confirmed by stashing my changes) — an inherent test race
|
||
(`waitFor(IsRunning())` returns once `running=true` is set, but `StopStack` may not have been called yet, so
|
||
`prov.stops` can read 0). Not touched (Tier-2, unrelated to this task). Worth a follow-up: have the test
|
||
`waitFor(prov.stops==1)` instead of asserting immediately.
|
||
- **Discovery vs the demo's reality (context for the supervised re-validation):** on the demo, `systemDataPath`
|
||
= `/mnt/sys_drive` (near-empty), the only registered schedulable path is `/mnt/felhom-drives/felhom-usb`, and
|
||
all recovery units live on that USB drive — so post-fix discovery will find them there regardless of any app's
|
||
(absent) live `app.yaml`. The declared `HDD_PATH` in the surviving backup artifacts (`/mnt/felhom-drives/felhom-flash`)
|
||
points at a drive that no longer exists — further reason the durable-registry scan (not the inferred drive) is correct.
|