diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index d240ffdb16..8bbeb8ad15 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -43,6 +43,21 @@ callbacks remain outside the manager lock. No retention duration is extended. sample and a short recovery/refire. This is synthetic lifecycle proof, not installed notification-destination acceptance. +Hourly tracking-map cleanup retains canonical acknowledgement records while +their incident is active, including legacy-keyed active snapshots and active +recurrences with an older inactive timestamp. A decision does not expire from +its acknowledgement age while the condition continues. Recovery starts the +existing inactive-retention window; the ordinary cleanup's one-hour expiry, +hourly stale-record fallback and explicit unacknowledgement remain unchanged. +`TestTrackingCleanupPreservesAcknowledgedProviderRecurrence` in +`internal/alerts/reducer_parity_ack_test.go` verifies manual and automatic +acknowledgement through provider-native pool incident reconciliation, affirmative +recovery, short recurrence, dispatch callbacks and JSON/durable checkpoint restart. +`TestTrackingCleanupCanonicalAckRetentionBounds` verifies active/legacy identity, +recent inactivity, both existing expiry paths and missing-timestamp fallback. +Modeled elapsed time and local callbacks do not prove installed recovery or +delivery to an external notification destination. + Mobile incident drawers transfer their exact context to Assistant and close through the shared explicit handoff callback. Keeping the source drawer above Assistant, or dropping its occurrence identity to make navigation work, fails diff --git a/internal/alerts/reducer_parity_ack_test.go b/internal/alerts/reducer_parity_ack_test.go index 934a35f753..477c1b993b 100644 --- a/internal/alerts/reducer_parity_ack_test.go +++ b/internal/alerts/reducer_parity_ack_test.go @@ -2,11 +2,13 @@ package alerts import ( "fmt" + "sync/atomic" "testing" "time" "github.com/rcourtman/pulse-go-rewrite/internal/alerts/reducer" alertspecs "github.com/rcourtman/pulse-go-rewrite/internal/alerts/specs" + "github.com/rcourtman/pulse-go-rewrite/internal/storagehealth" "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" ) @@ -262,3 +264,173 @@ func TestAutoAcknowledgementSurvivesMetricEvaluation(t *testing.T) { evaluate(90) assertAcknowledged("short recovery and refire") } + +// Age only the acknowledgement timestamps, keeping detector observations fresh. +// The hourly sweep must not discard a long-running operator decision before a +// real recovery can start the existing inactive-retention window. +func TestTrackingCleanupPreservesAcknowledgedProviderRecurrence(t *testing.T) { + for _, durable := range []bool{false, true} { + for _, automatic := range []bool{false, true} { + name := "JSON" + if durable { + name = "durable" + } + if automatic { + name += "/automatic" + } else { + name += "/manual" + } + t.Run(name, func(t *testing.T) { + dir := t.TempDir() + newManager := func() *Manager { + return newCleanupRetentionManager(t, dir, durable) + } + m := newManager() + resource := unifiedresources.Resource{ + ID: "storage:ack-tank", Type: unifiedresources.ResourceTypeStorage, + Name: "ack-tank", ParentName: "truenas-main", + Sources: []unifiedresources.DataSource{unifiedresources.SourceTrueNAS}, + Storage: &unifiedresources.StorageMeta{Platform: "truenas", Topology: "pool", Protection: "zfs", IsZFS: true}, + Incidents: []unifiedresources.ResourceIncident{{ + Provider: "truenas", NativeID: "ack-pool-alert", Code: "truenas_volume_status", + Severity: storagehealth.RiskWarning, Summary: "Pool ack-tank is DEGRADED", + StartedAt: time.Now().Add(-26 * time.Hour), + }}, + } + healthy := resource + healthy.Incidents = nil + evaluate := func(m *Manager, observed unifiedresources.Resource) { + m.SyncUnifiedResourceIncidents([]unifiedresources.Resource{observed}) + } + var deliveries atomic.Int32 + m.SetAlertCallback(func(*Alert) { deliveries.Add(1) }) + evaluate(m, resource) + initial := m.GetActiveAlerts() + if len(initial) != 1 || deliveries.Load() != 1 { + t.Fatalf("initial provider condition: %d alerts / %d dispatches, want 1 / 1", len(initial), deliveries.Load()) + } + id := initial[0].ID + user := "operator" + if automatic { + user = "system-auto" + m.mu.Lock() + m.config.AutoAcknowledgeAfterHours = 1 + m.mu.Unlock() + m.Cleanup(time.Hour) + } else if err := m.AcknowledgeAlert(id, user); err != nil { + t.Fatal(err) + } + ackAt := time.Now().Add(-25 * time.Hour) + m.mu.Lock() + active, ok := m.getActiveAlertNoLock(id) + if !ok || !active.Acknowledged { + m.mu.Unlock() + t.Fatal("detector acknowledgement was not established") + } + active.AckTime = &ackAt + record := m.ackStateByCanonical[id] + record.time = ackAt + m.ackStateByCanonical[id] = record + m.mirrorAcknowledgeNoLock(active, user, ackAt) + m.setActiveAlertNoLock(id, active) + m.mu.Unlock() + + checkCycle := func(m *Manager, count *atomic.Int32, stage string) { + t.Helper() + before := count.Load() + m.cleanupStaleMaps() + m.mu.RLock() + record, retained := m.ackStateByCanonical[id] + m.mu.RUnlock() + if !retained || record.user != user || !record.time.Equal(ackAt) { + t.Errorf("%s: hourly sweep lost the active acknowledgement: %+v, retained=%v", stage, record, retained) + } + evaluate(m, healthy) // Observed pool with no incident: affirmative recovery. + if got := len(m.GetActiveAlerts()); got != 0 { + t.Fatalf("%s: real recovery left %d active alerts", stage, got) + } + m.Cleanup(time.Hour) // Old acknowledgement, newly inactive: retain it. + evaluate(m, resource) + alerts := m.GetActiveAlerts() + if len(alerts) != 1 || alerts[0].ID != id || !alerts[0].Acknowledged || alerts[0].AckUser != user || alerts[0].AckTime == nil || !alerts[0].AckTime.Equal(ackAt) { + t.Errorf("%s: short recurrence lost acknowledgement identity: %+v", stage, alerts) + } + if got := count.Load() - before; got != 0 { + t.Errorf("%s: acknowledged recovery/recurrence dispatched %d duplicate notifications", stage, got) + } + t.Logf("%s: recovery/recurrence dispatch delta %d", stage, count.Load()-before) + } + checkCycle(m, &deliveries, "before restart") + m.Stop() // The actual final checkpoint, not a hand-built JSON fixture. + restarted := newManager() + var afterRestart atomic.Int32 + restarted.SetAlertCallback(func(*Alert) { afterRestart.Add(1) }) + checkCycle(restarted, &afterRestart, "after restart") + if err := restarted.UnacknowledgeAlert(id); err != nil { + t.Fatal(err) + } + before := afterRestart.Load() + evaluate(restarted, healthy) + evaluate(restarted, resource) + if got := afterRestart.Load() - before; got != 1 { + t.Errorf("explicit unacknowledge/recovery/recurrence dispatched %d notifications, want 1", got) + } + }) + } + } +} + +func TestTrackingCleanupCanonicalAckRetentionBounds(t *testing.T) { + for _, tc := range []struct { + name string + active bool + legacy bool + inactive time.Duration + wantHourly bool + wantNormal bool + }{ + {name: "active canonical", active: true, wantHourly: true, wantNormal: true}, + {name: "active legacy identity", active: true, legacy: true, wantHourly: true, wantNormal: true}, + {name: "active after earlier recurrence", active: true, inactive: 25 * time.Hour, wantHourly: true, wantNormal: true}, + {name: "recently inactive old acknowledgement", inactive: 30 * time.Minute, wantHourly: true, wantNormal: true}, + {name: "existing one-hour expiry", inactive: 2 * time.Hour, wantHourly: true}, + {name: "stale inactive", inactive: 25 * time.Hour}, + {name: "legacy missing inactive timestamp"}, + } { + t.Run(tc.name, func(t *testing.T) { + m := newTestManager(t) + now := time.Now() + old := now.Add(-25 * time.Hour) + id, alert := testNewCanonicalAlert("ack-bounds", "metric-threshold:cpu", "metric-threshold", "cpu") + alert.StartTime, alert.LastSeen = old, now + alert.Acknowledged, alert.AckUser, alert.AckTime = true, "operator", &old + record := ackRecord{acknowledged: true, user: "operator", time: old} + if tc.inactive > 0 { + record.inactiveAt = now.Add(-tc.inactive) + } + m.mu.Lock() + m.ackStateByCanonical[id] = record + if tc.active { + storageKey := id + if tc.legacy { + storageKey = "legacy-cpu-ack-bounds" + } + m.activeAlerts[storageKey] = alert + } + m.mu.Unlock() + assertRetained := func(stage string, want bool) { + t.Helper() + m.mu.RLock() + got, retained := m.ackStateByCanonical[id] + m.mu.RUnlock() + if retained != want || (retained && got != record) { + t.Errorf("%s: acknowledgement retained=%v (%+v), want %v with unchanged record", stage, retained, got, want) + } + } + m.cleanupStaleMaps() + assertRetained("hourly sweep", tc.wantHourly) + m.Cleanup(time.Hour) + assertRetained("normal one-hour inactive expiry", tc.wantNormal) + }) + } +} diff --git a/internal/alerts/tracking_cleanup.go b/internal/alerts/tracking_cleanup.go index 2628db03cb..7b7294b6d8 100644 --- a/internal/alerts/tracking_cleanup.go +++ b/internal/alerts/tracking_cleanup.go @@ -139,6 +139,11 @@ func (m *Manager) cleanupStaleMaps() { } } for canonicalID, record := range m.ackStateByCanonical { + // As with legacy records and Cleanup, an active incident owns its + // acknowledgement until recovery starts the inactive-retention window. + if m.hasActiveAlertTrackingKeyNoLock(canonicalID) { + continue + } checkTime := record.inactiveAt if checkTime.IsZero() { checkTime = record.time