Files
felhom-controller/REPORT.md
T

123 lines
7.1 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 — R-55 boot-gate stop-honesty (TASK-E Part 2), 2026-07-21
**Versions: v0.157.0 (R-55) + v0.157.1 (.gitignore anchor). Baseline `83f20c8` (v0.156.0), clean tree.
v0.157.0 is BUILT, PUSHED and DEPLOYED to guest 9201.**
## Scope correction — this part was specified against the wrong repo
TASK-E placed R-55 in `felhom-agent` (Parts 12 → felhom-agent) and separately forbade controller
changes ("felhom-controller untouched", "no controller changes at all"), with a supervised-lite leg
described as an *agent binary* deploy to felhom-pve.
**R-55 is not in the agent.** It is `felhom-controller/controller/internal/web/intermediary.go`
the in-guest boot bind gate. The agent's only related code is host-side `ReassertGuestBinds`, which
re-adds `pct set -mpN` binds and never touches apps. The GO ruling and the scope fence therefore
contradicted each other; the fence was evidently written believing the gate lived in the agent.
**Raised as a STOP rather than resolved unilaterally**, because it changes which repo ships and which
artifact deploys. **Operator ruling 2026-07-21: implement in the controller AND deploy to 9201.** Done.
## The defect
`shouldRecreateOnBoot` keyed on `Deployed && HDD_PATH && drive-present`. `Deployed` is a
deploy-lifecycle flag that stays true across a Stop, so the gate could not tell "the guest went down
under this app" from "the customer switched this off" — and resurrected both. Proven live by STOP-1's
R-52 leg, which was designed to prove the opposite: immich, stopped from the UI seconds earlier, came
back running after `pct reboot 9201`.
The operator's rationale for ruling this GO is worth keeping attached to the fix: an unconditional
recreate can **drown the node** when apps were deliberately stopped to free resources for others. This
is not only about honesty; it is about not fighting the customer's own capacity management.
## Spike-lite: a truthful source already existed — nothing was invented
The task required stopping rather than inventing state if no honest running-at-shutdown signal existed.
It does exist, and R-52 already trusts it:
- `AppConfig` carries **no** run-state. `Deployed` is lifecycle, not intent.
- `Stack.Containers` comes from `docker ps -a` (so `Exited` containers **count**), and docker's own
on-disk container records survive a reboot.
- A UI Stop is `compose down`, which **removes** the containers. Nothing else leaves a deployed app at
zero containers.
That is exactly `bootrecon.isBootOrphan`'s `existing-Exited vs absent` distinction. The gate now
additionally requires `len(Stack.Containers) > 0`.
**What deliberately did NOT change: container STATE is still not a filter.** That half of the original
design is load-bearing — a `State != stopped` filter misses an app docker has not auto-restarted yet,
or one stuck `Exited` on a create-time bind failure with `RestartCount=0`, which is the bug the boot-id
path exists to fix. `state` and `hasContainers` are different questions ("is it up right now" vs "does
docker still have records of it") and only the second survives a reboot as a statement of intent.
`TestShouldRecreateOnBoot` now pins both axes at once; they pull in opposite directions, which is the
entire difficulty of this gate.
**Ordering trap, found and handled:** the evidence is sampled into the `bootStack` snapshot **before**
any recreate runs, because `recreate` itself calls `StopStack` (`compose down`) and destroys it.
**No regression to the drive-absent gate:** apps it stopped are also at zero containers, so this path
now skips them — correctly. They are recorded in `StoragePath.StoppedStacks` and restarted by
`ReconcileDriveGates`' `Return` branch on the same `driveGateLoop` tick. Checked, not assumed.
**Honoured Stops are observable.** `leftStopped` is counted and logged separately from `skipped`, at
INFO. Conflating them would fire a WARN about a missing drive bind for an app behaving exactly as
asked — and a silent correct path is how an inert seam hides.
**No new privileged verb**, as required: `StopStack`/`StartStack` are plain `docker compose` execs in
the controller's own container, and the boot path's only agent calls are read-only (`agent.Disks`).
The agent sudoers/wrapper set is untouched — nothing in it relates to app start/stop.
### Red-proof — RUN
Dropped `hasContainers` from the predicate:
```
--- FAIL: TestShouldRecreateOnBoot
R-55 customer-stopped drive app (zero containers): shouldRecreateOnBoot = true, want false
R-55 gate-stopped app (zero containers; Return branch owns it): shouldRecreateOnBoot = true, want false
--- FAIL: TestRecreateDriveBackedApps_HonoursCustomerStop
recreated=[romm immich], want exactly [romm] — a customer-stopped app must never be restarted
```
`recreated=[romm immich]` is the live defect by name. Fix restored → green; `git diff` clean.
## Gates and deploy verification
`go build ./... && go vet ./... && go test ./...`**green**, whole module.
```
gitea.dooplex.hu/admin/felhom-controller:0.157.0 | Up (healthy)
```
**Not an inert seam** — the new code is verified present in the *running* binary, not just the image:
`grep -a -c "stopped them on purpose" /usr/local/bin/felhom-controller``1` inside the live
container. This check exists because three inert-seam defects shipped fully-green in three days.
### The live reboot leg is NOT done, and that is a deliberate stop
Proving R-55 end to end needs a drive-backed app stopped from the UI followed by `pct reboot 9201`.
9201 has two drive-backed apps (calibre-web, immich) on `hdd_1`, so the fixture is available — but the
demo box is **at a remote site until ~2026-08-02**, and rebooting it unprompted is not a call CC
should make (the 2026-07-20 dhclient incident cost 1h15m from a smaller action). **One operator-present
STOP away.** Until then R-55 is *shipped and wired*, not *proven live*; the capability-map row says so
in those words.
## v0.157.1 — `.gitignore` anchor (TASK-E Part 5)
`controller/.gitignore` carried a bare `controller`, which git matches against **directories** too, so
it also matched `cmd/controller/`. Two opposite failure modes, both manufacturing inert seams:
ripgrep silently skipped `cmd/controller/main.go` (a search for a setter's caller returns nothing and
reads as "unused"), and new files there needed `git add -f` or were never committed.
Anchored to `/controller` + `/controller.exe`. Verified **both** directions: `git check-ignore` no
longer matches `cmd/controller/main.go` and `rg` now finds it; a built binary at the module root is
still ignored. `git ls-files` confirms nothing had been hiding untracked behind the old rule.
## Observations
- `TestShouldRecreateOnBoot`'s old comment argued explicitly that a state filter would be a
regression — correct, and still true. The comment is now expanded to say why `hasContainers` is a
*different* axis, so the next reader does not "fix" it back.
- The gate calling `StopStack` before `StartStack` on every recreate is what erases the container
evidence, and is also why R-52's boot-orphan sweep only ever sees non-drive-backed apps. R-52's
practical scope on a real box remains narrower than its brief assumed; unchanged by this fix.