diff --git a/internal/alerts/active_cleanup.go b/internal/alerts/active_cleanup.go index 34d0935692..76cf8ef70c 100644 --- a/internal/alerts/active_cleanup.go +++ b/internal/alerts/active_cleanup.go @@ -349,6 +349,7 @@ func (m *Manager) ClearActiveAlerts() { m.dockerRestartTracking = make(map[string]*dockerRestartRecord) m.dockerUpdateFirstSeen = make(map[string]time.Time) m.dockerUpdateFirstSeenByIdentity = make(map[string]time.Time) + m.dockerUpdateLastObserved = make(map[string]time.Time) m.smartCounterSnapshots = make(map[string]smartCounterSnapshot) m.ackState = make(map[string]ackRecord) m.ackStateByCanonical = make(map[string]ackRecord) diff --git a/internal/alerts/config_runtime.go b/internal/alerts/config_runtime.go index 27202d3d0f..73b9fe2891 100644 --- a/internal/alerts/config_runtime.go +++ b/internal/alerts/config_runtime.go @@ -241,11 +241,13 @@ func (m *Manager) applyGlobalOfflineSettingsLocked() { m.dockerRestartTracking = make(map[string]*dockerRestartRecord) m.dockerUpdateFirstSeen = make(map[string]time.Time) m.dockerUpdateFirstSeenByIdentity = make(map[string]time.Time) + m.dockerUpdateLastObserved = make(map[string]time.Time) } if m.config.DockerDefaults.UpdateAlertDelayHours < 0 && !containersDisabled { m.clearDockerContainerUpdateAlertsLocked() m.dockerUpdateFirstSeen = make(map[string]time.Time) m.dockerUpdateFirstSeenByIdentity = make(map[string]time.Time) + m.dockerUpdateLastObserved = make(map[string]time.Time) } if servicesDisabled { var serviceAlerts []string diff --git a/internal/alerts/docker.go b/internal/alerts/docker.go index 1ab242fb08..e3a12331d5 100644 --- a/internal/alerts/docker.go +++ b/internal/alerts/docker.go @@ -1206,8 +1206,10 @@ func (m *Manager) clearDockerContainerMetricAlerts(resourceID string, metrics .. func (m *Manager) clearDockerContainerUpdateTracking(resourceID, trackingKey string) { m.mu.Lock() delete(m.dockerUpdateFirstSeen, resourceID) + delete(m.dockerUpdateLastObserved, resourceID) if trackingKey != "" { delete(m.dockerUpdateFirstSeenByIdentity, trackingKey) + delete(m.dockerUpdateLastObserved, trackingKey) } m.mu.Unlock() } @@ -1251,9 +1253,11 @@ func (m *Manager) clearDockerContainerUpdateStateLocked(alert *Alert) { if alert.ResourceID != "" { delete(m.dockerUpdateFirstSeen, alert.ResourceID) + delete(m.dockerUpdateLastObserved, alert.ResourceID) } if trackingKey := dockerUpdateTrackingKeyFromAlert(alert); trackingKey != "" { delete(m.dockerUpdateFirstSeenByIdentity, trackingKey) + delete(m.dockerUpdateLastObserved, trackingKey) } } @@ -1339,6 +1343,17 @@ func (m *Manager) checkDockerContainerImageUpdate(host models.DockerHost, contai return } + // Reports refresh tracking activity even when the registry result is cached + // or unavailable. Do not create a pending condition from unknown evidence. + m.mu.Lock() + if _, exists := m.dockerUpdateFirstSeen[resourceID]; exists { + m.dockerUpdateLastObserved[resourceID] = time.Now() + } + if _, exists := m.dockerUpdateFirstSeenByIdentity[updateTrackingKey]; exists { + m.dockerUpdateLastObserved[updateTrackingKey] = time.Now() + } + m.mu.Unlock() + // Check if this container has an update status reported if container.UpdateStatus == nil { // Missing update status means the condition is unknown, not resolved. @@ -1373,6 +1388,8 @@ func (m *Manager) checkDockerContainerImageUpdate(host models.DockerHost, contai } m.dockerUpdateFirstSeen[resourceID] = firstSeen m.dockerUpdateFirstSeenByIdentity[updateTrackingKey] = firstSeen + m.dockerUpdateLastObserved[resourceID] = time.Now() + m.dockerUpdateLastObserved[updateTrackingKey] = time.Now() m.mu.Unlock() // Check if we've exceeded the delay threshold @@ -1475,6 +1492,7 @@ func (m *Manager) cleanupDockerContainerAlertsWithTracking(host models.DockerHos if strings.HasPrefix(resourceID, prefix) { if _, exists := seen[resourceID]; !exists { delete(m.dockerUpdateFirstSeen, resourceID) + delete(m.dockerUpdateLastObserved, resourceID) } } } @@ -1485,6 +1503,7 @@ func (m *Manager) cleanupDockerContainerAlertsWithTracking(host models.DockerHos } if _, exists := seenUpdateTracking[trackingKey]; !exists { delete(m.dockerUpdateFirstSeenByIdentity, trackingKey) + delete(m.dockerUpdateLastObserved, trackingKey) } } } @@ -1520,11 +1539,13 @@ func (m *Manager) clearDockerHostContainerAlerts(host models.DockerHost) { for resourceID := range m.dockerUpdateFirstSeen { if strings.HasPrefix(resourceID, prefix) { delete(m.dockerUpdateFirstSeen, resourceID) + delete(m.dockerUpdateLastObserved, resourceID) } } for trackingKey := range m.dockerUpdateFirstSeenByIdentity { if strings.HasPrefix(trackingKey, updateTrackingPrefix) { delete(m.dockerUpdateFirstSeenByIdentity, trackingKey) + delete(m.dockerUpdateLastObserved, trackingKey) } } m.mu.Unlock() diff --git a/internal/alerts/docker_update_retention_test.go b/internal/alerts/docker_update_retention_test.go new file mode 100644 index 0000000000..87cd7a4b17 --- /dev/null +++ b/internal/alerts/docker_update_retention_test.go @@ -0,0 +1,72 @@ +package alerts + +import ( + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/models" +) + +// A cached registry observation is still an asserted condition on each report. +// Its first detection time must not be mistaken for tracking inactivity. +func TestDockerUpdateTrackingSurvivesDailyCleanup(t *testing.T) { + for _, delay := range []int{24, 48} { + t.Run((time.Duration(delay) * time.Hour).String(), func(t *testing.T) { + m := newTestManager(t) + m.config.DockerDefaults.UpdateAlertDelayHours = delay + host := models.DockerHost{ID: "host", DisplayName: "host"} + container := models.DockerContainer{ID: "container", Name: "web", Image: "mongo:7", UpdateStatus: &models.DockerContainerUpdateStatus{UpdateAvailable: true, CurrentDigest: "sha256:old", LatestDigest: "sha256:new", LastChecked: time.Now().Add(-6 * time.Hour)}} + resource := "docker:host/container" + key := dockerUpdateTrackingKey(host, container) + first := time.Now().Add(-25 * time.Hour) + m.dockerUpdateFirstSeen[resource] = first + m.dockerUpdateFirstSeenByIdentity[key] = first + check := func() { m.checkDockerContainerImageUpdate(host, container, resource, "web", "host", "host") } + check() + m.cleanupStaleMaps() + if !m.dockerUpdateFirstSeen[resource].Equal(first) || !m.dockerUpdateFirstSeenByIdentity[key].Equal(first) { + t.Fatal("daily cleanup discarded an observed pending update's first detection") + } + check() + if delay == 24 { + alerts := m.GetActiveAlerts() + if len(alerts) != 1 || !alerts[0].StartTime.Equal(first) { + t.Fatalf("continuing update lost its original incident: %+v", alerts) + } + } + // Unknown reports must neither resolve nor restart a pending condition. + container.UpdateStatus = nil + check() + m.cleanupStaleMaps() + if !m.dockerUpdateFirstSeen[resource].Equal(first) { + t.Fatal("missing status reset pending age") + } + container.UpdateStatus = &models.DockerContainerUpdateStatus{Error: "registry unavailable"} + check() + m.cleanupStaleMaps() + if !m.dockerUpdateFirstSeenByIdentity[key].Equal(first) { + t.Fatal("failed check reset pending age") + } + // An affirmative clear remains effective. + container.UpdateStatus = &models.DockerContainerUpdateStatus{UpdateAvailable: false, LastChecked: time.Now()} + check() + if len(m.GetActiveAlerts()) != 0 || len(m.dockerUpdateFirstSeen) != 0 || len(m.dockerUpdateFirstSeenByIdentity) != 0 || len(m.dockerUpdateLastObserved) != 0 { + t.Fatal("affirmative clear did not retire update") + } + }) + } +} + +func TestDockerUpdateTrackingExpiresOnlyAfterObservationStops(t *testing.T) { + m := newTestManager(t) + old := time.Now().Add(-25 * time.Hour) + for _, key := range []string{"docker:host/container", "docker-update:host/id:container"} { + m.dockerUpdateLastObserved[key] = old + } + m.dockerUpdateFirstSeen["docker:host/container"] = old.Add(-48 * time.Hour) + m.dockerUpdateFirstSeenByIdentity["docker-update:host/id:container"] = old.Add(-48 * time.Hour) + m.cleanupStaleMaps() + if len(m.dockerUpdateFirstSeen) != 0 || len(m.dockerUpdateFirstSeenByIdentity) != 0 || len(m.dockerUpdateLastObserved) != 0 { + t.Fatal("unobserved tracking was not reclaimed") + } +} diff --git a/internal/alerts/manager.go b/internal/alerts/manager.go index d263648743..747351f36e 100644 --- a/internal/alerts/manager.go +++ b/internal/alerts/manager.go @@ -82,6 +82,7 @@ type Manager struct { 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 + dockerUpdateLastObserved map[string]time.Time // Resource and stable identity keys; activity, not pending age. // PMG quarantine growth tracking pmgQuarantineHistory map[string][]pmgQuarantineSnapshot // Track quarantine snapshots for growth detection // SMART counter snapshots let alert evaluation distinguish historical @@ -227,6 +228,7 @@ func NewManagerWithDataDir(dataDir string, options ...ManagerOption) *Manager { dockerRestartTracking: make(map[string]*dockerRestartRecord), dockerUpdateFirstSeen: make(map[string]time.Time), dockerUpdateFirstSeenByIdentity: make(map[string]time.Time), + dockerUpdateLastObserved: make(map[string]time.Time), pmgQuarantineHistory: make(map[string][]pmgQuarantineSnapshot), smartCounterSnapshots: make(map[string]smartCounterSnapshot), pmgAnomalyTrackers: make(map[string]*pmgAnomalyTracker), diff --git a/internal/alerts/tracking_cleanup.go b/internal/alerts/tracking_cleanup.go index 9c546f0443..2628db03cb 100644 --- a/internal/alerts/tracking_cleanup.go +++ b/internal/alerts/tracking_cleanup.go @@ -70,14 +70,24 @@ func (m *Manager) cleanupStaleMaps() { } for containerID, firstSeen := range m.dockerUpdateFirstSeen { - if now.Sub(firstSeen) > staleThreshold { + lastObserved := m.dockerUpdateLastObserved[containerID] + if lastObserved.IsZero() { + lastObserved = firstSeen + } + if now.Sub(lastObserved) > staleThreshold { delete(m.dockerUpdateFirstSeen, containerID) + delete(m.dockerUpdateLastObserved, containerID) cleaned++ } } for containerID, firstSeen := range m.dockerUpdateFirstSeenByIdentity { - if now.Sub(firstSeen) > staleThreshold { + lastObserved := m.dockerUpdateLastObserved[containerID] + if lastObserved.IsZero() { + lastObserved = firstSeen + } + if now.Sub(lastObserved) > staleThreshold { delete(m.dockerUpdateFirstSeenByIdentity, containerID) + delete(m.dockerUpdateLastObserved, containerID) cleaned++ } } diff --git a/tests/qualification/docker-update-retention/README.md b/tests/qualification/docker-update-retention/README.md new file mode 100644 index 0000000000..d06bf83d3f --- /dev/null +++ b/tests/qualification/docker-update-retention/README.md @@ -0,0 +1,22 @@ +# Docker update tracking retention + +Run `go test -race ./internal/alerts -run 'TestDockerUpdate|TestCheckDockerContainerImageUpdate|TestCleanupStaleMaps|TestCleanupDockerContainerAlerts|TestCleanup$|TestUpdateConfig.*DockerContainerUpdate|TestEvaluateDockerContainerClearsUpdate' -count=1`. + +`TestDockerUpdateTrackingSurvivesDailyCleanup` seeds a first detection 25 hours +ago, with a cached registry result, and runs the real update evaluator and +hourly tracking cleanup. Both 24-hour (already firing) and 48-hour (pending) +delays must retain the detection time. Missing/error results retain it too; +an affirmative no-update result still clears the incident and both tracking +identities. Tracking is reclaimed after observations stop, and existing tests +cover host identity changes, removal and disabled update policy. + +The regression fails against the preceding source in both delay cases: cleanup +uses first detection as inactivity and discards live tracking. Keep pending age +separate from last observation. The new timestamps are ephemeral, bounded by the +same removal/configuration/24-hour inactivity paths as their tracking entries; +they are not registry-check timestamps or persisted evidence. + +This proves a tracking reset defect, not the cause of every open/resolved/open +cycle in issue #2048. The legacy recovery adapter labels an existing clear with +unknown confidence; it does not initiate that clear. No registry requests, +Docker updates, reporter installation or release availability are exercised.