From c4b6844d340e4f94441faffd3345dd80f3daf77b Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Mon, 2 Nov 2026 23:14:16 +0100 Subject: [PATCH] Traefik v3.7.17 (GHSA-xv5x-242w-7hf7); R-1347 a restore forgets the update-held flag; R-1348 a failed put-back at start-up mails; R-1339, R-1351, R-1323 truthful start-up lines and one lock over FileBrowser's compose. Commit only. --- CHANGELOG.md | 34 ++++++++ controller/cmd/controller/main.go | 9 ++- .../cmd/controller/r1347_r1348_wiring_test.go | 19 +++++ controller/internal/backup/appstop_marker.go | 4 + .../internal/backup/appstop_putback_wait.go | 33 ++++++++ controller/internal/backup/backup.go | 3 + .../backup/r1347_forget_update_held_test.go | 53 +++++++++++++ .../backup/r1348_putback_mail_test.go | 75 ++++++++++++++++++ controller/internal/backup/update_guard.go | 12 +++ controller/internal/infra/infra.go | 4 +- .../internal/stacks/filebrowser_gate.go | 23 +++++- controller/internal/stacks/infra.go | 25 +++++- controller/internal/stacks/manager.go | 7 +- .../stacks/r1323_gate_replaced_test.go | 78 +++++++++++++++++++ .../internal/stacks/r1347_watcher_test.go | 40 ++++++++++ .../stacks/r1351_filebrowser_serial_test.go | 58 ++++++++++++++ controller/internal/stacks/update.go | 2 +- .../internal/web/filebrowser_gate_test.go | 73 ++++++++++++++++- controller/internal/web/handlers.go | 26 ++++++- 19 files changed, 569 insertions(+), 9 deletions(-) create mode 100644 controller/cmd/controller/r1347_r1348_wiring_test.go create mode 100644 controller/internal/backup/r1347_forget_update_held_test.go create mode 100644 controller/internal/backup/r1348_putback_mail_test.go create mode 100644 controller/internal/stacks/r1323_gate_replaced_test.go create mode 100644 controller/internal/stacks/r1347_watcher_test.go create mode 100644 controller/internal/stacks/r1351_filebrowser_serial_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 62441e18..c88995b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,37 @@ +## Unreleased — next controller release (MinAgent unchanged, 0.158.1); commit only, 2026-11-02 night + +**Traefik v3.7.17 (GHSA-xv5x-242w-7hf7, high, published 2026-11-02).** v3.7.14–v3.7.16: an unauthenticated client that +sends a `Negotiate` or `NTLM` `Authorization` header gets a dedicated backend transport per connection, and it is not +released when the connection closes: many short connections exhaust Traefik's memory, whether or not an NTLM backend +exists. Traefik fronts every box. `TraefikImage` v3.7.16 → v3.7.17; a running box takes it by the reconcile at start +(`TestEnsureTraefik_ANewImageRecreatesTraefik`). The upgrade notes name Go 1.27 (GODEBUG TLS settings: we set none; 500 +request-header values at most, else 431) and the ingress-nginx provider (not used). Found by reading the release itself: +the watch had flagged it on a word-only line (felhom.eu R-1310). + +**R-1347 — a restore that lifts an update hold forgets the „update held" flag.** `clearUpdateHoldAfterRestore` cleared the +hold in settings, but `stacks.Manager` kept `updateHeld`: the crash-loop watcher did not watch the app and the night leg +skipped it until the controller restarted. New seam `backup.SetUpdateHeldForgetter` (`stackMgr.ForgetUpdateHeld`). Tests +`TestR1347_*` (the seam, the wiring, and the consequence: the same restarts trip the watcher once the flag is gone). + +**R-1348 — a failed put-back at start-up mails the operator.** `holdPutBackFailed` writes the D20 hold at `Recover`, before any +notifier exists, so only the hold reporter's info and the hub's later reminder said anything. `AppStopGuard.SetHeldNotify` +hands over what `Recover` held, once, and mails a later failure (a drive that returned) at once: the R-1340 sentence for +`MixedCauseInterruptedPutBack`, from the hold record. Tests `TestR1348_*`. + +**R-1339 — no false „drive … not mounted".** A per-app folder under a mounted drive is never a mountpoint itself; nine false lines +at every start on 9202. The folder still gets no bind and no skeleton of its own (the row's „not skipped" would write a +`userdata/` inside the app's folder and recreate the file manager, so the behaviour stays and only the line changes). A drive that +is really absent keeps its line, and a folder of it names the drive. `TestR1339_*`. + +**R-1351 — FileBrowser's two start paths at boot take one lock.** `EnsureBaseStack`'s `compose up` and +`web.syncFileBrowserMounts`' `compose up --force-recreate` in the same second gave „Conflict. The container name … is already in +use", two ERROR lines and a first-boot WARN for an app that then ran healthy. `stacks.Manager.LockFileBrowserStack`. +`TestR1351_*`. + +**R-1323 — „No such container" is not „every app can still reach it".** The checked container was replaced between the check and +the leave; the line now reads the CURRENT container's networks and says the truth. A current container on the app network, an +unreadable one, or any other failure keeps the alarm. `TestR1323_*`. + ## v0.349.0 (2026-10-29) — the household line after an operator reboot and the busy file for it (D65 = A); a hold clear records the wanted stop (R-1350) ### 2026-10-29 (evening) diff --git a/controller/cmd/controller/main.go b/controller/cmd/controller/main.go index c60847df..061ed123 100644 --- a/controller/cmd/controller/main.go +++ b/controller/cmd/controller/main.go @@ -682,6 +682,7 @@ func main() { backupMgr.SetVersionCheck(stackMgr.PinNotRunning) // R-671 (v0.272.0): a restore that lifts an update hold removes the undo copies that hold kept. backupMgr.SetUndoCopyRemover(stackMgr.RemoveUndoCopies) + backupMgr.SetUpdateHeldForgetter(stackMgr.ForgetUpdateHeld) // R-1347 } if n := stackMgr.ResumeInterruptedUpdates(ctx); n > 0 { logger.Printf("[WARN] [update] resumed %d interrupted update(s)", n) @@ -1556,7 +1557,7 @@ func main() { // 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) { + holdNotify := func(stack string, replayErr, rollbackErr error) { h := heldHoldFromNotify(sett.GetRestoreHold, stack, replayErr, rollbackErr) msg, leg, reason := heldStoppedOperatorText(h) runKind := "offsite-reconstitute" @@ -1566,7 +1567,11 @@ func main() { 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) - }) + } + backupMgr.SetRestoreHoldNotify(holdNotify) + // R-1348: the hold an interrupted put-back wrote at START-UP (Recover, before any notifier existed) or on a resume. + // The same sentence (R-1340, MixedCauseInterruptedPutBack), the same mail. + appStopGuard.SetHeldNotify(func(stack string) { holdNotify(stack, nil, nil) }) // R-893 option A (D20 = A): a failed off-site restore that was PUT BACK — the live volumes and definition poured // back and the app started. The same operator-tier event as the hold above (no new event type), its own leg. // D47 follow-up: a unit of an earlier install was set aside whole — the operator sees it (its own leg). diff --git a/controller/cmd/controller/r1347_r1348_wiring_test.go b/controller/cmd/controller/r1347_r1348_wiring_test.go new file mode 100644 index 00000000..1bc124c7 --- /dev/null +++ b/controller/cmd/controller/r1347_r1348_wiring_test.go @@ -0,0 +1,19 @@ +package main + +import "testing" + +// R-1347 and R-1348: each seam is CALLED from main.go (an AST walk) — a seam built and never wired is the project's +// oldest shape. +func TestR1347_UpdateHeldForgetterIsWiredAtStartup(t *testing.T) { + lines, _, _ := slice4CallLines(t) + if len(lines["SetUpdateHeldForgetter"]) == 0 { + t.Fatal("main.go never calls SetUpdateHeldForgetter — a restored app would stay unwatched until the controller restarts") + } +} + +func TestR1348_HeldNotifyIsWiredAtStartup(t *testing.T) { + lines, _, _ := slice4CallLines(t) + if len(lines["SetHeldNotify"]) == 0 { + t.Fatal("main.go never calls SetHeldNotify — a failed put-back at start-up would mail nobody") + } +} diff --git a/controller/internal/backup/appstop_marker.go b/controller/internal/backup/appstop_marker.go index 833ec311..decbd243 100644 --- a/controller/internal/backup/appstop_marker.go +++ b/controller/internal/backup/appstop_marker.go @@ -137,6 +137,10 @@ type AppStopGuard struct { // whether the drive a copy lives on is bound. nil = the pre-R-1251 behaviour. holds AppStopHolds undoReachable func(stack, dir string) bool + // heldNotify and heldPending (R-1348, appstop_putback_wait.go): the operator mail for a failed put-back. Recover runs + // before the notifier exists, so a hold written then waits in heldPending until SetHeldNotify. + heldNotify func(stack string) + heldPending []string // liveOp (R-1253) is the operation of THIS process that holds the marker now (set by Begin, cleared by End). The // marker has one slot; BeginExclusive refuses to replace a live one. A marker left by a crash has no live op. diff --git a/controller/internal/backup/appstop_putback_wait.go b/controller/internal/backup/appstop_putback_wait.go index 018f0724..80f82891 100644 --- a/controller/internal/backup/appstop_putback_wait.go +++ b/controller/internal/backup/appstop_putback_wait.go @@ -126,9 +126,42 @@ func (g *AppStopGuard) holdPutBackFailed(stack, dir string, cause error) bool { return false } g.logger.Printf("[ERROR] [appstop] %s: the interrupted restore could NOT be put back (%v) — the app is HELD for support, the copy stays at %s (R-1251 H2)", stack, cause, dir) + g.notifyHeld(stack) return true } +// SetHeldNotify wires the operator mail for a failed put-back (R-1348; main.go: the R-1340 sentence from the hold record) +// and hands over what Recover held before the notifier existed. Each held app is mailed once. Pinned by TestR1348_* and +// TestR1348_HeldNotifyIsWiredAtStartup. +func (g *AppStopGuard) SetHeldNotify(fn func(stack string)) { + if g == nil { + return + } + g.liveMu.Lock() + g.heldNotify = fn + waiting := g.heldPending + g.heldPending = nil + g.liveMu.Unlock() + if fn == nil { + return + } + for _, stack := range waiting { + fn(stack) + } +} + +func (g *AppStopGuard) notifyHeld(stack string) { + g.liveMu.Lock() + fn := g.heldNotify + if fn == nil { + g.heldPending = append(g.heldPending, stack) + } + g.liveMu.Unlock() + if fn != nil { + fn(stack) + } +} + // clearPutBackPending lifts this app's waiting-put-back hold, and only that kind. func (g *AppStopGuard) clearPutBackPending(stack string) { if g.holds == nil { diff --git a/controller/internal/backup/backup.go b/controller/internal/backup/backup.go index c99e8428..23deadd1 100644 --- a/controller/internal/backup/backup.go +++ b/controller/internal/backup/backup.go @@ -219,6 +219,9 @@ type Manager struct { // undoCopyRemover (R-671, v0.272.0) deletes an app's leftover undo copies — stacks.Manager.RemoveUndoCopies, // wired in main.go (SetUndoCopyRemover). nil-safe: without it the copies stay, as before. undoCopyRemover func(stackName string) int + // updateHeldForgetter (R-1347) forgets the stack manager's „last update ended held" flag when a restore lifts the update + // hold — stacks.Manager.ForgetUpdateHeld, wired in main.go (SetUpdateHeldForgetter). nil-safe. + updateHeldForgetter func(stackName string) bool // R-475 update-precondition seams, one per tier. Nil → the real Tier2UnitRestorePoint / // ListRestorePoints / OffsiteSnapshotTimes. They let a test reach Tier 1 and Tier 3 without a drive diff --git a/controller/internal/backup/r1347_forget_update_held_test.go b/controller/internal/backup/r1347_forget_update_held_test.go new file mode 100644 index 00000000..fad59217 --- /dev/null +++ b/controller/internal/backup/r1347_forget_update_held_test.go @@ -0,0 +1,53 @@ +package backup + +import ( + "io" + "log" + "testing" + "time" + + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" +) + +// R-1347: a restore that lifts an UPDATE hold must also forget the stack manager's in-memory „last update ended held" +// flag. Left set, the crash-loop watcher does not watch the app and the night leg skips it until the controller restarts +// (D64's sibling: the operator's clear_hold already forgets it, R-1343; the restore did not). +// +// The consequence on the real flag — ObserveUnhealthy and legCandidate — is TestR1347_RestoreLiftsTheWatchersBlindSpot +// (cmd/controller), and the seam's wiring TestR1347_UpdateHeldForgetterIsWiredAtStartup. +// +// RED-PROOF (2026-11-02): seen failing before the seam existed — the build failed on SetUpdateHeldForgetter; with the +// call dropped from clearUpdateHoldAfterRestore it fails on „the flag was not forgotten". +func TestR1347_ARestoreThatLiftsAnUpdateHoldForgetsTheHeldFlag(t *testing.T) { + sett := slice4Settings(t) + m := &Manager{logger: log.New(io.Discard, "", 0), settings: sett} + forgot := map[string]int{} + m.SetUpdateHeldForgetter(func(s string) bool { forgot[s]++; return true }) + + _ = m.HoldAfterFailedUpdate("nextcloud", time.Now(), time.Now(), UpdateTierLocal) + m.clearUpdateHoldAfterRestore("nextcloud") + if _, held := sett.GetRestoreHold("nextcloud"); held { + t.Fatal("the update hold must be lifted") + } + if forgot["nextcloud"] != 1 { + t.Fatalf("the restore lifted the update hold, but the flag was not forgotten (calls: %v)", forgot) + } + + // A RESTORE hold (R-379) stays, so nothing about it is forgotten. + _ = sett.SetRestoreHold(settings.RestoreHold{Stack: "rst", At: "2026-08-22T14:00:00Z"}) + m.clearUpdateHoldAfterRestore("rst") + if _, held := sett.GetRestoreHold("rst"); !held || forgot["rst"] != 0 { + t.Fatalf("a restore hold is not lifted by a restore (held %v, forget calls %d)", held, forgot["rst"]) + } + + // No hold at all: nothing to forget (a plain restore of a healthy app). + m.clearUpdateHoldAfterRestore("plain") + if forgot["plain"] != 0 { + t.Fatalf("no hold was lifted, yet the flag was touched %d time(s)", forgot["plain"]) + } + + // nil seam: no crash (the pre-R-1347 behaviour). + m2 := &Manager{logger: log.New(io.Discard, "", 0), settings: sett} + _ = m2.HoldAfterFailedUpdate("other", time.Now(), time.Now(), UpdateTierLocal) + m2.clearUpdateHoldAfterRestore("other") +} diff --git a/controller/internal/backup/r1348_putback_mail_test.go b/controller/internal/backup/r1348_putback_mail_test.go new file mode 100644 index 00000000..469a7f52 --- /dev/null +++ b/controller/internal/backup/r1348_putback_mail_test.go @@ -0,0 +1,75 @@ +package backup + +import ( + "errors" + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" +) + +// R-1348: the restore_mixed hold written at START-UP after an interrupted put-back fails must reach the operator as the +// R-1340 mail. Recover runs long before the notifier exists (main.go), so the guard keeps the name and hands it over when +// SetHeldNotify is wired; a failure after that (a drive that returned at run time) goes out at once. +// +// RED-PROOF (2026-11-02): seen failing before the seam existed (build: SetHeldNotify); with the call dropped from +// holdPutBackFailed these fail on „no mail for a held app". +func TestR1348_AFailedPutBackAtStartupMailsOnceTheNotifierIsWired(t *testing.T) { + r := newR1251Rig(t) + r.putErr = errors.New("compose down: timeout") + g := r.guard() + if res := g.Recover(); res == nil || len(res.Held) != 1 { + t.Fatalf("recovery must hold the app: %+v", res) + } + var mailed []string + g.SetHeldNotify(func(stack string) { mailed = append(mailed, stack) }) + if len(mailed) != 1 || mailed[0] != "immich" { + t.Fatalf("no mail for a held app: the hold from Recover must be handed over when the notifier is wired, got %v", mailed) + } + h, ok := r.sett.GetRestoreHold("immich") + if !ok || h.Reason != settings.HoldReasonRestoreMixed || h.MixedCause != settings.MixedCauseInterruptedPutBack || + !strings.Contains(h.FailedStep, "compose down: timeout") { + t.Fatalf("the mail reads this record, and it is wrong: %+v", h) + } + g.SetHeldNotify(func(stack string) { mailed = append(mailed, "again:"+stack) }) + if len(mailed) != 1 { + t.Fatalf("a held app is mailed once, not at every wiring: %v", mailed) + } +} + +// A clean put-back, or one still waiting for its drive, is no failure: no mail. +func TestR1348_NoMailWithoutAFailedPutBack(t *testing.T) { + r := newR1251Rig(t) + g := r.guard() + g.Recover() + var mailed []string + g.SetHeldNotify(func(stack string) { mailed = append(mailed, stack) }) + if len(mailed) != 0 { + t.Fatalf("a put-back that worked mailed: %v", mailed) + } + + w := newR1251Rig(t) + w.unbind(t) + gw := w.guard() + gw.Recover() + gw.SetHeldNotify(func(stack string) { mailed = append(mailed, stack) }) + if len(mailed) != 0 { + t.Fatalf("a put-back that waits for its drive has its own hold and reminder, not this mail: %v", mailed) + } +} + +// After the wiring a failed put-back (the drive returned at run time: ResumePutBack) mails at once. +func TestR1348_AFailedResumeMailsAtOnce(t *testing.T) { + r := newR1251Rig(t) + r.unbind(t) + g := r.guard() + g.Recover() // waiting for its drive + var mailed []string + g.SetHeldNotify(func(stack string) { mailed = append(mailed, stack) }) + r.bind(t) + r.putErr = errors.New("tar xf failed") + g.ResumePutBack("immich") + if len(mailed) != 1 || mailed[0] != "immich" { + t.Fatalf("a put-back that failed on resume must mail: %v", mailed) + } +} diff --git a/controller/internal/backup/update_guard.go b/controller/internal/backup/update_guard.go index 55f85f33..3d87f1eb 100644 --- a/controller/internal/backup/update_guard.go +++ b/controller/internal/backup/update_guard.go @@ -674,6 +674,12 @@ func (m *Manager) clearUpdateHoldAfterRestore(stackName string) { return } m.logger.Printf("[INFO] [backup] %s: restore completed — the update hold (set %s) is CLEARED", stackName, h.At) + // R-1347: the hold store and the stack manager's in-memory flag go together, as in the operator's clear (D64 = B, R-1343). + // Left set, the crash-loop watcher would not watch the restored app and the night leg would skip it until the next + // controller restart. A running update is not touched (ForgetUpdateHeld refuses it). Pinned by TestR1347_*. + if m.updateHeldForgetter != nil && m.updateHeldForgetter(stackName) { + m.logger.Printf("[INFO] [backup] %s: the restored app is watched and updated again — its update-held flag is forgotten (R-1347)", stackName) + } // R-671 (v0.272.0): the undo copies the hold kept describe the state this restore just replaced. They // were kept so the hold's data stayed recoverable; once the app is restored whole they are dead weight // (measured 2026-09-24 on 9202: three nextcloud copies, ~0.9 GiB, outlived the hold and nothing named @@ -691,6 +697,12 @@ func (m *Manager) SetUndoCopyRemover(fn func(stackName string) int) { m.undoCopyRemover = fn } +// SetUpdateHeldForgetter wires R-1347's seam: stacks.Manager.ForgetUpdateHeld. INIT-ONLY, main.go — pinned by +// TestR1347_UpdateHeldForgetterIsWiredAtStartup (an AST walk). The backup package cannot import stacks. +func (m *Manager) SetUpdateHeldForgetter(fn func(stackName string) bool) { + m.updateHeldForgetter = fn +} + // UpdateHeldStacks is the set of apps held stopped after a failed update (R-660, v0.268.0) — the // FOURTH way the product stops an app on purpose, and until v0.268.0 the one `classifyRunStates` did // not know: each hold's `app_update_held` was followed ~11 s later by an `app_start_failed` for the diff --git a/controller/internal/infra/infra.go b/controller/internal/infra/infra.go index b65e9a78..4a6652e9 100644 --- a/controller/internal/infra/infra.go +++ b/controller/internal/infra/infra.go @@ -27,8 +27,10 @@ import ( // notices (felhom.eu R-1304, R-1305; 1.5.7-stable is skipped — it broke existing TOTP sign-ins). Each new image reaches // a running box by the reconcile at start: TestEnsureTraefik_ANewImageRecreatesTraefik, // TestEnsureCloudflared_ANewImageRecreatesTheTunnel, TestReconcileFileBrowserImage_MovesOnlyTheImage. +// traefik v3.7.17 since 2026-11-02: GHSA-xv5x-242w-7hf7 (high, v3.7.14–v3.7.16): an unauthenticated client with a +// Negotiate/NTLM header holds a backend transport per connection until memory runs out, NTLM backend or not. const ( - TraefikImage = "traefik:v3.7.16" + TraefikImage = "traefik:v3.7.17" CloudflaredImage = "cloudflare/cloudflared:2026.10.4" FileBrowserImage = "gtstef/filebrowser:1.5.8-stable" // FileBrowserImportMount is the in-container mount point NAME for the canonical drop-zone diff --git a/controller/internal/stacks/filebrowser_gate.go b/controller/internal/stacks/filebrowser_gate.go index b15ea992..8d838867 100644 --- a/controller/internal/stacks/filebrowser_gate.go +++ b/controller/internal/stacks/filebrowser_gate.go @@ -109,7 +109,7 @@ func (m *Manager) ensureFileBrowserPublic() { } if m.fbNetOn(target, traefikNetwork) { if err := m.fbNetDisconnect(target, traefikNetwork); err != nil { - m.logger.Printf("[WARN] [infra] filebrowser: could not leave %s — every app can still reach it (R-1281): %v", traefikNetwork, err) + m.reportFileBrowserNotLeft(target, err) } else { m.logger.Printf("[INFO] [infra] filebrowser: left %s — no app container reaches it any more (R-1281)", traefikNetwork) } @@ -117,6 +117,27 @@ func (m *Manager) ensureFileBrowserPublic() { m.ensureOfficeOnFilesNetwork() } +// reportFileBrowserNotLeft says what a failed leave means (R-1323). „No such container" means the CHECKED container was +// replaced between the check and the leave (the image moved at start, a mounts sync): the line then reads the CURRENT +// container's networks and says the truth. A false „every app can still reach it" teaches the reader to ignore the real +// one, which stays: any other failure, a current container on the app network, or one that cannot be read. The new +// container is checked and published by the next round. Pinned by TestR1323_*. +func (m *Manager) reportFileBrowserNotLeft(target string, err error) { + if !strings.Contains(err.Error(), "No such container") { + m.logger.Printf("[WARN] [infra] filebrowser: could not leave %s — every app can still reach it (R-1281): %v", traefikNetwork, err) + return + } + cur := strings.Fields(m.fbStartedAt()) + switch { + case len(cur) == 0: + m.logger.Printf("[WARN] [infra] filebrowser: the checked container %s is gone and the current one cannot be read — whether every app can reach it is not known (R-1281, R-1323)", target) + case m.fbNetOn(cur[0], traefikNetwork): + m.logger.Printf("[WARN] [infra] filebrowser: could not leave %s — every app can still reach it (R-1281): the checked container %s was replaced, and the current one %s IS on it", traefikNetwork, target, cur[0]) + default: + m.logger.Printf("[INFO] [infra] filebrowser: the checked container %s was replaced before it left %s; the current one %s is not on %s (R-1323)", target, traefikNetwork, cur[0], traefikNetwork) + } +} + // traefikContainer is traefik's container (infra/templates/traefik-compose.yml.tmpl container_name). const traefikContainer = "traefik" diff --git a/controller/internal/stacks/infra.go b/controller/internal/stacks/infra.go index 4100e762..9f16a871 100644 --- a/controller/internal/stacks/infra.go +++ b/controller/internal/stacks/infra.go @@ -254,8 +254,28 @@ func changedInfraFiles(dir string, files map[string]infra.FileSpec) []string { return changed } +// LockFileBrowserStack takes the lock over every `docker compose` on FileBrowser's stack at a boot (R-1351): this file's +// ensureFileBrowser and web.syncFileBrowserMounts. Returns the unlock. Lock order: infraMu (the base stack) before this; +// the web sync takes only this one, and starts the gate (infraMu) in a goroutine. Pinned by TestR1351_* and +// TestR1351_TheWebSyncTakesTheLockBeforeCompose (internal/web). +func (m *Manager) LockFileBrowserStack() (unlock func()) { + m.fbStackMu.Lock() + return m.fbStackMu.Unlock +} + +// WithFileBrowserStack runs fn under LockFileBrowserStack. +func (m *Manager) WithFileBrowserStack(fn func()) { + defer m.LockFileBrowserStack()() + fn() +} + func (m *Manager) ensureFileBrowser(dir string) error { - if containerRunning("filebrowser") { + defer m.LockFileBrowserStack()() + running := containerRunning + if m.fbRunningFn != nil { + running = func(string) bool { return m.fbRunningFn() } + } + if running("filebrowser") { return m.reconcileFileBrowserImage(dir) } composePath := filepath.Join(dir, "docker-compose.yml") @@ -263,6 +283,9 @@ func (m *Manager) ensureFileBrowser(dir string) error { // Already provisioned but not running — bring it up WITHOUT regenerating, so the storage // mounts that web.SyncFileBrowserMounts manages are preserved. Just `compose up -d`. m.logger.Printf("[INFO] [infra] filebrowser compose exists — starting without regenerating") + if m.fbComposeUpFn != nil { + return m.fbComposeUpFn(dir) + } return m.composeUp(dir) } m.logger.Printf("[INFO] [infra] deploying filebrowser → %s", dir) diff --git a/controller/internal/stacks/manager.go b/controller/internal/stacks/manager.go index 51cc8a03..1d3e466b 100644 --- a/controller/internal/stacks/manager.go +++ b/controller/internal/stacks/manager.go @@ -294,7 +294,12 @@ type Manager struct { fbStarted func() string fbProbedRun string // fbNetOnFn / fbNetConnectFn replace the docker network reads and connects of the R-1126 gate in tests. - fbNetOnFn func(container, network string) bool + fbNetOnFn func(container, network string) bool + // fbStackMu (R-1351) is the lock over `docker compose` on FileBrowser's stack: LockFileBrowserStack. fbRunningFn and + // fbComposeUpFn replace the docker read and the bring-up of ensureFileBrowser in tests. + fbStackMu sync.Mutex + fbRunningFn func() bool + fbComposeUpFn func(dir string) error fbNetConnectFn func(container, network string) error // R-1281: the disconnect, the network create, and „is OnlyOffice linked to FileBrowser and running" — test seams. fbNetDisconnectFn func(container, network string) error diff --git a/controller/internal/stacks/r1323_gate_replaced_test.go b/controller/internal/stacks/r1323_gate_replaced_test.go new file mode 100644 index 00000000..ca6ec7b9 --- /dev/null +++ b/controller/internal/stacks/r1323_gate_replaced_test.go @@ -0,0 +1,78 @@ +package stacks + +import ( + "bytes" + "errors" + "log" + "net/http/httptest" + "strings" + "testing" +) + +// R-1323: the checked FileBrowser container can be replaced between the check and the leave (the image moved at start). +// „No such container" then means the OLD one is gone, not that every app still reaches FileBrowser. The line reads the +// CURRENT container's networks and says what is true; a current container that IS on the app network keeps the alarm. +// +// RED-PROOF (2026-11-02): before the fix the first case logged „could not leave traefik-public — every app can still reach +// it … No such container" (seen on 9202, 0.346.1-rc.1, 2026-10-27), and this fails on that line. +func r1323Rig(t *testing.T, currentOnApps bool) (*Manager, *bytes.Buffer) { + t.Helper() + fb := &fakeFB{password: "admin"} + srv := httptest.NewServer(fb.handler()) + t.Cleanup(srv.Close) + m, _, _ := fbManager(t, srv.URL) + var buf bytes.Buffer + m.logger = log.New(&buf, "", 0) + runs := []string{"old111 2026-10-27T14:58:15Z"} + m.fbStarted = func() string { + r := runs[0] + if len(runs) > 1 { + runs = runs[1:] + } + return r + } + nets := map[string]bool{"old111@traefik-public": true, "new222@traefik-public": currentOnApps} + m.fbNetOnFn = func(c, n string) bool { return nets[c+"@"+n] } + m.fbNetConnectFn = func(c, n string) error { nets[c+"@"+n] = true; return nil } + m.fbNetDisconnectFn = func(c, n string) error { + if c == "old111" { + runs = []string{"new222 2026-10-27T14:58:17Z"} // the recreate won the race + return errors.New("Error response from daemon: No such container: old111") + } + return nil + } + m.fbNetEnsureFn = func(string) error { return nil } + m.fbOfficeLinkedFn = func() (bool, bool) { return false, false } + return m, &buf +} + +func TestR1323_AReplacedContainerIsNotReportedAsExposed(t *testing.T) { + m, buf := r1323Rig(t, false) + m.fileBrowserGateLocked() + got := buf.String() + if strings.Contains(got, "every app can still reach it") { + t.Fatalf("a false alarm for a container that is gone: %q", got) + } + if !strings.Contains(got, "new222") || !strings.Contains(got, "not on traefik-public") { + t.Fatalf("the line must name the current container and the truth: %q", got) + } +} + +func TestR1323_ACurrentContainerOnTheAppNetworkKeepsTheAlarm(t *testing.T) { + m, buf := r1323Rig(t, true) + m.fileBrowserGateLocked() + got := buf.String() + if !strings.Contains(got, "every app can still reach it") || !strings.Contains(got, "new222") { + t.Fatalf("the replacement IS on the app network: the alarm stays, naming it: %q", got) + } +} + +// Any other failure to leave keeps the alarm as before (control). +func TestR1323_AnotherDisconnectFailureKeepsTheAlarm(t *testing.T) { + m, buf := r1323Rig(t, false) + m.fbNetDisconnectFn = func(c, n string) error { return errors.New("daemon busy") } + m.fileBrowserGateLocked() + if got := buf.String(); !strings.Contains(got, "every app can still reach it") || !strings.Contains(got, "daemon busy") { + t.Fatalf("a real failure to leave must keep its alarm: %q", got) + } +} diff --git a/controller/internal/stacks/r1347_watcher_test.go b/controller/internal/stacks/r1347_watcher_test.go new file mode 100644 index 00000000..b012b885 --- /dev/null +++ b/controller/internal/stacks/r1347_watcher_test.go @@ -0,0 +1,40 @@ +package stacks + +import ( + "fmt" + "io" + "log" + "testing" + "time" +) + +// R-1347, the consequence: an app whose last update ended held is not watched for a crash loop; once its hold is lifted by +// a restore and the flag forgotten (backup.clearUpdateHoldAfterRestore → ForgetUpdateHeld) the same restarts trip the +// watcher again. +func TestR1347_TheWatcherWatchesAgainOnceTheFlagIsForgotten(t *testing.T) { + m := &Manager{logger: log.New(io.Discard, "", 0), stacks: map[string]*Stack{ + "nextcloud": {Name: "nextcloud", Deployed: true, Containers: []ContainerInfo{{Name: "nextcloud-web-1"}}}, + }} + m.markUpdateHeld("nextcloud") + count := int64(0) + m.execFn = func(string, ...string) (string, error) { return fmt.Sprintf("/nextcloud-web-1|%d\n", count), nil } + t0 := time.Date(2026, 11, 2, 4, 20, 0, 0, time.UTC) + sample := func(from int) []UnhealthyVerdict { + var got []UnhealthyVerdict + for i := from; i < from+12 && len(got) == 0; i++ { + count += 3 + got = m.ObserveUnhealthy(t0.Add(time.Duration(i)*time.Minute), nil) + } + return got + } + if got := sample(0); len(got) != 0 { + t.Fatalf("control: an app whose update ended held must not be judged: %+v", got) + } + if !m.ForgetUpdateHeld("nextcloud") { + t.Fatal("the flag was set, so it must be forgotten") + } + got := sample(20) + if len(got) != 1 || got[0].Stack != "nextcloud" || got[0].Kind != UnhealthyCrashLoop { + t.Fatalf("after the restore lifted the hold the watcher must watch the app again; verdicts %+v", got) + } +} diff --git a/controller/internal/stacks/r1351_filebrowser_serial_test.go b/controller/internal/stacks/r1351_filebrowser_serial_test.go new file mode 100644 index 00000000..b25cfc75 --- /dev/null +++ b/controller/internal/stacks/r1351_filebrowser_serial_test.go @@ -0,0 +1,58 @@ +package stacks + +import ( + "io" + "log" + "os" + "path/filepath" + "sync" + "testing" + "time" +) + +// R-1351: at a guest boot EnsureBaseStack's `compose up` and web.SyncFileBrowserMounts' `compose up --force-recreate` ran +// in the same second; one lost with „Conflict. The container name … is already in use" and a false first-boot WARN. The +// two paths now share one lock: the base stack's bring-up holds it around its FileBrowser step, the web sync around its +// compose call (TestFileBrowserSyncRunsInsideTheStackLock pins that side, internal/web). +// +// RED-PROOF (2026-11-02): seen failing before the lock existed (build: WithFileBrowserStack); with ensureFileBrowser not +// taking the lock it fails on „overlap". +func TestR1351_TheTwoFileBrowserStartPathsNeverOverlap(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "docker-compose.yml"), []byte("services: {}\n"), 0o644); err != nil { + t.Fatal(err) + } + m := &Manager{logger: log.New(io.Discard, "", 0)} + var mu sync.Mutex + inside, overlaps := 0, 0 + enter := func() { + mu.Lock() + inside++ + if inside > 1 { + overlaps++ + } + mu.Unlock() + time.Sleep(30 * time.Millisecond) + mu.Lock() + inside-- + mu.Unlock() + } + m.fbComposeUpFn = func(string) error { enter(); return nil } + m.fbRunningFn = func() bool { return false } + + var wg sync.WaitGroup + for i := 0; i < 4; i++ { + wg.Add(2) + go func() { + defer wg.Done() + if err := m.ensureFileBrowser(dir); err != nil { + t.Error(err) + } + }() + go func() { defer wg.Done(); m.WithFileBrowserStack(enter) }() + } + wg.Wait() + if overlaps != 0 { + t.Fatalf("overlap: the two start paths ran compose at the same time %d time(s) — the „Conflict” of R-1351", overlaps) + } +} diff --git a/controller/internal/stacks/update.go b/controller/internal/stacks/update.go index 71a15a71..a51880e3 100644 --- a/controller/internal/stacks/update.go +++ b/controller/internal/stacks/update.go @@ -214,7 +214,7 @@ func (m *Manager) ClearUpdateState(name string) { // (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). +// Pinned by TestD64_ForgetUpdateHeld (this package) and TestD64_ClearHoldViaTheHub (cmd/controller). func (m *Manager) ForgetUpdateHeld(name string) bool { m.mu.Lock() defer m.mu.Unlock() diff --git a/controller/internal/web/filebrowser_gate_test.go b/controller/internal/web/filebrowser_gate_test.go index aa010356..8af2f51e 100644 --- a/controller/internal/web/filebrowser_gate_test.go +++ b/controller/internal/web/filebrowser_gate_test.go @@ -1,6 +1,14 @@ package web -import "testing" +import ( + "bytes" + "log" + "os" + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" +) // T4: skipFileBrowserPath skips ONLY an external drive path (under StableParentDir) that is not a live // mountpoint. A mounted external path and any system/local path are never skipped. @@ -49,3 +57,66 @@ func TestFbNeedsRecreate(t *testing.T) { } } } + +// R-1339: a per-app folder under a MOUNTED drive (/mnt/felhom-drives/x/userdata/) is never a mountpoint itself. It is +// shown through the drive's own bind, so it still gets no bind and no skeleton of its own (a skeleton there would write +// userdata/ inside the app's folder), and the log no longer says its drive is „not mounted". A drive that really is +// absent keeps its line, and a folder of such a drive names the absent drive. +// +// RED-PROOF (2026-11-02): before the fix the sub-folder logged „drive …/userdata/nextcloud not mounted" (nine lines at every +// start on 9202, scratch_hdd a live mountpoint) and this fails on that line. +func TestR1339_AFolderOfAMountedDriveIsNotReportedAsUnmounted(t *testing.T) { + sub := StableParentDir + "/scratch_hdd/userdata/nextcloud" + mounted := StableParentDir + "/scratch_hdd" + absent := StableParentDir + "/usb" + absentSub := absent + "/userdata/romm" + isMount := func(p string) bool { return p == mounted } + var buf bytes.Buffer + var skeletons []string + mounts, cfg := buildFileBrowserPaths([]settings.StoragePath{{Path: sub}, {Path: mounted}, {Path: absent}, {Path: absentSub}}, fbPathDeps{ + isMount: isMount, + ensureSkeleton: func(p string) error { skeletons = append(skeletons, p); return nil }, + logger: log.New(&buf, "", 0), + }) + logged := buf.String() + if strings.Contains(logged, sub) { + t.Errorf("a folder of a mounted drive is reported: %q", logged) + } + if !strings.Contains(logged, "drive "+absent+" not mounted") { + t.Errorf("a drive that is really absent keeps its line: %q", logged) + } + if !strings.Contains(logged, "drive "+absent+" not mounted") || strings.Count(logged, "not mounted") != 2 || !strings.Contains(logged, absentSub) { + t.Errorf("a folder of an absent drive is reported, naming that drive: %q", logged) + } + // The behaviour stays: only the mounted drive itself gets a skeleton and a bind. + if len(skeletons) != 1 || skeletons[0] != mounted { + t.Errorf("skeletons %v — only the mounted drive gets one", skeletons) + } + if len(mounts) != 1 || !strings.Contains(mounts[0], ":/srv/scratch_hdd") { + t.Errorf("mounts %v — only the mounted drive's userdata is bound", mounts) + } + if len(cfg) != 4 { + t.Errorf("the source list keeps every registered path: %v", cfg) + } +} + +// R-1351: the web sync's compose call runs inside the stack lock it shares with the base stack's bring-up. The lock's +// behaviour is TestR1351_TheTwoFileBrowserStartPathsNeverOverlap (internal/stacks); this pins the web side's use of it. +func TestR1351_TheWebSyncTakesTheLockBeforeCompose(t *testing.T) { + b, err := os.ReadFile("handlers.go") + if err != nil { + t.Fatal(err) + } + src := string(b) + start := strings.Index(src, "func (s *Server) syncFileBrowserMounts(") + if start < 0 { + t.Fatal("syncFileBrowserMounts not found") + } + body := src[start:] + lock := strings.Index(body, "s.stackMgr.LockFileBrowserStack()") + down := strings.Index(body, `"docker", "compose", "down", "-v"`) + up := strings.Index(body, `[]string{"compose", "up", "-d", "--remove-orphans"}`) + if lock < 0 || down < 0 || up < 0 || lock > down || lock > up { + t.Fatalf("the lock must come before both compose calls (lock %d, down %d, up %d)", lock, down, up) + } +} diff --git a/controller/internal/web/handlers.go b/controller/internal/web/handlers.go index 229b5b30..2ba140aa 100644 --- a/controller/internal/web/handlers.go +++ b/controller/internal/web/handlers.go @@ -3769,6 +3769,15 @@ func skipFileBrowserPath(path string, isMount func(string) bool) bool { return strings.HasPrefix(path, StableParentDir+"/") && !isMount(path) } +// fbDriveRoot is the drive a path under StableParentDir lives on (…/scratch_hdd/userdata/x → …/scratch_hdd). +func fbDriveRoot(path string) string { + rest := strings.TrimPrefix(path, StableParentDir+"/") + if i := strings.Index(rest, "/"); i >= 0 { + rest = rest[:i] + } + return StableParentDir + "/" + rest +} + func (s *Server) syncFileBrowserMounts(resetDBOnChange bool) { // R-695 (v0.275.0): SINGLE-FLIGHT. A request is covered by any sync that STARTS reading the state // after the request was made: a caller that waited behind a running sync returns as soon as one @@ -3917,6 +3926,13 @@ func (s *Server) syncFileBrowserMounts(resetDBOnChange bool) { } s.fbForceMu.Unlock() + // R-1351: the base stack's bring-up may run `compose up` on this same stack in the same second (a guest boot); the loser + // logged „Conflict. The container name … is already in use". One lock over both compose paths. + if s.stackMgr != nil { + unlock := s.stackMgr.LockFileBrowserStack() + defer unlock() + } + // If sources changed and caller requested a DB reset (restore flow), // nuke the data volume so FileBrowser re-reads config.yaml from scratch. // Normal operations skip this to preserve user accounts, permissions, and share links. @@ -4015,8 +4031,16 @@ func buildFileBrowserPaths(paths []settings.StoragePath, d fbPathDeps) (storageM // FileBrowser this pass. It returns on the next sync after reconnect. Matches planDriveGates' // external-only rule (system paths, not under StableParentDir, are never skipped). if skipFileBrowserPath(sp.Path, d.isMount) { + // R-1339: a per-app folder (/userdata/) is never a mountpoint itself. Under a MOUNTED drive it is shown + // through the drive's own bind, so it gets no bind of its own and there is nothing to report: the old „not mounted" + // line was false nine times at every start on a box whose drive worked. Pinned by TestR1339_*. if d.logger != nil { - d.logger.Printf("[INFO] [web] FileBrowser: drive %s not mounted — skipping userdata skeleton", sp.Path) + switch root := fbDriveRoot(sp.Path); { + case root == sp.Path: + d.logger.Printf("[INFO] [web] FileBrowser: drive %s not mounted — skipping userdata skeleton", sp.Path) + case !d.isMount(root): + d.logger.Printf("[INFO] [web] FileBrowser: drive %s not mounted — skipping its folder %s", root, sp.Path) + } } continue }