From eed23e7594202eddc26b5e68566cc405fa151202 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Wed, 28 Oct 2026 23:22:46 +0100 Subject: [PATCH] R-528: the 30-s OOM scan also reads the host's per-container kernel counter (agent GET /guest/oom-kills) A count above 0 in a running container's current run fires the existing app_oom line (source cgroup/docker in the details), whatever Docker's OOMKilled says. Old agent: 404 back-off; no agent: Docker's flag only. Tests TestR528_* red first / red-proved. Commit only. --- CHANGELOG.md | 9 ++ controller/cmd/controller/hostoom_wiring.go | 50 +++++++++++ controller/cmd/controller/main.go | 7 +- .../controller/r528_hostoom_wiring_test.go | 60 +++++++++++++ controller/internal/agentapi/guestoom.go | 31 +++++++ controller/internal/notify/notifier.go | 10 ++- controller/internal/stacks/manager.go | 2 + controller/internal/stacks/oom.go | 51 ++++++++++- .../internal/stacks/r528_host_oom_test.go | 84 +++++++++++++++++++ 9 files changed, 297 insertions(+), 7 deletions(-) create mode 100644 controller/cmd/controller/hostoom_wiring.go create mode 100644 controller/cmd/controller/r528_hostoom_wiring_test.go create mode 100644 controller/internal/agentapi/guestoom.go create mode 100644 controller/internal/stacks/r528_host_oom_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index b62beb47..058f9a9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,15 @@ night report (`audits/night-2026-10-28/`). `api.escrow.confirm_store_unknown` („Nem tudjuk biztosan… Őrizd meg mindkét kódot…", hu + en) and never promises the previous code. `TestR1151_ConfirmAnswersInTheBoxLanguage`'s status-0 case changes with it, on purpose. Tests `TestR1254_*` (agentapi, web), red before the fix. +- **R-528 — the OOM line sees a kill Docker does not report (felhom.eu R-528; agent `GET /guest/oom-kills`).** The + 30-s dead-app scan (`stacks.ScanOOMKilled`) now also asks the host's agent once per scan for the kernel's per-container + `oom_kill` counter (`agentapi.GuestOOMKills`, wired in `cmd/controller/hostoom_wiring.go`, 5 s cap). A running + container whose current run has a count above 0 is an OOM kill whatever Docker's `OOMKilled` says: the existing + `app_oom` line (once per run, R-514/R-636) fires, its details carry `source` (`docker`, `cgroup` or `docker,cgroup`), + and the host's count is the run's kill count. An agent without the route (404) is asked again only after 10 minutes + and said once; no agent (9202) = Docker's flag only, as before. The inspect now reads `{{.Id}}` too. Tests + `TestR528_*` (stacks, cmd/controller), red before the fix (the host-counted kill was not seen) or red-proved by + mutation (the back-off, the wiring). - **H8 — `internal/backup/restore_copy_sweep.go`:** an unused restore copy goes after 14 days. Every 6 hours (`restore-copy-sweep`) a copy is removed only when the app has no restore hold of any kind, the app-stop marker is readable and does not name the app, no restore runs, and the copy is older than 14 days. Tests `TestR1256_H8_*` diff --git a/controller/cmd/controller/hostoom_wiring.go b/controller/cmd/controller/hostoom_wiring.go new file mode 100644 index 00000000..5a1c0a26 --- /dev/null +++ b/controller/cmd/controller/hostoom_wiring.go @@ -0,0 +1,50 @@ +package main + +import ( + "context" + "errors" + "log" + "net/http" + "sync" + "time" + + "gitea.dooplex.hu/admin/felhom-controller/internal/agentapi" +) + +// hostOOMSource (R-528) is the 30-s dead-app scan's host-side OOM counter: one agent call per scan (5 s cap). An agent +// without the route (404) is not asked again for hostOOMRetryAfter, and said once; any other failure is a nil answer +// for this scan only (Docker's flag still works). Pinned by TestR528_HostOOMSource*. +const hostOOMRetryAfter = 10 * time.Minute + +func hostOOMSource(ask func(context.Context) (agentapi.GuestOOMKillsState, error), logger *log.Logger, now func() time.Time) func() map[string]int64 { + var mu sync.Mutex + var until time.Time + said := false + return func() map[string]int64 { + mu.Lock() + defer mu.Unlock() + if now().Before(until) { + return nil + } + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + st, err := ask(ctx) + if err != nil { + var se *agentapi.StatusError + if errors.As(err, &se) && se.Code == http.StatusNotFound { + until = now().Add(hostOOMRetryAfter) + if !said { + logger.Printf("[INFO] [deadapp] the agent has no OOM counter route (R-528) — Docker's OOM flag only; asked again in %s", hostOOMRetryAfter) + said = true + } + return nil + } + logger.Printf("[WARN] [deadapp] the host OOM counter could not be read: %v — Docker's OOM flag only for this scan", err) + return nil + } + if !st.Available { + return nil + } + return st.Containers + } +} diff --git a/controller/cmd/controller/main.go b/controller/cmd/controller/main.go index 00b6b526..fff9a610 100644 --- a/controller/cmd/controller/main.go +++ b/controller/cmd/controller/main.go @@ -1035,6 +1035,9 @@ func main() { logger.Printf("[WARN] [deadapp] agent client init failed (%v) — the boot grace cannot learn of a crash boot; 90 s", err) } else { crashProbe = crashboot.AgentProbe(ac.CrashGuard) + // R-528: the 30-s scan also asks the host for the kernel's per-container OOM counter (Docker's own flag can + // stay false inside an LXC guest). TestR528_HostOOMSourceIsWiredAtStartup. + stackMgr.SetHostOOMKills(hostOOMSource(ac.GuestOOMKills, logger, time.Now)) } } bootGrace := crashboot.New(startTime, crashProbe, logger) @@ -1094,8 +1097,8 @@ func main() { logger.Printf("[WARN] [deadapp] OOM scan failed: %v", oerr) } else { for _, o := range ooms { - logger.Printf("[WARN] [deadapp] %s: container %s was OOM-killed (started %s; kills %d, limit %s, peak %s)", o.Stack, o.Container, o.StartedAt, o.Kills, o.MemLimit, o.Peak) - notifier.NotifyAppOOM(o.Stack, o.Container, o.StartedAt, o.Kills, o.MemLimit, o.Peak) + 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) + notifier.NotifyAppOOMFrom(o.Stack, o.Container, o.StartedAt, o.Kills, o.MemLimit, o.Peak, o.Source) } } // v0.269.0 (`09` §3 decision 28, R-667): a crash loop or an OOM storm is STOPPED by the box. diff --git a/controller/cmd/controller/r528_hostoom_wiring_test.go b/controller/cmd/controller/r528_hostoom_wiring_test.go new file mode 100644 index 00000000..7718b3e3 --- /dev/null +++ b/controller/cmd/controller/r528_hostoom_wiring_test.go @@ -0,0 +1,60 @@ +package main + +import ( + "context" + "errors" + "io" + "log" + "net/http" + "testing" + "time" + + "gitea.dooplex.hu/admin/felhom-controller/internal/agentapi" +) + +// R-528: the scan's host counter source. An old agent (404) is asked again only after hostOOMRetryAfter; a transient +// failure is retried at the next scan; the counters pass through. +// RED-PROOF: drop the 404 back-off → an old agent is asked at every 30-s scan (calls 3, not 1) → FAILS. +func TestR528_HostOOMSourceBacksOffAnOldAgent(t *testing.T) { + clock := time.Date(2026, 10, 28, 23, 0, 0, 0, time.UTC) + calls := 0 + var answer error = &agentapi.StatusError{Path: "/guest/oom-kills", Code: http.StatusNotFound} + src := hostOOMSource(func(context.Context) (agentapi.GuestOOMKillsState, error) { + calls++ + if answer != nil { + return agentapi.GuestOOMKillsState{}, answer + } + return agentapi.GuestOOMKillsState{Available: true, Containers: map[string]int64{"id": 2}}, nil + }, log.New(io.Discard, "", 0), func() time.Time { return clock }) + for i := 0; i < 3; i++ { + if got := src(); got != nil { + t.Fatalf("an old agent: %v", got) + } + clock = clock.Add(30 * time.Second) + } + if calls != 1 { + t.Fatalf("an old agent was asked %d times in 90 s, want 1", calls) + } + clock = clock.Add(hostOOMRetryAfter) + answer = nil + if got := src(); got["id"] != 2 || calls != 2 { + t.Fatalf("after the back-off, an upgraded agent: %v (calls %d)", got, calls) + } + answer = errors.New("timeout") + if got := src(); got != nil || calls != 3 { + t.Fatalf("a transient failure: %v", got) + } + answer = nil + if got := src(); got["id"] != 2 || calls != 4 { + t.Fatalf("the next scan after a transient failure must ask again: %v (calls %d)", got, calls) + } +} + +// The source is wired into the stack manager at startup (the seam-built-but-never-wired class). +// RED-PROOF: delete the SetHostOOMKills line in main.go → FAILS. +func TestR528_HostOOMSourceIsWiredAtStartup(t *testing.T) { + lines, _, _ := slice4CallLines(t) + if len(lines["SetHostOOMKills"]) == 0 || len(lines["hostOOMSource"]) == 0 { + t.Fatal("the host OOM counter is never wired — the 30-s scan would read Docker's flag only (R-528)") + } +} diff --git a/controller/internal/agentapi/guestoom.go b/controller/internal/agentapi/guestoom.go new file mode 100644 index 00000000..e3d66842 --- /dev/null +++ b/controller/internal/agentapi/guestoom.go @@ -0,0 +1,31 @@ +package agentapi + +import ( + "context" + "encoding/json" + "fmt" +) + +// GuestOOMKillsState mirrors the agent's GET /guest/oom-kills (R-528, agent ≥ the R-528 commit): the kernel's oom_kill +// counter of each of THIS guest's containers, read on the host — Docker's own OOMKilled flag can stay false inside an +// LXC guest. Containers maps the full 64-hex container id to the count for the container's current run. +// Available=false: the guest has no cgroup folder the host can read. An agent that predates the route answers 404 (a +// *StatusError) — the controller then keeps Docker's flag only. +type GuestOOMKillsState struct { + VMID int `json:"vmid"` + Available bool `json:"available"` + Containers map[string]int64 `json:"containers"` +} + +// GuestOOMKills calls GET /guest/oom-kills. +func (c *Client) GuestOOMKills(ctx context.Context) (GuestOOMKillsState, error) { + var out GuestOOMKillsState + body, err := c.get(ctx, "/guest/oom-kills") + if err != nil { + return out, err + } + if err := json.Unmarshal(body, &out); err != nil { + return out, fmt.Errorf("agentapi: decode /guest/oom-kills: %w", err) + } + return out, nil +} diff --git a/controller/internal/notify/notifier.go b/controller/internal/notify/notifier.go index 10efc0ed..af099677 100644 --- a/controller/internal/notify/notifier.go +++ b/controller/internal/notify/notifier.go @@ -163,6 +163,8 @@ type UpdateDetails struct { type AppDetails struct { StackName string `json:"stack_name,omitempty"` DisplayName string `json:"display_name,omitempty"` + // Source (app_oom only, R-528): what saw the kill — "docker" (State.OOMKilled) or "cgroup" (the host's kernel counter). + Source string `json:"source,omitempty"` } // CrossDriveDetails holds structured data for cross-drive backup events. @@ -804,6 +806,12 @@ func (n *Notifier) NotifyAppStartFailures(apps []AppRunState) { // that stays OOM-marked does not repeat. "warning" — the hub vocabulary (R-329). Operator-only // hub-side (hub >= v0.114.0 registers it): the household sees the dashboard tag. func (n *Notifier) NotifyAppOOM(stack, container, startedAt string, kills int64, memLimit, peak string) { + n.NotifyAppOOMFrom(stack, container, startedAt, kills, memLimit, peak, "") +} + +// NotifyAppOOMFrom is NotifyAppOOM with the source that saw the kill (R-528: "docker" or "cgroup"), carried in the +// event's details. One event per container run, whichever source saw it. +func (n *Notifier) NotifyAppOOMFrom(stack, container, startedAt string, kills int64, memLimit, peak, source string) { key := container + "|" + startedAt n.mu.Lock() if n.oomSeen == nil { @@ -816,7 +824,7 @@ func (n *Notifier) NotifyAppOOM(stack, container, startedAt string, kills int64, if first { n.emit("app_oom", "warning", fmt.Sprintf("Alkalmazás memóriája elfogyott: %s (%s) — egy folyamatát a memóriakorlát leállította", stack, container), - AppDetails{StackName: stack, DisplayName: container}) + AppDetails{StackName: stack, DisplayName: container, Source: source}) } if storm { n.logger.Printf("[ERROR] [notify] %s: container %s OOM STORM — %d kills in %d min (limit %s, peak %s)", diff --git a/controller/internal/stacks/manager.go b/controller/internal/stacks/manager.go index fb9843d3..51cc8a03 100644 --- a/controller/internal/stacks/manager.go +++ b/controller/internal/stacks/manager.go @@ -284,6 +284,8 @@ type Manager struct { // R-514: last OOM scan (oom.go), for the dashboard. oomMu sync.Mutex oomCache map[string][]string + // hostOOMKills (R-528) is the host agent's per-container OOM counter; nil = Docker's flag only (SetHostOOMKills). + hostOOMKills func() map[string]int64 // R-513: FileBrowser admin-password seams (filebrowser_password.go); nil/"" in production. fbHTTP fbHTTPDo fbBaseURL string diff --git a/controller/internal/stacks/oom.go b/controller/internal/stacks/oom.go index d56a6c5b..18ea1bac 100644 --- a/controller/internal/stacks/oom.go +++ b/controller/internal/stacks/oom.go @@ -27,6 +27,21 @@ type OOMContainer struct { Kills int64 // MemLimit / Peak are the cgroup's memory.max and memory.peak, as "M" (or "max"); "" unread. MemLimit, Peak string + // Source names what flagged it: OOMSourceDocker (State.OOMKilled), OOMSourceCgroup (R-528 — only the host's + // per-container kernel counter saw it; Docker's flag stayed false), or "docker,cgroup" (both). + Source string +} + +const ( + OOMSourceDocker = "docker" + OOMSourceCgroup = "cgroup" +) + +// SetHostOOMKills wires the host-side OOM counter (R-528): container id → the kernel's oom_kill for the container's +// CURRENT run (its cgroup scope is new at each start), read by the host's agent. nil, or a nil answer (no agent, an +// agent without the route, a failed call) = Docker's flag only, as before. INIT-ONLY (main.go). +func (m *Manager) SetHostOOMKills(fn func() map[string]int64) { + m.hostOOMKills = fn } // ScanOOMKilled inspects the containers of every deployed, running-ish stack in ONE docker call and @@ -52,20 +67,45 @@ func (m *Manager) ScanOOMKilled() ([]OOMContainer, error) { return nil, nil } sort.Strings(names) - args := append([]string{"inspect", "-f", "{{.Name}}|{{.State.OOMKilled}}|{{.State.StartedAt}}"}, names...) + args := append([]string{"inspect", "-f", "{{.Name}}|{{.State.OOMKilled}}|{{.State.StartedAt}}|{{.Id}}"}, names...) out, err := m.execCommand("docker", args...) if err != nil && strings.TrimSpace(out) == "" { return nil, err } + // R-528: the host's counter, asked once per scan (an HTTPS call to the agent); nil = Docker's flag only. + var host map[string]int64 + if m.hostOOMKills != nil { + host = m.hostOOMKills() + } var found []OOMContainer + hostKills := map[int]int64{} for _, line := range strings.Split(strings.TrimSpace(out), "\n") { - f := strings.SplitN(strings.TrimSpace(line), "|", 3) - if len(f) != 3 || f[1] != "true" { + f := strings.SplitN(strings.TrimSpace(line), "|", 4) + if len(f) < 3 { + continue + } + id := "" + if len(f) == 4 { + id = strings.TrimSpace(f[3]) + } + hk, counted := host[id] + source := "" + switch { + case f[1] == "true" && id != "" && counted && hk > 0: + source = OOMSourceDocker + "," + OOMSourceCgroup // both saw it + case f[1] == "true": + source = OOMSourceDocker + case id != "" && counted && hk > 0: + source = OOMSourceCgroup + default: continue } cname := strings.TrimPrefix(f[0], "/") if st, ok := owner[cname]; ok { - found = append(found, OOMContainer{Stack: st, Container: cname, StartedAt: f[2], Kills: -1}) + if counted && id != "" { + hostKills[len(found)] = hk + } + found = append(found, OOMContainer{Stack: st, Container: cname, StartedAt: f[2], Kills: -1, Source: source}) } } // R-636: only for the containers already flagged — one `docker exec` each, and the flagged set is @@ -75,6 +115,9 @@ func (m *Manager) ScanOOMKilled() ([]OOMContainer, error) { out, _ := m.execCommand("docker", "exec", found[i].Container, "cat", "/sys/fs/cgroup/memory.events", "/sys/fs/cgroup/memory.max", "/sys/fs/cgroup/memory.peak") found[i].Kills, found[i].MemLimit, found[i].Peak = parseCgroupMemory(out) + if hk, ok := hostKills[i]; ok && hk > found[i].Kills { + found[i].Kills = hk // the host's count is the kernel's own; the in-container read can be -1 (no cat) + } } m.setOOMCache(found) return found, nil diff --git a/controller/internal/stacks/r528_host_oom_test.go b/controller/internal/stacks/r528_host_oom_test.go new file mode 100644 index 00000000..35158eb5 --- /dev/null +++ b/controller/internal/stacks/r528_host_oom_test.go @@ -0,0 +1,84 @@ +package stacks + +import ( + "io" + "log" + "strings" + "testing" +) + +// R-528 — inside a Felhom LXC guest Docker can leave State.OOMKilled false after a memory-limit kill (measured +// 2026-09-15/16). The host's agent reads the kernel's own per-container counter (GET /guest/oom-kills); a counter above +// 0 for the RUNNING container's id is an OOM kill in this run, whatever Docker says. The scan flags it with +// Source "cgroup" and the host's count, and the existing app_oom line (once per run) fires for it. + +const r528ID = "4f5e6d7c8b9a0f1e2d3c4b5a69788796a5b4c3d2e1f00112233445566778899a" + +// RED-PROOF: ignore the host counter in ScanOOMKilled (only `OOMKilled == true` flags) → the killed container is not +// seen → FAILS. +func TestR528_AHostCounterAboveZeroIsAnOOMKillWhateverDockerSays(t *testing.T) { + var execd []string + m := &Manager{logger: log.New(io.Discard, "", 0), stacks: map[string]*Stack{ + "hog": {Name: "hog", Deployed: true, Containers: []ContainerInfo{{Name: "hog", State: StateRunning}}}, + "bookstack": {Name: "bookstack", Deployed: true, Containers: []ContainerInfo{{Name: "bookstack", State: StateRunning}}}, + }} + m.execFn = func(name string, args ...string) (string, error) { + if args[0] == "exec" { + execd = append(execd, args[1]) + return "oom_kill 1\n67108864\n67100000\n", nil + } + if !strings.Contains(strings.Join(args, " "), "{{.Id}}") { + t.Errorf("the inspect must ask for the container id: %v", args) + } + return "/bookstack|false|t0|" + strings.Repeat("0", 64) + "\n/hog|false|2026-10-28T23:00:00Z|" + r528ID + "\n", nil + } + m.SetHostOOMKills(func() map[string]int64 { + return map[string]int64{r528ID: 2, strings.Repeat("0", 64): 0} + }) + ooms, err := m.ScanOOMKilled() + if err != nil { + t.Fatal(err) + } + if len(ooms) != 1 || ooms[0].Stack != "hog" || ooms[0].Container != "hog" { + t.Fatalf("the host-counted kill was not seen: %+v", ooms) + } + if ooms[0].Source != OOMSourceCgroup || ooms[0].Kills != 2 || ooms[0].StartedAt != "2026-10-28T23:00:00Z" { + t.Fatalf("want source cgroup, the host's count 2 and the run's start: %+v", ooms[0]) + } + if ooms[0].MemLimit != "64M" { + t.Errorf("limit/peak are still read for the flagged container: %+v", ooms[0]) + } + if len(execd) != 1 || execd[0] != "hog" { + t.Errorf("exec must reach the flagged container only, got %v", execd) + } +} + +// Docker's flag alone still flags (Source docker); no host source (an old agent, 9202 without an agent) = today's +// behaviour; a host counter of 0 never flags. +func TestR528_WithoutAHostCounterTheScanIsAsBefore(t *testing.T) { + m := &Manager{logger: log.New(io.Discard, "", 0), stacks: map[string]*Stack{ + "a": {Name: "a", Deployed: true, Containers: []ContainerInfo{{Name: "a", State: StateRunning}}}, + "b": {Name: "b", Deployed: true, Containers: []ContainerInfo{{Name: "b", State: StateRunning}}}, + }} + m.execFn = func(name string, args ...string) (string, error) { + if args[0] == "exec" { + return "oom_kill 3\n", nil + } + return "/a|true|s1|" + strings.Repeat("a", 64) + "\n/b|false|s2|" + strings.Repeat("b", 64) + "\n", nil + } + ooms, _ := m.ScanOOMKilled() + if len(ooms) != 1 || ooms[0].Stack != "a" || ooms[0].Source != OOMSourceDocker || ooms[0].Kills != 3 { + t.Fatalf("no host source: %+v", ooms) + } + m.SetHostOOMKills(func() map[string]int64 { return map[string]int64{strings.Repeat("b", 64): 0} }) + ooms, _ = m.ScanOOMKilled() + if len(ooms) != 1 || ooms[0].Stack != "a" { + t.Fatalf("a host counter of 0 flagged a container: %+v", ooms) + } + // Both saw it: the source names both, the host's count wins over a lower in-container read. + m.SetHostOOMKills(func() map[string]int64 { return map[string]int64{strings.Repeat("a", 64): 7} }) + ooms, _ = m.ScanOOMKilled() + if len(ooms) != 1 || ooms[0].Source != OOMSourceDocker+","+OOMSourceCgroup || ooms[0].Kills != 7 { + t.Fatalf("both sources: %+v", ooms) + } +}