From 0b93e1ad1e52caa30b61b1f77dc38b828de42b52 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Mon, 5 Oct 2026 21:32:36 +0200 Subject: [PATCH] R-621: a held app's logs show the log the hold kept MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GetLogs (logs page, /api/stacks//logs, the operator's remote diagnostics) returned "" for a held app — no containers — while captureHoldLogs had kept the app's own log under hold-logs//compose-logs.txt. It now serves the newest non-empty kept log, under a header naming the file, when the live log is empty; a running app's own output always wins. GetLogs gains a test seam (logsComposeFn) so the test never reaches docker. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS --- controller/internal/stacks/manager.go | 42 ++++++++++++++- .../stacks/r621_hold_log_fallback_test.go | 53 +++++++++++++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 controller/internal/stacks/r621_hold_log_fallback_test.go diff --git a/controller/internal/stacks/manager.go b/controller/internal/stacks/manager.go index 6c45716..ab2496b 100644 --- a/controller/internal/stacks/manager.go +++ b/controller/internal/stacks/manager.go @@ -291,6 +291,8 @@ type Manager struct { // --- guarded update (slice 4, update.go) --- updateGuards UpdateGuards // init-only, SetUpdateGuards; nil ⇒ every update is REFUSED updateComposeFn func(dir string, env []string, args ...string) (string, error) + // logsComposeFn is GetLogs' compose seam (tests only; nil → the real compose). R-621. + logsComposeFn func(dir string, args ...string) (string, error) // composeDownFn is RecoverInterruptedInstalls' seam (R-681): nil → the real `compose down`. Tests set a // fake so they never reach real Docker (R-650). composeDownFn func(dir string) error @@ -1448,11 +1450,26 @@ func (m *Manager) GetLogs(name string, lines int) (string, error) { m.logger.Printf("[INFO] [stacks] Fetching logs for stack %s (tail=%d)", name, lines) dir := filepath.Dir(stack.ComposePath) - output, err := m.composeExec(dir, "logs", "--tail", fmt.Sprintf("%d", lines), "--no-color") + run := m.composeExec + if m.logsComposeFn != nil { + run = m.logsComposeFn + } + output, err := run(dir, "logs", "--tail", fmt.Sprintf("%d", lines), "--no-color") if err != nil { m.logger.Printf("[WARN] [stacks] Failed to fetch logs for %s: %v", name, err) return "", fmt.Errorf("getting logs for %s: %w", name, err) } + // R-621: a HELD app has no containers, so its live log is empty — but the hold kept the app's own + // log before the `down` (captureHoldLogs). Serve the newest kept log then, under a header naming + // the file, so the logs page, the API and the operator's remote diagnostics all show WHY the + // update failed instead of nothing. Only when the live log is empty: a running app's own output + // always wins. Pinned by TestR621_GetLogsFallsBackToTheHoldLog. + if strings.TrimSpace(output) == "" { + if kept, ts, ok := latestHoldLog(dir); ok { + m.logger.Printf("[INFO] [stacks] Logs for %s: no live output — serving the log kept at the update hold (%s, %d bytes)", name, ts, len(kept)) + return "--- hold-logs/" + ts + "/compose-logs.txt ---\n" + kept, nil + } + } if len(output) == 0 { m.logger.Printf("[DEBUG] Logs result for %s: 0 bytes returned (empty)", name) @@ -1462,6 +1479,29 @@ func (m *Manager) GetLogs(name string, lines int) (string, error) { return output, nil } +// latestHoldLog returns the newest non-empty hold-logs//compose-logs.txt under a stack dir. The +// directory names are UTC timestamps (20060102T150405Z), so the lexical order is the time order. +func latestHoldLog(dir string) (content, ts string, ok bool) { + entries, err := os.ReadDir(filepath.Join(dir, "hold-logs")) + if err != nil { + return "", "", false + } + names := make([]string, 0, len(entries)) + for _, e := range entries { + if e.IsDir() { + names = append(names, e.Name()) + } + } + sort.Sort(sort.Reverse(sort.StringSlice(names))) + for _, n := range names { + b, rerr := os.ReadFile(filepath.Join(dir, "hold-logs", n, "compose-logs.txt")) + if rerr == nil && strings.TrimSpace(string(b)) != "" { + return string(b), n, true + } + } + return "", "", false +} + // --- Env and compose helpers --- // stackEnv builds the full OS env slice for a stack, merging app.yaml values. diff --git a/controller/internal/stacks/r621_hold_log_fallback_test.go b/controller/internal/stacks/r621_hold_log_fallback_test.go new file mode 100644 index 0000000..c2d59d3 --- /dev/null +++ b/controller/internal/stacks/r621_hold_log_fallback_test.go @@ -0,0 +1,53 @@ +package stacks + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// R-621 (the open half): the hold keeps the app's own log, and the logs a person can READ — the logs +// page, /api/stacks//logs, the operator's remote diagnostics, all through GetLogs — now show it +// when the held app has no live output. Before this, GetLogs returned "" for a held app (no +// containers), so the kept file was reachable only by a shell on the box. +func TestR621_GetLogsFallsBackToTheHoldLog(t *testing.T) { + m, dir := newPinManager(t, pinTplOld, pinTplNew, "deployed: true\nenv: {}\n") + live := "" + m.logsComposeFn = func(string, ...string) (string, error) { return live, nil } + + // No hold log yet: an empty live log stays empty (nothing invented). + if got, err := m.GetLogs("nextcloud", 200); err != nil || got != "" { + t.Fatalf("no hold log: got %q, %v", got, err) + } + + for ts, body := range map[string]string{ + "20260921T211700Z": "web-1 | OLDER hold\n", + "20260922T080000Z": "web-1 | Applying migration 0009... OK\nweb-1 | never bound :8000\n", + "20260923T000000Z": " \n", // a newer, EMPTY capture must not hide the useful one + } { + p := filepath.Join(dir, "hold-logs", ts) + if err := os.MkdirAll(p, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(p, "compose-logs.txt"), []byte(body), 0o644); err != nil { + t.Fatal(err) + } + } + got, err := m.GetLogs("nextcloud", 200) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(got, "never bound :8000") || !strings.HasPrefix(got, "--- hold-logs/20260922T080000Z/compose-logs.txt ---\n") { + t.Fatalf("R-621: a held app's logs must show the newest non-empty kept log under its header, got %q", got) + } + if strings.Contains(got, "OLDER hold") { + t.Errorf("only the newest kept log is served, got %q", got) + } + + // A running app's own output always wins over a kept log. + live = "web-1 | serving\n" + if got, _ := m.GetLogs("nextcloud", 200); got != live { + t.Errorf("live output must win over the hold log, got %q", got) + } +}