From f7a43d27946a48a61b3c84a5bd1ab1fc561eab97 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Mon, 28 Sep 2026 05:03:26 +0100 Subject: [PATCH] Retain observed Docker update alert age through cleanup Hourly cleanup previously aged pending image updates by first detection, so a continuing update older than 24 hours lost its tracking and could restart its delay. Track observation activity separately, keep unknown checks pending, and reclaim state when observations stop or an affirmative clear arrives. Cover 24- and 48-hour delays and cleanup. Contract-Neutral: Preserve the existing Docker update alert policy and public contract while correcting internal tracking expiry. Change-source: pulse-maintainer --- internal/alerts/active_cleanup.go | 1 + internal/alerts/config_runtime.go | 2 + internal/alerts/docker.go | 21 ++++++ .../alerts/docker_update_retention_test.go | 72 +++++++++++++++++++ internal/alerts/manager.go | 2 + internal/alerts/tracking_cleanup.go | 14 +++- .../docker-update-retention/README.md | 22 ++++++ 7 files changed, 132 insertions(+), 2 deletions(-) create mode 100644 internal/alerts/docker_update_retention_test.go create mode 100644 tests/qualification/docker-update-retention/README.md 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.