diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d66bebb..637354db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,49 @@ +## Unreleased (2026-10-29, afternoon) — clear_hold (D64 = B), the box command through the running controller (R-1343), R-1340, R-1341; commit only, rides the next release + +**MinAgent: 0.158.1** (unchanged). The next release also carries R-1342 (`652a690`, already on main, the entry below). +**No new event type** — the hub records the operator action's result as its own `operator_action` event. + +- **`clear_hold` — a fifth verb in the CLOSED operator-action list (operator ruling D64 = B, 2026-10-29 12:10)** + (`internal/report/opactions.go`, wired in `cmd/controller/opactions_wiring.go`; arg = the app's stack name, letters, + digits, `-`, `_`, ≤64 — checked before any handler). It lifts the app's hold of ANY reason in the RUNNING controller + (`backup.AppStopGuard.ClearHoldByOperator` → `settings.ClearRestoreHold`: memory and `settings.json` together), so no + later save can write it back. It starts nothing. Refused, nothing changed: a bad name, an app with no hold. One log + line names who: „hold CLEARED by the operator via the hub, action #". Every reader of the hold (page, start gate, + API, `RestoreHoldFor`, hold reporter) reads the same settings record; the ONE held flag cached elsewhere — + `stacks.Manager`'s „the last update ended held" (it made the crash-loop observer and the nightly update leg skip the + app) — is forgotten too (`ForgetUpdateHeld`). A `putback_pending` hold is also dropped from the app-stop marker, or the + next start with the drive bound would pour the old copy over what the app wrote after the clear; the copy stays on + disk. The hold reporter needs no change: its next tick (≤1 min) sends `app_hold_cleared`. + Tests: `TestOpActions_ClosedList` (updated deliberately), `TestOpActions_ClearHoldBadArgumentIsRefusedAndCallsNothing`, + `TestOpActions_ClearHoldDoneRefusedFailed`, `TestD64_ClearHoldViaTheHub`, `TestD64_ClearHoldRefusals`, + `TestD64_ClearHoldStaysGoneAfterALaterSave`, `TestD64_ClearPutBackPendingDropsTheMarkerEntry`, + `TestD64_TheHoldReporterTellsTheHubAfterTheClear`, `TestD64_ForgetUpdateHeld`, `TestD64_MainWiresTheNewPieces`. +- **R-1343 — the box command `--clear-restore-hold ` goes through the running controller** (`cmd/controller/holdclear.go`): + a unix socket `/ctl/hold.sock`, 0600 in a 0700 dir, peer uid checked (SO_PEERCRED), nothing on TCP — not + reachable through traefik, the LAN or an app container. Output: „Cleared by the RUNNING controller … no restart is + needed". Only when no controller answers (no socket, or a stale one) does it edit `settings.json`, and it says so, + naming the restart INSIDE the guest (`pct exec -- systemctl restart felhom-controller-bootstrap.service`). The + flag's help names the exact guest command. `--restore-holds` (the list) is unchanged. + Tests: `TestR1343_TheBoxCommandClearsThroughTheRunningController` (red with the socket skipped: „the hold came back + after a later save of the running controller" — R-1343 reproduced), `TestR1343_TheBoxCommandFallsBackWhenNoControllerRuns`, + `TestHoldCtl_NotReachableFromOutside`, `TestHoldCtl_UnknownRequestIsRefused`, `TestHoldCtl_ANonSocketFileIsNotReplaced`. +- **R-1340 — the operator's „HELD STOPPED" mail names the real cause** (`cmd/controller/holdmail.go` + `heldStoppedOperatorText`, built from the hold record). Hold writers now record `RestoreKind` (off-site / local), + `MixedCause` (`definition_write` / `put_back` / `interrupted_put_back`), `FailedStep` (the put-back step that failed, + with its error), `DBReplay`, and the kept copy's folder (`settings.RestoreHold`, all `omitempty`). The mail never says + „database" unless the failure was a database replay, never „off-site" for a local restore; a pre-R-1340 mixed hold + names only what its record says. Legs unchanged (`restore-hold`, `restore-hold-mixed`); a local restore's RunKind is + `restore`. Tests (red with the old sentence/record): `TestR1340_UnhealthyStop`, `_UpdateFailed`, + `_RestoreFailedWithADatabase`, `_PutBackWithoutADatabase` (the measured Radicale case), `_PutBackAfterADatabaseReplay`, + `_DefinitionWrite`, `_LocalRestore`, `_PutBackPending`, `_InterruptedPutBack`, `_LegacyMixedHoldAssumesNothing`, + `_NotifyReadsTheHoldRecord`; writer side `TestR1340_AVolumeFailureWhosePutBackStopFailsRecordsTheStepAndNoDatabase`, + `_ADatabaseReplayFailureRecordsTheDatabase`, `_ADefinitionWriteFailureRecordsItsCause`, `_ALocalRestoreRecordsItsKind`, + `_AnInterruptedPutBackRecordsItsCause`. `TestEventMessageWireTextIsFrozen` is untouched (this text is passed through). +- **R-1341 — the OOM line once per NEW kill** (`cmd/controller/oomlog.go` `oomLogGate`, key = container + start time + + kill count): a second scan of the same kill logs nothing; a new kill or a restarted container logs again. The + `app_oom` event call is unchanged. Tests: `TestR1341_ASecondScanOfTheSameKillLogsNothing`, `_ANewKillLogsAgain`, + `_ARestartedContainerLogsAgain`, `_ContainersAreJudgedApart`. + ## Unreleased (2026-10-29, after v0.347.0) — R-1342: the restore copy is safe during the stable-start watch; a removed app does not alarm; commit only, rides the next release **MinAgent: 0.158.1** (unchanged). Found live on Tester 1 at 10:39 CET: a false `backup_run_failures` mail („Gokapi did diff --git a/REUSE.md b/REUSE.md index 75dce7b9..f2ec3463 100644 --- a/REUSE.md +++ b/REUSE.md @@ -82,12 +82,13 @@ | `restoreOpBlocked` | controller/internal/web/restore_wizard.go | `() (msg string, blocked bool)` | THE refusal gate before starting ANY restore | **Use this, never a bare `IsRunning()`.** It reads BOTH flags: `RestoreStatus().Running` (set synchronously by `BeginRestoreOp`, true for the whole off-box restore) and `IsRunning()` (the concurrency flag, the only one the nightly backup holds). **R-351: all seven handlers read only `IsRunning()`, which the goroutine acquires AFTER the handler returns — a second press started a second run and was told „…elindult".** Returns the Hungarian refusal, which names the running app and a route | **R-360 (v0.226.0): `offboxVerifyCopyDeleteHandler` was the LAST holdout and its doc comment claimed it already did this — the sentence is why nobody looked. `IsRunning()` is FALSE for the whole of a verification restore, so the copy a restore was writing into could be deleted from the UI (observed live 2026-08-21 22:35). No app-name comparison: refusing during ANY restore is stronger and uniform.** | `Server.beginRestoreOp` → `backup.TryBeginRestoreOp` / `RestoreBusy` (R-1256 H6) | controller/internal/web/restore_wizard.go; controller/internal/backup/opstatus.go | `(op, stack) (msg string, ok bool)` / `(op, stack) (busy, mirrorApp string)` | THE way a restore op begins: the check for every other operation (a restore op, the backup run, a second-drive copy) and the record in ONE step under `m.mu` — the mirror asks under the same lock (`beginTier2Run`) | **Never call `BeginRestoreOp` from a handler** (`TestR1256_H6_NoHandlerBeginsARestoreUnchecked`): `restoreOpBlocked` asked early can be stale by the time the form is read, and a mirror that began in between ran beside the restore. `restoreOpBlocked` stays as the EARLY refusal; the kept loads begin inside `LoadKeptApp` / `LoadKeptOffsite` and return `ErrRestoreBusy` | | `Manager.RestoreFromRecoveryUnitAt` / `RestoreFromRecoveryUnit` + `UnitRestoreResult` (R-353 v0.226.0; R-102 v0.229.0) | controller/internal/backup/restore_unit.go | `(stack, unitDir string) (UnitRestoreResult, error)` / `(stack string) (…)` | THE local recovery-unit restore, and the facts its surface must state | **Returns a RESULT, not just an error** — volumes replayed, DBs replayed, and what the manifest LISTED. The Manifest* counts are load-bearing: zero-replayed has two causes (the backup held no data / the backup listed data that did not come back) and they are opposite news. Pair it with `unitRestoreOutcomeMsg`; do NOT write a new sentence. **A claim about the APP is forbidden on this path** — it has no `SafetyDump` discriminator, unlike the off-site twin (CONTEXT.md ruling, 07-backup-architecture §6.3). `restoreDockerVolumes` now returns `(int, error)`; `restoreDockerVolumesFrom` is unchanged and still the shared implementation. **R-102: `…At` holds the whole body and takes the unit DIRECTORY; the one-argument form is the thin caller naming the primary unit.** THE SOURCE MOVES; THE DESTINATION DOES NOT — `GetAppDrivePath` still resolves where the data lands. The R-47 mutation order, the unit-over-guest secret precedence, the fail-closed data-key gate and the no-unit `RestoreApp` fallback are all pinned and unchanged. The volume leg goes through the R-354 `volumeReplayFrom` seam so the source directory is assertable without Docker | -| `Manager.putBackAfterFailedRestore` + `liveVolumeCopy` (R-893 option A, v0.336.0) | controller/internal/backup/offbox_restore_undo.go | `(stack, cause, live, copy, volumesMoved, filesPlaced, restart, holdKey) error` | THE restore undo — off-site (v0.336.0) and, since D22 (unreleased), the LOCAL unit restore (`RestoreFromRecoveryUnitAtWith`, so also the second drive's and a kept load): the live volumes it will replace, measured before the stop (`planLiveVolumeCopy`), copied right after it (`copyLiveVolumes`), poured back on any failure | One copy per restore under `/backups/restore-undo//` (no capture set reads `backups/`); the put-back uses the restore's own `volumeReplayFrom`/`restoreDockerVolumesFrom`, so there is one volume-replay implementation. A put-back that fails sets `keep` and HOLDS (option C) — never start on volumes that are not known to be back. Seams for tests without Docker: `undoVolumeProbeFn`, `undoVolumeTarFn`, `undoVolumeRemoveFn`; operator notice `SetRestorePutBackNotify` (leg `restore-put-back`). Pinned by `r893_putback_test.go` and `r1177_local_putback_test.go` | +| `Manager.putBackAfterFailedRestore` + `liveVolumeCopy` (R-893 option A, v0.336.0) | controller/internal/backup/offbox_restore_undo.go | `(stack, cause, live, copy, volumesMoved, filesPlaced, restart, holdKey, restoreKind) error` (R-1340: `restoreKind` = `settings.RestoreKindOffsite`/`Local`; a failed put-back records its step in the hold's `MixedCause`/`FailedStep`/`DBReplay`) | THE restore undo — off-site (v0.336.0) and, since D22 (unreleased), the LOCAL unit restore (`RestoreFromRecoveryUnitAtWith`, so also the second drive's and a kept load): the live volumes it will replace, measured before the stop (`planLiveVolumeCopy`), copied right after it (`copyLiveVolumes`), poured back on any failure | One copy per restore under `/backups/restore-undo//` (no capture set reads `backups/`); the put-back uses the restore's own `volumeReplayFrom`/`restoreDockerVolumesFrom`, so there is one volume-replay implementation. A put-back that fails sets `keep` and HOLDS (option C) — never start on volumes that are not known to be back. Seams for tests without Docker: `undoVolumeProbeFn`, `undoVolumeTarFn`, `undoVolumeRemoveFn`; operator notice `SetRestorePutBackNotify` (leg `restore-put-back`). Pinned by `r893_putback_test.go` and `r1177_local_putback_test.go` | | `Manager.SweepUnusedRestoreCopies` (R-1256 H8) | controller/internal/backup/restore_copy_sweep.go | `(now) []string` | THE only AUTOMATIC remover of a restore copy nothing names any more (`restore-copy-sweep`, every 6 h); the household's „Törlés most" is `DeleteRestoreCopy` (D62, restore_copies.go — same „needed" rule via `restoreCopyNeededFor`, check + removal under `m.mu`) | Removes a copy only when ALL hold: older than 30 days (D62), the app has no restore hold of ANY kind, the app-stop marker is readable and does not name the app, no restore op runs. It answers „no" when it cannot answer — a held or waiting copy is support's way back | | `CrashPutBack` + `Manager.persistUndoForCrash` + `AppStopGuard.SetUndo` (D22 / R-1195, v0.338.0) | controller/internal/backup/restore_crash_putback.go | `NewCrashPutBack(stop, writeDef, logger)`; `PutBackInterruptedRestore(stack, undoDir) error` | A restore cut by a power loss is put back at start: the copy (volumes + `definition/` + `undo.json`) is recorded in the app-stop marker once whole; `Recover` pours it back BEFORE the restart | Call `persistUndoForCrash` right after `copyLiveVolumes` in any new restore path. `Recover` runs before the backup manager exists, so the put-back is wired in main.go with `stackMgr.StopStack` and the stack adapter's definition writer. A failed put-back = a FAILED recovery (app not started, marker kept). The household reads `note.restore.interrupted_put_back` (`LoadRestoreRecordAfterRecovery`). Pinned by `TestR1195_*` | | `tier2CaptureSetFull(...) (legs, missing, warns, classified)` + `keptCopyRels` + `dropKeptCopiesUnder` (R-1252, unreleased) | controller/internal/backup/tier2_capture.go, tier2_kept.go | `classified` = the legs come from a VALID backup block | **The second-drive reconcile deletes only under a valid classification**; a rejected or absent block keeps every dest dir; kept copies (D25) are always in the keep set; a mirror deleted by Remove drops its kept records | A new reconcile caller must take `classified` and the kept rels too — never reconcile against the legacy legs. A rejected block is told by `Manager.BackupBlockRejected` (stacks; via the adapter's optional `BackupBlockRejected` method) and signalled once a day (`SetTier2SettingsUnreadableNotify`). Pinned by `r1252_rejected_block_keeps_copies_test.go` | | `holdreport.Reporter` + `settings.RestoreHold.Why` / `WireReason` (D53, unreleased) | controller/internal/holdreport/holdreport.go · controller/internal/settings/settings.go | `New(store, notifier.PushEvent, logger)`; `Tick(now)` from the scheduler | Tells the hub every hold in place (`app_held`), every hold gone (`app_hold_cleared`), each again daily | **One observer of the hold store — never a send inside a hold writer.** A new hold writer needs nothing here. The hold sentence is `RestoreHold.Why()` (one per reason) — never a second copy. Pinned by `internal/holdreport/holdreport_test.go` | | `AppStopGuard.ResumePutBack` / `ResumeWaitingPutBacks` + `HoldReasonPutBackPending` (R-1251, unreleased) | controller/internal/backup/appstop_putback_wait.go | `ResumePutBack(stack) bool` (true = nothing owed, the caller may start); `ResumeWaitingPutBacks() []string` (puts back, then starts through the gated starter) | A cut restore's copy on a drive that is not bound is WAITING: Recover writes the restore hold `putback_pending` (copy in `SafetyDump`) and starts nothing; the put-back runs when the drive binds, before any start | **Any new path that starts an app when its drive arrives calls `ResumePutBack` first** (the boot recreate and `restartStacks` do, in web via `Server.resumePutBack`). Every other start path is covered by the hold store it already honours — do not add a second check. A failed put-back (at start or on resume) writes the D20 hold (`restore_mixed`) and drops the app from the marker. Needs `SetHolds` + `SetUndoReachable` before `Recover` (main.go). Pinned by `r1251_putback_waits_test.go`, `TestR1251_DriveStartPathsPutBackBeforeTheyStart`, `TestR1251_MainWiresThePutBackWait` | +| `AppStopGuard.ClearHoldByOperator` + `clearHoldNow` / `listenHoldCtl` / `clearRestoreHoldCLI` (D64 = B, R-1343, unreleased) | controller/internal/backup/hold_clear.go · controller/cmd/controller/holdclear.go | `ClearHoldByOperator(stack) (RestoreHold, error)`; `clearHoldNow(deps, stack, who) (msg, error)` | THE operator's hold clear, any reason — the hub's `clear_hold` and the box command `--clear-restore-hold` both go through `clearHoldNow` in the RUNNING controller | **Never clear a hold from a second process** — the running controller's next save writes it back (R-1343, measured). The box command reaches the running controller over `/ctl/hold.sock` (unix, 0600, peer uid checked; never TCP/HTTP). A `putback_pending` clear also drops the app from the app-stop marker (the copy stays). Refusals (`ErrHoldBadStack`, `ErrNotHeld`) change nothing. The update's in-memory held flag goes via `stacks.Manager.ForgetUpdateHeld`. Pinned by `cmd/controller/holdclear_test.go` | | `Manager.RestoreTier2Unit` + `Tier2Coverage.CanRestoreUnit` / `Tier2CopyDate` (R-102/R-103, v0.229.0) | controller/internal/backup/tier2_restore.go | `(stack string) (UnitRestoreResult, error)` / `() bool` / `() (string, bool)` | THE full restore from the SECOND DRIVE's mirror, and the predicate that gates it | **Do NOT widen `CanRestore()`** — it answers only "can the additive file restore run?" and one predicate answering two questions is R-356. Fail-closed: `tier2UnitIsOpenable` requires a parseable manifest, because a directory is not a package. The single-writer flag is taken INSIDE `RestoreFromRecoveryUnitAt` — a second `acquireRunning()` here would refuse the restore it guards. `Tier2CopyDate` prefers the SUCCESS anchor over the attempt clock (R-101) and reports which it returned | | `unitRestoreOutcomeMsg` + its three message constants (R-353, v0.226.0) | controller/internal/web/handlers.go | `(app string, res backup.UnitRestoreResult) string` | THE customer sentence for a completed LOCAL restore | Twin of `reconstituteOutcomeMsg`; copy its SHAPE (clauses earned by having done the thing, no filesystem path, base names only), never its text. The constants are named because `r353_unit_outcome_test.go` asserts them verbatim — a silent edit is how an honest message drifts back into a comforting one, which is the documented history of the sentence it replaces | | `offsiteNoSpaceMsgFmt` + `offsiteSizeUnknownMsg` (R-357, v0.226.0) | controller/internal/backup/offbox_restore.go | two consts | EVERY headroom refusal on the off-site restore surface | **All four gates share these** (prepare, scratch, place, and the destructive reconstitute). A customer meeting one wording on one path and a different one on another has to work out whether it is the same problem. **The reconstitute gate uses NO ×1.1 margin** — it copies a measured tree; `OffboxRestorePrepareFull`'s ×1.1 predicts a download. **Fail closed when either probe reads ≤ 0**: `free < need` with `need == 0` is FALSE, so an unmeasurable input sails through — a gate present and inert | @@ -456,7 +457,7 @@ Cross-repo edges: | timeAgo switch body ×2 | controller/internal/web/funcmap.go `timeAgo` vs `timeAgoStr` (identical formatting logic) | | Budapest timezone loader ×4 | controller/internal/scheduler/scheduler.go `getBudapestLocation` vs controller/internal/web/funcmap.go `getTimezone` vs controller/internal/quiesce/quiesce.go `budapestLocation` vs controller/internal/api/kept_install.go `budapest` (v0.277.0, the kept choice's dates) (v0.168.0 window gate — Budapest wall-clock, kept local to avoid a scheduler↔quiesce import edge) | | JSON writers ×5, 3 envelope shapes | api `writeJSON`; web `writeDiskJSON`, `jsonResponse`/`jsonError`, `writeDebugJSON` | -| Safe-name validators ×4 | controller/internal/web/validate.go `validStackName`; controller/internal/api/router.go `validStackParam` (same body — api↔web import cycle); controller/internal/backup/offbox.go `isSafeStackName`; controller/internal/appexport/validate.go `ValidateSegment` (strictest) | +| Safe-name validators ×5 | controller/internal/web/validate.go `validStackName`; controller/internal/api/router.go `validStackParam` (same body — api↔web import cycle); controller/internal/backup/offbox.go `isSafeStackName` (exported as `IsValidHoldStack` for the hold commands); controller/internal/report/opactions.go `validHoldStack` (same alphabet as `isSafeStackName` — report cannot import backup; clear_hold's argument check); controller/internal/appexport/validate.go `ValidateSegment` (strictest) | | DB wait/import ×2 | controller/internal/appbackup/dbdump.go `waitDBReady`/`ImportDump` vs controller/internal/appexport/restore.go `waitForDB`/`importDBDump` | | qBittorrent Web API login ×2 | controller/internal/mediawire/mediawire.go `qbLogin` (writes the drive wiring) vs controller/internal/upwatch/explain.go `QBittorrent.login` (reads every minute; keeps its session cookie, which mediawire's per-run login does not need) | | compose exec ×2 | controller/internal/stacks/manager.go `composeExecCustomEnv` vs controller/internal/appexport/restore.go `composeExecEnv` (the latter has ctx+timeout; the former has the userdata belt) | diff --git a/controller/README.md b/controller/README.md index 368cb07d..476dc56d 100644 --- a/controller/README.md +++ b/controller/README.md @@ -1017,7 +1017,8 @@ Each app can define rich metadata in `.felhom.yml`: the hold is persisted in `Settings.RestoreHolds`, every start path refuses it (customer button, app-stop `Recover()`, boot sweep, via `driveStartGate` **above** its driveless early return), the app-stop marker is ended so nothing auto-restarts it, and the row goes red. Cleared with - `--clear-restore-hold `, **which requires a controller restart**. `--single-transaction` on + `--clear-restore-hold ` (since 2026-10-29, R-1343: through the running controller, no restart — see + „Operator actions") or the hub's „Clear hold" (`clear_hold`). `--single-transaction` on the Postgres import is a belt only; MariaDB DDL is not transactional, which is why the rollback is the fix. - **Off-site restore of one app: a failed restore puts the app back EXACTLY as it was and starts it (v0.336.0, @@ -3655,6 +3656,19 @@ Anything else is answered `refused` and calls nothing. Each id runs once per pro (`operator_action_results: [{id, outcome, message}]`, outcome `done` / `refused` / `failed`). No action deletes data, starts a countdown or shortens one (`TestOpActions_ClosedList`). The box's log records every action it receives. +- **`clear_hold` (2026-10-29, operator ruling D64 = B; arg = the app's stack name, letters/digits/`-`/`_`, ≤64):** lifts + the app's hold of ANY reason, in the RUNNING controller (`backup.AppStopGuard.ClearHoldByOperator`, the door + `clearHoldNow` in `cmd/controller/holdclear.go`): memory and `settings.json` together, so no later save writes it back + (R-1343). It starts nothing — the household's Start (or a restore) does. A waiting put-back (`putback_pending`) is + dropped from the app-stop marker too; its copy stays on disk. The update's in-memory „held" flag + (`stacks.Manager.ForgetUpdateHeld`) goes with it; the hold reporter's next tick (≤1 min) sends `app_hold_cleared`. + Refused, nothing changed: a bad name, or an app with no hold. No new event type: the hub records the result. +- **The box command `--clear-restore-hold `** (`pct exec -- docker exec felhom-controller felhom-controller + --clear-restore-hold `) asks the running controller over a unix socket, `/ctl/hold.sock` (0600 in a + 0700 dir, peer uid checked, nothing on TCP — not reachable through traefik, the LAN or an app), and says no restart is + needed. Only when no controller answers does it edit `settings.json` itself, and it says so, naming the in-guest + restart. `--restore-holds` lists the holds (read-only). + #### Report Push (`internal/report/`) Periodic JSON push (default every 15 min) to the central felhom-hub service: diff --git a/controller/cmd/controller/holdclear.go b/controller/cmd/controller/holdclear.go new file mode 100644 index 00000000..01e15db0 --- /dev/null +++ b/controller/cmd/controller/holdclear.go @@ -0,0 +1,260 @@ +package main + +import ( + "bufio" + "encoding/json" + "errors" + "fmt" + "io" + "log" + "net" + "os" + "path/filepath" + "strings" + "time" + + "gitea.dooplex.hu/admin/felhom-controller/internal/backup" + "gitea.dooplex.hu/admin/felhom-controller/internal/report" + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" +) + +// ── D64 = B + R-1343: the operator clears a hold in the RUNNING controller ───────────────────────────────────────── +// +// Two routes reach the same door (clearHoldNow): +// - the hub's „Clear hold" → the operator action clear_hold (internal/report/opactions.go), run in this process; +// - the box command `felhom-controller --clear-restore-hold ` (run with `docker exec` inside the guest) → the +// local socket below → the running controller. +// The running controller clears the hold in memory AND saves settings.json at once (backup.ClearHoldByOperator), so +// no later save — the hold reporter's, a hub report's — can write it back (R-1343). The household's „held" sentence +// goes with it: every reader (the page, the start gate, the API, the hold reporter) reads the same settings record. +// The ONE held flag kept elsewhere — stacks.Manager's „the last update ended held" — is forgotten here too. + +// holdClearDeps are clearHoldNow's seams (production: holdClearDepsFor). +type holdClearDeps struct { + clear func(stack string) (settings.RestoreHold, error) // backup.AppStopGuard.ClearHoldByOperator + forgetUpdateHeld func(stack string) bool // stacks.Manager.ForgetUpdateHeld (nil = none) + logger *log.Logger +} + +// clearHoldNow lifts stack's hold and returns the result sentence (English, operator-facing). who names the person +// and the route for the box's own log, e.g. „the operator via the hub, action #12". A refusal (bad name, no hold) is +// wrapped in report.ErrActionRefused, and nothing was changed. +func clearHoldNow(d holdClearDeps, stack, who string) (string, error) { + if d.clear == nil { + return "", fmt.Errorf("%w: clearing a hold is not available on this box", report.ErrActionRefused) + } + h, err := d.clear(stack) + switch { + case errors.Is(err, backup.ErrHoldBadStack): + return "", fmt.Errorf("%w: %q is not a valid app name — nothing was changed", report.ErrActionRefused, stack) + case errors.Is(err, backup.ErrNotHeld): + return "", fmt.Errorf("%w: %s has no hold — nothing was changed", report.ErrActionRefused, stack) + case err != nil: + return "", fmt.Errorf("clearing the hold on %s failed: %v", stack, err) + } + if d.forgetUpdateHeld != nil && d.forgetUpdateHeld(stack) { + if d.logger != nil { + d.logger.Printf("[INFO] [hold] %s: the update's held state is forgotten with the hold", stack) + } + } + if d.logger != nil { + d.logger.Printf("[WARN] [hold] %s: hold CLEARED by %s — it was %s (set %s); the app is not started by this", stack, who, h.WireReason(), h.At) + } + msg := fmt.Sprintf("the hold on %s is cleared (it was: %s). The app is NOT started by this — the household's Start, or a restore, starts it", stack, h.Why()) + if h.Reason == settings.HoldReasonPutBackPending { + dir := h.SafetyDump + if dir == "" { + dir = "(not named in the hold)" + } + msg += ". Its waiting put-back is dropped: the pre-restore copy stays on disk at " + dir + " and is never poured back by itself" + } + return msg, nil +} + +// ── the local channel (R-1343) ──────────────────────────────────────────────────────────────────────────────────── +// +// WHY A UNIX SOCKET IN THE CONTROLLER'S DATA DIR, mode 0600 in a 0700 directory, and nothing on TCP: +// - traefik, the LAN and every app reach the controller only over TCP (:8080, :8081). A unix socket has no port, +// so none of them can reach it, whatever a proxy header or a forwarded request says; +// - the data dir is the controller's own volume: no app container mounts it, and only root in the guest +// (`pct exec -- docker exec felhom-controller …`, the operator gate this project already uses) can enter +// the controller's container to use it; +// - the directory is created 0700 BEFORE the socket is bound, so the moment between bind and chmod is not a window; +// - the peer's uid must equal this process's (SO_PEERCRED, holdctl_linux.go) — a second lock on the same door. +// The customer's HTTP surface is deliberately NOT used: it authenticates as the household, and a household clearing +// its own hold is what the hold exists to prevent. Pinned by TestHoldCtl_NotReachableFromOutside. + +const ( + holdCtlDir = "ctl" + holdCtlSock = "hold.sock" + // holdCtlOpClear is the one request this channel answers. + holdCtlOpClear = "clear_hold" + // holdCtlMaxRequest bounds what one request may send. + holdCtlMaxRequest = 4096 +) + +// holdCtlPath is the socket's path under the controller's data dir. +func holdCtlPath(dataDir string) string { return filepath.Join(dataDir, holdCtlDir, holdCtlSock) } + +type holdCtlRequest struct { + Op string `json:"op"` + Stack string `json:"stack"` +} + +type holdCtlResponse struct { + Outcome string `json:"outcome"` // done | refused | failed (report.Outcome*) + Message string `json:"message"` +} + +// listenHoldCtl binds the socket and serves it until the listener is closed (closing it removes the socket file). +// clear is clearHoldNow bound to the running controller's deps. +func listenHoldCtl(path string, clear func(stack, who string) (string, error), logger *log.Logger) (net.Listener, error) { + dir := filepath.Dir(path) + if err := os.MkdirAll(dir, 0o700); err != nil { + return nil, err + } + if err := os.Chmod(dir, 0o700); err != nil { + return nil, err + } + // A socket left by a controller that died is replaced; anything else at that path is refused, never removed. + if fi, err := os.Lstat(path); err == nil { + if fi.Mode()&os.ModeSocket == 0 { + return nil, fmt.Errorf("%s exists and is not a socket — refusing to replace it", path) + } + if err := os.Remove(path); err != nil { + return nil, err + } + } + ln, err := net.Listen("unix", path) + if err != nil { + return nil, err + } + if err := os.Chmod(path, 0o600); err != nil { + ln.Close() + return nil, err + } + go func() { + for { + conn, err := ln.Accept() + if err != nil { + if !errors.Is(err, net.ErrClosed) && logger != nil { + logger.Printf("[WARN] [hold] local channel stopped: %v", err) + } + return + } + go serveHoldCtl(conn, clear, logger) + } + }() + return ln, nil +} + +func serveHoldCtl(conn net.Conn, clear func(stack, who string) (string, error), logger *log.Logger) { + defer conn.Close() + _ = conn.SetDeadline(time.Now().Add(30 * time.Second)) + answer := func(r holdCtlResponse) { _ = json.NewEncoder(conn).Encode(r) } + if ok, why := holdCtlPeerAllowed(conn); !ok { + if logger != nil { + logger.Printf("[WARN] [hold] local channel: a request was refused — %s", why) + } + answer(holdCtlResponse{Outcome: report.OutcomeRefused, Message: "refused: " + why}) + return + } + line, err := bufio.NewReader(io.LimitReader(conn, holdCtlMaxRequest)).ReadBytes('\n') + if err != nil && len(line) == 0 { + answer(holdCtlResponse{Outcome: report.OutcomeRefused, Message: "no request was read"}) + return + } + var req holdCtlRequest + if err := json.Unmarshal(line, &req); err != nil || req.Op != holdCtlOpClear { + answer(holdCtlResponse{Outcome: report.OutcomeRefused, Message: "unknown request — this channel only clears a hold"}) + return + } + msg, err := clear(req.Stack, "the operator on the box (--clear-restore-hold)") + switch { + case err == nil: + answer(holdCtlResponse{Outcome: report.OutcomeDone, Message: msg}) + case errors.Is(err, report.ErrActionRefused): + answer(holdCtlResponse{Outcome: report.OutcomeRefused, Message: strings.TrimPrefix(err.Error(), report.ErrActionRefused.Error()+": ")}) + default: + answer(holdCtlResponse{Outcome: report.OutcomeFailed, Message: err.Error()}) + } +} + +// askRunningControllerToClear sends one clear request. reached=false: no controller answered at path (no socket, or +// a socket nobody listens on) — nothing was asked, so the caller may edit settings.json itself. reached=true with an +// error: the request may have been applied; the caller must not edit the file. +func askRunningControllerToClear(path, stack string) (resp holdCtlResponse, reached bool, err error) { + conn, err := net.DialTimeout("unix", path, 3*time.Second) + if err != nil { + return holdCtlResponse{}, false, err + } + defer conn.Close() + _ = conn.SetDeadline(time.Now().Add(60 * time.Second)) + if err := json.NewEncoder(conn).Encode(holdCtlRequest{Op: holdCtlOpClear, Stack: stack}); err != nil { + return holdCtlResponse{}, true, err + } + if err := json.NewDecoder(io.LimitReader(conn, 64*1024)).Decode(&resp); err != nil { + return holdCtlResponse{}, true, err + } + return resp, true, nil +} + +// clearRestoreHoldCLI is the box command `--clear-restore-hold ` (R-379, R-1343). It asks the running +// controller first; only when none answers does it edit settings.json itself, and it says which one happened. +// Returns the process exit code: 0 cleared, 2 refused (nothing changed), 1 failed or unknown. +func clearRestoreHoldCLI(dataDir, stack string, stdout, stderr io.Writer) int { + path := holdCtlPath(dataDir) + resp, reached, err := askRunningControllerToClear(path, stack) + if reached { + if err != nil { + fmt.Fprintf(stderr, "the running controller was reached, but its answer was lost: %v\n", err) + fmt.Fprintf(stderr, " The hold may or may not be cleared. Check with: felhom-controller --restore-holds\n") + fmt.Fprintf(stderr, " settings.json was NOT edited by this command.\n") + return 1 + } + switch resp.Outcome { + case report.OutcomeDone: + fmt.Fprintf(stdout, "%s.\n", resp.Message) + fmt.Fprintf(stdout, " Cleared by the RUNNING controller (memory and settings.json together) — no restart is needed.\n") + return 0 + case report.OutcomeRefused: + fmt.Fprintf(stderr, "%s\n", resp.Message) + return 2 + default: + fmt.Fprintf(stderr, "the running controller could not clear the hold: %s\n", resp.Message) + return 1 + } + } + // No controller answered: it is not running (or is older than this channel). The file is the only copy. + fmt.Fprintf(stdout, "No running controller answered on %s (%v).\n", path, err) + fmt.Fprintf(stdout, " Editing settings.json directly instead.\n") + if !backup.IsValidHoldStack(stack) { + fmt.Fprintf(stderr, "%q is not a valid app name — nothing was changed\n", stack) + return 2 + } + lg := log.New(stderr, "", 0) + sett, lerr := settings.Load(filepath.Join(dataDir, "settings.json"), lg) + if lerr != nil { + fmt.Fprintf(stderr, "restore-hold: loading settings: %v\n", lerr) + return 1 + } + cleared, cerr := sett.ClearRestoreHold(stack) + if cerr != nil { + fmt.Fprintf(stderr, "restore-hold: clearing %s: %v\n", stack, cerr) + return 1 + } + if !cleared { + // "there was nothing to clear" is not success — a silent 0 here would let an operator believe they had + // unblocked an app whose name they mistyped. + fmt.Fprintf(stderr, "no restore hold in force for %q — nothing was changed\n", stack) + return 2 + } + fmt.Fprintf(stdout, "restore hold cleared for %s in settings.json (no controller was running to ask).\n", stack) + fmt.Fprintf(stdout, " The controller reads it when it starts. If a controller IS running in this guest after all, restart it NOW,\n") + fmt.Fprintf(stdout, " INSIDE THE GUEST — from the Proxmox host: pct exec -- systemctl restart felhom-controller-bootstrap.service\n") + fmt.Fprintf(stdout, " — or its next settings save writes the hold back (R-1343).\n") + return 0 +} + +// holdClearCommandHint is the exact guest command, for the help text and the operator's mail. +const holdClearCommandHint = "pct exec -- docker exec felhom-controller felhom-controller --clear-restore-hold " diff --git a/controller/cmd/controller/holdclear_test.go b/controller/cmd/controller/holdclear_test.go new file mode 100644 index 00000000..0bde0630 --- /dev/null +++ b/controller/cmd/controller/holdclear_test.go @@ -0,0 +1,516 @@ +package main + +import ( + "bytes" + "context" + "encoding/json" + "go/ast" + "go/parser" + "go/token" + "io" + "log" + "net" + "os" + "path/filepath" + "strconv" + "strings" + "sync" + "testing" + "time" + + "gitea.dooplex.hu/admin/felhom-controller/internal/backup" + "gitea.dooplex.hu/admin/felhom-controller/internal/config" + "gitea.dooplex.hu/admin/felhom-controller/internal/holdreport" + "gitea.dooplex.hu/admin/felhom-controller/internal/report" + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" +) + +// D64 = B (operator ruling 2026-10-29 12:10) + R-1343: the operator clears a hold — from the hub (clear_hold) or with +// the box command — IN THE RUNNING CONTROLLER, so the hold is gone from memory and from settings.json together and no +// later save brings it back. +// +// RED-PROOFS (2026-10-29): +// - TestR1343_TheBoxCommandClearsThroughTheRunningController: with clearRestoreHoldCLI's socket call skipped (the +// old second-process edit of settings.json), it fails on „the hold came back after a later save of the running +// controller" — the R-1343 measurement, in a test; +// - TestD64_ClearPutBackPendingDropsTheMarkerEntry: with the dropFromMarker call removed from +// ClearHoldByOperator, it fails on „a later controller start poured the old copy back"; +// - TestD64_ClearHoldViaTheHub: with the forgetUpdateHeld call removed from clearHoldNow, it fails on „the +// update's held flag was not forgotten". +// - TestD64_ClearHoldViaTheHub + TestD64_ClearHoldStaysGoneAfterALaterSave: with ClearHoldByOperator clearing a +// SECOND Settings loaded from disk (the old CLI's shape) instead of the running one, they fail on „the running +// controller still holds the app" / „the hold came back after a later save of the running controller". + +// d64Rig is a running controller's hold side: its Settings, the app-stop guard wired to them, and a log. +type d64Rig struct { + dir string + sett *settings.Settings + guard *backup.AppStopGuard + logBuf *syncBuffer + logger *log.Logger + forgot []string + holdDeps holdClearDeps +} + +type syncBuffer struct { + mu sync.Mutex + b bytes.Buffer +} + +func (s *syncBuffer) Write(p []byte) (int, error) { + s.mu.Lock() + defer s.mu.Unlock() + return s.b.Write(p) +} +func (s *syncBuffer) String() string { s.mu.Lock(); defer s.mu.Unlock(); return s.b.String() } + +func newD64Rig(t *testing.T) *d64Rig { + t.Helper() + r := &d64Rig{dir: t.TempDir(), logBuf: &syncBuffer{}} + r.logger = log.New(r.logBuf, "", 0) + sett, err := settings.Load(filepath.Join(r.dir, "settings.json"), r.logger) + if err != nil { + t.Fatal(err) + } + r.sett = sett + r.guard = backup.NewAppStopGuard(filepath.Join(r.dir, "appstop-state.json"), r.logger) + r.guard.SetHolds(sett) + r.holdDeps = holdClearDeps{ + clear: r.guard.ClearHoldByOperator, + forgetUpdateHeld: func(s string) bool { r.forgot = append(r.forgot, s); return true }, + logger: r.logger, + } + return r +} + +func (r *d64Rig) hold(t *testing.T, h settings.RestoreHold) { + t.Helper() + if h.At == "" { + h.At = "2026-10-29T08:18:00Z" + } + if err := r.sett.SetRestoreHold(h); err != nil { + t.Fatal(err) + } +} + +func (r *d64Rig) onDisk(t *testing.T, stack string) bool { + t.Helper() + s, err := settings.Load(filepath.Join(r.dir, "settings.json"), log.New(io.Discard, "", 0)) + if err != nil { + t.Fatal(err) + } + _, held := s.GetRestoreHold(stack) + return held +} + +func (r *d64Rig) actions() *report.OperatorActions { + return report.NewOperatorActions(context.Background(), operatorActionHandlers(nil, nil, r.holdDeps, nil, r.logger), r.logger) +} + +func onlyResult(t *testing.T, o *report.OperatorActions, id int64) report.OperatorActionResult { + t.Helper() + for _, x := range o.Results() { + if x.ID == id { + return x + } + } + t.Fatalf("no result for #%d: %+v", id, o.Results()) + return report.OperatorActionResult{} +} + +// The hub's clear_hold: the hold is gone (memory and disk), the household's held sentence is gone, the update's +// held flag is forgotten, the box logs who did it, and the result names what was cleared. +func TestD64_ClearHoldViaTheHub(t *testing.T) { + r := newD64Rig(t) + r.hold(t, settings.RestoreHold{Stack: "paperless", ReplayError: "ERROR: relation exists", RollbackErr: "psql: refused"}) + cfg := &config.Config{} + cfg.Paths.DataDir = r.dir + mgr := backup.NewManager(cfg, r.sett, r.logger) + if held, why := mgr.RestoreHoldForLang("paperless", "hu"); !held || why == "" { + t.Fatal("fixture: the household must read a held sentence before the clear") + } + + o := r.actions() + o.Reconcile([]report.OperatorAction{{ID: 12, Action: report.OpClearHold, Arg: "paperless"}}) + res := onlyResult(t, o, 12) + if res.Outcome != report.OutcomeDone { + t.Fatalf("result %+v", res) + } + for _, want := range []string{"the hold on paperless is cleared", "held after a failed restore whose rollback also failed", "NOT started by this"} { + if !strings.Contains(res.Message, want) { + t.Errorf("result message %q lacks %q", res.Message, want) + } + } + if _, held := r.sett.GetRestoreHold("paperless"); held { + t.Fatal("the running controller still holds the app") + } + if r.onDisk(t, "paperless") { + t.Fatal("settings.json still holds the app") + } + if held, why := mgr.RestoreHoldForLang("paperless", "hu"); held || why != "" { + t.Fatalf("the household still reads a held sentence: %q", why) + } + if len(r.forgot) != 1 || r.forgot[0] != "paperless" { + t.Fatalf("the update's held flag was not forgotten: %v", r.forgot) + } + if lg := r.logBuf.String(); !strings.Contains(lg, "paperless: hold CLEARED by the operator via the hub, action #12") { + t.Fatalf("the box's log must say who cleared it:\n%s", lg) + } +} + +// Refusals change nothing: an app nobody ever held, a second press on an app already cleared, a bad name. +func TestD64_ClearHoldRefusals(t *testing.T) { + r := newD64Rig(t) + r.hold(t, settings.RestoreHold{Stack: "kimai", Reason: settings.HoldReasonUpdateFailed}) + o := r.actions() + o.Reconcile([]report.OperatorAction{{ID: 1, Action: report.OpClearHold, Arg: "nosuchapp"}}) + if res := onlyResult(t, o, 1); res.Outcome != report.OutcomeRefused || !strings.Contains(res.Message, "nosuchapp has no hold") { + t.Fatalf("unknown app: %+v", res) + } + if _, held := r.sett.GetRestoreHold("kimai"); !held || !r.onDisk(t, "kimai") { + t.Fatal("a refusal changed another app's hold") + } + if len(r.forgot) != 0 { + t.Fatalf("a refusal touched the update state: %v", r.forgot) + } + o.Reconcile([]report.OperatorAction{{ID: 2, Action: report.OpClearHold, Arg: "kimai"}}) + o.Reconcile([]report.OperatorAction{{ID: 3, Action: report.OpClearHold, Arg: "kimai"}}) + if res := onlyResult(t, o, 3); res.Outcome != report.OutcomeRefused || !strings.Contains(res.Message, "kimai has no hold") { + t.Fatalf("not held (second press): %+v", res) + } + if _, err := clearHoldNow(r.holdDeps, "../kimai", "test"); err == nil || !strings.Contains(err.Error(), "not a valid app name") { + t.Fatalf("a bad name must be refused by the door itself too: %v", err) + } +} + +// R-1343's order: clear, then ANOTHER component saves settings (the hold reporter, a hub report), then the file is +// read back — the hold must stay gone, in memory and on disk. +func TestD64_ClearHoldStaysGoneAfterALaterSave(t *testing.T) { + r := newD64Rig(t) + r.hold(t, settings.RestoreHold{Stack: "radicale", Reason: settings.HoldReasonRestoreMixed}) + o := r.actions() + o.Reconcile([]report.OperatorAction{{ID: 5, Action: report.OpClearHold, Arg: "radicale"}}) + if res := onlyResult(t, o, 5); res.Outcome != report.OutcomeDone { + t.Fatalf("result %+v", res) + } + if err := r.sett.SetReportedHolds(map[string]string{"other": "x|y"}); err != nil { // another component's save + t.Fatal(err) + } + if r.onDisk(t, "radicale") { + t.Fatal("the hold came back after a later save of the running controller") + } + if _, held := r.sett.GetRestoreHold("radicale"); held { + t.Fatal("the running controller holds the app again") + } +} + +// The same order through the BOX COMMAND: it reaches the running controller over the socket; no restart. +func TestR1343_TheBoxCommandClearsThroughTheRunningController(t *testing.T) { + r := newD64Rig(t) + r.hold(t, settings.RestoreHold{Stack: "radicale", Reason: settings.HoldReasonRestoreMixed}) + ln, err := listenHoldCtl(holdCtlPath(r.dir), func(stack, who string) (string, error) { return clearHoldNow(r.holdDeps, stack, who) }, r.logger) + if err != nil { + t.Fatal(err) + } + defer ln.Close() + + var out, errb bytes.Buffer + if code := clearRestoreHoldCLI(r.dir, "radicale", &out, &errb); code != 0 { + t.Fatalf("exit %d, stdout %q, stderr %q", code, out.String(), errb.String()) + } + if err := r.sett.SetReportedHolds(map[string]string{"other": "x|y"}); err != nil { // the running controller's next save + t.Fatal(err) + } + if r.onDisk(t, "radicale") { + t.Fatal("the hold came back after a later save of the running controller") + } + if !strings.Contains(out.String(), "Cleared by the RUNNING controller") || !strings.Contains(out.String(), "no restart is needed") { + t.Fatalf("the command must say it went through the running controller and needs no restart:\n%s", out.String()) + } + if !strings.Contains(r.logBuf.String(), "hold CLEARED by the operator on the box (--clear-restore-hold)") { + t.Fatalf("the box's log must say who cleared it:\n%s", r.logBuf.String()) + } + + // A second run: refused through the socket, exit 2, nothing edited. + out.Reset() + errb.Reset() + if code := clearRestoreHoldCLI(r.dir, "radicale", &out, &errb); code != 2 || !strings.Contains(errb.String(), "radicale has no hold") { + t.Fatalf("not held: exit %d, stderr %q", code, errb.String()) + } +} + +// No controller runs (no socket, or a socket nobody listens on): the command edits settings.json and says so plainly, +// naming the guest restart for the case a controller runs after all. +func TestR1343_TheBoxCommandFallsBackWhenNoControllerRuns(t *testing.T) { + for _, stale := range []bool{false, true} { + r := newD64Rig(t) + r.hold(t, settings.RestoreHold{Stack: "kimai", Reason: settings.HoldReasonUpdateFailed}) + if stale { // a socket left by a controller that died + ln, err := listenHoldCtl(holdCtlPath(r.dir), func(string, string) (string, error) { return "", nil }, r.logger) + if err != nil { + t.Fatal(err) + } + ln.(*net.UnixListener).SetUnlinkOnClose(false) + ln.Close() + } + var out, errb bytes.Buffer + if code := clearRestoreHoldCLI(r.dir, "kimai", &out, &errb); code != 0 { + t.Fatalf("stale=%v: exit %d, stderr %q", stale, code, errb.String()) + } + for _, want := range []string{"No running controller answered", "Editing settings.json directly", "INSIDE THE GUEST", "pct exec -- systemctl restart felhom-controller-bootstrap.service"} { + if !strings.Contains(out.String(), want) { + t.Errorf("stale=%v: output lacks %q:\n%s", stale, want, out.String()) + } + } + if r.onDisk(t, "kimai") { + t.Fatalf("stale=%v: settings.json still holds the app", stale) + } + out.Reset() + errb.Reset() + if code := clearRestoreHoldCLI(r.dir, "kimai", &out, &errb); code != 2 { + t.Fatalf("stale=%v: not held must exit 2, got %d", stale, code) + } + if code := clearRestoreHoldCLI(r.dir, "../x", &out, &errb); code != 2 { + t.Fatalf("stale=%v: a bad name must exit 2, got %d", stale, code) + } + } +} + +// A put-back WAITING for its drive is also in the app-stop marker: clearing the hold must drop it there too, or the +// next controller start with the drive bound would pour the old copy over what the app wrote after the clear. The +// copy itself stays on disk (nothing is deleted). +func TestD64_ClearPutBackPendingDropsTheMarkerEntry(t *testing.T) { + r := newD64Rig(t) + undo := filepath.Join(r.dir, "drive", "felhom-data", "backups", "restore-undo", "immich", "20261029T080000Z") + if err := os.MkdirAll(undo, 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(undo, "undo.json"), []byte(`{"stack":"immich"}`), 0o600); err != nil { + t.Fatal(err) + } + if err := r.guard.Begin("unit-restore:immich", backup.ReasonUnitRestore, []string{"immich"}); err != nil { + t.Fatal(err) + } + if err := r.guard.SetUndo("immich", undo); err != nil { + t.Fatal(err) + } + r.hold(t, settings.RestoreHold{Stack: "immich", Reason: settings.HoldReasonPutBackPending, SafetyDump: undo}) + + o := r.actions() + o.Reconcile([]report.OperatorAction{{ID: 3, Action: report.OpClearHold, Arg: "immich"}}) + res := onlyResult(t, o, 3) + if res.Outcome != report.OutcomeDone || !strings.Contains(res.Message, "waiting put-back is dropped") || !strings.Contains(res.Message, undo) { + t.Fatalf("result %+v", res) + } + if _, err := os.Stat(filepath.Join(undo, "undo.json")); err != nil { + t.Fatal("the copy must stay on disk") + } + // The consequence: a later controller start with the drive bound pours nothing back. + poured := &recordingPutBacker{} + g2 := backup.NewAppStopGuard(filepath.Join(r.dir, "appstop-state.json"), r.logger) + g2.SetHolds(r.sett) + g2.SetPutBacker(poured) + g2.SetStarter(nopStarter{}) + g2.SetUndoReachable(func(string, string) bool { return true }) + g2.Recover() + if len(poured.stacks) != 0 { + t.Fatalf("a later controller start poured the old copy back over the app: %v", poured.stacks) + } +} + +type recordingPutBacker struct{ stacks []string } + +func (p *recordingPutBacker) PutBackInterruptedRestore(stack, _ string) error { + p.stacks = append(p.stacks, stack) + return nil +} + +type nopStarter struct{} + +func (nopStarter) StartStack(string) error { return nil } + +// The hold reporter's next tick tells the hub the hold is gone (it reads the same settings record; nothing cached). +func TestD64_TheHoldReporterTellsTheHubAfterTheClear(t *testing.T) { + r := newD64Rig(t) + r.hold(t, settings.RestoreHold{Stack: "kimai", Reason: settings.HoldReasonUpdateFailed}) + var events []string + rep := holdreport.New(r.sett, func(et, _, _ string, _ interface{}) { events = append(events, et) }, r.logger) + rep.Tick(timeAt(0)) + o := r.actions() + o.Reconcile([]report.OperatorAction{{ID: 1, Action: report.OpClearHold, Arg: "kimai"}}) + rep.Tick(timeAt(60)) + if strings.Join(events, ",") != holdreport.EventHeld+","+holdreport.EventCleared { + t.Fatalf("events %v — want app_held then app_hold_cleared", events) + } +} + +// „Not reachable from outside": the channel is a unix socket (no TCP anywhere in the file), 0600 in a 0700 directory +// — even when the directory existed with a looser mode — and main opens it once, under the data dir. +func TestHoldCtl_NotReachableFromOutside(t *testing.T) { + dir := t.TempDir() + if err := os.MkdirAll(filepath.Join(dir, holdCtlDir), 0o755); err != nil { + t.Fatal(err) + } + ln, err := listenHoldCtl(holdCtlPath(dir), func(string, string) (string, error) { return "", nil }, nil) + if err != nil { + t.Fatal(err) + } + defer ln.Close() + if n := ln.Addr().Network(); n != "unix" { + t.Fatalf("the channel listens on %q, want unix", n) + } + fi, err := os.Lstat(holdCtlPath(dir)) + if err != nil || fi.Mode()&os.ModeSocket == 0 || fi.Mode().Perm() != 0o600 { + t.Fatalf("socket mode %v (err %v), want a socket with 0600", fi.Mode(), err) + } + di, _ := os.Stat(filepath.Join(dir, holdCtlDir)) + if di.Mode().Perm() != 0o700 { + t.Fatalf("socket dir mode %v, want 0700", di.Mode().Perm()) + } + + // No TCP and no HTTP in the channel's file: every net.Listen/Dial names "unix". + f, err := parser.ParseFile(token.NewFileSet(), "holdclear.go", nil, parser.ImportsOnly) + if err != nil { + t.Fatal(err) + } + for _, im := range f.Imports { + if p, _ := strconv.Unquote(im.Path.Value); strings.HasPrefix(p, "net/http") { + t.Fatalf("holdclear.go imports %s — the channel must not be HTTP", p) + } + } + full, err := parser.ParseFile(token.NewFileSet(), "holdclear.go", nil, 0) + if err != nil { + t.Fatal(err) + } + nets := 0 + ast.Inspect(full, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok { + return true + } + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return true + } + if x, ok := sel.X.(*ast.Ident); ok && x.Name == "net" && len(call.Args) > 0 { + nets++ + if lit, ok := call.Args[0].(*ast.BasicLit); !ok || lit.Value != `"unix"` { + t.Errorf("net.%s is called with a network other than \"unix\"", sel.Sel.Name) + } + } + return true + }) + if nets < 2 { + t.Fatalf("expected the listen and the dial in holdclear.go, found %d net calls", nets) + } + + // main opens it exactly once, at holdCtlPath(cfg.Paths.DataDir). + mf, err := parser.ParseFile(token.NewFileSet(), "main.go", nil, 0) + if err != nil { + t.Fatal(err) + } + opened := 0 + ast.Inspect(mf, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok { + return true + } + if id, ok := call.Fun.(*ast.Ident); ok && id.Name == "listenHoldCtl" { + opened++ + arg, ok := call.Args[0].(*ast.CallExpr) + if fn, ok2 := arg.Fun.(*ast.Ident); !ok || !ok2 || fn.Name != "holdCtlPath" { + t.Error("main must open the channel at holdCtlPath()") + } + } + return true + }) + if opened != 1 { + t.Fatalf("main.go opens the channel %d time(s), want 1", opened) + } +} + +// The socket answers only its one request; anything else is refused and calls nothing. +func TestHoldCtl_UnknownRequestIsRefused(t *testing.T) { + dir := t.TempDir() + called := 0 + ln, err := listenHoldCtl(holdCtlPath(dir), func(string, string) (string, error) { called++; return "x", nil }, nil) + if err != nil { + t.Fatal(err) + } + defer ln.Close() + conn, err := net.Dial("unix", holdCtlPath(dir)) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + if _, err := conn.Write([]byte(`{"op":"set_hold","stack":"x"}` + "\n")); err != nil { + t.Fatal(err) + } + var resp holdCtlResponse + if err := json.NewDecoder(conn).Decode(&resp); err != nil { + t.Fatal(err) + } + if resp.Outcome != report.OutcomeRefused || called != 0 { + t.Fatalf("response %+v, calls %d — want refused and nothing called", resp, called) + } +} + +// Something at the socket's path that is NOT a socket is refused, never removed. +func TestHoldCtl_ANonSocketFileIsNotReplaced(t *testing.T) { + dir := t.TempDir() + p := holdCtlPath(dir) + if err := os.MkdirAll(filepath.Dir(p), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte("keep me"), 0o600); err != nil { + t.Fatal(err) + } + if _, err := listenHoldCtl(p, func(string, string) (string, error) { return "", nil }, nil); err == nil { + t.Fatal("a regular file at the socket path must be refused") + } + if b, _ := os.ReadFile(p); string(b) != "keep me" { + t.Fatal("the file at the socket path was changed") + } +} + +// main wires every piece once (the seam-built-but-never-wired trap): the clear door into the operator actions, the +// R-1340 text into the hold notification, the R-1341 gate into the OOM loop, and the box command. +func TestD64_MainWiresTheNewPieces(t *testing.T) { + f, err := parser.ParseFile(token.NewFileSet(), "main.go", nil, 0) + if err != nil { + t.Fatal(err) + } + found := map[string]int{} + var opArgs []ast.Expr + ast.Inspect(f, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok { + return true + } + switch fn := call.Fun.(type) { + case *ast.SelectorExpr: + if x, ok := fn.X.(*ast.Ident); ok { + found[x.Name+"."+fn.Sel.Name]++ + } + case *ast.Ident: + found[fn.Name]++ + if fn.Name == "operatorActionHandlers" { + opArgs = call.Args + } + } + return true + }) + for _, want := range []string{"heldStoppedOperatorText", "heldHoldFromNotify", "oomLog.fresh", "clearRestoreHoldCLI", "listenHoldCtl", "clearHoldNow"} { + if found[want] != 1 { + t.Errorf("main.go calls %s %d time(s), want exactly 1", want, found[want]) + } + } + if len(opArgs) < 3 { + t.Fatal("operatorActionHandlers call not found") + } + if id, ok := opArgs[2].(*ast.Ident); !ok || id.Name != "holdDeps" { + t.Error("main must hand holdDeps (the running controller's clear door) to operatorActionHandlers") + } +} + +func timeAt(sec int) time.Time { return time.Date(2026, 10, 29, 12, 0, sec, 0, time.UTC) } diff --git a/controller/cmd/controller/holdctl_linux.go b/controller/cmd/controller/holdctl_linux.go new file mode 100644 index 00000000..5ed693ef --- /dev/null +++ b/controller/cmd/controller/holdctl_linux.go @@ -0,0 +1,34 @@ +//go:build linux + +package main + +import ( + "fmt" + "net" + "os" + "syscall" +) + +// holdCtlPeerAllowed (R-1343) admits a peer on the hold socket only when its uid equals this process's (SO_PEERCRED). +// The socket's 0600 mode in a 0700 directory already says so; this is the second lock on the same door. +func holdCtlPeerAllowed(conn net.Conn) (bool, string) { + uc, ok := conn.(*net.UnixConn) + if !ok { + return false, "not a unix-socket peer" + } + raw, err := uc.SyscallConn() + if err != nil { + return false, "the peer's identity could not be read" + } + var cred *syscall.Ucred + var cerr error + if err := raw.Control(func(fd uintptr) { + cred, cerr = syscall.GetsockoptUcred(int(fd), syscall.SOL_SOCKET, syscall.SO_PEERCRED) + }); err != nil || cerr != nil || cred == nil { + return false, "the peer's identity could not be read" + } + if int(cred.Uid) != os.Geteuid() { + return false, fmt.Sprintf("the peer's uid %d is not this controller's", cred.Uid) + } + return true, "" +} diff --git a/controller/cmd/controller/holdctl_other.go b/controller/cmd/controller/holdctl_other.go new file mode 100644 index 00000000..61cd5c3e --- /dev/null +++ b/controller/cmd/controller/holdctl_other.go @@ -0,0 +1,14 @@ +//go:build !linux + +package main + +import "net" + +// holdCtlPeerAllowed: the peer check needs SO_PEERCRED (Linux, where the controller runs). Elsewhere the socket's +// 0600 mode in a 0700 directory is the only lock. +func holdCtlPeerAllowed(conn net.Conn) (bool, string) { + if _, ok := conn.(*net.UnixConn); !ok { + return false, "not a unix-socket peer" + } + return true, "" +} diff --git a/controller/cmd/controller/holdmail.go b/controller/cmd/controller/holdmail.go new file mode 100644 index 00000000..170de1ca --- /dev/null +++ b/controller/cmd/controller/holdmail.go @@ -0,0 +1,155 @@ +package main + +import ( + "fmt" + "strings" + + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" +) + +// ── R-1340: the operator's „HELD STOPPED" mail names the hold's REAL cause ──────────────────────────────────────── +// +// Until 2026-10-29 the restore-hold mail told one story for every restore_mixed hold — R-893 option C's „its off-site +// database restore failed. The database was rolled back …, but its files and named volumes are the snapshot's" — +// written when option C was the only mixed hold. Measured on Tester 1 (D63's H5 proof): Radicale, which has no +// database, whose put-back stopped at „the app could not be stopped" with nothing moved, got exactly that text. +// Support read a wrong account of the box. +// +// The text is now built from the HOLD RECORD the writer persisted just before the mail (its reason and the R-1340 +// fields: which restore, which branch, which step failed, whether the failure was a database replay, where the copy +// is kept). It never says „database" unless the restore's failure WAS a database replay, and never „off-site" for a +// restore from the app's own backup. One sentence per reason; pinned by TestR1340_* (holdmail_test.go). + +// heldStoppedOperatorText is the operator mail for a held app: the message, its leg (the hub reads the leg, so the +// two existing ones are kept: restore-hold for a restore hold, restore-hold-mixed for a mixed one) and a short reason. +func heldStoppedOperatorText(h settings.RestoreHold) (msg, leg, reason string) { + stack := h.Stack + const tail = " The app will not start from any path until the hold is cleared (the hub's Clear hold button, or on the box: " + holdClearCommandHint + ")." + kind := restoreKindPhrase(h.RestoreKind) + switch h.Reason { + case settings.HoldReasonUnhealthyStop: + what := "a crash loop" + if h.UnhealthyKind == "oom_storm" { + what = "an out-of-memory storm" + } + trip := "" + if h.Trip >= 2 { + trip = fmt.Sprintf(" This is stop %d within 24 hours.", h.Trip) + } + msg = fmt.Sprintf("App %q is HELD STOPPED: the box stopped it for %s at %s.%s No restore or update was involved and no data was touched. "+ + "The household's Start gives it one more try; clearing the hold lifts it too.", stack, what, h.At, trip) + return msg, "unhealthy-stop", "stopped for " + what + case settings.HoldReasonUpdateFailed: + undo := "" + switch h.UndoState { + case "untouched": + undo = " The automatic undo failed; the data was not touched." + case "half": + undo = " The automatic undo failed half-way; the data may be partly the new version's." + case "not_started": + undo = " The automatic undo did not start." + } + copyLine := " Restoring it from its backup starts it." + if h.NoWholeCopy { + copyLine = " No copy on this box brings it back whole — it needs support." + } else if h.CopyDate != "" { + copyLine = fmt.Sprintf(" Restoring it from its backup (tier %d, %s) starts it.", h.CopyTier, h.CopyDate) + } + msg = fmt.Sprintf("App %q is HELD STOPPED: its update at %s did not come up healthy.%s%s", stack, h.At, undo, copyLine) + tail + return msg, "update-hold", "update failed" + case settings.HoldReasonPutBackPending: + msg = fmt.Sprintf("App %q is HELD STOPPED: a restore was cut after its copy of the live data was taken, and that copy (%s) is on a drive that is not bound. "+ + "The copy is poured back and the app started when the drive is bound. Clearing the hold DROPS that put-back; the copy stays on disk.", stack, orNone(h.SafetyDump)) + return msg, "putback-pending", "put-back waits for its drive" + case settings.HoldReasonRestoreMixed: + return mixedHoldText(h, kind, tail) + default: // HoldReasonRestoreFailed: a database replay failed AND its rollback failed (holdAppAfterFailedRollback) + what := "its " + kind + " failed" + if h.DBReplay || h.RestoreKind == "" { // every restore hold before R-1340 was written by the database branch + what = "its " + kind + " failed at the database replay AND the rollback to the pre-restore database copy also failed — the database may be half-written" + } + msg = fmt.Sprintf("App %q is HELD STOPPED: %s.", stack, what) + tail + + fmt.Sprintf(" Replay error: %s. Rollback error: %s", orNone(h.ReplayError), orNone(h.RollbackErr)) + reason = h.RollbackErr + if reason == "" { + reason = "rollback failed" + } + return msg, "restore-hold", reason + } +} + +func mixedHoldText(h settings.RestoreHold, kind, tail string) (msg, leg, reason string) { + stack := h.Stack + leg = "restore-hold-mixed" + failedAt := "" + if h.DBReplay { + failedAt = " at the database replay" + } + copyLine := " There is no copy of the live volumes." + if h.SafetyDump != "" { + copyLine = " The copy of the live data is kept at " + h.SafetyDump + "." + } + switch h.MixedCause { + case settings.MixedCauseDefinitionWrite: + msg = fmt.Sprintf("App %q is HELD STOPPED for support: its %s failed while writing the backup's app definition, before any data was touched, "+ + "and %s. The app's data is the live data; the definition on disk may be the backup's.", stack, kind, orNone(h.FailedStep)) + tail + + " Error: " + orNone(h.ReplayError) + return msg, leg, "definition write failed; writing the live definition back failed too" + case settings.MixedCausePutBack: + msg = fmt.Sprintf("App %q is HELD STOPPED for support: its %s failed%s, and putting the live data back FAILED — %s. "+ + "The app may be on a mix of the backup's and the live data.%s", stack, kind, failedAt, orNone(h.FailedStep), copyLine) + tail + + " Restore error: " + orNone(h.ReplayError) + return msg, leg, "put-back failed: " + firstClause(h.FailedStep) + case settings.MixedCauseInterruptedPutBack: + msg = fmt.Sprintf("App %q is HELD STOPPED for support: a restore cut by a stop could not be put back at the next start — %s. "+ + "The app may be on a mix of the backup's and the live data.%s", stack, orNone(h.FailedStep), copyLine) + tail + return msg, leg, "interrupted restore: put-back failed" + } + // A mixed hold written before R-1340 carries no cause: name only what the record says, and assume nothing. + msg = fmt.Sprintf("App %q is HELD STOPPED for support: a restore stopped half-way (set %s); the app may be on a mix of the backup's and the live data. "+ + "The hold record says: %s.", stack, h.At, orNone(h.ReplayError)) + tail + return msg, leg, "restore stopped half-way" +} + +func restoreKindPhrase(k string) string { + switch k { + case settings.RestoreKindLocal: + return "restore from its own backup" + case settings.RestoreKindOffsite: + return "off-site restore" + } + return "restore" +} + +func orNone(s string) string { + if strings.TrimSpace(s) == "" { + return "(none recorded)" + } + return s +} + +// firstClause is a step's text up to its error (the short reason field). +func firstClause(s string) string { + if i := strings.Index(s, ":"); i > 0 { + return s[:i] + } + return s +} + +// heldHoldFromNotify is the hold the notification describes: the record the writer persisted just before it, or — +// when that write failed — one rebuilt from the errors handed over (the pre-R-1340 rule: no rollback error = mixed). +func heldHoldFromNotify(get func(string) (settings.RestoreHold, bool), stack string, replayErr, rollbackErr error) settings.RestoreHold { + if get != nil { + if h, ok := get(stack); ok && (h.Reason == settings.HoldReasonRestoreFailed || h.Reason == settings.HoldReasonRestoreMixed) { + return h + } + } + h := settings.RestoreHold{Stack: stack, Reason: settings.HoldReasonRestoreMixed} + if rollbackErr != nil { + h.Reason, h.RollbackErr = settings.HoldReasonRestoreFailed, rollbackErr.Error() + } + if replayErr != nil { + h.ReplayError = replayErr.Error() + } + return h +} diff --git a/controller/cmd/controller/holdmail_test.go b/controller/cmd/controller/holdmail_test.go new file mode 100644 index 00000000..c5325da1 --- /dev/null +++ b/controller/cmd/controller/holdmail_test.go @@ -0,0 +1,140 @@ +package main + +import ( + "errors" + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" +) + +// R-1340: the operator's „HELD STOPPED" mail names the hold's real cause — one test per reason. Each asserts the words +// that MUST be there and the words that must NOT (the option-C database story was the bug). +// +// RED-PROOF (2026-10-29): with mixedHoldText's body replaced by the pre-R-1340 sentence („its off-site database restore +// failed. The database was rolled back to the pre-restore copy and the live definition written back, but its files +// and named volumes are the snapshot's"), TestR1340_PutBackWithoutADatabase, _DefinitionWrite, _LocalRestore and +// _InterruptedPutBack fail on „database" / „off-site" being present and their own step missing. + +const optionCStory = "files and named volumes are the snapshot's" + +func mustHave(t *testing.T, msg string, want ...string) { + t.Helper() + for _, w := range want { + if !strings.Contains(msg, w) { + t.Errorf("the mail must say %q:\n%s", w, msg) + } + } +} + +func mustNotHave(t *testing.T, msg string, bad ...string) { + t.Helper() + for _, b := range bad { + if strings.Contains(strings.ToLower(msg), strings.ToLower(b)) { + t.Errorf("the mail must NOT say %q:\n%s", b, msg) + } + } +} + +func TestR1340_UnhealthyStop(t *testing.T) { + msg, leg, _ := heldStoppedOperatorText(settings.RestoreHold{Stack: "romm", At: "2026-10-29T08:00:00Z", Reason: settings.HoldReasonUnhealthyStop, UnhealthyKind: "crash_loop"}) + mustHave(t, msg, `"romm"`, "crash loop", "no data was touched", "Start gives it one more try") + mustNotHave(t, msg, "database", "restore failed", optionCStory) + if leg != "unhealthy-stop" { + t.Errorf("leg %q", leg) + } + oom, _, _ := heldStoppedOperatorText(settings.RestoreHold{Stack: "romm", Reason: settings.HoldReasonUnhealthyStop, UnhealthyKind: "oom_storm", Trip: 2}) + mustHave(t, oom, "out-of-memory storm", "stop 2 within 24 hours") +} + +func TestR1340_UpdateFailed(t *testing.T) { + msg, leg, _ := heldStoppedOperatorText(settings.RestoreHold{Stack: "kimai", At: "2026-10-29T03:10:00Z", Reason: settings.HoldReasonUpdateFailed, + UndoState: "half", CopyTier: 1, CopyDate: "2026-10-29T02:00:00Z"}) + mustHave(t, msg, "update at 2026-10-29T03:10:00Z did not come up healthy", "undo failed half-way", "tier 1, 2026-10-29T02:00:00Z", "--clear-restore-hold") + mustNotHave(t, msg, "database", optionCStory) + if leg != "update-hold" { + t.Errorf("leg %q", leg) + } +} + +func TestR1340_RestoreFailedWithADatabase(t *testing.T) { + msg, leg, reason := heldStoppedOperatorText(settings.RestoreHold{Stack: "paperless", Reason: settings.HoldReasonRestoreFailed, RestoreKind: settings.RestoreKindOffsite, + DBReplay: true, ReplayError: "ERROR: relation exists", RollbackErr: "psql: connection refused"}) + mustHave(t, msg, "off-site restore failed at the database replay", "rollback to the pre-restore database copy also failed", "ERROR: relation exists", "psql: connection refused") + mustNotHave(t, msg, optionCStory) + if leg != "restore-hold" || reason != "psql: connection refused" { + t.Errorf("leg %q reason %q", leg, reason) + } +} + +// The measured case: Radicale has no database; the volume could not be removed and the put-back's stop failed. +func TestR1340_PutBackWithoutADatabase(t *testing.T) { + msg, leg, reason := heldStoppedOperatorText(settings.RestoreHold{Stack: "radicale", Reason: settings.HoldReasonRestoreMixed, RestoreKind: settings.RestoreKindOffsite, + MixedCause: settings.MixedCausePutBack, FailedStep: "stopping the app before the put-back failed, so nothing was put back: compose down: timeout", + ReplayError: "volume rm -f radicale_data: volume is in use", SafetyDump: "/mnt/hdd/felhom-data/backups/restore-undo/radicale/20261029T081800Z"}) + mustHave(t, msg, `"radicale"`, "off-site restore failed, and putting the live data back FAILED", "stopping the app before the put-back failed", + "compose down: timeout", "kept at /mnt/hdd/felhom-data/backups/restore-undo/radicale/20261029T081800Z", "volume is in use") + mustNotHave(t, msg, "database", "rolled back", optionCStory) + if leg != "restore-hold-mixed" || reason != "put-back failed: stopping the app before the put-back failed, so nothing was put back" { + t.Errorf("leg %q reason %q", leg, reason) + } +} + +func TestR1340_PutBackAfterADatabaseReplay(t *testing.T) { + msg, _, _ := heldStoppedOperatorText(settings.RestoreHold{Stack: "immich", Reason: settings.HoldReasonRestoreMixed, RestoreKind: settings.RestoreKindOffsite, + MixedCause: settings.MixedCausePutBack, DBReplay: true, FailedStep: "putting the live volumes back failed: tar xf failed"}) + mustHave(t, msg, "failed at the database replay", "putting the live volumes back failed: tar xf failed", "There is no copy of the live volumes") +} + +func TestR1340_DefinitionWrite(t *testing.T) { + msg, _, _ := heldStoppedOperatorText(settings.RestoreHold{Stack: "gokapi", Reason: settings.HoldReasonRestoreMixed, RestoreKind: settings.RestoreKindOffsite, + MixedCause: settings.MixedCauseDefinitionWrite, FailedStep: "writing the live app definition back failed: app.yaml write failed", ReplayError: "app.yaml write failed"}) + mustHave(t, msg, "while writing the backup's app definition, before any data was touched", "writing the live app definition back failed", "data is the live data") + mustNotHave(t, msg, "database", optionCStory) +} + +func TestR1340_LocalRestore(t *testing.T) { + msg, _, _ := heldStoppedOperatorText(settings.RestoreHold{Stack: "app", Reason: settings.HoldReasonRestoreMixed, RestoreKind: settings.RestoreKindLocal, + MixedCause: settings.MixedCausePutBack, FailedStep: "putting the live volumes back failed: tar xf failed", SafetyDump: "/srv/undo/app/1"}) + mustHave(t, msg, "restore from its own backup failed", "putting the live volumes back failed") + mustNotHave(t, msg, "off-site", "database", optionCStory) +} + +func TestR1340_PutBackPending(t *testing.T) { + msg, leg, _ := heldStoppedOperatorText(settings.RestoreHold{Stack: "immich", Reason: settings.HoldReasonPutBackPending, SafetyDump: "/mnt/hdd/undo/immich/1"}) + mustHave(t, msg, "copy (/mnt/hdd/undo/immich/1) is on a drive that is not bound", "poured back and the app started when the drive is bound", "DROPS that put-back") + mustNotHave(t, msg, "database", optionCStory) + if leg != "putback-pending" { + t.Errorf("leg %q", leg) + } +} + +func TestR1340_InterruptedPutBack(t *testing.T) { + msg, _, _ := heldStoppedOperatorText(settings.RestoreHold{Stack: "immich", Reason: settings.HoldReasonRestoreMixed, MixedCause: settings.MixedCauseInterruptedPutBack, + FailedStep: "putting the interrupted restore's copy back failed: compose down: timeout", SafetyDump: "/mnt/hdd/undo/immich/1"}) + mustHave(t, msg, "cut by a stop could not be put back", "compose down: timeout", "kept at /mnt/hdd/undo/immich/1") + mustNotHave(t, msg, "database", "off-site", optionCStory) +} + +// A mixed hold written before R-1340 names only what its record says. +func TestR1340_LegacyMixedHoldAssumesNothing(t *testing.T) { + msg, _, _ := heldStoppedOperatorText(settings.RestoreHold{Stack: "x", At: "2026-10-01T00:00:00Z", Reason: settings.HoldReasonRestoreMixed, ReplayError: "boom"}) + mustHave(t, msg, "stopped half-way", "The hold record says: boom") + mustNotHave(t, msg, "database", optionCStory) +} + +// The notification reads the hold the writer persisted; when that write failed, it rebuilds one from the errors. +func TestR1340_NotifyReadsTheHoldRecord(t *testing.T) { + rec := settings.RestoreHold{Stack: "radicale", Reason: settings.HoldReasonRestoreMixed, MixedCause: settings.MixedCausePutBack, FailedStep: "x"} + get := func(s string) (settings.RestoreHold, bool) { return rec, s == "radicale" } + if h := heldHoldFromNotify(get, "radicale", errors.New("e"), nil); h.MixedCause != settings.MixedCausePutBack { + t.Fatalf("the persisted record was not used: %+v", h) + } + h := heldHoldFromNotify(get, "other", errors.New("replay"), errors.New("rollback")) + if h.Reason != settings.HoldReasonRestoreFailed || h.RollbackErr != "rollback" || h.ReplayError != "replay" { + t.Fatalf("fallback hold %+v", h) + } + if h := heldHoldFromNotify(nil, "other", errors.New("replay"), nil); h.Reason != settings.HoldReasonRestoreMixed { + t.Fatalf("no rollback error = mixed, got %+v", h) + } +} diff --git a/controller/cmd/controller/main.go b/controller/cmd/controller/main.go index 90dbc6fc..7d981b01 100644 --- a/controller/cmd/controller/main.go +++ b/controller/cmd/controller/main.go @@ -92,8 +92,9 @@ func main() { abandonStatus := flag.Bool("abandon-status", false, "R-241: print the state of this box's off-site abandonment countdown (set-aside path, due date, days left) and exit. Read-only.") abandonExtend := flag.Int("abandon-extend", 0, "R-241 (operator): extend a running abandonment countdown by N days from now, then exit. Refuses when no countdown is running.") abandonStop := flag.Bool("abandon-stop", false, "R-241 (operator): stop a running abandonment countdown, then exit. The set-aside history is kept and nothing is deleted. Refuses when no countdown is running.") - listRestoreHolds := flag.Bool("restore-holds", false, "R-379 (operator): list apps this controller is deliberately holding stopped after a failed database restore whose rollback also failed, then exit. Read-only.") - clearRestoreHold := flag.String("clear-restore-hold", "", "R-379 (operator): clear the restore hold on APP so it can start again, then exit. Clearing does NOT repair the database — check the app's data first; the undo copies are in its unit's db-dumps dir.") + listRestoreHolds := flag.Bool("restore-holds", false, "R-379 (operator): list the apps this controller is holding stopped (every reason, with why), then exit. Read-only.") + clearRestoreHold := flag.String("clear-restore-hold", "", "R-379/D64 (operator): clear the hold on APP (any reason), then exit. Run it inside the guest: "+holdClearCommandHint+ + ". It asks the RUNNING controller, so no restart is needed; only when no controller runs does it edit settings.json. Clearing repairs nothing and starts nothing — check the app's data first.") flag.Parse() if *showVersion { @@ -172,21 +173,23 @@ func main() { // starts, exactly like the other CLI subcommands: each loads config + settings, acts, and exits. // R-379 (operator): the way OUT of a hold. A CLI subcommand rather than an API route, matching // --abandon-stop above: the controller's HTTP surface is authenticated as the CUSTOMER, and - // clearing a hold is a decision that should follow an operator looking at the database. Reaching + // clearing a hold is a decision that should follow an operator looking at the data. Reaching // this needs `docker exec` into the guest, which is the operator gate this project already uses. + // R-1343: the clear goes to the RUNNING controller over its local socket (holdclear.go); the list reads + // settings.json, which the running controller saves at every hold change. if *listRestoreHolds || *clearRestoreHold != "" { cfg, err := config.LoadPermissive(*configPath) if err != nil { fmt.Fprintf(os.Stderr, "restore-hold: loading config: %v\n", err) os.Exit(1) } - lg := log.New(os.Stderr, "", 0) - sett, err := settings.Load(cfg.Paths.DataDir+"/settings.json", lg) - if err != nil { - fmt.Fprintf(os.Stderr, "restore-hold: loading settings: %v\n", err) - os.Exit(1) - } if *listRestoreHolds { + lg := log.New(os.Stderr, "", 0) + sett, err := settings.Load(cfg.Paths.DataDir+"/settings.json", lg) + if err != nil { + fmt.Fprintf(os.Stderr, "restore-hold: loading settings: %v\n", err) + os.Exit(1) + } holds := sett.ListRestoreHolds() if len(holds) == 0 { fmt.Println("no restore holds in force") @@ -197,34 +200,9 @@ func main() { } os.Exit(0) } - cleared, err := sett.ClearRestoreHold(*clearRestoreHold) - if err != nil { - fmt.Fprintf(os.Stderr, "restore-hold: clearing %s: %v\n", *clearRestoreHold, err) - os.Exit(1) - } - if !cleared { - // "there was nothing to clear" is not success — a silent 0 here would let an operator - // believe they had unblocked an app whose name they mistyped. - fmt.Fprintf(os.Stderr, "no restore hold in force for %q — nothing was changed\n", *clearRestoreHold) - os.Exit(2) - } - // THE RESTART IS NOT OPTIONAL, and saying so is the difference between a route that works - // and one an operator believes worked. This runs as a SECOND process: it clears the hold in - // settings.json, but the RUNNING controller holds its own in-memory Settings and keeps - // refusing until it reloads. Measured on demo-hp 2026-08-22 — the clear succeeded, the file - // was correct, and the customer's start button still refused until the controller restarted. - // - // There is also a lost-update window while both processes hold the file: if the running - // controller saves settings.json from its own copy before the restart, the clear is undone. - // Restarting promptly closes it. A route that clears through the running controller would - // remove both problems and is the right shape later; it needs an operator tier the - // controller's HTTP surface does not have today (it authenticates as the CUSTOMER, and a - // customer clearing their own hold is precisely what the hold exists to prevent). - fmt.Printf("restore hold cleared for %s in settings.json.\n", *clearRestoreHold) - fmt.Printf(" NOW RESTART THE CONTROLLER, or it will keep refusing to start the app:\n") - fmt.Printf(" systemctl restart felhom-controller-bootstrap.service\n") - fmt.Printf(" Check the app's data first — the undo copies are in its unit's db-dumps dir.\n") - os.Exit(0) + // R-1343: through the RUNNING controller (holdclear.go) — the old second-process edit of settings.json was + // written back by the running controller's next save. Only when no controller answers is the file edited here. + os.Exit(clearRestoreHoldCLI(cfg.Paths.DataDir, *clearRestoreHold, os.Stdout, os.Stderr)) } if *abandonStatus || *abandonExtend > 0 || *abandonStop { @@ -485,6 +463,18 @@ func main() { return stackMgr.DriveLive(hdd) }) appStopRecovery := appStopGuard.Recover() + // D64 = B / R-1343: the operator's hold clear — ONE door for the hub's clear_hold and the box command, run in THIS + // process so memory and settings.json agree (holdclear.go). The box command reaches it over a unix socket in the + // data dir (0600, nothing on TCP). A socket that cannot be bound is logged; the command then says plainly that no + // running controller answered. + holdDeps := holdClearDeps{clear: appStopGuard.ClearHoldByOperator, forgetUpdateHeld: stackMgr.ForgetUpdateHeld, logger: logger} + if holdLn, herr := listenHoldCtl(holdCtlPath(cfg.Paths.DataDir), func(stack, who string) (string, error) { + return clearHoldNow(holdDeps, stack, who) + }, logger); herr != nil { + logger.Printf("[ERROR] [hold] the local hold-clear channel could not be opened: %v — `--clear-restore-hold` will edit settings.json and need a restart", herr) + } else { + defer holdLn.Close() + } // R-1251: a put-back that waited for its drive (a hold from an earlier start) runs now if that drive is bound. if started := appStopGuard.ResumeWaitingPutBacks(); len(started) > 0 { logger.Printf("[WARN] [appstop] %d app(s) whose interrupted restore waited for its drive are put back and started: %v", len(started), started) @@ -1081,6 +1071,7 @@ func main() { holdReporter.Tick(time.Now()) return nil }) + oomLog := &oomLogGate{} // R-1341 sched.Every("deadapp-check", 30*time.Second, func(ctx context.Context) error { if bootGrace.Within(time.Now()) { return nil // still inside the startup settle window (90 s, or ~15 min after a crash boot) @@ -1096,8 +1087,12 @@ func main() { if oerr != nil { logger.Printf("[WARN] [deadapp] OOM scan failed: %v", oerr) } else { - for _, o := range ooms { + // R-1341: the line once per NEW kill (oomlog.go); the event call stays per scan — the notifier's cooldown + // keeps it to one, as before. + for _, o := range oomLog.fresh(ooms) { logger.Printf("[WARN] [deadapp] %s: container %s was OOM-killed (started %s; kills %d, limit %s, peak %s; seen by %s)", o.Stack, o.Container, o.StartedAt, o.Kills, o.MemLimit, o.Peak, o.Source) + } + for _, o := range ooms { notifier.NotifyAppOOMFrom(o.Stack, o.Container, o.StartedAt, o.Kills, o.MemLimit, o.Peak, o.Source) } } @@ -1217,7 +1212,7 @@ func main() { // R-314/R-279/R-177 (`09` §3 decision 185): the operator's closed list of actions, carried in the // report reply and run in THIS process (opactions_wiring.go). The result rides the next report; // a finished result fires an out-of-cycle report through the trigger once it exists. - opActions := report.NewOperatorActions(ctx, operatorActionHandlers(backupMgr, sched, func() { + opActions := report.NewOperatorActions(ctx, operatorActionHandlers(backupMgr, sched, holdDeps, func() { if f, ok := opActionReportFire.Load().(func()); ok && f != nil { f() } @@ -1548,22 +1543,18 @@ func main() { // DELIBERATELY NOT `backup_failed`: customer-enabled by default, Hungarian "A biztonsági // mentés sikertelen!", and this is a deliberate hold rather than a backup failure. R-171 one // path over. + // R-1340: the text is built from the HOLD RECORD the writer persisted just before this call (holdmail.go) — its + // reason, which restore, which branch, which put-back step failed, whether it was a database replay — never the + // option-C database story for every cause. backupMgr.SetRestoreHoldNotify(func(stack string, replayErr, rollbackErr error) { - d := notify.BackupRunFailuresDetails{RunKind: "offsite-reconstitute", Failed: 1, Attempted: 1} - if rollbackErr == nil { // R-893: the rollback WORKED, but files/volumes/version had already moved - d.Apps = append(d.Apps, notify.RunFailureDetail{App: stack, Leg: "restore-hold-mixed", Reason: "replay failed after files/volumes/version moved"}) - msg := fmt.Sprintf("App %q is HELD STOPPED for support: its off-site database restore failed. The database was rolled back to the pre-restore copy and the live definition written back, "+ - "but its files and named volumes are the snapshot's — a mixed state (R-893). The app will not start from any path until the hold is cleared. Replay error: %v", stack, replayErr) - notifier.NotifyBackupRunFailures(msg, d) - return + h := heldHoldFromNotify(sett.GetRestoreHold, stack, replayErr, rollbackErr) + msg, leg, reason := heldStoppedOperatorText(h) + runKind := "offsite-reconstitute" + if h.RestoreKind == settings.RestoreKindLocal { + runKind = "restore" } - reason := "rollback failed" - if rollbackErr != nil { - reason = rollbackErr.Error() - } - d.Apps = append(d.Apps, notify.RunFailureDetail{App: stack, Leg: "restore-hold", Reason: reason}) - msg := fmt.Sprintf("App %q is HELD STOPPED: its off-site database restore failed AND the rollback to the customer's own pre-restore copy also failed. "+ - "The app will not start from any path until the hold is cleared. Replay error: %v. Rollback error: %v", stack, replayErr, rollbackErr) + d := notify.BackupRunFailuresDetails{RunKind: runKind, Failed: 1, Attempted: 1} + d.Apps = append(d.Apps, notify.RunFailureDetail{App: stack, Leg: leg, Reason: reason}) notifier.NotifyBackupRunFailures(msg, d) }) // R-893 option A (D20 = A): a failed off-site restore that was PUT BACK — the live volumes and definition poured diff --git a/controller/cmd/controller/oomlog.go b/controller/cmd/controller/oomlog.go new file mode 100644 index 00000000..384259af --- /dev/null +++ b/controller/cmd/controller/oomlog.go @@ -0,0 +1,43 @@ +package main + +import ( + "strconv" + + "gitea.dooplex.hu/admin/felhom-controller/internal/stacks" +) + +// ── R-1341: the OOM line is logged once per NEW kill ────────────────────────────────────────────────────────────── +// +// ScanOOMKilled reports a container for as long as it lives after a kill (Docker's OOMKilled flag is sticky, and the +// kernel's kill counter never goes down), so the 30-s dead-app scan logged „[WARN] [deadapp] … was OOM-killed" about +// 2 880 times a day per container — measured 2026-10-29 on Tester 1 — while the notifier's cooldown kept the event to +// one. The line now goes out when the kill is NEW: the key is (container, its start time, the kill count), and a line +// is logged only when a container's key changes — a second scan of the same kill logs nothing, a new kill (the count +// went up) or a restarted container (a new start time) logs again. The event call is unchanged (every scan, the +// notifier decides). Pinned by TestR1341_*. + +// oomLogGate remembers the last logged key per container. Not safe for concurrent use: the dead-app job runs serially. +type oomLogGate struct { + last map[string]string +} + +func oomLogKey(o stacks.OOMContainer) string { + return o.StartedAt + "|" + strconv.FormatInt(o.Kills, 10) +} + +// fresh returns the containers whose kill is new since the last scan, and remembers this scan's keys. A container +// absent from this scan is forgotten (it was removed or restarted clean), which also bounds the memory. +func (g *oomLogGate) fresh(ooms []stacks.OOMContainer) []stacks.OOMContainer { + next := make(map[string]string, len(ooms)) + var out []stacks.OOMContainer + for _, o := range ooms { + k := oomLogKey(o) + next[o.Container] = k + if prev, ok := g.last[o.Container]; ok && prev == k { + continue + } + out = append(out, o) + } + g.last = next + return out +} diff --git a/controller/cmd/controller/oomlog_test.go b/controller/cmd/controller/oomlog_test.go new file mode 100644 index 00000000..f1b6c119 --- /dev/null +++ b/controller/cmd/controller/oomlog_test.go @@ -0,0 +1,64 @@ +package main + +import ( + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/stacks" +) + +// R-1341: the „[WARN] [deadapp] … was OOM-killed" line goes out once per NEW kill, not every 30-s scan. +// +// RED-PROOF (2026-10-29): with oomLogGate.fresh returning its input unchanged (the pre-R-1341 loop logged every +// container every scan), TestR1341_ASecondScanOfTheSameKillLogsNothing fails on „logged 1 line(s), want 0". + +func oom(container, started string, kills int64) stacks.OOMContainer { + return stacks.OOMContainer{Stack: "gokapi", Container: container, StartedAt: started, Kills: kills} +} + +func TestR1341_ASecondScanOfTheSameKillLogsNothing(t *testing.T) { + g := &oomLogGate{} + scan := []stacks.OOMContainer{oom("gokapi", "2026-10-29T09:02:00Z", 1)} + if got := g.fresh(scan); len(got) != 1 { + t.Fatalf("the first sighting must log, logged %d line(s)", len(got)) + } + for i := 0; i < 5; i++ { + if got := g.fresh(scan); len(got) != 0 { + t.Fatalf("scan %d of the same kill logged %d line(s), want 0", i+2, len(got)) + } + } +} + +func TestR1341_ANewKillLogsAgain(t *testing.T) { + g := &oomLogGate{} + g.fresh([]stacks.OOMContainer{oom("gokapi", "2026-10-29T09:02:00Z", 1)}) + got := g.fresh([]stacks.OOMContainer{oom("gokapi", "2026-10-29T09:02:00Z", 2)}) + if len(got) != 1 || got[0].Kills != 2 { + t.Fatalf("a new kill (count 1→2) must log once, got %+v", got) + } + if got := g.fresh([]stacks.OOMContainer{oom("gokapi", "2026-10-29T09:02:00Z", 2)}); len(got) != 0 { + t.Fatalf("the same new kill must not log twice, got %+v", got) + } +} + +func TestR1341_ARestartedContainerLogsAgain(t *testing.T) { + g := &oomLogGate{} + g.fresh([]stacks.OOMContainer{oom("gokapi", "2026-10-29T09:02:00Z", 1)}) + if got := g.fresh([]stacks.OOMContainer{oom("gokapi", "2026-10-29T09:30:00Z", 1)}); len(got) != 1 { + t.Fatalf("a restarted container's kill must log, got %+v", got) + } +} + +// Two containers are judged apart; one that leaves the scan is forgotten, so its return logs again. +func TestR1341_ContainersAreJudgedApart(t *testing.T) { + g := &oomLogGate{} + a, b := oom("a", "t1", 1), oom("b", "t1", 1) + if got := g.fresh([]stacks.OOMContainer{a, b}); len(got) != 2 { + t.Fatalf("first scan: %+v", got) + } + if got := g.fresh([]stacks.OOMContainer{a}); len(got) != 0 { + t.Fatalf("a seen again must not log: %+v", got) + } + if got := g.fresh([]stacks.OOMContainer{a, b}); len(got) != 1 || got[0].Container != "b" { + t.Fatalf("b came back after leaving the scan: %+v", got) + } +} diff --git a/controller/cmd/controller/opactions_wiring.go b/controller/cmd/controller/opactions_wiring.go index 4532741c..a6b7f929 100644 --- a/controller/cmd/controller/opactions_wiring.go +++ b/controller/cmd/controller/opactions_wiring.go @@ -17,18 +17,26 @@ import ( // executor is built earlier than the trigger, so it reads it through this holder. var opActionReportFire atomic.Value -// operatorActionHandlers wires the four operator actions (R-314/R-279/R-177, `09` §3 decision 185) +// operatorActionHandlers wires the five operator actions (R-314/R-279/R-177, `09` §3 decision 185) // to the running process. ONE wiring site, called from main and from the wiring test, so the test // exercises the doors production uses (the seam-built-but-never-wired trap). // // offsite_backup_now makes the SAME four checks as the household's „back up now" // (offboxRunHandler) and then runs the same call, so an operator press can never do what the // household's own press cannot. -func operatorActionHandlers(backupMgr *backup.Manager, sched *scheduler.Scheduler, resultReady func(), logger *log.Logger) report.OperatorActionHandlers { +// +// clear_hold (D64 = B) goes through clearHoldNow with hold — the same door the box command's socket uses +// (holdclear.go), so the hub's button and the box command can never differ. hold.clear nil = not available. +func operatorActionHandlers(backupMgr *backup.Manager, sched *scheduler.Scheduler, hold holdClearDeps, resultReady func(), logger *log.Logger) report.OperatorActionHandlers { h := report.OperatorActionHandlers{ResultReady: resultReady} if sched != nil { h.RunJob = sched.RunNow } + if hold.clear != nil { + h.ClearHold = func(stack string, actionID int64) (string, error) { + return clearHoldNow(hold, stack, fmt.Sprintf("the operator via the hub, action #%d", actionID)) + } + } if backupMgr == nil { return h } diff --git a/controller/cmd/controller/opactions_wiring_test.go b/controller/cmd/controller/opactions_wiring_test.go index 11c71784..423ce5c7 100644 --- a/controller/cmd/controller/opactions_wiring_test.go +++ b/controller/cmd/controller/opactions_wiring_test.go @@ -57,7 +57,7 @@ func TestD1_AbandonStopActsOnTheRunningManagerAndDisk(t *testing.T) { t.Fatal("fixture: a countdown must be running") } - o := report.NewOperatorActions(context.Background(), operatorActionHandlers(mgr, nil, nil, logger), logger) + o := report.NewOperatorActions(context.Background(), operatorActionHandlers(mgr, nil, holdClearDeps{}, nil, logger), logger) o.Reconcile([]report.OperatorAction{{ID: 1, Action: report.OpAbandonStop}}) if mgr.AbandonStatus().Active { @@ -97,7 +97,7 @@ func TestD1_OffsiteBackupNowRefusesWithoutATarget(t *testing.T) { t.Fatal("no restic may run") return nil, nil }) - o := report.NewOperatorActions(context.Background(), operatorActionHandlers(mgr, nil, nil, logger), logger) + o := report.NewOperatorActions(context.Background(), operatorActionHandlers(mgr, nil, holdClearDeps{}, nil, logger), logger) o.Reconcile([]report.OperatorAction{{ID: 9, Action: report.OpOffsiteBackupNow}}) if r := o.Results(); len(r) != 1 || r[0].Outcome != report.OutcomeRefused { t.Fatalf("result = %+v", r) @@ -115,7 +115,7 @@ func TestD1_RunJobUsesTheScheduler(t *testing.T) { defer cancel() sched.Start(ctx) defer sched.Stop() - o := report.NewOperatorActions(ctx, operatorActionHandlers(nil, sched, nil, logger), logger) + o := report.NewOperatorActions(ctx, operatorActionHandlers(nil, sched, holdClearDeps{}, nil, logger), logger) o.Reconcile([]report.OperatorAction{{ID: 1, Action: report.OpRunJob, Arg: "fill-watch"}, {ID: 2, Action: report.OpRunJob, Arg: "offsite-proof"}}) select { case od := <-ran: diff --git a/controller/internal/backup/appstop_putback_wait.go b/controller/internal/backup/appstop_putback_wait.go index dc64f48f..018f0724 100644 --- a/controller/internal/backup/appstop_putback_wait.go +++ b/controller/internal/backup/appstop_putback_wait.go @@ -119,7 +119,8 @@ func (g *AppStopGuard) holdPutBackFailed(stack, dir string, cause error) bool { return false } h := settings.RestoreHold{Stack: stack, At: g.now().UTC().Format(time.RFC3339), Reason: settings.HoldReasonRestoreMixed, - ReplayError: "interrupted restore, put-back failed: " + cause.Error(), SafetyDump: dir} + ReplayError: "interrupted restore, put-back failed: " + cause.Error(), SafetyDump: dir, + MixedCause: settings.MixedCauseInterruptedPutBack, FailedStep: "putting the interrupted restore's copy back failed: " + cause.Error()} if err := g.holds.SetRestoreHold(h); err != nil { g.logger.Printf("[ERROR] [appstop] %s: the failed put-back could not be recorded as a hold: %v — the marker is kept", stack, err) return false diff --git a/controller/internal/backup/hold_clear.go b/controller/internal/backup/hold_clear.go new file mode 100644 index 00000000..b24b6c21 --- /dev/null +++ b/controller/internal/backup/hold_clear.go @@ -0,0 +1,66 @@ +package backup + +import ( + "errors" + + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" +) + +// ── D64 = B (operator ruling 2026-10-29 12:10) + R-1343: the operator clears a hold IN THE RUNNING CONTROLLER ────── +// +// One door for both of the operator's routes — the hub's „Clear hold" (operator action clear_hold) and the box command +// `--clear-restore-hold`, which now asks the running controller over its local socket (cmd/controller/holdctl.go). +// +// WHY IN THIS PROCESS (R-1343, measured 2026-10-29 on Tester 1): the CLI used to run as a SECOND process that edited +// settings.json while the running controller kept the hold in its own in-memory Settings — and that controller's next +// save (after a hub report) wrote the hold back. Clearing through the running Settings removes the hold from memory +// AND saves the file at once, so no later save of any component can bring it back. +// Pinned by TestD64_ClearHoldStaysGoneAfterALaterSave (cmd/controller). +// +// EVERY REASON (D64 = B): the hold's reason does not matter — R-379 now reads „a person — on the box, or the operator +// on the hub". One reason needs a second act: a put-back WAITING for its drive (HoldReasonPutBackPending) is also +// recorded in the app-stop marker, and a controller start with the drive bound would pour that old copy back over +// whatever the app wrote after the clear. So the clear also drops the app from the marker; the copy stays on disk +// (nothing is deleted). It runs under resumeMu, so it can never interleave with a put-back of the same app. +// Pinned by TestD64_ClearPutBackPendingDropsTheMarkerEntry. + +// ErrHoldBadStack: the argument is not a valid stack name. ErrNotHeld: the app has no hold. Both are refusals — +// nothing was changed. +var ( + ErrHoldBadStack = errors.New("not a valid app name") + ErrNotHeld = errors.New("this app has no hold") +) + +// IsValidHoldStack reports whether s may name an app for the hold commands (the backup package's strict +// isSafeStackName: letters, digits, '-' and '_', at most 64). +func IsValidHoldStack(s string) bool { return isSafeStackName(s) } + +// ClearHoldByOperator lifts stack's hold, whatever its reason, in the hold store the running controller uses, and +// returns the hold it lifted. A waiting put-back is dropped from the app-stop marker too (see above). Refusals +// (ErrHoldBadStack, ErrNotHeld) change nothing. +func (g *AppStopGuard) ClearHoldByOperator(stack string) (settings.RestoreHold, error) { + if !isSafeStackName(stack) { + return settings.RestoreHold{}, ErrHoldBadStack + } + if g == nil || g.holds == nil { + return settings.RestoreHold{}, errors.New("no hold store is wired on this box") + } + resumeMu.Lock() + defer resumeMu.Unlock() + h, ok := g.holds.GetRestoreHold(stack) + if !ok { + return settings.RestoreHold{}, ErrNotHeld + } + cleared, err := g.holds.ClearRestoreHold(stack) + if err != nil { + return h, err + } + if !cleared { + return settings.RestoreHold{}, ErrNotHeld + } + if h.Reason == settings.HoldReasonPutBackPending { + g.dropFromMarker(stack) + g.logger.Printf("[WARN] [appstop] %s: its waiting put-back is DROPPED with the hold — the copy stays on disk at %s and is never poured back by itself", stack, h.SafetyDump) + } + return h, nil +} diff --git a/controller/internal/backup/offbox_reconstitute.go b/controller/internal/backup/offbox_reconstitute.go index 979845dd..3acac3fb 100644 --- a/controller/internal/backup/offbox_reconstitute.go +++ b/controller/internal/backup/offbox_reconstitute.go @@ -425,9 +425,12 @@ func (m *Manager) holdAppAfterFailedRollback(stack string, replayErr, rollbackEr m.logger.Printf("[ERROR] [offbox] %s: cannot persist the restore hold — no settings wired; the app is stopped but NOTHING will refuse a restart", stack) return } + // R-1340: this branch is only ever an off-site restore's database replay (offbox_reconstitute's rollback). h := settings.RestoreHold{ - Stack: stack, - At: time.Now().UTC().Format(time.RFC3339), + Stack: stack, + At: time.Now().UTC().Format(time.RFC3339), + RestoreKind: settings.RestoreKindOffsite, + DBReplay: true, } if replayErr != nil { h.ReplayError = replayErr.Error() @@ -447,6 +450,15 @@ func (m *Manager) holdAppAfterFailedRollback(stack string, replayErr, rollbackEr } } +// mixedHoldWhy (R-1340) is what a restore_mixed hold records about its cause (settings.RestoreHold's R-1340 fields). +type mixedHoldWhy struct { + kind string // settings.RestoreKind* + cause string // settings.MixedCause* + step string // the step that failed, with its error + dbReplay bool // the restore's own failure was its database replay + copyDir string // where the copy of the live volumes is kept; "" = none kept +} + // liveDefinition is the app's definition as it ran before an off-site restore wrote the snapshot's // (R-893): a copy of its compose files in a temporary folder, and its full env held in memory only. type liveDefinition struct { @@ -511,7 +523,11 @@ func (m *Manager) captureLiveDefinition(stack string) (*liveDefinition, error) { // written back (when the snapshot's was written), the database service is stopped again, and the app is // held with HoldReasonRestoreMixed. The operator is notified with no rollback error — that is how the // notification tells this case from R-379's double failure. -func (m *Manager) holdAppAfterMixedRestore(stack string, replayErr error, live *liveDefinition) { +// +// why (R-1340) is what the hold records about its cause, so the operator's mail tells the real story — this hold is +// written by three different branches now (a definition write, a put-back that failed at one of its steps), not +// only by option C's database replay. +func (m *Manager) holdAppAfterMixedRestore(stack string, replayErr error, live *liveDefinition, why mixedHoldWhy) { // The hold is PERSISTED FIRST (security review 2026-10-08): a controller that dies between the stop and // the hold would otherwise restart the app from the R-166 marker with nothing refusing it. When the hold // cannot be persisted, the app-stop marker is KEPT, so nothing reads the window as finished. @@ -519,7 +535,8 @@ func (m *Manager) holdAppAfterMixedRestore(stack string, replayErr error, live * if m.settings == nil { m.logger.Printf("[ERROR] [offbox] %s: cannot persist the restore hold — no settings wired; the app is stopped but NOTHING will refuse a restart", stack) } else { - h := settings.RestoreHold{Stack: stack, At: time.Now().UTC().Format(time.RFC3339), Reason: settings.HoldReasonRestoreMixed} + h := settings.RestoreHold{Stack: stack, At: time.Now().UTC().Format(time.RFC3339), Reason: settings.HoldReasonRestoreMixed, + RestoreKind: why.kind, MixedCause: why.cause, FailedStep: why.step, DBReplay: why.dbReplay, SafetyDump: why.copyDir} if replayErr != nil { h.ReplayError = replayErr.Error() } @@ -557,7 +574,8 @@ func (m *Manager) restartAtLiveDefinition(stack string, live *liveDefinition, ca if live != nil { if err := m.stackProvider.RecreateStackDefinitionFromUnit(stack, live.dir, live.env); err != nil { m.logger.Printf("[ERROR] [offbox] %s: the definition write failed (%v) and writing the live definition back failed too: %v — holding the app stopped", stack, cause, err) - m.holdAppAfterMixedRestore(stack, cause, nil) + m.holdAppAfterMixedRestore(stack, cause, nil, mixedHoldWhy{kind: settings.RestoreKindOffsite, cause: settings.MixedCauseDefinitionWrite, + step: "writing the live app definition back failed: " + err.Error()}) return } m.logger.Printf("[INFO] [offbox] %s: the definition write failed (%v); the live definition is written back before the start", stack, cause) @@ -1060,7 +1078,7 @@ func (m *Manager) ReconstituteFromOffsite(ctx context.Context, stack string, ack // R-893 option A: no volume has moved yet, so only the live definition is written back and the app starts. // The failing copy may have placed part of its files, so the sentence names placed files (+1). if definitionMoved { - return res, m.putBackAfterFailedRestore(stack, cErr, liveDef, undo, false, res.FilesPlaced+1, restartStack, "err.backup.restore_failed_held_mixed") + return res, m.putBackAfterFailedRestore(stack, cErr, liveDef, undo, false, res.FilesPlaced+1, restartStack, "err.backup.restore_failed_held_mixed", settings.RestoreKindOffsite) } // Nothing moved yet: best-effort bring-up — a failed restore must not also be an outage. if sErr := restartStack(); sErr != nil { @@ -1095,7 +1113,7 @@ func (m *Manager) ReconstituteFromOffsite(ctx context.Context, stack string, ack // R-893 option A: the replay removes a volume BEFORE it recreates it, so even nVols == 0 may have lost one — // with a copy, it is always poured back; with no copy the old rule decides (and holds when one moved). if definitionMoved || nVols > 0 || undo != nil { - return res, m.putBackAfterFailedRestore(stack, vErr, liveDef, undo, nVols > 0 || undo != nil, res.FilesPlaced, restartStack, "err.backup.restore_failed_held_mixed") + return res, m.putBackAfterFailedRestore(stack, vErr, liveDef, undo, nVols > 0 || undo != nil, res.FilesPlaced, restartStack, "err.backup.restore_failed_held_mixed", settings.RestoreKindOffsite) } // Nothing replaced yet: bring the app back up rather than leaving an outage, then surface it. if sErr := restartStack(); sErr != nil { @@ -1114,7 +1132,7 @@ func (m *Manager) ReconstituteFromOffsite(ctx context.Context, stack string, ack if err := m.stackProvider.StartStackServices(stack, dbServices); err != nil { // R-893 option C (security review 2026-10-08): volumes or the definition already moved → HOLD. if definitionMoved || res.VolumesReplayed > 0 { - return res, m.putBackAfterFailedRestore(stack, err, liveDef, undo, res.VolumesReplayed > 0, res.FilesPlaced, restartStack, "err.backup.restore_failed_held_mixed") + return res, m.putBackAfterFailedRestore(stack, err, liveDef, undo, res.VolumesReplayed > 0, res.FilesPlaced, restartStack, "err.backup.restore_failed_held_mixed", settings.RestoreKindOffsite) } // Best-effort bring-up: a failed restore must not also be an outage. if sErr := restartStack(); sErr != nil { @@ -1166,7 +1184,7 @@ func (m *Manager) ReconstituteFromOffsite(ctx context.Context, stack string, ack // under them — the copy of the live volumes is poured back over the rolled-back database, the live // definition written back, and the app starts exactly as it was. A put-back that fails holds, as before. if definitionMoved || res.VolumesReplayed > 0 { - return res, m.putBackAfterFailedRestore(stack, iErr, liveDef, undo, res.VolumesReplayed > 0, res.FilesPlaced, restartStack, "err.backup.db_restore_failed_held_mixed") + return res, m.putBackAfterFailedRestore(stack, iErr, liveDef, undo, res.VolumesReplayed > 0, res.FilesPlaced, restartStack, "err.backup.db_restore_failed_held_mixed", settings.RestoreKindOffsite) } if sErr := restartStack(); sErr != nil { m.logger.Printf("[WARN] [offbox] %s: start after a successful rollback failed: %v", stack, sErr) diff --git a/controller/internal/backup/offbox_restore_undo.go b/controller/internal/backup/offbox_restore_undo.go index de0dc2cc..2e7a5fe8 100644 --- a/controller/internal/backup/offbox_restore_undo.go +++ b/controller/internal/backup/offbox_restore_undo.go @@ -11,6 +11,7 @@ import ( "time" "gitea.dooplex.hu/admin/felhom-controller/internal/dockerexec" + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" "gitea.dooplex.hu/admin/felhom-controller/internal/util" ) @@ -219,11 +220,23 @@ func (m *Manager) putBack(stack string, c *liveVolumeCopy) error { // failure's own held sentence) and the copy stays on disk. volumesMoved with no copy is such a case: nothing can // put them back. filesPlaced chooses the household's sentence: placed files are not taken back (as ruled for // option C, a file mix does not hold), so the sentence then says so instead of „nothing changed". +// +// restoreKind (R-1340) is settings.RestoreKindOffsite or RestoreKindLocal: the hold records it, with the step that +// failed and whether the restore's failure was a database replay (holdKey says so — it is the callers' own choice of +// the household's sentence), so the operator's mail names the real cause. func (m *Manager) putBackAfterFailedRestore(stack string, cause error, live *liveDefinition, c *liveVolumeCopy, volumesMoved bool, - filesPlaced int, restart func() error, holdKey string) error { + filesPlaced int, restart func() error, holdKey, restoreKind string) error { + why := func(step string) mixedHoldWhy { + w := mixedHoldWhy{kind: restoreKind, cause: settings.MixedCausePutBack, step: step, + dbReplay: holdKey == "err.backup.db_restore_failed_held_mixed"} + if c != nil && c.keep { + w.copyDir = c.dir + } + return w + } if volumesMoved && c == nil { m.logger.Printf("[ERROR] [offbox] %s: volumes were replaced and there is no copy to put back — holding the app", stack) - m.holdAppAfterMixedRestore(stack, cause, live) + m.holdAppAfterMixedRestore(stack, cause, live, why("the restore had already changed the app's data and there was no copy of the live volumes to put back")) return util.MsgError(holdKey, stack) } // R-1256 H5: a put-back under an app that could not be stopped could mix the copy with what the app writes. @@ -231,14 +244,14 @@ func (m *Manager) putBackAfterFailedRestore(stack string, cause error, live *liv if err := m.stackProvider.StopStack(stack); err != nil { c.keepIfAny() m.logger.Printf("[ERROR] [offbox] %s: stopping the app before the put-back FAILED: %v — nothing is put back; the copy stays at %s; holding the app", stack, err, c.dirOrNone()) - m.holdAppAfterMixedRestore(stack, cause, live) + m.holdAppAfterMixedRestore(stack, cause, live, why("stopping the app before the put-back failed, so nothing was put back: "+err.Error())) return util.MsgError(holdKey, stack) } if volumesMoved { if err := m.putBack(stack, c); err != nil { c.keep = true m.logger.Printf("[ERROR] [offbox] %s: putting the live volumes back FAILED: %v — the copy stays at %s; holding the app", stack, err, c.dir) - m.holdAppAfterMixedRestore(stack, cause, live) + m.holdAppAfterMixedRestore(stack, cause, live, why("putting the live volumes back failed: "+err.Error())) return util.MsgError(holdKey, stack) } } @@ -246,7 +259,7 @@ func (m *Manager) putBackAfterFailedRestore(stack string, cause error, live *liv if err := m.stackProvider.RecreateStackDefinitionFromUnit(stack, live.dir, live.env); err != nil { c.keepIfAny() // R-1256 H5 (D20): a put-back that fails keeps its copy for support m.logger.Printf("[ERROR] [offbox] %s: writing the live definition back FAILED: %v — the copy stays at %s; holding the app", stack, err, c.dirOrNone()) - m.holdAppAfterMixedRestore(stack, cause, live) + m.holdAppAfterMixedRestore(stack, cause, live, why("writing the live app definition back failed: "+err.Error())) return util.MsgError(holdKey, stack) } } diff --git a/controller/internal/backup/r1340_hold_cause_test.go b/controller/internal/backup/r1340_hold_cause_test.go new file mode 100644 index 00000000..a340669e --- /dev/null +++ b/controller/internal/backup/r1340_hold_cause_test.go @@ -0,0 +1,133 @@ +package backup + +import ( + "context" + "errors" + "io" + "log" + "os" + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" +) + +// R-1340: a restore_mixed hold records WHAT put it in place — which restore, which branch, which put-back step failed, +// whether the restore's failure was a database replay, and where the copy is kept — so the operator's mail +// (cmd/controller heldStoppedOperatorText, pinned by TestR1340_* there) tells the real story. Measured 2026-10-29 on +// Tester 1: Radicale (no database), its volume could not be removed, the put-back's stop failed, and the mail said its +// DATABASE restore failed and was rolled back. +// +// RED-PROOF (2026-10-29): with the R-1340 fields dropped from holdAppAfterMixedRestore's hold literal and from +// holdPutBackFailed's (the pre-R-1340 record, which carried no cause), all five tests below fail on „mixed cause """ / +// „kind """. + +func holdOf(t *testing.T, m *Manager, stack string) settings.RestoreHold { + t.Helper() + if m.settings == nil { + t.Fatal("fixture: no settings wired") + } + h, ok := m.settings.GetRestoreHold(stack) + if !ok { + t.Fatalf("%s is not held", stack) + } + return h +} + +// The Radicale shape: a VOLUME restore fails (no database involved), then the put-back's stop fails. +func TestR1340_AVolumeFailureWhosePutBackStopFailsRecordsTheStepAndNoDatabase(t *testing.T) { + m, vp, rig, _, notified, _ := r893PutBackFixture(t) + rig.snapReplayErr = errors.New("volume rm -f immich_pgdata: volume is in use") + p := &r1256StopFailProvider{r893Provider: vp, failAt: 2} // 1 = the restore's stop, 2 = the put-back's + m.SetStackProvider(p) + if _, err := m.ReconstituteFromOffsite(context.Background(), "immich", false); err == nil { + t.Fatal("a failed restore must be surfaced") + } + h := holdOf(t, m, "immich") + if h.Reason != settings.HoldReasonRestoreMixed || h.MixedCause != settings.MixedCausePutBack { + t.Fatalf("reason %q, mixed cause %q — want restore_mixed / put_back", h.Reason, h.MixedCause) + } + if !strings.Contains(h.FailedStep, "stopping the app before the put-back failed") || !strings.Contains(h.FailedStep, "compose down: timeout") { + t.Errorf("failed step %q — want the stop and its error", h.FailedStep) + } + if h.DBReplay { + t.Error("a volume failure must not be recorded as a database replay") + } + if h.RestoreKind != settings.RestoreKindOffsite { + t.Errorf("restore kind %q, want offsite", h.RestoreKind) + } + if h.SafetyDump == "" { + t.Error("the kept copy's folder must be recorded") + } else if _, err := os.Stat(h.SafetyDump); err != nil { + t.Errorf("the recorded copy %s is not on disk: %v", h.SafetyDump, err) + } + if *notified != 1 { + t.Errorf("hold notices %d, want 1", *notified) + } +} + +// The database replay fails (option C's shape), then the put-back's stop fails: DBReplay is recorded. +func TestR1340_ADatabaseReplayFailureRecordsTheDatabase(t *testing.T) { + m, vp, _, _, _, _ := r893PutBackFixture(t) + m.SetStackProvider(&r1256StopFailProvider{r893Provider: vp, failAt: 2}) + if _, err := m.ReconstituteFromOffsite(context.Background(), "immich", false); err == nil { + t.Fatal("a failed restore must be surfaced") + } + h := holdOf(t, m, "immich") + if h.MixedCause != settings.MixedCausePutBack || !h.DBReplay { + t.Fatalf("mixed cause %q, db replay %v — want put_back and true", h.MixedCause, h.DBReplay) + } +} + +// R-1256 H4: the backup's definition cannot be written and the live one cannot be written back — no data moved. +func TestR1340_ADefinitionWriteFailureRecordsItsCause(t *testing.T) { + m, _, _ := r1256DefFailFixture(t, 2) + if _, err := m.ReconstituteFromOffsite(context.Background(), "immich", false); err == nil { + t.Fatal("a failed definition write must be surfaced") + } + h := holdOf(t, m, "immich") + if h.MixedCause != settings.MixedCauseDefinitionWrite || h.DBReplay { + t.Fatalf("mixed cause %q, db replay %v — want definition_write and false", h.MixedCause, h.DBReplay) + } + if !strings.Contains(h.FailedStep, "writing the live app definition back failed") { + t.Errorf("failed step %q", h.FailedStep) + } +} + +// A LOCAL restore (from the app's own backup) records itself as local, never off-site. +func TestR1340_ALocalRestoreRecordsItsKind(t *testing.T) { + m, _, rig, _ := r1177Fixture(t) + if m.settings == nil { + sett, err := settings.Load(t.TempDir()+"/settings.json", log.New(io.Discard, "", 0)) + if err != nil { + t.Fatal(err) + } + m.settings = sett + } + rig.unitErr = errors.New("tar: short read") + rig.putBackErr = errors.New("tar xf failed") + if _, err := m.RestoreFromRecoveryUnit("app"); err == nil { + t.Fatal("a failed put-back must be surfaced") + } + h := holdOf(t, m, "app") + if h.RestoreKind != settings.RestoreKindLocal || h.MixedCause != settings.MixedCausePutBack || h.DBReplay { + t.Fatalf("kind %q, cause %q, db %v — want local / put_back / false", h.RestoreKind, h.MixedCause, h.DBReplay) + } + if !strings.Contains(h.FailedStep, "putting the live volumes back failed") || !strings.Contains(h.FailedStep, "tar xf failed") { + t.Errorf("failed step %q", h.FailedStep) + } +} + +// A restore cut by a stop whose put-back fails at the next start (R-1251 H2) records its own cause. +func TestR1340_AnInterruptedPutBackRecordsItsCause(t *testing.T) { + r := newR1251Rig(t) + r.putErr = errors.New("compose down: timeout") + r.guard().Recover() + h, ok := r.sett.GetRestoreHold("immich") + if !ok || h.Reason != settings.HoldReasonRestoreMixed || h.MixedCause != settings.MixedCauseInterruptedPutBack { + t.Fatalf("hold %+v (held %v) — want restore_mixed / interrupted_put_back", h, ok) + } + if !strings.Contains(h.FailedStep, "compose down: timeout") { + t.Errorf("failed step %q", h.FailedStep) + } +} diff --git a/controller/internal/backup/restore_unit.go b/controller/internal/backup/restore_unit.go index eebee576..c3dc7f22 100644 --- a/controller/internal/backup/restore_unit.go +++ b/controller/internal/backup/restore_unit.go @@ -3,6 +3,7 @@ package backup import ( "errors" "fmt" + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" "gitea.dooplex.hu/admin/felhom-controller/internal/util" "os" "path/filepath" @@ -530,7 +531,7 @@ func (m *Manager) RestoreFromRecoveryUnitAtWith(stackName, unitDir string, opt U // without one (the unit holds no volume) only a database replay that began can have changed data — and with no // copy to pour back that is the R-379 half state: held, never started (operator ruling 2026-08-22). putBack := func(cause error, holdKey string, moved bool) (UnitRestoreResult, error) { - return res, m.putBackAfterFailedRestore(stackName, cause, liveDef, undo, moved || undo != nil, opt.FilesPlaced, restartStack, holdKey) + return res, m.putBackAfterFailedRestore(stackName, cause, liveDef, undo, moved || undo != nil, opt.FilesPlaced, restartStack, holdKey, settings.RestoreKindLocal) } // R-353: the count is captured even when the replay errors — a partial replay is a fact the // customer's sentence has to be built from, and discarding it on the error path is how Scenario C diff --git a/controller/internal/report/opactions.go b/controller/internal/report/opactions.go index 742ea258..d58e15ea 100644 --- a/controller/internal/report/opactions.go +++ b/controller/internal/report/opactions.go @@ -21,9 +21,11 @@ import ( // problem, main.go). The result rides the next report as `operator_action_results: [{id, outcome, // message}]`; the hub lists an action until its result has arrived, then stops. // -// THE LIST IS CLOSED. Four actions, and run_job names one of four jobs. Nothing here deletes data, +// THE LIST IS CLOSED. Five actions, and run_job names one of four jobs. Nothing here deletes data, // starts a countdown or shortens one — `03` §4 asks for a signing key only to destroy or overwrite the -// only copy, and none of these does. An unknown action, an unknown job or an argument out of range is +// only copy, and none of these does. clear_hold (D64 = B, operator ruling 2026-10-29 12:10) lifts an app's +// hold of ANY reason with no operator signature: it deletes nothing and starts nothing — the app stays +// stopped until the household (or the operator) starts it, and a waiting put-back's copy stays on disk. An unknown action, an unknown job or an argument out of range is // answered `refused` and NOTHING is called. Pinned by TestOpActions_ClosedList (which fails when an // entry is added, so a new action is a decision, not an edit) and TestOpActions_UnknownIsRefused. // @@ -38,6 +40,7 @@ const ( OpAbandonStop = "abandon_stop" OpAbandonExtend = "abandon_extend" OpRunJob = "run_job" + OpClearHold = "clear_hold" OutcomeDone = "done" OutcomeRefused = "refused" @@ -57,7 +60,7 @@ const ( ) // operatorActionNames is THE closed list. Adding an entry fails TestOpActions_ClosedList by design. -var operatorActionNames = []string{OpOffsiteBackupNow, OpAbandonStop, OpAbandonExtend, OpRunJob} +var operatorActionNames = []string{OpOffsiteBackupNow, OpAbandonStop, OpAbandonExtend, OpRunJob, OpClearHold} // operatorJobNames is the fixed set run_job may name — the scheduler's own job names (main.go). var operatorJobNames = []string{"fill-watch", "offsite-integrity", "offsite-proof", "disk-health-check"} @@ -83,6 +86,21 @@ func contains(list []string, v string) bool { return false } +// validHoldStack is clear_hold's argument check: the same alphabet as backup.isSafeStackName (report cannot import +// backup; REUSE.md „Safe-name validators"). +func validHoldStack(s string) bool { + if s == "" || len(s) > 64 { + return false + } + for _, c := range s { + if (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '-' || c == '_' { + continue + } + return false + } + return true +} + // ErrActionRefused, wrapped by a handler, turns its error into a `refused` outcome (the action could // not START — e.g. an off-site run already in flight) instead of `failed` (it started and went wrong). var ErrActionRefused = errors.New("refused") @@ -101,7 +119,7 @@ type OperatorActionResult struct { Message string `json:"message"` } -// OperatorActionHandlers are the four doors, wired by main.go. A nil handler answers `refused` +// OperatorActionHandlers are the five doors, wired by main.go. A nil handler answers `refused` // („not available on this box"). type OperatorActionHandlers struct { // OffsiteBackupNow returns a refusal sentence when the run may not start (the household's @@ -111,6 +129,9 @@ type OperatorActionHandlers struct { AbandonExtend func(days int) (time.Time, error) // RunJob is Scheduler.RunNow: an error = not started (unknown/running); the channel = the job's end. RunJob func(name string) (<-chan error, error) + // ClearHold lifts the app's hold in the RUNNING controller (D64 = B, R-1343) and returns the result + // sentence. actionID names the action in the box's log. An error wrapping ErrActionRefused = nothing changed. + ClearHold func(stack string, actionID int64) (string, error) // ResultReady sends an out-of-cycle report so a result reaches the hub in seconds (optional). ResultReady func() } @@ -317,6 +338,23 @@ func (o *OperatorActions) dispatch(a OperatorAction) { } o.finish(a, outcomeOf(jerr), msg) }() + case OpClearHold: + // The argument is checked HERE, before any handler is touched (the closed list's rule): one app name, + // the backup package's strict alphabet (letters, digits, '-' and '_', at most 64). + if !validHoldStack(a.Arg) { + o.finish(a, OutcomeRefused, "the argument must be one app name (letters, digits, '-' and '_')") + return + } + if o.h.ClearHold == nil { + o.finish(a, OutcomeRefused, "not available on this box") + return + } + msg, err := o.h.ClearHold(a.Arg, a.ID) + if err != nil { + o.finish(a, outcomeOf(err), err.Error()) + return + } + o.finish(a, OutcomeDone, msg) default: o.finish(a, OutcomeRefused, "unknown action") } diff --git a/controller/internal/report/opactions_test.go b/controller/internal/report/opactions_test.go index a7f6573d..fe9b999c 100644 --- a/controller/internal/report/opactions_test.go +++ b/controller/internal/report/opactions_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "reflect" "strings" "sync" @@ -15,9 +16,11 @@ import ( // The list is CLOSED. This test fails when an action or a job is added, by design: a new entry is an // operator decision (no action may delete data, start a countdown or shorten one), not an edit. The -// hub pins the same four in its own test (hub internal/store opactions_test.go). +// hub pins the same list in its own test (hub internal/store opactions_test.go). +// 2026-10-29: clear_hold added deliberately — operator ruling D64 = B (12:10): the hub's „Clear hold" works for +// every hold reason with no operator signature. It deletes nothing and starts nothing. func TestOpActions_ClosedList(t *testing.T) { - if got, want := OperatorActionNames(), []string{"abandon_extend", "abandon_stop", "offsite_backup_now", "run_job"}; !reflect.DeepEqual(got, want) { + if got, want := OperatorActionNames(), []string{"abandon_extend", "abandon_stop", "clear_hold", "offsite_backup_now", "run_job"}; !reflect.DeepEqual(got, want) { t.Fatalf("operator actions = %v, want exactly %v — a new action needs the operator's word (decision 185)", got, want) } if got, want := OperatorJobNames(), []string{"disk-health-check", "fill-watch", "offsite-integrity", "offsite-proof"}; !reflect.DeepEqual(got, want) { @@ -56,6 +59,10 @@ func recordingHandlers(c *calls) OperatorActionHandlers { close(ch) return ch, nil }, + ClearHold: func(stack string, id int64) (string, error) { + c.add("clear:" + stack) + return "the hold on " + stack + " is cleared", nil + }, } } @@ -197,3 +204,73 @@ func TestOpActions_WireShapes(t *testing.T) { t.Fatalf("report carried %s", b) } } + +// clear_hold (D64 = B): the argument is one app name in the strict alphabet — never a path, never empty, never two +// names. A bad one is refused and the handler is NOT called (the closed list's rule: validate before any door). +// RED-PROOF (2026-10-29): with validHoldStack returning true, this fails on „a refused action called [clear:…]". +func TestOpActions_ClearHoldBadArgumentIsRefusedAndCallsNothing(t *testing.T) { + c := &calls{} + o := NewOperatorActions(context.Background(), recordingHandlers(c), nil) + list := []OperatorAction{ + {ID: 1, Action: OpClearHold, Arg: ""}, + {ID: 2, Action: OpClearHold, Arg: "../settings"}, + {ID: 3, Action: OpClearHold, Arg: "immich paperless"}, + {ID: 4, Action: OpClearHold, Arg: "immich/x"}, + {ID: 5, Action: OpClearHold, Arg: strings.Repeat("a", 65)}, + {ID: 6, Action: OpClearHold, Arg: "immich\n"}, + {ID: 7, Action: OpClearHold, Arg: "."}, + } + o.Reconcile(list) + if got := c.get(); len(got) != 0 { + t.Fatalf("a refused action called %v", got) + } + for _, a := range list { + if r := resultFor(t, o, a.ID); r.Outcome != OutcomeRefused || !strings.Contains(r.Message, "one app name") { + t.Errorf("#%d clear_hold(%q): %+v, want refused naming the rule", a.ID, a.Arg, r) + } + } + // control: a good name does reach the handler + o.Reconcile([]OperatorAction{{ID: 8, Action: OpClearHold, Arg: "paperless-ngx_2"}}) + if got := c.get(); len(got) != 1 || got[0] != "clear:paperless-ngx_2" { + t.Fatalf("a valid name must reach the handler, calls %v", got) + } +} + +// clear_hold (D64 = B): a valid name reaches the handler with the action's id, and its sentence is the result; a +// handler refusal (the app has no hold) is answered refused, an error that is not a refusal failed. +// RED-PROOF (2026-10-29): with the OpClearHold case removed from dispatch, this fails on „unknown action". +func TestOpActions_ClearHoldDoneRefusedFailed(t *testing.T) { + var gotStack string + var gotID int64 + h := OperatorActionHandlers{ClearHold: func(stack string, id int64) (string, error) { + gotStack, gotID = stack, id + switch stack { + case "notheld": + return "", fmt.Errorf("%w: notheld has no hold — nothing was changed", ErrActionRefused) + case "broken": + return "", errors.New("saving settings: disk full") + } + return "the hold on " + stack + " is cleared (it was: held after a failed update)", nil + }} + o := NewOperatorActions(context.Background(), h, nil) + o.Reconcile([]OperatorAction{{ID: 7, Action: OpClearHold, Arg: "kimai"}}) + if gotStack != "kimai" || gotID != 7 { + t.Fatalf("handler called with (%q, %d), want (kimai, 7)", gotStack, gotID) + } + if r := resultFor(t, o, 7); r.Outcome != OutcomeDone || r.Message != "the hold on kimai is cleared (it was: held after a failed update)" { + t.Fatalf("result %+v", r) + } + o.Reconcile([]OperatorAction{{ID: 8, Action: OpClearHold, Arg: "notheld"}, {ID: 9, Action: OpClearHold, Arg: "broken"}}) + if r := resultFor(t, o, 8); r.Outcome != OutcomeRefused || !strings.Contains(r.Message, "has no hold") { + t.Errorf("not held: %+v", r) + } + if r := resultFor(t, o, 9); r.Outcome != OutcomeFailed { + t.Errorf("a save error must be failed, not refused: %+v", r) + } + // nil handler → refused, not a crash + o2 := NewOperatorActions(context.Background(), OperatorActionHandlers{}, nil) + o2.Reconcile([]OperatorAction{{ID: 1, Action: OpClearHold, Arg: "kimai"}}) + if r := resultFor(t, o2, 1); r.Outcome != OutcomeRefused { + t.Errorf("nil handler: %+v", r) + } +} diff --git a/controller/internal/settings/settings.go b/controller/internal/settings/settings.go index e16326b8..a117b165 100644 --- a/controller/internal/settings/settings.go +++ b/controller/internal/settings/settings.go @@ -1982,8 +1982,36 @@ type RestoreHold struct { // first stop or a repeat within 24 h (Trip >= 2 → the sentence says support is informed). UnhealthyKind string `json:"unhealthy_kind,omitempty"` Trip int `json:"trip,omitempty"` + + // R-1340 (2026-10-29): what put a restore hold in place, so the operator's „HELD STOPPED" mail tells the real + // story instead of the option-C database story for every cause. All empty on a hold written before them: the mail + // then names only what the record carries (cmd/controller heldStoppedOperatorText). + // + // RestoreKind is which restore: RestoreKindOffsite or RestoreKindLocal. + RestoreKind string `json:"restore_kind,omitempty"` + // MixedCause (restore_mixed only) is the branch that held the app: MixedCause*. + MixedCause string `json:"mixed_cause,omitempty"` + // FailedStep is the put-back step that failed, with its error (MixedCausePutBack). + FailedStep string `json:"failed_step,omitempty"` + // DBReplay: the restore's own failure was its database replay. False = the app's database was never touched + // by the failure, so the mail never says „database". + DBReplay bool `json:"db_replay,omitempty"` } +// Restore kinds and mixed-hold causes (R-1340). See RestoreHold.RestoreKind / MixedCause. +const ( + RestoreKindOffsite = "offsite" + RestoreKindLocal = "local" + + // MixedCauseDefinitionWrite: the backup's definition could not be written (no data moved yet) and writing the + // live definition back failed too (R-1256 H4). + MixedCauseDefinitionWrite = "definition_write" + // MixedCausePutBack: the restore failed and putting the live data back failed at FailedStep (D20/R-1256 H5). + MixedCausePutBack = "put_back" + // MixedCauseInterruptedPutBack: a restore cut by a stop could not be put back at the next start (R-1251 H2). + MixedCauseInterruptedPutBack = "interrupted_put_back" +) + // Hold reasons. See RestoreHold.Reason. const ( HoldReasonRestoreFailed = "" // R-379/R-380: a restore AND its rollback failed diff --git a/controller/internal/stacks/d64_forget_update_held_test.go b/controller/internal/stacks/d64_forget_update_held_test.go new file mode 100644 index 00000000..9918cf90 --- /dev/null +++ b/controller/internal/stacks/d64_forget_update_held_test.go @@ -0,0 +1,46 @@ +package stacks + +import ( + "io" + "log" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/config" +) + +// D64 = B / R-1343: when the operator clears a hold, the ONE held flag kept outside the hold store — „the last update +// ended held" — goes too. Left set, the crash-loop observer and the nightly update leg kept skipping an app whose hold +// was gone, until the controller restarted. +// RED-PROOF (2026-10-29): with ForgetUpdateHeld's body reduced to `return false`, this fails on „a held update must be forgotten". +func TestD64_ForgetUpdateHeld(t *testing.T) { + m := &Manager{cfg: &config.Config{}, logger: log.New(io.Discard, "", 0), stacks: map[string]*Stack{ + "kimai": {Name: "kimai", Deployed: true}, + "running": {Name: "running", Deployed: true, Updating: true}, + }} + m.finishUpdateKey("kimai", UpdatePhaseFailed, UpdateErrorKeyHeld, "a kimai leállítva marad") + m.markUpdateHeld("kimai") + m.markUpdateHeld("running") + + if !m.ForgetUpdateHeld("kimai") { + t.Fatal("a held update must be forgotten") + } + st, _ := m.GetStack("kimai") + if st.updateHeld || st.UpdateErrorKey != "" || st.UpdateError != "" || st.UpdatePhase != "" { + t.Fatalf("held state left: held=%v key=%q err=%q phase=%q", st.updateHeld, st.UpdateErrorKey, st.UpdateError, st.UpdatePhase) + } + // The two readers of the flag (ObserveUnhealthy's skip, unhealthy.go; legCandidate's LegSkipHeld, unattended.go) + // read the stored stack — not called here, because ObserveUnhealthy reaches docker (R-650). + m.mu.RLock() + skipped := m.stacks["kimai"].updateHeld || m.stacks["kimai"].UpdateErrorKey == UpdateErrorKeyHeld + m.mu.RUnlock() + if skipped { + t.Fatal("the observer and the update leg still skip kimai") + } + // A running update is never touched; an app with nothing held reports false. + if m.ForgetUpdateHeld("running") { + t.Fatal("a running update must not be touched") + } + if m.ForgetUpdateHeld("kimai") || m.ForgetUpdateHeld("nosuch") { + t.Fatal("nothing held → false") + } +} diff --git a/controller/internal/stacks/update.go b/controller/internal/stacks/update.go index 9b784f82..71a15a71 100644 --- a/controller/internal/stacks/update.go +++ b/controller/internal/stacks/update.go @@ -209,6 +209,25 @@ func (m *Manager) ClearUpdateState(name string) { s.HealthProbe = nil } +// ForgetUpdateHeld (D64 = B, R-1343) forgets that the app's last update ended HELD, once the hold itself is cleared +// by the operator. updateHeld is the ONE held flag kept outside the hold store: it makes the crash-loop observer +// (ObserveUnhealthy) and the nightly update leg (legCandidate) skip the app, and keeps the hold's sentence as the +// update error. Left set, an app whose hold was cleared would stay unobserved and never be updated by night until +// the controller restarts. A running update is not touched. Returns whether anything was forgotten. +// Pinned by TestD64_ForgetUpdateHeld (this package) and TestD64_ClearHoldForgetsTheUpdateHeldFlag (cmd/controller). +func (m *Manager) ForgetUpdateHeld(name string) bool { + m.mu.Lock() + defer m.mu.Unlock() + s, ok := m.stacks[name] + if !ok || s.Updating || (!s.updateHeld && s.UpdateErrorKey != UpdateErrorKeyHeld) { + return false + } + s.updateHeld = false + s.UpdatePhase, s.UpdatePhaseLabel = "", "" + s.UpdateError, s.UpdateErrorKey, s.UpdateErrorArgs = "", "", nil + return true +} + func (m *Manager) markUpdateHeld(name string) { m.mu.Lock() if s, ok := m.stacks[name]; ok {