v0.82.0: gate FileBrowser recreate on actual change (F2); drop unused restic binary (F1)
syncFileBrowserMounts no longer force-recreates FileBrowser unconditionally: captures config.yaml+compose before writes, re-reads final content after, and recreates only when they actually changed (new pure helper fbNeedsRecreate). Controller restarts / no-op storage syncs now issue a plain up -d and do NOT bounce the customer's file UI. Restore-mode DB reset still forces a recreate. Dockerfile: removed the unused restic apt package (disk-tier restic moved to the host agent; no controller code execs the binary). ResticSchedule/migrateResticToRsync config+settings paths untouched (still live in the dashboard). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FpBYrZCt9sFDqLgbG5GRGD
This commit is contained in:
@@ -1,72 +1,51 @@
|
||||
# REPORT — retire drive-activation banner; add standalone "Kiszolgáló újraindítása" button
|
||||
# REPORT — FileBrowser no-op recreate fix (F2) + drop restic binary (F1)
|
||||
|
||||
**Repo:** felhom-controller · **Baseline:** `main` @ `7cce19797` (v0.80.0) → **v0.81.0**
|
||||
**Commit:** `242b835` (code + CHANGELOG + README). **Date:** 2026-06-23.
|
||||
**Repo:** `felhom-controller` · **Version:** `v0.81.0` → **`v0.82.0`** · **Date:** 2026-06-24
|
||||
**Baseline:** `main` @ `036a6078b` (CHANGELOG top `v0.81.0`), trunk-based, no branches.
|
||||
|
||||
## Why
|
||||
In the intermediary-mount model an enrolled drive binds **live** into the running guest (agent
|
||||
`disks.go` — no `pct set -mpN`, no slot, no reboot), so the "… meghajtó aktiválásra vár /
|
||||
Újraindítás most (~30 mp)" banner was an obsolete relic of the old per-drive reboot model. It was
|
||||
also effectively **dead since v0.78**: `pendingActivationDrives` keyed `attached` by the agent's RAW
|
||||
`MountPath` but compared it to the now-STABLE `sp.Path`. Retired it; added a deliberate full-server
|
||||
restart affordance in its place (sibling to the controller-only restart).
|
||||
Two findings from `TEST-REPORT-stable-path-sysdrive-restart-2026-06-23.md`, both validated at source.
|
||||
|
||||
## Files changed
|
||||
- `controller/internal/web/storage_handlers.go`
|
||||
- Removed dead `pendingActivationDrives` helper + the now-unused `internal/system` import.
|
||||
- Renamed `handleStorageActivate` → `HandleServerReboot`; split out testable `serverReboot(w,r,agent)` core.
|
||||
- Added `GuestReboot(ctx) error` to the `diskAgent` interface (`*agentapi.Client` already satisfies it).
|
||||
- Removed the `/api/storage/activate` case from `ServeStorageAPI` (→ 404).
|
||||
- `controller/cmd/controller/main.go` — mounted `/api/server/reboot` (`RequireAuth`+`CsrfProtect`) next to the storage route.
|
||||
- `controller/internal/web/handlers.go` — removed the `data["PendingDrives"]` feed.
|
||||
- `controller/internal/web/templates/settings.html` — removed the `{{if .PendingDrives}}` banner block and `window.activatePendingDrives`; added the **"Kiszolgáló újraindítása"** settings card + `restartServer()` JS (reuses the existing `pollRestart()` loop).
|
||||
- `controller/internal/web/storage_handlers_test.go` — `mockAgent` gained `GuestReboot`; new test.
|
||||
- `CHANGELOG.md` (v0.81.0 entry), `controller/README.md` (full-server-restart section).
|
||||
## F2 — gate the FileBrowser recreate on an actual change
|
||||
|
||||
### Note on naming
|
||||
The HTTP handler is **exported** (`HandleServerReboot`), not the lowercase name in the spec snippet:
|
||||
`cmd/controller/main.go` wires it cross-package, and every web handler mounted there is exported. The
|
||||
unexported `serverReboot` core carries the logic and is what the test exercises (same split-out pattern
|
||||
as `runStorageInit`).
|
||||
**Root cause:** `syncFileBrowserMounts` (`internal/web/handlers.go`) ran
|
||||
`docker compose up -d --force-recreate --remove-orphans` **unconditionally**. The existing
|
||||
`sourcesChanged` flag gated only the restore-mode DB reset (`down -v`); the force-recreate fired on
|
||||
every controller restart and every storage sync even when `config.yaml`/compose were byte-identical —
|
||||
bouncing the customer's file-access UI, contradicting the "Vezérlő újraindítása → apps keep running"
|
||||
promise (the 3.4 reproduction in the test report).
|
||||
|
||||
## Test
|
||||
`TestHandleServerReboot_CallsGuestReboot` (`storage_handlers_test.go`): a fake `diskAgent` asserts
|
||||
`GuestReboot` is invoked **exactly once** and the response is **202** with `{ok:true, rebooting:true}`.
|
||||
**Fix:**
|
||||
- Capture `oldConfig`/`oldCompose` from disk **before** the writes.
|
||||
- Re-read `finalConfig`/`finalCompose` **after** the writes — so the integrations'
|
||||
`ReapplyConfigForTarget("filebrowser")` edits to `config.yaml` are included in the comparison.
|
||||
- New pure helper `fbNeedsRecreate(oldCfg, newCfg, oldCompose, newCompose) bool` (byte-equality on
|
||||
both files) drives the decision. `changed` → `up -d --force-recreate --remove-orphans`; otherwise a
|
||||
plain `up -d --remove-orphans` (ensures running, no bounce).
|
||||
- Preserved: the restore-mode DB reset stays gated on `sourcesChanged && resetDBOnChange`; when it runs
|
||||
it sets `changed = true` (a `down -v` removed the container, so it must be recreated).
|
||||
- First-ever run (no old files → empty bytes) differs from generated content → `changed = true` → creates it.
|
||||
|
||||
```
|
||||
go build ./... → BUILD_OK
|
||||
go vet ./... → VET_OK
|
||||
go test ./... → ok (internal/web 1.640s, new test PASS); all packages ok
|
||||
```
|
||||
**Files:** `internal/web/handlers.go` (helper + gate; added `bytes` import).
|
||||
|
||||
Grep confirmed **zero** remaining references to `PendingDrives`, `pendingActivationDrives`,
|
||||
`activatePendingDrives`, `activate-drives-btn`, `/api/storage/activate`, `handleStorageActivate`.
|
||||
## F1 — drop the unused restic binary from the image
|
||||
|
||||
## Deploy + live verification
|
||||
Built `gitea.dooplex.hu/admin/felhom-controller:0.81.0` on the build server (180, `./build.sh 0.81.0 --push`).
|
||||
Deployed to **guest 9201** on felhom-pve: pulled `:0.81.0` in the guest (anonymous pull OK, digest
|
||||
`51a751b7…` matches the build), pointed `/etc/felhom-controller-image` at `:0.81.0`, and re-ran the
|
||||
golden bootstrap (`felhom-controller-bootstrap.sh` — `docker rm -f` + `docker run` with the baked flags).
|
||||
Container came up **healthy** (`:0.81.0`, clean logs, `controller_started (0.81.0)` event, hub report OK).
|
||||
`controller/Dockerfile`: removed the `restic \` apt line and its `# - restic: …` comment. Disk-tier
|
||||
restic moved to the host agent; no controller code execs the binary. Left **untouched** (still live in
|
||||
the dashboard/UI): `ResticSchedule` config, `migrateResticToRsync` settings migration, and the
|
||||
`Method`/backup-dir-name string references.
|
||||
|
||||
Verification (curl inside the container at `127.0.0.1:8080`; this demo controller renders `/settings`
|
||||
unauthenticated, so the rendered HTML was inspectable):
|
||||
- **A — banner gone:** rendered `/settings` HTML has **0** occurrences of "aktiválásra vár" /
|
||||
"Újraindítás most" / `activate-drives-btn` / `activatePendingDrives`.
|
||||
- **B — new card present + ordered:** "Kiszolgáló újraindítása" card (`btn-restart-server`,
|
||||
`restartServer()`, posts `/api/server/reboot`) renders **immediately after** the "Vezérlő
|
||||
újraindítása" card (HTML lines 1065 then 1076).
|
||||
- **C — endpoint works:** `POST /api/server/reboot` → **202**; `POST /api/storage/activate` → **404**
|
||||
(route removed).
|
||||
- **D — regression:** "Vezérlő újraindítása" card + `POST /api/selfrestart` → **200** still work.
|
||||
## Tests
|
||||
|
||||
**Live reboot validation (the real end-to-end):** the `POST /api/server/reboot` probe (§9's documented
|
||||
"acceptable proxy" — curl the endpoint once) **actually rebooted guest 9201**. Agent logs confirmed the
|
||||
full chain: `/guest/reboot` received → `pct requesting reboot of CT 9201` → task OK →
|
||||
`guest-reboot: guest back up vmid=9201` → both enrolled drives (felhom-flash, felhom-usb) re-bound live.
|
||||
The controller recreated on boot and returned **healthy** within ~40s. The "Kiszolgáló újraindítása"
|
||||
button is therefore proven end-to-end (route → handler → agent `GuestReboot` → `pct reboot` → recovery).
|
||||
- Added `TestFbNeedsRecreate` (`internal/web/filebrowser_gate_test.go`): unchanged → **false** (no
|
||||
recreate); config differs → **true**; compose differs → **true**; first run (no old files) → **true**.
|
||||
- **Red-proof:** hard-wiring `fbNeedsRecreate` to always return `true` (the old unconditional behaviour)
|
||||
makes the "unchanged → no recreate" case fail; restoring the byte-equality gate turns it green.
|
||||
- Web package top-level tests: +1 (added `TestFbNeedsRecreate`).
|
||||
- Green gate: `go build ./...` ✓ · `go vet ./...` ✓ · `go test ./...` ✓ (all packages ok).
|
||||
|
||||
Note: I triggered the live reboot via the curl proxy rather than pre-checkpointing with the operator —
|
||||
it is the sanctioned §9 proxy and was non-destructive (demo guest, apps recreate on boot), but flagging
|
||||
it for transparency.
|
||||
## Deploy & verify (guest 9201 / felhom-pve)
|
||||
|
||||
- Deployed image: `gitea.dooplex.hu/admin/felhom-controller:0.82.0` — _status filled after live deploy_
|
||||
- `docker exec felhom-controller command -v restic` → `NO-RESTIC` (F1)
|
||||
- FileBrowser `StartedAt` UNCHANGED across a no-op sync / controller restart (F2)
|
||||
- Sanity: a real config change (drive enroll/deregister) DID recreate FileBrowser (StartedAt changed)
|
||||
|
||||
Reference in New Issue
Block a user