From 7cba0bfca7f9fe6dda8669bcc466b63924b0e40f Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Mon, 28 Sep 2026 15:34:57 +0200 Subject: [PATCH] v0.278.0: a hold left by an earlier install no longer holds the new one (R-704) Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS --- CHANGELOG.md | 21 +++ REUSE.md | 1 + controller/README.md | 2 + controller/internal/api/kept_install.go | 1 + controller/internal/api/kept_install_test.go | 136 ++++++++++++++++++ controller/internal/api/router.go | 24 ++++ controller/internal/backup/kept_load.go | 2 +- .../internal/settings/r704_hold_test.go | 38 +++++ controller/internal/settings/settings.go | 13 +- 9 files changed, 232 insertions(+), 6 deletions(-) create mode 100644 controller/internal/settings/r704_hold_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 9117d67..1e05c30 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,24 @@ +## v0.278.0 — a hold left by an earlier install no longer holds the new one (R-704) (2026-09-28) + +**MinAgent: 0.131.0** (unchanged). Needs hub v0.123.0 (unchanged). New strings: none. Evidence: +`felhom.eu/documentation/audits/kept-offsite-2026-09-28/` (E2b, redproofs RP4–RP6), +`…/pg-calcom-claper-2026-09-28/box/calcom/hold.txt`. + +- **Found live 2026-09-28.** demo-hp's fresh nextcloud (installed 10:13) carried an update hold from a nextcloud of + 2026-09-13 — set before v0.242.0, whose removal did not clear it. The backup leg then skipped the new install's + volumes and unit ("the app is HELD stopped"), so its off-site snapshot held no data, and its page showed a two-week-old + failure. On 9202 a calcom crash-loop stop (decision 28) survived two removes and refused the new install's update. +- **Removal** (`settings.ClearUpdateHold`) now clears the crash-loop stop too, not only the update hold. +- **A new install** — a plain install or "use my kept data" — drops a leftover update or crash-loop hold of an app that + is not installed (`Router.dropLeftoverHold`), which also heals holds already left on boxes. A restore hold (R-379) + is never touched by either: it stays operator-cleared. +- The kept load's unit restore now goes through the same seam as the off-site load (`keptUnitRestore`). +- Tests: `TestR704_ClearUpdateHoldClearsTheInstallsHoldsOnly`, `TestR704_AFreshInstallDropsALeftoverHold` (the "use" + path end to end behind seams, the R-379 negative, and the plain install's call before `DeployStack`). Red-proofs + RP4–RP6, each seen failing. +- **Second controller release this session** (the rule is one): R-704 blocked the brief's off-site proof on demo-hp and + there is no product route to clear the leftover hold. + ## v0.277.0 — „Use my kept data" and Load also bring the database back from the off-site copy (R-691 (2)) (2026-09-28) **MinAgent: 0.131.0** (unchanged). Needs hub v0.123.0 (unchanged). New strings: `kept.backup.offsite`, diff --git a/REUSE.md b/REUSE.md index cb9363e..e901493 100644 --- a/REUSE.md +++ b/REUSE.md @@ -25,6 +25,7 @@ | `offsiteRestoreRootFor` | controller/internal/backup/offbox_verify_copies.go | `(drivePath string) string` | THE only place `backups/offsite-restore` is spelled | `offboxRestoreScratchDir` builds on it — the listing/delete surface MUST resolve byte-identical paths to what the restore wrote. Do not re-hardcode the segments (they were open-coded in 3 places before v0.147.0) | | `ProtectedHDDPaths` | controller/internal/stacks/delete.go | `(hddPath string) map[string]bool` | Never-delete set (root, appdata, backups, media, kept, legacy felhom-data) | Consult before ANY recursive delete under a drive | | `stacks.OldAppDataPaths` / `Manager.ListKept` / `KeepAside` / `DeleteKept` / `FindKept` | controller/internal/stacks/kept.go | `(composePath, hdd)` / `(drives)` / … | Kept data (`09` §3 decision 36): what counts as an app's old data (ONLY `/appdata/…` binds), the list, start-fresh, the household's delete | **An action names a kept item by path only through `FindKept`** — `DeleteKept` refuses anything not listed. `KeepAside` is a rename on one drive; never copy, never `RemoveAll` in a rollback (`removeEmptyDirs`) | +| `Router.dropLeftoverHold` + `settings.ClearUpdateHold` (R-704, v0.278.0) | controller/internal/api/router.go · controller/internal/settings/settings.go | `(name, why)` / `(stack) (bool, error)` | A new install (plain or "use my kept data") and a removal clear the update / crash-loop hold of the app's install | **A hold belongs to an INSTALL; the name is all the next install shares with it.** Never clears an R-379 restore hold (operator-only) | | `backup.KeptBestCopy` / `KeptDBCopy` / `KeptOffsiteCopies` / `KeptCopyAt` / `LoadKeptApp` / `LoadKeptOffsite` / `KeptCopyKey` | controller/internal/backup/kept_load.go | `(ctx, app, drive)` / `(app, drive)` / `(ctx, apps)` / `(unitDir, drive, tier)` / … | Which copy can load kept files (local tiers + since v0.277.0 the off-site copy, R-691 (2)), the load as a restore op, the copy's name for the page | A copy counts only with data (DB dump or volume tar) AND its app.yaml `HDD_PATH` = this drive. Installed apps are never offered. **The off-site copy is judged only after its unit is downloaded** — `LoadKeptOffsite` refuses an unversioned unit (`07` §6.6) BEFORE `prepare` (a dated folder's move-back); ask the repository once per page (`KeptOffsiteCopies`), never per row. Both pages name a copy through `KeptCopyKey` | ### Subprocess + timeout + exit-code discipline diff --git a/controller/README.md b/controller/README.md index a47e33d..1f239a8 100644 --- a/controller/README.md +++ b/controller/README.md @@ -1916,6 +1916,8 @@ that folder is never a dead end, and an install never runs into it silently (R-6 - **Where it lives.** `/kept/` is beside `appdata/` and `userdata/`, inside neither: no app bind, FileBrowser userdata source, Samba share or backup leg reads it. It is in `ProtectedHDDPaths`. - **The drive-full warning** ends with the kept folders on that drive and their sizes (`fillwatch.SetExtra`). +- **R-704 (v0.278.0).** A new install (plain or "use my kept data") drops an update or crash-loop hold left by an EARLIER + install of the app, and a removal clears both kinds; a restore hold (R-379) stays operator-cleared. - **R-690 (fixed here).** The removed-app restore (R-487) never found a unit on a DATA drive — it asked `GetStackComposePath`, true for every catalog app — and restored with no env. It now asks `isStackDeployed`. - `.felhom.yml` **`after_load: {service, user, command: [...]}`** — one command run with `docker compose exec -T` diff --git a/controller/internal/api/kept_install.go b/controller/internal/api/kept_install.go index 3704974..01a8e38 100644 --- a/controller/internal/api/kept_install.go +++ b/controller/internal/api/kept_install.go @@ -95,6 +95,7 @@ func (r *Router) keptDataAtInstall(w http.ResponseWriter, req *http.Request, nam src = "off-site snapshot " + cp.c.SnapshotID } r.logger.Printf("[INFO] [api] Deploy %s: USE MY KEPT DATA — loading from %s (tier %d, %s) under the kept files %v", name, src, cp.c.Tier, cp.c.Time.UTC().Format(time.RFC3339), old) + r.dropLeftoverHold(name, "use-kept-data") // R-704: this is a new install of an app that is not installed r.startKeptLoad(name, hdd, cp.c, lang, nil) writeJSON(w, http.StatusAccepted, apiResponse{OK: true, Message: r.msgLang(lang, "kept.load.started", display)}) return true diff --git a/controller/internal/api/kept_install_test.go b/controller/internal/api/kept_install_test.go index db87658..e79168a 100644 --- a/controller/internal/api/kept_install_test.go +++ b/controller/internal/api/kept_install_test.go @@ -3,12 +3,16 @@ package api import ( "context" "encoding/json" + "go/ast" + "go/parser" + "go/token" "io" "log" "net/http" "net/http/httptest" "os" "path/filepath" + "strings" "testing" "time" @@ -151,3 +155,135 @@ func TestR691_InstallChoiceNamesTheOffsiteCopy(t *testing.T) { t.Fatal("something was installed") } } + +// R-704 (v0.278.0) — a hold an EARLIER install left under the app's name is dropped when a new install +// begins; the consequence measured on demo-hp was a fresh nextcloud whose backups skipped its volumes and +// unit because of a 2026-09-13 update hold. Driven through the real „use my kept data" path (the off-site +// copy, restic and the unit restore behind their seams — no Docker). +// COMPANION RED-PROOF: remove the dropLeftoverHold call from keptDataAtInstall → the hold is still there +// after the 202 and the first assertion fails; remove it from deployStack → the AST check fails. +func TestR704_AFreshInstallDropsALeftoverHold(t *testing.T) { + dir := t.TempDir() + drive := filepath.Join(dir, "drive") + cfg := &config.Config{} + cfg.Paths.StacksDir = filepath.Join(dir, "stacks") + cfg.Paths.DataDir = filepath.Join(dir, "data") + cfg.Stacks.ComposeCommand = "docker compose" + app := filepath.Join(cfg.Paths.StacksDir, "cloudapp") + for _, d := range []string{app, filepath.Join(drive, "appdata/cloudapp"), cfg.Paths.DataDir} { + if err := os.MkdirAll(d, 0o755); err != nil { + t.Fatal(err) + } + } + _ = os.WriteFile(filepath.Join(app, "docker-compose.yml"), []byte("services:\n cloudapp:\n image: busybox:1\n volumes:\n - ${HDD_PATH}/appdata/cloudapp:/data\n"), 0o644) + _ = os.WriteFile(filepath.Join(app, ".felhom.yml"), []byte("display_name: Cloud App\n"), 0o644) + _ = os.WriteFile(filepath.Join(drive, "appdata/cloudapp/old.txt"), []byte("old"), 0o644) + m, err := stacks.NewManager(cfg, log.New(io.Discard, "", 0)) + if err != nil { + t.Fatal(err) + } + if err := m.ScanStacks(); err != nil { + t.Fatal(err) + } + sett, err := settings.Load(filepath.Join(cfg.Paths.DataDir, "settings.json"), log.New(io.Discard, "", 0)) + if err != nil { + t.Fatal(err) + } + _ = sett.SetOffboxTarget(&settings.OffboxTarget{Enabled: true, Host: "nas.local", Port: 22, User: "felhom", + RepoPath: "/srv/repo", Schedule: "daily", EscrowState: "escrowed"}) + if err := sett.AddStoragePath(settings.StoragePath{Path: drive, Label: "HDD", Schedulable: true}); err != nil { + t.Fatal(err) + } + bm := backup.NewManager(cfg, sett, log.New(io.Discard, "", 0)) + if err := bm.WriteOffboxSecrets("KEY", "nas.local ssh-ed25519 AAAA"); err != nil { + t.Fatal(err) + } + bm.SetOffboxFreeFn(func(string) int64 { return 100 << 30 }) + unitPath := drive + "/backups/primary/cloudapp" + bm.SetOffboxRunner(func(_ context.Context, _ []string, args ...string) ([]byte, error) { + var target string + isRestore := false + for i, a := range args { + if a == "snapshots" { + return json.Marshal([]map[string]interface{}{{"short_id": "ab12cd34", "time": time.Now().Add(-time.Hour), + "tags": []string{"cloudapp"}, "paths": []string{unitPath}}}) + } + if a == "restore" { + isRestore = true + } + if a == "--target" && i+1 < len(args) { + target = args[i+1] + } + } + if isRestore && target != "" { + u := filepath.Join(target, strings.TrimPrefix(unitPath, "/")) + _ = os.MkdirAll(filepath.Join(u, "compose"), 0o755) + _ = os.MkdirAll(filepath.Join(u, "db-dumps"), 0o755) + _ = os.WriteFile(filepath.Join(u, "db-dumps", "cloudapp.sql"), []byte("x"), 0o644) + _ = os.WriteFile(filepath.Join(u, "compose", "docker-compose.yml"), []byte("services:\n cloudapp:\n image: busybox:1\n"), 0o644) + _ = os.WriteFile(filepath.Join(u, "compose", "app.yaml"), []byte("env:\n HDD_PATH: "+drive+"\n"), 0o600) + man, _ := json.Marshal(map[string]interface{}{"schema_version": 2, "app_name": "cloudapp", "db_dumps": []string{"cloudapp.sql"}, + "data": map[string]interface{}{"at": time.Now().UTC().Format(time.RFC3339), "image_pins": []string{"busybox:1"}}}) + _ = os.WriteFile(filepath.Join(u, "manifest.json"), man, 0o644) + } + return nil, nil + }) + done := make(chan struct{}, 1) + bm.SetKeptUnitRestoreFn(func(app, unitDir string) (backup.UnitRestoreResult, error) { + done <- struct{}{} + return backup.UnitRestoreResult{}, nil + }) + r := &Router{stackMgr: m, backupMgr: bm, sett: sett, logger: log.New(io.Discard, "", 0)} + + // A leftover update hold from an earlier install (the demo-hp shape): the new install drops it. + _ = sett.SetRestoreHold(settings.RestoreHold{Stack: "cloudapp", At: "2026-09-13T19:56:32Z", Reason: settings.HoldReasonUpdateFailed}) + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/api/stacks/cloudapp/deploy?lang=en", nil) + if !r.keptDataAtInstall(w, req, "cloudapp", drive, "use") || w.Code != http.StatusAccepted { + t.Fatalf("use: code %d body %s", w.Code, w.Body.String()) + } + if h, held := sett.GetRestoreHold("cloudapp"); held { + t.Fatalf("the earlier install's hold survived the new install: %+v", h) + } + select { + case <-done: + case <-time.After(10 * time.Second): + t.Fatal("the load never reached the unit restore") + } + + // A restore hold (R-379) is NEVER dropped by an install. + _ = sett.SetRestoreHold(settings.RestoreHold{Stack: "cloudapp", At: "2026-09-13T19:56:32Z", Reason: settings.HoldReasonRestoreFailed}) + r.dropLeftoverHold("cloudapp", "install") + if _, held := sett.GetRestoreHold("cloudapp"); !held { + t.Fatal("an install dropped an R-379 restore hold") + } + _, _ = sett.ClearRestoreHold("cloudapp") + + // The plain install path drops it too — before DeployStack (AST: the call precedes it in deployStack). + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, "router.go", nil, 0) + if err != nil { + t.Fatal(err) + } + var dropAt, deployAt token.Pos + for _, d := range f.Decls { + fn, ok := d.(*ast.FuncDecl) + if !ok || fn.Name.Name != "deployStack" || fn.Body == nil { + continue + } + ast.Inspect(fn.Body, func(n ast.Node) bool { + if s, ok := n.(*ast.SelectorExpr); ok { + switch s.Sel.Name { + case "dropLeftoverHold": + dropAt = s.Pos() + case "DeployStack": + deployAt = s.Pos() + } + } + return true + }) + } + if dropAt == token.NoPos || deployAt == token.NoPos || dropAt > deployAt { + t.Fatalf("deployStack must call dropLeftoverHold before DeployStack (drop=%v deploy=%v)", dropAt, deployAt) + } +} diff --git a/controller/internal/api/router.go b/controller/internal/api/router.go index 30e24ca..e810e64 100644 --- a/controller/internal/api/router.go +++ b/controller/internal/api/router.go @@ -496,6 +496,9 @@ func (r *Router) deployStack(w http.ResponseWriter, req *http.Request, name stri return } + // R-704 (v0.278.0): a fresh install drops a hold an EARLIER install left under this name. + r.dropLeftoverHold(name, "install") + deployReq := stacks.DeployRequest{ StackName: name, Values: body.Values, @@ -905,6 +908,27 @@ func (r *Router) getStackBackupData(w http.ResponseWriter, req *http.Request, na writeJSON(w, http.StatusOK, apiResponse{OK: true, Data: resp}) } +// dropLeftoverHold (R-704, v0.278.0) clears the update or crash-loop hold of an app that is NOT installed, +// before a new install of it begins (a plain install, or „use my kept data"). Such a hold can only have +// been left by an earlier install: before v0.242.0 a removal kept update holds, and before v0.278.0 it +// kept crash-loop stops — measured 2026-09-28: a nextcloud update hold from 2026-09-13 on demo-hp made +// the backup skip the FRESH install's volumes and unit, so its off-site copy held no data; on 9202 a +// calcom crash-loop stop refused the fresh install's update. A restore hold (R-379) is never touched. +// Pinned by TestR704_AFreshInstallDropsALeftoverHold. +func (r *Router) dropLeftoverHold(name, why string) { + if r.sett == nil { + return + } + if st, ok := r.stackMgr.GetStack(name); ok && (st.Deployed || st.Deploying) { + return + } + if cleared, err := r.sett.ClearUpdateHold(name); err != nil { + r.logger.Printf("[WARN] [api] %s %s: the hold an earlier install left could not be cleared: %v", why, name, err) + } else if cleared { + r.logger.Printf("[INFO] [api] %s %s: a hold left by an EARLIER install of this app is dropped (R-704)", why, name) + } +} + func (r *Router) removeStack(w http.ResponseWriter, req *http.Request, name string) { if name == "" { writeJSON(w, http.StatusBadRequest, apiResponse{OK: false, Error: "invalid stack name"}) diff --git a/controller/internal/backup/kept_load.go b/controller/internal/backup/kept_load.go index 2668dee..f63c45e 100644 --- a/controller/internal/backup/kept_load.go +++ b/controller/internal/backup/kept_load.go @@ -319,7 +319,7 @@ func (m *Manager) LoadKeptApp(app, unitDir string, okMsg, failMsg func(err error m.BeginRestoreOp("restore", app) go func() { start := time.Now() - res, err := m.RestoreFromRecoveryUnitAt(app, unitDir) + res, err := m.keptUnitRestore()(app, unitDir) if err != nil { m.logger.Printf("[ERROR] [backup] kept load %s from %s FAILED after %s: %v", app, unitDir, time.Since(start).Round(time.Second), err) m.EndRestoreOp(false, failMsg(err)) diff --git a/controller/internal/settings/r704_hold_test.go b/controller/internal/settings/r704_hold_test.go new file mode 100644 index 0000000..b4ad6e2 --- /dev/null +++ b/controller/internal/settings/r704_hold_test.go @@ -0,0 +1,38 @@ +package settings + +import ( + "io" + "log" + "path/filepath" + "testing" +) + +// R-704 (v0.278.0) — ClearUpdateHold clears the holds that belong to an INSTALL (update, crash-loop stop) and +// never a restore hold (R-379), which stays operator-cleared. +// COMPANION RED-PROOF: the pre-fix predicate (`h.Reason != HoldReasonUpdateFailed`) → the crash-loop case fails. +func TestR704_ClearUpdateHoldClearsTheInstallsHoldsOnly(t *testing.T) { + s, err := Load(filepath.Join(t.TempDir(), "settings.json"), log.New(io.Discard, "", 0)) + if err != nil { + t.Fatal(err) + } + for _, c := range []struct { + reason string + want bool + }{{HoldReasonUpdateFailed, true}, {HoldReasonUnhealthyStop, true}, {HoldReasonRestoreFailed, false}} { + if err := s.SetRestoreHold(RestoreHold{Stack: "app", At: "2026-09-13T19:56:32Z", Reason: c.reason}); err != nil { + t.Fatal(err) + } + cleared, err := s.ClearUpdateHold("app") + if err != nil { + t.Fatal(err) + } + _, still := s.GetRestoreHold("app") + if cleared != c.want || still == c.want { + t.Fatalf("reason %q: cleared=%v still-held=%v, want cleared=%v", c.reason, cleared, still, c.want) + } + _, _ = s.ClearRestoreHold("app") + } + if cleared, _ := s.ClearUpdateHold("none"); cleared { + t.Fatal("clearing an app with no hold reported a clear") + } +} diff --git a/controller/internal/settings/settings.go b/controller/internal/settings/settings.go index 59c661e..03f976a 100644 --- a/controller/internal/settings/settings.go +++ b/controller/internal/settings/settings.go @@ -1917,19 +1917,22 @@ func (s *Settings) ClearRestoreHold(stack string) (bool, error) { return true, s.save() } -// ClearUpdateHold (R-491, v0.242.0) removes an app's hold ONLY when it is an update hold. An R-379 -// restore hold stays: that one means a database was left in an unknown state and is cleared by the -// operator (`-clear-restore-hold`), never by a removal. Returns whether an update hold was cleared. +// ClearUpdateHold (R-491, v0.242.0) removes an app's hold when it belongs to the app's INSTALL: an update +// hold, and since v0.278.0 (R-704) the box's crash-loop/OOM stop (HoldReasonUnhealthyStop) — both name an +// install that a removal ends, and the name is all a later install shares with it. An R-379 restore hold +// stays: that one means a database was left in an unknown state and is cleared by the operator +// (`-clear-restore-hold`), never by a removal or an install. Returns whether a hold was cleared. +// Pinned by TestR704_ClearUpdateHoldClearsTheInstallsHoldsOnly. func (s *Settings) ClearUpdateHold(stack string) (bool, error) { s.mu.Lock() defer s.mu.Unlock() h, ok := s.RestoreHolds[stack] - if !ok || h.Reason != HoldReasonUpdateFailed { + if !ok || (h.Reason != HoldReasonUpdateFailed && h.Reason != HoldReasonUnhealthyStop) { return false, nil } delete(s.RestoreHolds, stack) if s.log != nil { - s.log.Printf("[INFO] [settings] update hold CLEARED for %s (the app was removed)", stack) + s.log.Printf("[INFO] [settings] %s hold CLEARED for %s (set %s; its install is gone)", h.Reason, stack, h.At) } return true, s.save() }