diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index b62ac92ea..9cdd9d02d 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -170,6 +170,12 @@ percent because that is the collector's source evidence. Lifecycle surfaces may transport that report, but must not reinterpret it as host-capacity utilization; monitoring and unified resources own the normalized CPU contract used by history, alerts, and canonical app-container metrics. +The same module authors the container OOM evidence boundary from Docker inspect. +Current agents must serialize a non-null `OOMKilled` boolean for inspected +containers, preserving explicit false; absence is reserved for older or +reduced-fidelity report producers. Lifecycle transport must not synthesize OOM +state from exit code 137 or collapse absent and false, because monitoring and +alerts depend on that distinction to fail closed. Inside-guest Docker / Podman visibility is also a privacy boundary: full Docker / Podman inventory may come from a guest-local agent or another explicit guest reporting path. LXC Docker inventory may also come from the Proxmox host diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index e3b1c822f..3cbba5e92 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -21,6 +21,12 @@ Docker and Podman container CPU thresholds evaluate host-capacity-normalized CPU percent, not Docker's runtime-native per-core percent. Alert metadata may carry the raw per-core value and reporting host CPU count for evidence, but the threshold value and canonical `cpuPercent` metadata remain normalized. +Docker and Podman OOM alerts require authoritative runtime evidence: the +container must be stopped (`exited` or `dead`) and its reported `OOMKilled` +state must be explicitly true. Exit code 137 alone is only SIGKILL evidence; +explicit false and unavailable/legacy OOM state both fail closed without an OOM +alert. Recovery clears an existing OOM alert when the authoritative predicate +is no longer true. ## Canonical Files @@ -421,7 +427,10 @@ Docker alert evaluation now lives in `internal/alerts/docker.go`. That file owns Docker host connectivity, container state and health, container metric projection, service gap/update-state checks, image-update timing, and Docker tracking cleanup; future Docker alert behavior should extend that resource -checker owner rather than expanding the central Manager file. +checker owner rather than expanding the central Manager file. It must not keep +shadow last-exit-code state or infer an OOM kill from exit 137; the accepted +container model's nullable runtime-authored `OOMKilled` field is the sole OOM +classification input. PBS alert evaluation now lives in `internal/alerts/pbs.go`. That file owns PBS connectivity normalization, PBS metric projection, PBS metric cleanup, and PBS offline lifecycle handling; future PBS alert behavior should extend that diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 815bad23d..fba6502ba 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -61,6 +61,14 @@ per-core CPU percent, but monitoring-owned history and alert threshold evaluation use host-capacity-normalized CPU percent when host CPU capacity is known. Raw runtime CPU remains alert/resource metadata, not the canonical threshold value. +Docker and Podman container OOM state is runtime-authored evidence, not an +exit-code inference. Current agents must publish Docker inspect's `OOMKilled` +boolean for every inspected container, including explicit `false`; monitoring +must preserve that nullable boolean through report ingest, internal/frontend +models, and unified resources. An absent value means an older or reduced-fidelity +report did not provide the evidence and must remain distinguishable from both +confirmed OOM and confirmed non-OOM state. Exit code 137 proves only SIGKILL and +must not be promoted into OOM truth by monitoring. Proxmox read-state rehydration is the inverse boundary: canonical unified-resource CPU metrics are 0..100 percentages, while legacy `models.Node.CPU`, `models.VM.CPU`, and `models.Container.CPU` remain Proxmox @@ -216,6 +224,11 @@ resource health. and Docker container CPU alerts must pass through the shared normalized capacity helper so an 80% threshold means 80% of the reporting host capacity, not 0.8 of one core on a multi-core host. + Container OOM evidence must come from the inspected runtime state. The report + wire field is nullable for compatibility with older agents, but a current + collector must set it to the exact Docker inspect boolean even when false; + report ingest and model conversion must clone and preserve the pointer so + concurrent state replacement cannot alter previously accepted evidence. 8. Add or change Proxmox Ceph compatibility payload decoding through `pkg/proxmox/ceph.go` 9. Add or change Proxmox ZFS compatibility payload decoding and vdev-role normalization through `pkg/proxmox/zfs.go` 10. Add or change mock chart synthesis, seeded history continuity, or mock-owned diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 359f735ef..492672b07 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -115,6 +115,11 @@ storage/recovery-adjacent only because `DockerData` is a shared resource payload. Raw per-core CPU evidence and normalized capacity CPU must not be used as backup coverage, restore readiness, storage-health, or recovery-point signals. +The same dependency rule applies to nullable Docker `OOMKilled` evidence. +Storage and recovery consumers may preserve and display it as runtime workload +context, but must not reinterpret confirmed OOM, confirmed non-OOM, missing +evidence, or exit code 137 as backup damage, protection health, restore +readiness, or a recovery-point signal. Local subscription-agent settings in `internal/api/ai_handlers.go` remain an ai-runtime transport choice even when Patrol observes storage or recovery @@ -1236,7 +1241,8 @@ recovery scope, or a storage/recovery-owned secret source. only; they must not reinterpret it as backup freshness, recovery point recency, or protection cadence. Docker / Podman `DockerData` container lifecycle, Podman metadata, and - cumulative block I/O totals remain unified-resource runtime context. + cumulative block I/O totals, including nullable runtime-authored OOM state, + remain unified-resource runtime context. Storage and recovery may use those fields only as workload description when linking to an owning runtime/platform page; they must not reinterpret container block I/O totals as backup throughput, recovery-point evidence, diff --git a/docs/release-control/v6/internal/subsystems/unified-resources.md b/docs/release-control/v6/internal/subsystems/unified-resources.md index bd6a8f409..5e018a17d 100644 --- a/docs/release-control/v6/internal/subsystems/unified-resources.md +++ b/docs/release-control/v6/internal/subsystems/unified-resources.md @@ -528,7 +528,10 @@ engine storage rows must stay host-scoped with table proof hooks so browser proof can distinguish a populated disk-usage tab from an empty fixture. Runtime container detail payloads must preserve the agent-reported lifecycle timestamps, Podman pod/compose/auto-update/user-namespace metadata, and -cumulative block I/O totals on `DockerData`; frontend detail summaries and +cumulative block I/O totals on `DockerData`. They must also preserve nullable, +runtime-authored `OOMKilled` evidence without converting absent state or exit +code 137 into a positive classification; typed Docker views must return an +independent copy of that value. Frontend detail summaries and Docker page search consume those backend-authored fields before falling back to legacy labels. Docker network rows must consume canonical runtime attachment relationships, diff --git a/internal/alerts/active_cleanup.go b/internal/alerts/active_cleanup.go index bd42faa8c..d5ba6d01c 100644 --- a/internal/alerts/active_cleanup.go +++ b/internal/alerts/active_cleanup.go @@ -330,7 +330,6 @@ func (m *Manager) ClearActiveAlerts() { m.dockerOfflineCount = make(map[string]int) m.dockerStateConfirm = make(map[string]int) m.dockerRestartTracking = make(map[string]*dockerRestartRecord) - m.dockerLastExitCode = make(map[string]int) m.dockerUpdateFirstSeen = make(map[string]time.Time) m.dockerUpdateFirstSeenByIdentity = make(map[string]time.Time) m.ackState = make(map[string]ackRecord) diff --git a/internal/alerts/alerts_test.go b/internal/alerts/alerts_test.go index 3107935d6..3f427bdd0 100644 --- a/internal/alerts/alerts_test.go +++ b/internal/alerts/alerts_test.go @@ -778,7 +778,6 @@ func TestHandleDockerHostRemovedClearsAlertsAndTracking(t *testing.T) { m.dockerOfflineCount[host.ID] = 2 m.dockerStateConfirm[containerResourceID] = 1 m.dockerRestartTracking[containerResourceID] = &dockerRestartRecord{} - m.dockerLastExitCode[containerResourceID] = 137 m.mu.Unlock() m.HandleDockerHostRemoved(host) @@ -801,9 +800,6 @@ func TestHandleDockerHostRemovedClearsAlertsAndTracking(t *testing.T) { if _, exists := m.dockerRestartTracking[containerResourceID]; exists { t.Fatalf("expected restart tracking to be cleared") } - if _, exists := m.dockerLastExitCode[containerResourceID]; exists { - t.Fatalf("expected last exit code tracking to be cleared") - } } func TestCheckHostGeneratesMetricAlerts(t *testing.T) { @@ -3424,7 +3420,6 @@ func TestCheckDockerHostIgnoredPrefixClearsExistingAlerts(t *testing.T) { m.activeAlerts[restartAlertID] = &Alert{ID: restartAlertID, ResourceID: resourceID} m.dockerStateConfirm[resourceID] = 2 m.dockerRestartTracking[resourceID] = &dockerRestartRecord{} - m.dockerLastExitCode[resourceID] = 137 m.mu.Unlock() m.CheckDockerHost(host) @@ -3447,9 +3442,6 @@ func TestCheckDockerHostIgnoredPrefixClearsExistingAlerts(t *testing.T) { if _, exists := m.dockerRestartTracking[resourceID]; exists { t.Fatalf("expected restart tracking cleared") } - if _, exists := m.dockerLastExitCode[resourceID]; exists { - t.Fatalf("expected last exit code cleared") - } } func TestUpdateConfigNormalizesDockerIgnoredPrefixes(t *testing.T) { @@ -6853,7 +6845,6 @@ func TestClearActiveAlertsWithExistingAlerts(t *testing.T) { m.dockerOfflineCount["docker-1"] = 1 m.dockerStateConfirm["docker-1"] = 1 m.dockerRestartTracking["docker-1"] = &dockerRestartRecord{} - m.dockerLastExitCode["docker-1"] = 137 m.dockerUpdateFirstSeen["docker-1"] = time.Now() m.dockerUpdateFirstSeenByIdentity["docker-1"] = time.Now() m.ackState["test-alert-1"] = ackRecord{acknowledged: true, user: "testuser", time: time.Now()} @@ -6908,9 +6899,6 @@ func TestClearActiveAlertsWithExistingAlerts(t *testing.T) { if len(m.dockerRestartTracking) != 0 { t.Errorf("expected dockerRestartTracking to be empty, got %d", len(m.dockerRestartTracking)) } - if len(m.dockerLastExitCode) != 0 { - t.Errorf("expected dockerLastExitCode to be empty, got %d", len(m.dockerLastExitCode)) - } if len(m.dockerUpdateFirstSeen) != 0 { t.Errorf("expected dockerUpdateFirstSeen to be empty, got %d", len(m.dockerUpdateFirstSeen)) } @@ -7314,7 +7302,6 @@ func TestApplyGlobalOfflineSettingsLocked(t *testing.T) { // Add tracking state m.dockerStateConfirm["c1"] = 2 m.dockerRestartTracking["c1"] = &dockerRestartRecord{count: 5} - m.dockerLastExitCode["c1"] = 137 m.config.DisableAllDockerContainers = true @@ -7340,9 +7327,6 @@ func TestApplyGlobalOfflineSettingsLocked(t *testing.T) { if len(m.dockerRestartTracking) != 0 { t.Errorf("expected dockerRestartTracking to be empty, got %d entries", len(m.dockerRestartTracking)) } - if len(m.dockerLastExitCode) != 0 { - t.Errorf("expected dockerLastExitCode to be empty, got %d entries", len(m.dockerLastExitCode)) - } }) t.Run("DisableAllDockerServices clears docker service alerts", func(t *testing.T) { @@ -12945,7 +12929,31 @@ func TestDockerContainerOOMKillAlert(t *testing.T) { } }) - t.Run("exited container with exit code 137 - critical OOM alert", func(t *testing.T) { + for _, tc := range []struct { + name string + oomKilled *bool + }{ + {name: "explicitly not OOM killed", oomKilled: boolPtr(false)}, + {name: "OOM state unavailable from older agent", oomKilled: nil}, + } { + t.Run("exit code 137 without authoritative OOM evidence - "+tc.name, func(t *testing.T) { + m := newTestManager(t) + host := models.DockerHost{ + ID: "host-sigkill", Containers: []models.DockerContainer{{ + ID: "container-sigkill", Name: "sigkill-app", State: "exited", ExitCode: 137, OOMKilled: tc.oomKilled, + }}, + } + + m.CheckDockerHost(host) + + resourceID := dockerResourceID(host.ID, host.Containers[0].ID) + if testHasActiveAlert(t, m, fmt.Sprintf("docker-container-oom-%s", resourceID)) { + t.Fatal("exit code 137 alone must not create an OOM alert") + } + }) + } + + t.Run("runtime-confirmed OOM kill - critical alert", func(t *testing.T) { m := newTestManager(t) host := models.DockerHost{ @@ -12959,6 +12967,7 @@ func TestDockerContainerOOMKillAlert(t *testing.T) { State: "exited", Status: "Exited (137) 1 minute ago", ExitCode: 137, + OOMKilled: boolPtr(true), MemoryUsage: 512 * 1024 * 1024, MemoryLimit: 512 * 1024 * 1024, }, @@ -12971,7 +12980,7 @@ func TestDockerContainerOOMKillAlert(t *testing.T) { alertID := fmt.Sprintf("docker-container-oom-%s", resourceID) alert, exists := testLookupActiveAlert(t, m, alertID) if !exists { - t.Fatal("expected OOM alert for container with exit code 137") + t.Fatal("expected OOM alert for runtime-confirmed OOM kill") } if alert.Level != AlertLevelCritical { t.Fatalf("expected critical OOM alert, got %s", alert.Level) @@ -12985,9 +12994,12 @@ func TestDockerContainerOOMKillAlert(t *testing.T) { if got := alert.Metadata["canonicalSpecID"]; got != resourceID+"-oom-kill" { t.Fatalf("canonicalSpecID = %v, want %s", got, resourceID+"-oom-kill") } + if got := alert.Metadata["oomKilled"]; got != true { + t.Fatalf("oomKilled metadata = %v, want true", got) + } }) - t.Run("dead container with exit code 137 - critical OOM alert", func(t *testing.T) { + t.Run("dead container with runtime-confirmed OOM kill - critical alert", func(t *testing.T) { m := newTestManager(t) host := models.DockerHost{ @@ -12996,11 +13008,12 @@ func TestDockerContainerOOMKillAlert(t *testing.T) { Hostname: "docker.local", Containers: []models.DockerContainer{ { - ID: "container-dead", - Name: "dead-oom-app", - State: "dead", - Status: "Dead", - ExitCode: 137, + ID: "container-dead", + Name: "dead-oom-app", + State: "dead", + Status: "Dead", + ExitCode: 137, + OOMKilled: boolPtr(true), }, }, } @@ -13018,7 +13031,7 @@ func TestDockerContainerOOMKillAlert(t *testing.T) { } }) - t.Run("repeated 137 exit code - no new alert", func(t *testing.T) { + t.Run("repeated runtime-confirmed OOM state - no new alert", func(t *testing.T) { m := newTestManager(t) hostID := "host-oom-4" @@ -13030,11 +13043,12 @@ func TestDockerContainerOOMKillAlert(t *testing.T) { Hostname: "docker.local", Containers: []models.DockerContainer{ { - ID: containerID, - Name: "oom-killed-app", - State: "exited", - Status: "Exited (137) 1 minute ago", - ExitCode: 137, + ID: containerID, + Name: "oom-killed-app", + State: "exited", + Status: "Exited (137) 1 minute ago", + ExitCode: 137, + OOMKilled: boolPtr(true), }, }, } @@ -13075,11 +13089,12 @@ func TestDockerContainerOOMKillAlert(t *testing.T) { Hostname: "docker.local", Containers: []models.DockerContainer{ { - ID: containerID, - Name: "recovering-app", - State: "exited", - Status: "Exited (137) 1 minute ago", - ExitCode: 137, + ID: containerID, + Name: "recovering-app", + State: "exited", + Status: "Exited (137) 1 minute ago", + ExitCode: 137, + OOMKilled: boolPtr(true), }, }, } @@ -13128,11 +13143,12 @@ func TestDockerContainerOOMKillAlert(t *testing.T) { Hostname: "docker.local", Containers: []models.DockerContainer{ { - ID: containerID, - Name: "multi-exit-app", - State: "exited", - Status: "Exited (137) 1 minute ago", - ExitCode: 137, + ID: containerID, + Name: "multi-exit-app", + State: "exited", + Status: "Exited (137) 1 minute ago", + ExitCode: 137, + OOMKilled: boolPtr(true), }, }, } diff --git a/internal/alerts/cleanup_test.go b/internal/alerts/cleanup_test.go index bb7fcf21f..e0b8b7934 100644 --- a/internal/alerts/cleanup_test.go +++ b/internal/alerts/cleanup_test.go @@ -107,7 +107,6 @@ func TestCleanupStaleMaps(t *testing.T) { // Docker restart tracking m.dockerRestartTracking["stale-restart"] = &dockerRestartRecord{lastChecked: oldTime} - m.dockerLastExitCode["stale-restart"] = 1 m.dockerRestartTracking["recent-restart"] = &dockerRestartRecord{lastChecked: recentTime} // Alert rate limit @@ -165,9 +164,6 @@ func TestCleanupStaleMaps(t *testing.T) { if _, exists := m.dockerRestartTracking["stale-restart"]; exists { t.Error("stale-restart should be removed") } - if _, exists := m.dockerLastExitCode["stale-restart"]; exists { - t.Error("stale-restart exit code should be removed") - } if _, exists := m.dockerRestartTracking["recent-restart"]; !exists { t.Error("recent-restart should NOT be removed") } diff --git a/internal/alerts/config_runtime.go b/internal/alerts/config_runtime.go index 8be490b27..846567dac 100644 --- a/internal/alerts/config_runtime.go +++ b/internal/alerts/config_runtime.go @@ -212,7 +212,6 @@ func (m *Manager) applyGlobalOfflineSettingsLocked() { } m.dockerStateConfirm = make(map[string]int) m.dockerRestartTracking = make(map[string]*dockerRestartRecord) - m.dockerLastExitCode = make(map[string]int) m.dockerUpdateFirstSeen = make(map[string]time.Time) m.dockerUpdateFirstSeenByIdentity = make(map[string]time.Time) } diff --git a/internal/alerts/docker.go b/internal/alerts/docker.go index dd4339035..be28ada6b 100644 --- a/internal/alerts/docker.go +++ b/internal/alerts/docker.go @@ -228,7 +228,6 @@ func (m *Manager) CheckDockerHost(host models.DockerHost) { m.clearAlert(fmt.Sprintf("docker-container-memory-limit-%s", resourceID)) m.mu.Lock() delete(m.dockerRestartTracking, resourceID) - delete(m.dockerLastExitCode, resourceID) m.mu.Unlock() m.clearDockerContainerUpdateTracking(resourceID, updateTrackingKey) continue @@ -982,18 +981,13 @@ func (m *Manager) checkDockerContainerRestartLoop(host models.DockerHost, contai } } -// checkDockerContainerOOMKill detects when a container was killed due to out of memory +// checkDockerContainerOOMKill detects when the runtime explicitly reports that +// a container was killed due to out of memory. func (m *Manager) checkDockerContainerOOMKill(host models.DockerHost, container models.DockerContainer, resourceID, containerName, instanceName, nodeName string) { alertID := fmt.Sprintf("docker-container-oom-%s", resourceID) - // Exit code 137 means the container was killed by SIGKILL, often due to OOM - // Only alert if the container exited (not running) with exit code 137 state := strings.ToLower(strings.TrimSpace(container.State)) - if (state == "exited" || state == "dead") && container.ExitCode == 137 { - m.mu.Lock() - m.dockerLastExitCode[resourceID] = 137 - m.mu.Unlock() - + if (state == "exited" || state == "dead") && container.OOMKilled != nil && *container.OOMKilled { spec, err := buildCanonicalHealthAssessmentSpec(resourceID+"-oom-kill", resourceID, containerName, unifiedresources.ResourceTypeAppContainer, "docker-container-exit", []string{"oom-kill"}, false) if err != nil { log.Warn(). @@ -1006,6 +1000,7 @@ func (m *Manager) checkDockerContainerOOMKill(host models.DockerHost, container metadata := dockerContainerAlertMetadata(host, container, containerName) metadata["exitCode"] = container.ExitCode + metadata["oomKilled"] = true metadata["memoryUsageBytes"] = container.MemoryUsage metadata["memoryLimitBytes"] = container.MemoryLimit @@ -1038,13 +1033,8 @@ func (m *Manager) checkDockerContainerOOMKill(host models.DockerHost, container Int64("memoryLimit", container.MemoryLimit). Msg("Docker container OOM killed") } else { - // Update last exit code if it changed - if container.ExitCode != 0 { - m.mu.Lock() - m.dockerLastExitCode[resourceID] = container.ExitCode - m.mu.Unlock() - } - // Clear OOM alert if container is running or exited with different code + // Unknown reports from older agents and explicit false reports both fail + // closed. Exit code 137 proves SIGKILL only; it is not OOM evidence. m.clearAlert(buildCanonicalStateID(resourceID, resourceID+"-oom-kill")) } } @@ -1437,11 +1427,6 @@ func (m *Manager) clearDockerHostContainerAlerts(host models.DockerHost) { delete(m.dockerRestartTracking, resourceID) } } - for resourceID := range m.dockerLastExitCode { - if strings.HasPrefix(resourceID, prefix) { - delete(m.dockerLastExitCode, resourceID) - } - } for resourceID := range m.dockerUpdateFirstSeen { if strings.HasPrefix(resourceID, prefix) { delete(m.dockerUpdateFirstSeen, resourceID) diff --git a/internal/alerts/manager.go b/internal/alerts/manager.go index 34f8008e5..95f6bc082 100644 --- a/internal/alerts/manager.go +++ b/internal/alerts/manager.go @@ -52,7 +52,6 @@ type Manager struct { dockerOfflineCount map[string]int // Track consecutive offline counts for Docker hosts dockerStateConfirm map[string]int // Track consecutive state confirmations for Docker containers dockerRestartTracking map[string]*dockerRestartRecord // Track restart counts and times for restart loop detection - dockerLastExitCode map[string]int // Track last exit code for OOM detection dockerUpdateFirstSeen map[string]time.Time // Track when image updates were first detected for alert delay // Stable identity tracking prevents update-delay resets when host IDs churn. dockerUpdateFirstSeenByIdentity map[string]time.Time @@ -132,7 +131,6 @@ func NewManagerWithDataDir(dataDir string) *Manager { dockerOfflineCount: make(map[string]int), dockerStateConfirm: make(map[string]int), dockerRestartTracking: make(map[string]*dockerRestartRecord), - dockerLastExitCode: make(map[string]int), dockerUpdateFirstSeen: make(map[string]time.Time), dockerUpdateFirstSeenByIdentity: make(map[string]time.Time), pmgQuarantineHistory: make(map[string][]pmgQuarantineSnapshot), diff --git a/internal/alerts/offline_toggle_test.go b/internal/alerts/offline_toggle_test.go index 78e36df16..ef25df525 100644 --- a/internal/alerts/offline_toggle_test.go +++ b/internal/alerts/offline_toggle_test.go @@ -98,7 +98,6 @@ func TestUpdateConfigClearsDockerContainerAlertsWhenDisabled(t *testing.T) { } manager.dockerStateConfirm[containerResourceID] = 2 manager.dockerRestartTracking[containerResourceID] = &dockerRestartRecord{} - manager.dockerLastExitCode[containerResourceID] = 137 manager.mu.Unlock() config := manager.GetConfig() @@ -120,9 +119,6 @@ func TestUpdateConfigClearsDockerContainerAlertsWhenDisabled(t *testing.T) { if len(manager.dockerRestartTracking) != 0 { t.Fatalf("expected dockerRestartTracking map to be cleared when DisableAllDockerContainers is enabled") } - if len(manager.dockerLastExitCode) != 0 { - t.Fatalf("expected dockerLastExitCode map to be cleared when DisableAllDockerContainers is enabled") - } } func TestUpdateConfigClearsDockerServiceAlertsWhenDisabled(t *testing.T) { diff --git a/internal/alerts/tracking_cleanup.go b/internal/alerts/tracking_cleanup.go index e7951ec86..88295116b 100644 --- a/internal/alerts/tracking_cleanup.go +++ b/internal/alerts/tracking_cleanup.go @@ -130,7 +130,6 @@ func (m *Manager) cleanupStaleMaps() { for containerID, record := range m.dockerRestartTracking { if record != nil && now.Sub(record.lastChecked) > staleThreshold { delete(m.dockerRestartTracking, containerID) - delete(m.dockerLastExitCode, containerID) cleaned++ } } diff --git a/internal/dockeragent/agent_collect_test.go b/internal/dockeragent/agent_collect_test.go index 2ee3b3187..d15672f55 100644 --- a/internal/dockeragent/agent_collect_test.go +++ b/internal/dockeragent/agent_collect_test.go @@ -305,6 +305,9 @@ func TestCollectContainer(t *testing.T) { if container.Health != "healthy" { t.Fatalf("expected health status, got %q", container.Health) } + if container.OOMKilled == nil || *container.OOMKilled { + t.Fatalf("expected authoritative non-OOM state, got %v", container.OOMKilled) + } if container.BlockIO == nil { t.Fatalf("expected block IO to be populated") } diff --git a/internal/dockeragent/collect.go b/internal/dockeragent/collect.go index 1a5a18712..bd740324f 100644 --- a/internal/dockeragent/collect.go +++ b/internal/dockeragent/collect.go @@ -621,6 +621,7 @@ func (a *Agent) collectContainer(ctx context.Context, summary containertypes.Sum } } + oomKilled := inspect.State.OOMKilled container := agentsdocker.Container{ ID: summary.ID, Name: trimLeadingSlash(summary.Names), @@ -637,6 +638,7 @@ func (a *Agent) collectContainer(ctx context.Context, summary containertypes.Sum UptimeSeconds: uptimeSeconds, RestartCount: inspect.RestartCount, ExitCode: inspect.State.ExitCode, + OOMKilled: &oomKilled, StartedAt: startedPtr, FinishedAt: finishedPtr, Ports: ports, diff --git a/internal/models/converters.go b/internal/models/converters.go index b77074cb6..0b49a4123 100644 --- a/internal/models/converters.go +++ b/internal/models/converters.go @@ -605,6 +605,7 @@ func (c DockerContainer) ToFrontend() DockerContainerFrontend { UptimeSeconds: c.UptimeSeconds, RestartCount: c.RestartCount, ExitCode: c.ExitCode, + OOMKilled: cloneBoolPtr(c.OOMKilled), CreatedAt: c.CreatedAt.Unix() * 1000, Labels: nil, WritableLayerBytes: c.WritableLayerBytes, diff --git a/internal/models/converters_test.go b/internal/models/converters_test.go index 623fc60ed..336a45062 100644 --- a/internal/models/converters_test.go +++ b/internal/models/converters_test.go @@ -413,6 +413,7 @@ func TestContainerToFrontend(t *testing.T) { func TestDockerContainerToFrontend(t *testing.T) { now := time.Now() startedAt := now.Add(-1 * time.Hour) + oomKilled := false container := DockerContainer{ ID: "abc123def456", @@ -427,6 +428,7 @@ func TestDockerContainerToFrontend(t *testing.T) { MemoryPercent: 50.0, UptimeSeconds: 3600, RestartCount: 0, + OOMKilled: &oomKilled, CreatedAt: now.Add(-2 * time.Hour), StartedAt: &startedAt, Ports: []DockerContainerPort{ @@ -479,6 +481,13 @@ func TestDockerContainerToFrontend(t *testing.T) { if frontend.StartedAt == nil { t.Error("StartedAt should not be nil") } + if frontend.OOMKilled == nil || *frontend.OOMKilled { + t.Fatalf("OOMKilled = %v, want explicit false", frontend.OOMKilled) + } + oomKilled = true + if *frontend.OOMKilled { + t.Fatal("frontend OOMKilled must not alias the internal model") + } } func TestDockerServiceToFrontend(t *testing.T) { diff --git a/internal/models/deepcopy.go b/internal/models/deepcopy.go index abcd236af..6f8695294 100644 --- a/internal/models/deepcopy.go +++ b/internal/models/deepcopy.go @@ -422,6 +422,7 @@ func cloneDockerContainerUpdateStatus(src *DockerContainerUpdateStatus) *DockerC func cloneDockerContainer(src DockerContainer) DockerContainer { dest := src + dest.OOMKilled = cloneBoolPtr(src.OOMKilled) dest.StartedAt = cloneTimePtr(src.StartedAt) dest.FinishedAt = cloneTimePtr(src.FinishedAt) dest.Ports = append([]DockerContainerPort(nil), src.Ports...) diff --git a/internal/models/deepcopy_test.go b/internal/models/deepcopy_test.go index d2f1977b7..bfe17fc56 100644 --- a/internal/models/deepcopy_test.go +++ b/internal/models/deepcopy_test.go @@ -24,6 +24,20 @@ func TestCloneBoolPtr_Value(t *testing.T) { } } +func TestCloneDockerContainer_PreservesIndependentOOMEvidence(t *testing.T) { + oomKilled := false + src := DockerContainer{ID: "container-1", OOMKilled: &oomKilled} + + got := cloneDockerContainer(src) + if got.OOMKilled == nil || *got.OOMKilled { + t.Fatalf("OOMKilled = %v, want explicit false", got.OOMKilled) + } + oomKilled = true + if *got.OOMKilled { + t.Fatal("cloned Docker container must own its OOM evidence value") + } +} + func TestCloneFloat64Ptr_Nil(t *testing.T) { if cloneFloat64Ptr(nil) != nil { t.Error("nil should clone to nil") diff --git a/internal/models/models.go b/internal/models/models.go index 736b9f1c0..cca8e227c 100644 --- a/internal/models/models.go +++ b/internal/models/models.go @@ -908,6 +908,7 @@ type DockerContainer struct { UptimeSeconds int64 `json:"uptimeSeconds"` RestartCount int `json:"restartCount"` ExitCode int `json:"exitCode"` + OOMKilled *bool `json:"oomKilled,omitempty"` CreatedAt time.Time `json:"createdAt"` StartedAt *time.Time `json:"startedAt,omitempty"` FinishedAt *time.Time `json:"finishedAt,omitempty"` diff --git a/internal/models/models_frontend.go b/internal/models/models_frontend.go index d8517e9fe..8cbb89556 100644 --- a/internal/models/models_frontend.go +++ b/internal/models/models_frontend.go @@ -454,6 +454,7 @@ type DockerContainerFrontend struct { UptimeSeconds int64 `json:"uptimeSeconds"` RestartCount int `json:"restartCount"` ExitCode int `json:"exitCode"` + OOMKilled *bool `json:"oomKilled,omitempty"` CreatedAt int64 `json:"createdAt"` StartedAt *int64 `json:"startedAt,omitempty"` FinishedAt *int64 `json:"finishedAt,omitempty"` diff --git a/internal/monitoring/monitor_agents.go b/internal/monitoring/monitor_agents.go index f8b7b7738..266179eab 100644 --- a/internal/monitoring/monitor_agents.go +++ b/internal/monitoring/monitor_agents.go @@ -1200,6 +1200,7 @@ func (m *Monitor) ApplyDockerReport(report agentsdocker.Report, tokenRecord *con UptimeSeconds: payload.UptimeSeconds, RestartCount: payload.RestartCount, ExitCode: payload.ExitCode, + OOMKilled: cloneReportBoolPtr(payload.OOMKilled), CreatedAt: payload.CreatedAt, StartedAt: payload.StartedAt, FinishedAt: payload.FinishedAt, diff --git a/internal/monitoring/monitor_docker_test.go b/internal/monitoring/monitor_docker_test.go index acfa56f9d..42682f715 100644 --- a/internal/monitoring/monitor_docker_test.go +++ b/internal/monitoring/monitor_docker_test.go @@ -264,6 +264,7 @@ func TestApplyDockerReportSkipsMetricsHistoryInMockMode(t *testing.T) { func TestApplyDockerReportIncludesContainerDiskDetails(t *testing.T) { timestamp := time.Now().UTC() + oomKilled := false report := agentsdocker.Report{ Agent: agentsdocker.AgentInfo{ ID: "agent-1", @@ -277,6 +278,7 @@ func TestApplyDockerReportIncludesContainerDiskDetails(t *testing.T) { { ID: "ctr-1", Name: "app", + OOMKilled: &oomKilled, WritableLayerBytes: 512 * 1024 * 1024, RootFilesystemBytes: 2 * 1024 * 1024 * 1024, BlockIO: &agentsdocker.ContainerBlockIO{ @@ -311,6 +313,13 @@ func TestApplyDockerReportIncludesContainerDiskDetails(t *testing.T) { } container := host.Containers[0] + if container.OOMKilled == nil || *container.OOMKilled { + t.Fatalf("expected explicit non-OOM state, got %v", container.OOMKilled) + } + oomKilled = true + if *container.OOMKilled { + t.Fatal("expected monitoring state to own an independent OOM evidence value") + } if container.WritableLayerBytes != 512*1024*1024 { t.Fatalf("expected writable layer bytes to match, got %d", container.WritableLayerBytes) } diff --git a/internal/unifiedresources/adapters.go b/internal/unifiedresources/adapters.go index b6c4d527d..2944f0b3b 100644 --- a/internal/unifiedresources/adapters.go +++ b/internal/unifiedresources/adapters.go @@ -2162,6 +2162,7 @@ func resourceFromDockerContainer(ct models.DockerContainer, host models.DockerHo Health: ct.Health, RestartCount: ct.RestartCount, ExitCode: ct.ExitCode, + OOMKilled: cloneBoolPtr(ct.OOMKilled), CPURawPercent: ct.CPUPercent, CPUCapacityPercent: models.DockerContainerCPUCapacityPercent(ct, host.CPUs), CPUCapacityCores: host.CPUs, diff --git a/internal/unifiedresources/adapters_test.go b/internal/unifiedresources/adapters_test.go index cb46d6320..b070235bc 100644 --- a/internal/unifiedresources/adapters_test.go +++ b/internal/unifiedresources/adapters_test.go @@ -581,6 +581,7 @@ func TestResourceFromDockerHostOmitsStandaloneInactiveSwarm(t *testing.T) { } func TestResourceFromDockerContainerIncludesContainerID(t *testing.T) { + oomKilled := false container := models.DockerContainer{ ID: "aurora-3-abcdef123456", Name: "web", @@ -589,6 +590,7 @@ func TestResourceFromDockerContainerIncludesContainerID(t *testing.T) { ImageDigest: "sha256:current", UptimeSeconds: 1234, CPUPercent: 12.5, + OOMKilled: &oomKilled, } host := models.DockerHost{ @@ -616,6 +618,13 @@ func TestResourceFromDockerContainerIncludesContainerID(t *testing.T) { if got, want := resource.Docker.ImageID, container.ImageDigest; got != want { t.Fatalf("imageId = %q, want %q", got, want) } + if resource.Docker.OOMKilled == nil || *resource.Docker.OOMKilled { + t.Fatalf("oomKilled = %v, want explicit false", resource.Docker.OOMKilled) + } + oomKilled = true + if *resource.Docker.OOMKilled { + t.Fatal("unified resource must own an independent OOM evidence value") + } } func TestResourceFromDockerContainerPreservesRuntimeMetadata(t *testing.T) { diff --git a/internal/unifiedresources/clone.go b/internal/unifiedresources/clone.go index 18ae3834a..684b6cd75 100644 --- a/internal/unifiedresources/clone.go +++ b/internal/unifiedresources/clone.go @@ -225,6 +225,7 @@ func cloneDockerData(in *DockerData) *DockerData { return nil } out := *in + out.OOMKilled = cloneBoolPtr(in.OOMKilled) out.Temperature = cloneFloat64Ptr(in.Temperature) out.LoadAverage = cloneFloat64Slice(in.LoadAverage) out.TokenLastUsedAt = cloneTimePtr(in.TokenLastUsedAt) diff --git a/internal/unifiedresources/code_standards_test.go b/internal/unifiedresources/code_standards_test.go index cf760ad09..422adfbe5 100644 --- a/internal/unifiedresources/code_standards_test.go +++ b/internal/unifiedresources/code_standards_test.go @@ -177,6 +177,38 @@ func TestOperationReceiptProtocolRemainsInternalToIngestAndRuntime(t *testing.T) } } +func TestDockerOOMEvidenceRemainsAuthoritativeAcrossCanonicalIngest(t *testing.T) { + monitorSource, err := os.ReadFile(filepath.Join("..", "monitoring", "monitor_agents.go")) + if err != nil { + t.Fatal(err) + } + if !regexp.MustCompile(`OOMKilled:\s+cloneReportBoolPtr\(payload\.OOMKilled\)`).Match(monitorSource) { + t.Fatal("Docker report ingest must preserve and clone nullable runtime OOM evidence") + } + + oomKilled := false + resource, _ := resourceFromDockerContainer(models.DockerContainer{ + ID: "container-oom-contract", + Name: "contract-container", + State: "exited", + ExitCode: 137, + OOMKilled: &oomKilled, + }, models.DockerHost{ID: "docker-host-contract"}) + if resource.Docker == nil || resource.Docker.OOMKilled == nil || *resource.Docker.OOMKilled { + t.Fatalf("canonical DockerData lost explicit non-OOM evidence: %+v", resource.Docker) + } + + oomKilled = true + viewEvidence := NewDockerContainerView(&resource).OOMKilled() + if viewEvidence == nil || *viewEvidence { + t.Fatalf("typed Docker view aliased its source evidence: %v", viewEvidence) + } + *viewEvidence = true + if got := NewDockerContainerView(&resource).OOMKilled(); got == nil || *got { + t.Fatalf("typed Docker view must return an independent OOM evidence value: %v", got) + } +} + func TestProductionActionLifecycleDoesNotUseRecordActionAuditAsUpsert(t *testing.T) { paths := []string{"../actionlifecycle/service.go", "../ai/tools/action_audit.go", "../api/patrol_action_broker.go"} for _, path := range paths { diff --git a/internal/unifiedresources/types.go b/internal/unifiedresources/types.go index 1d8f800c9..eb18a24f4 100644 --- a/internal/unifiedresources/types.go +++ b/internal/unifiedresources/types.go @@ -1009,6 +1009,7 @@ type DockerData struct { Health string `json:"health,omitempty"` RestartCount int `json:"restartCount,omitempty"` ExitCode int `json:"exitCode,omitempty"` + OOMKilled *bool `json:"oomKilled,omitempty"` CPURawPercent float64 `json:"cpuRawPercent,omitempty"` CPUCapacityPercent float64 `json:"cpuCapacityPercent,omitempty"` CPUCapacityCores int `json:"cpuCapacityCores,omitempty"` diff --git a/internal/unifiedresources/views.go b/internal/unifiedresources/views.go index ae11a9a49..6a9235cc1 100644 --- a/internal/unifiedresources/views.go +++ b/internal/unifiedresources/views.go @@ -3812,6 +3812,13 @@ func (v DockerContainerView) ExitCode() int { return v.r.Docker.ExitCode } +func (v DockerContainerView) OOMKilled() *bool { + if v.r == nil || v.r.Docker == nil { + return nil + } + return cloneBoolPtr(v.r.Docker.OOMKilled) +} + func (v DockerContainerView) CPUPercent() float64 { if v.r == nil { return 0 diff --git a/internal/unifiedresources/views_test.go b/internal/unifiedresources/views_test.go index 86df4367d..994fb6dd2 100644 --- a/internal/unifiedresources/views_test.go +++ b/internal/unifiedresources/views_test.go @@ -1186,6 +1186,7 @@ func TestView_StoragePoolViewUsesCanonicalMetricsTargetAndGenericNodeHints(t *te func TestView_DockerContainerViewMetricsTargetAccessor(t *testing.T) { now := time.Date(2026, 3, 31, 12, 0, 0, 0, time.UTC) parentID := "host-1" + oomKilled := false r := &Resource{ ID: "app-1", Type: ResourceTypeAppContainer, @@ -1201,6 +1202,7 @@ func TestView_DockerContainerViewMetricsTargetAccessor(t *testing.T) { ContainerID: "nextcloud", Image: "docker.io/library/nextcloud:29.0.7", ContainerState: "running", + OOMKilled: &oomKilled, }, } @@ -1221,6 +1223,15 @@ func TestView_DockerContainerViewMetricsTargetAccessor(t *testing.T) { if got := v.MetricsTarget(); got == nil || got.ResourceID != "nextcloud-web-1" { t.Fatalf("expected cloned docker container metrics target, got %+v", got) } + + oom := v.OOMKilled() + if oom == nil || *oom { + t.Fatalf("expected explicit non-OOM view state, got %v", oom) + } + *oom = true + if got := v.OOMKilled(); got == nil || *got { + t.Fatalf("expected cloned OOM evidence, got %v", got) + } } func TestView_PBSAndPMGInstanceViewAccessors(t *testing.T) { diff --git a/pkg/agents/docker/report.go b/pkg/agents/docker/report.go index b400dfa5c..1e49b732a 100644 --- a/pkg/agents/docker/report.go +++ b/pkg/agents/docker/report.go @@ -80,6 +80,7 @@ type Container struct { UptimeSeconds int64 `json:"uptimeSeconds"` RestartCount int `json:"restartCount"` ExitCode int `json:"exitCode"` + OOMKilled *bool `json:"oomKilled,omitempty"` StartedAt *time.Time `json:"startedAt,omitempty"` FinishedAt *time.Time `json:"finishedAt,omitempty"` Ports []ContainerPort `json:"ports,omitempty"` diff --git a/pkg/agents/docker/report_test.go b/pkg/agents/docker/report_test.go index 7b7484889..e8a8e74c1 100644 --- a/pkg/agents/docker/report_test.go +++ b/pkg/agents/docker/report_test.go @@ -2,6 +2,7 @@ package dockeragent import ( "encoding/json" + "strings" "testing" "time" ) @@ -58,6 +59,7 @@ func TestReport_AgentKey(t *testing.T) { func TestReport_JSONMarshal(t *testing.T) { now := time.Now() + oomKilled := false report := Report{ Agent: AgentInfo{ ID: "agent-docker-1", @@ -73,10 +75,11 @@ func TestReport_JSONMarshal(t *testing.T) { }, Containers: []Container{ { - ID: "abc123", - Name: "nginx", - Image: "nginx:latest", - State: "running", + ID: "abc123", + Name: "nginx", + Image: "nginx:latest", + State: "running", + OOMKilled: &oomKilled, }, }, Timestamp: now, @@ -98,6 +101,26 @@ func TestReport_JSONMarshal(t *testing.T) { if len(decoded.Containers) != 1 { t.Errorf("Containers count = %d, want 1", len(decoded.Containers)) } + if decoded.Containers[0].OOMKilled == nil || *decoded.Containers[0].OOMKilled { + t.Fatalf("OOMKilled = %v, want explicit false", decoded.Containers[0].OOMKilled) + } + if !json.Valid(data) || !containsJSONField(data, `"oomKilled":false`) { + t.Fatalf("expected false OOM evidence on the wire: %s", data) + } +} + +func TestContainer_JSONOmitsUnknownOOMEvidence(t *testing.T) { + data, err := json.Marshal(Container{ID: "legacy-container"}) + if err != nil { + t.Fatalf("marshal Container: %v", err) + } + if containsJSONField(data, `"oomKilled"`) { + t.Fatalf("unknown OOM evidence must remain absent: %s", data) + } +} + +func containsJSONField(data []byte, field string) bool { + return strings.Contains(string(data), field) } func TestAgentInfo_Fields(t *testing.T) {