fix(alerts): re-stamp lifecycle first-matched time when a new confirmation run starts

The slice-2 first-matched preservation kept an entry as long as one
existed, but several callers reset the confirmation-count maps directly
without the evaluator path — clearResourceOfflineAlert among them — so
a stale entry from a prior run backdated the next run's alert to the
previous run's first observation. Stamp the first-matched time whenever
the pre-evaluation count is zero, making stale entries harmless at
every reset site. Found by the recovery-gate parity harness.
This commit is contained in:
rcourtman
2026-08-26 23:12:02 +01:00
parent de489abc65
commit 44b85f754b
2 changed files with 71 additions and 1 deletions
+7 -1
View File
@@ -390,7 +390,13 @@ func (m *Manager) evaluateCanonicalLifecycleAlert(params canonicalLifecycleAlert
if params.Tracking != nil {
if result.State.ConsecutiveMatches > 0 {
params.Tracking[params.TrackingKey] = result.State.ConsecutiveMatches
if _, tracked := m.lifecycleFirstMatched[params.TrackingKey]; !tracked && !result.State.FirstMatchedAt.IsZero() {
// Stamp the first-matched time whenever this observation starts
// a new confirmation run (the pre-evaluation count was zero).
// Several callers reset the count maps directly without going
// through this path — clearResourceOfflineAlert among them — so
// a keep-if-present guard would let a stale entry from a prior
// run backdate the next run's alert.
if confirmations == 0 && !result.State.FirstMatchedAt.IsZero() {
m.lifecycleFirstMatched[params.TrackingKey] = result.State.FirstMatchedAt
}
} else {
@@ -226,3 +226,67 @@ func confirmedProviderIncidentResource(observedAt time.Time) unifiedresources.Re
}},
}
}
// Regression: several callers reset the confirmation-count maps directly
// without the evaluator path (clearResourceOfflineAlert among them), which
// left a stale lifecycleFirstMatched entry that backdated the next run's
// alert to the previous run's first observation. A new run (pre-evaluation
// count zero) must re-stamp the first-matched time.
func TestLifecycleFirstMatchedRestampsAfterDirectCountReset(t *testing.T) {
manager := NewManagerWithDataDir(t.TempDir(), WithoutPersistedAlertRestore())
t.Cleanup(manager.Stop)
manager.mu.Lock()
manager.config.Enabled = true
manager.mu.Unlock()
epoch := time.Date(2026, 8, 26, 12, 0, 0, 0, time.UTC)
observe := func(observed string, at time.Time) {
spec, err := buildCanonicalDiscreteStateSpec(
"node-9", "node-9", unifiedresources.ResourceTypeAgent,
AlertLevelCritical, 3, false, "connectivity", []string{"offline"},
)
if err != nil {
t.Fatalf("build spec: %v", err)
}
if _, ok := manager.evaluateCanonicalLifecycleAlert(canonicalLifecycleAlertParams{
Spec: spec,
Evidence: alertspecs.AlertEvidence{
ObservedAt: at,
DiscreteState: &alertspecs.DiscreteStateEvidence{StateKey: "connectivity", Observed: observed},
},
Tracking: manager.offlineConfirmations,
TrackingKey: "conn:node-9",
AlertID: canonicalDiscreteStateStateID("node-9", "connectivity"),
AlertType: "connectivity",
ResourceID: "node-9",
ResourceName: "node-9",
Message: "node offline",
}); !ok {
t.Fatal("evaluation rejected")
}
}
// Two matches begin a run, then the count is reset directly, the way
// the healthy-poll paths do — leaving lifecycleFirstMatched stale.
observe("offline", epoch)
observe("offline", epoch.Add(30*time.Second))
manager.mu.Lock()
delete(manager.offlineConfirmations, "conn:node-9")
manager.mu.Unlock()
restart := epoch.Add(2 * time.Minute)
observe("offline", restart)
observe("offline", restart.Add(30*time.Second))
observe("offline", restart.Add(60*time.Second))
manager.mu.Lock()
alert, exists := manager.getActiveAlertNoLock(canonicalDiscreteStateStateID("node-9", "connectivity"))
manager.mu.Unlock()
if !exists || alert == nil {
t.Fatal("expected alert to fire after the second run")
}
if !alert.StartTime.Equal(restart) {
t.Fatalf("StartTime = %v, want the second run's first observation %v", alert.StartTime, restart)
}
}