Use authoritative Docker OOM evidence

This commit is contained in:
rcourtman
2026-07-15 17:55:26 +01:00
parent e499f92360
commit e3382c8bcb
32 changed files with 234 additions and 81 deletions
-1
View File
@@ -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)
+56 -40
View File
@@ -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),
},
},
}
-4
View File
@@ -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")
}
-1
View File
@@ -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)
}
+6 -21
View File
@@ -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)
-2
View File
@@ -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),
-4
View File
@@ -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) {
-1
View File
@@ -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++
}
}