Files
felhom-controller/REPORT.md
T

93 lines
7.4 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 — CRITICAL C6B-F1: hollow .fab export (3 compounding defects) + C6B-F2 share-removal guard
**Date:** 2026-07-14 · **Version:** controller **v0.130.0** (baseline `eb3bf4a` on `main`) · Controller-only; no agent/hub/felhom.eu change.
## What was broken (CAMPAIGN-6B, reviewer-validated at source)
`.fab` export produced a **config-only, data-free bundle** for **12/13 `needs_hdd` catalog apps**
(the `${USERDATA_PATH}` convention: audiobookshelf, calibre-web, emby, immich, jellyfin, komga,
navidrome, paperless-ngx, plex, radarr, romm, sonarr), reported success, and passed the v0.125.0
anti-hollow guard. Live proof: sonarr, 4.17 GB / 7 files → a **2308-byte** bundle. Cross-box or
fresh-box restore = silent total data loss. **Confirmed scope: the SCHEDULED/tier-2 backup path was
NOT affected** (it copies the `felhom-data` namespace wholesale via `GetAppDrivePath`); only the
`.fab` export adapter had the mount-parse + either/or combination — the backup-side `stackAdapter`
is deliberately untouched.
## Commits (each part green-gated: `go build ./... && go vet ./... && go test ./...`)
| Commit | Part | Change |
|---|---|---|
| `8967ba7` | 1 (cause 2) | `stacks.ExportDataMounts` (new, delete.go) + `exportAdapter.GetStackHDDMounts` rewired to it — `${HDD_PATH}` binds UNIONed with the `${USERDATA_PATH}` ROOT, containment-deduped both directions. Also fixes the estimate's `data=0 B`. |
| `c6d8bc8` | 2 (cause 1 + §8) | `executeExport` additive (needs_hdd apps run `exportHDDData` AND `exportVolumeData`); `exportHDDData` returns error + fails LOUDLY on a basename collision; `EstimateExport` additive to match (fits-on-dest counts both). |
| `a829cdc` | 3 (cause 3) | `assertBundleDataComplete`: a `needs_hdd` manifest with neither HDD data nor volume data fails the job — "a mentés nem tartalmaz alkalmazásadatot (0 adatkönyvtár, 0 kötet…)". |
| `b49076d` | 4 (C6B-F2) | `handleNetStorageRemove` refuses (409, Hungarian, names the apps) while a DEPLOYED stack's HDD_PATH is the share root or a subpath; new `deployedAppsOnPath` helper; remove resolves the agent via the `netAgent` seam. |
| (this commit) | docs/ship | CHANGELOG v0.130.0, README export section, REUSE.md (2 new helpers), CONTEXT.md, this REPORT. |
## ⚠ Design deviation from the task (deliberate, load-bearing)
The task's Part 1 letter said: union the **per-bind** `ParseComposeUserdataMounts` results, and §8
said namespace colliding tar names "by a sanitized full-relative path or index". **Both are
incompatible with §12 "do NOT improve the import side":** the manifest keys HDD tars by basename,
and the untouched import maps a basename either to a resolved `${HDD_PATH}` mount or to
`<HDD_PATH>/<basename>` (restore.go `restoreHDDData` + `resolveHDDMounts`, which is itself
`${HDD_PATH}`-only). A per-bind mount `…/userdata/media/tv` bases to `tv` → the import would restore
it to `<HDD_PATH>/tv` — wrong place, a subtler variant of the same data loss; a namespaced tar name
could not be mapped back at all. **Resolution:** capture the userdata subtree at its ROOT (one
mount, basename `userdata`, a direct child of HDD_PATH) — it round-trips through the existing import
fallback exactly, captures at least as much data (the whole per-app userdata subtree, same
philosophy as tier-2's namespace-wholesale copy), and needs zero import changes. Proven by
`TestFabRoundTrip_UserdataPlacement` (export → wipe → import → file back at
`<HDD_PATH>/userdata/media/tv/show.bin`, byte-identical). For §8, colliding basenames now **fail
loudly** instead of being renamed (a rename cannot round-trip either); no catalog app collides today.
## Part 4 scope note (C6B-F2)
The task guessed the defect was removal-order in the controller. Investigation: the agent's
`RemoveNetworkMount` (felhom-agent `internal/storage/netmount.go:419`) already stops the automount
BEFORE unlinking — the real defect is **tolerate-and-continue** (netmount.go:434-443: every stop
step's failure is logged at Debug and execution proceeds), so a busy mount (live app bind — the C6B
live event) gets its unit files deleted anyway → the unreapable orphaned autofs. The agent is out of
this task's scope ("Repos touched: felhom-controller ONLY"), so the shipped fix is the
controller-side guard that cuts the trigger off at the product flow (refuse removal while a deployed
app binds the share — also the C6B audit's explicit fix direction). **Follow-up needed (felhom-agent
task):** after the stop steps, verify the mountpoint is actually released and ABORT the unit-file
removal if not.
## Tests + red-proofs (all four run→fail→revert, recorded)
| # | Test(s) | Red-proof |
|---|---|---|
| 1 | `stacks/export_mounts_test.go` ×6 (union incl. baked-in pre-fix-finds-0 contrast, HDD-direct regression B, mixed, covering-root, literal-userdata dedupe, empty-HDD) | reverted `ExportDataMounts` to `${HDD_PATH}`-only → 3 tests FAIL (UserdataConvention, MixedBindsUnion, LiteralUserdataBindDeduped) → restored green |
| 2 | `appexport/export_additive_test.go`: scenario A (bundle has BOTH `data/hdd/userdata.tar` + `data/volumes/hdd-app_config.tar`, both manifest flags) | reverted `executeExport` to the either/or → scenario A FAILS (volume tar absent) → restored green |
| 2b | §8 collision test (loud Hungarian failure naming `config`) | removed the collision check → FAILS ("got success (silent overwrite)") → restored green |
| 3 | scenario D (`needs_hdd` + zero discovered data → job fails "nem tartalmaz alkalmazásadatot", no bundle) | removed the guard assertion → FAILS ("got success (the hollow bundle)") → restored green |
| 4 | `web/netstorage_remove_guard_test.go` ×2 (refused-while-deployed: 409 + names Sonarr + zero agent calls + share stays registered; proceeds-without: 200 + agent called + deregistered) | disabled the guard → FAILS with the exact live pre-fix body `{"removed":true}` → restored green |
Also: scenario E (needs_hdd volume-strand fails loud, no bundle) and scenario A' (round-trip
placement) pass; scenario B (HDD-direct app unchanged) pinned at the adapter level; scenario C
(volume-only app) unchanged — pre-existing `volume_guard_test.go` suite still green. Full gate:
**23/23 packages ok** (build+vet+test). Test fixtures use `t.TempDir()` + the `dockerExec`/
`netAgentFn` seams — no real docker anywhere.
## Deploy + live self-verify
- Built `v0.130.0` on 180, deployed to demo 9201 + drill guest (see verification below).
- Live self-verify on demo (the C6B-F1 repro flipped): deploy a `${USERDATA_PATH}` needs_hdd app with
marker data → export → the `.fab` is NOT config-only (manifest `has_hdd_data:true` +
`has_volume_data:true`, size ≫ 2308 B). Evidence inline below.
## For CAMPAIGN-6C
The flagship `.fab` full-circle (sonarr → 4 GB → export → download → delete → upload → import →
**byte-compare, zero mismatches**) that was BLOCKED-BY-BUG in 6B is now verifiable end-to-end — it
should be 6C's FIRST live acceptance test. The C6B-F2 clean-order teardown re-test (remove app THEN
share → no orphan) is also now guard-assisted.
## Observations
- `estimate.go` had the same either/or shape as the export; left un-additive it would have
under-reported needs_hdd apps by their volume size against the new bundle content — fixed in the
same commit as cause 1 (the fits-on-dest gate consumes it inside `executeExport`).
- `netAgentForAdd` now serves the whole share lifecycle (remove resolves through it too) — comment
updated; a rename was skipped for minimal-diff discipline.