diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index 60d37911dd..8eea5b7709 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -1411,6 +1411,21 @@ per-alert rate limiting; future notification-gating changes should extend that policy owner rather than burying new checks inside metric or resource-specific evaluators. +### Configured flapping thresholds remain reachable + +Every accepted positive `FlappingThreshold`, including values above ten, must +be reachable by current observations inside `FlappingWindowSeconds`. Per-key +history retains at most `max(10, FlappingThreshold)` timestamps after window +pruning; it must not grow with repeated attempts during an active cooldown. +Only the threshold-crossing attempt opens that cooldown. Later attempts keep +its original deadline even after the observation window drains, and a served +cooldown resets the episode so a later storm can arm it again. Lowering the +threshold applies the new history bound and threshold on the next check. The +read-only delivery diagnosis must explain the same threshold and suppression +actually enforced at dispatch. `internal/alerts/flapping_threshold_test.go` +pins these boundaries through normal configuration updates and notification +callbacks, alongside the existing flapping cooldown and one-shot callback tests. + ### Monitor-only delivery is terminal Monitor-only alerts remain visible, but neither a firing nor a recovery diff --git a/docs/release-control/v6/internal/subsystems/registry.json b/docs/release-control/v6/internal/subsystems/registry.json index 8df54c6569..7f8b616f6b 100644 --- a/docs/release-control/v6/internal/subsystems/registry.json +++ b/docs/release-control/v6/internal/subsystems/registry.json @@ -2768,6 +2768,7 @@ "internal/alerts/eventlog/reader_isolation_test.go", "internal/alerts/external_probe_test.go", "internal/alerts/filter_evaluation_test.go", + "internal/alerts/flapping_threshold_test.go", "internal/alerts/guest_snapshot_test.go", "internal/alerts/history_concurrency_test.go", "internal/alerts/history_migration_test.go", diff --git a/internal/alerts/flapping_threshold_test.go b/internal/alerts/flapping_threshold_test.go new file mode 100644 index 0000000000..25872dca0e --- /dev/null +++ b/internal/alerts/flapping_threshold_test.go @@ -0,0 +1,154 @@ +package alerts + +import ( + "fmt" + "testing" + "time" +) + +// A positive threshold survives the normal configuration update. Its history +// bound must not silently make that accepted storm-control setting unreachable. +func TestFlappingConfiguredThresholdGatesDispatch(t *testing.T) { + for _, threshold := range []int{1, 5, 10, 11, 25, 100} { + t.Run(fmt.Sprintf("threshold_%d", threshold), func(t *testing.T) { + m := NewManagerWithDataDir(t.TempDir()) + t.Cleanup(m.Stop) + cfg := m.GetConfig() + cfg.Enabled = true + cfg.ActivationState = ActivationActive + cfg.FlappingEnabled = true + cfg.FlappingThreshold = threshold + cfg.FlappingWindowSeconds = 300 + cfg.FlappingCooldownMinutes = 15 + m.UpdateConfig(cfg) + if got := m.GetConfig().FlappingThreshold; got != threshold { + t.Fatalf("accepted threshold = %d, want %d", got, threshold) + } + + _, alert := testNewCanonicalAlert("vm-100", "vm-100-cpu", "vm", "cpu") + alert.Level = AlertLevelWarning + alert.StartTime = time.Now() + alert.LastSeen = alert.StartTime + m.mu.Lock() + m.setActiveAlertNoLock(alert.ID, alert) + m.mu.Unlock() + + delivered := 0 + m.SetAlertCallback(func(*Alert) { delivered++ }) + for attempt := 1; attempt <= threshold; attempt++ { + m.mu.Lock() + dispatched := m.dispatchAlert(alert, false) + m.mu.Unlock() + if want := attempt < threshold; dispatched != want { + t.Fatalf("dispatch %d = %v, want %v at threshold %d", attempt, dispatched, want, threshold) + } + } + if delivered != threshold-1 { + t.Fatalf("notifications admitted = %d, want %d", delivered, threshold-1) + } + + // Repeated attempts during the same cooldown must neither escape to a + // destination nor grow the retained history or extend the deadline. + key := canonicalTrackingKeyForAlert(alert) + m.mu.Lock() + deadline := m.suppressedUntil[key] + lastNotified := alert.LastNotified + for attempt := 0; attempt < 500; attempt++ { + if m.dispatchAlert(alert, false) { + m.mu.Unlock() + t.Fatal("notification escaped the flapping cooldown") + } + } + historySize := len(m.flappingHistory[key]) + unchanged := m.suppressedUntil[key].Equal(deadline) && alert.LastNotified == lastNotified + m.mu.Unlock() + if delivered != threshold-1 || historySize != threshold || !unchanged { + t.Fatalf("cooldown: delivered=%d history=%d unchanged=%v", delivered, historySize, unchanged) + } + + diagnosis, found := m.DiagnoseAlertDelivery(alert.ID) + if !found || diagnosis.Reason != AlertDeliveryReasonFlapping || !diagnosis.FlappingActive || + diagnosis.FlappingThreshold != threshold || diagnosis.FlappingHistoryInWindow != threshold || + diagnosis.SuppressedUntil == nil || !diagnosis.SuppressedUntil.Equal(deadline) { + t.Fatalf("delivery diagnosis does not explain the enforced setting: %+v", diagnosis) + } + }) + } +} + +func TestFlappingHighThresholdPrunesExpiresAndRearms(t *testing.T) { + m := NewManagerWithDataDir(t.TempDir()) + t.Cleanup(m.Stop) + cfg := m.GetConfig() + cfg.FlappingEnabled = true + cfg.FlappingThreshold = 25 + cfg.FlappingWindowSeconds = 60 + cfg.FlappingCooldownMinutes = 15 + m.UpdateConfig(cfg) + + const key = "high-threshold-window" + m.mu.Lock() + defer m.mu.Unlock() + now := time.Now() + for i := 0; i < 100; i++ { + m.flappingHistory[key] = append(m.flappingHistory[key], now.Add(-2*time.Minute)) + } + for i := 0; i < 23; i++ { + m.flappingHistory[key] = append(m.flappingHistory[key], now.Add(-time.Second)) + } + suppressed, transitioned := m.checkFlappingLocked(key) + if suppressed || transitioned || len(m.flappingHistory[key]) != 24 { + t.Fatalf("old changes counted or current changes lost: suppressed=%v transitioned=%v history=%d", suppressed, transitioned, len(m.flappingHistory[key])) + } + suppressed, transitioned = m.checkFlappingLocked(key) + if !suppressed || !transitioned || len(m.flappingHistory[key]) != 25 { + t.Fatalf("25th current change did not arm cooldown: suppressed=%v transitioned=%v history=%d", suppressed, transitioned, len(m.flappingHistory[key])) + } + + deadline := m.suppressedUntil[key] + m.flappingHistory[key] = nil // The observation window drains before cooldown ends. + suppressed, transitioned = m.checkFlappingLocked(key) + if !suppressed || transitioned || !m.suppressedUntil[key].Equal(deadline) { + t.Fatal("cooldown did not hold its original deadline after the window drained") + } + m.suppressedUntil[key] = now.Add(-time.Second) + suppressed, transitioned = m.checkFlappingLocked(key) + if suppressed || transitioned || m.flappingActive[key] || len(m.flappingHistory[key]) != 1 { + t.Fatal("served cooldown did not release and reset the episode") + } + for change := 2; change <= 25; change++ { + suppressed, transitioned = m.checkFlappingLocked(key) + if want := change == 25; suppressed != want || transitioned != want { + t.Fatalf("new episode change %d: suppressed=%v transitioned=%v", change, suppressed, transitioned) + } + } +} + +func TestFlappingLoweredThresholdBoundsRetainedHistory(t *testing.T) { + m := NewManagerWithDataDir(t.TempDir()) + t.Cleanup(m.Stop) + cfg := m.GetConfig() + cfg.FlappingEnabled = true + cfg.FlappingThreshold = 100 + m.UpdateConfig(cfg) + + const key = "threshold-edited" + m.mu.Lock() + for change := 0; change < 30; change++ { + if suppressed, _ := m.checkFlappingLocked(key); suppressed { + m.mu.Unlock() + t.Fatal("notification suppressed below the original threshold") + } + } + m.mu.Unlock() + cfg.FlappingThreshold = 12 + m.UpdateConfig(cfg) + + m.mu.Lock() + suppressed, transitioned := m.checkFlappingLocked(key) + historySize := len(m.flappingHistory[key]) + m.mu.Unlock() + if !suppressed || !transitioned || historySize != 12 { + t.Fatalf("lowered threshold not enforced with bounded history: suppressed=%v transitioned=%v history=%d", suppressed, transitioned, historySize) + } +} diff --git a/internal/alerts/notification_policy.go b/internal/alerts/notification_policy.go index ca0218adb8..24a3b49510 100644 --- a/internal/alerts/notification_policy.go +++ b/internal/alerts/notification_policy.go @@ -122,8 +122,12 @@ func (m *Manager) checkFlappingLocked(trackingKey string) (suppress bool, justTr validHistory = append(validHistory, t) } } - // Limit to max 10 entries to prevent unbounded growth - const maxFlappingHistory = 10 + // Keep enough observations to reach the configured threshold. A fixed + // ten-entry cap silently disabled flapping suppression for accepted + // thresholds above ten. Retain the old diagnostic allowance for smaller + // thresholds, while still bounding history by configuration rather than + // the number of dispatch attempts. + maxFlappingHistory := max(10, m.config.FlappingThreshold) if len(validHistory) > maxFlappingHistory { validHistory = validHistory[len(validHistory)-maxFlappingHistory:] }