Forward-port guest alert node-move continuity

This commit is contained in:
rcourtman 2026-04-01 13:18:15 +01:00
parent 5e2807d8f3
commit f5a878016c
8 changed files with 547 additions and 0 deletions

View file

@ -75,6 +75,13 @@ operator-facing alert routing behavior for live runtime alerts.
Canonical alert identity and evaluation are the live runtime model. Remaining
legacy references should exist only in explicit migration or negative test
boundaries.
Guest metric canonical state remains resource-backed and therefore node-scoped
for Proxmox guests, so node moves must not strand active alert state on the
previous resource ID. When a guest metric alert survives a node move, alerts
runtime must migrate the active alert, history entry, acknowledgment record,
suppression/rate-limit/flapping tracking, and guest per-disk metric identity
to the current canonical state instead of reopening a duplicate alert or
resolving only the stale node-scoped identity.
Alert history persistence is also part of that canonical boundary. The history
manager may choose the owned runtime data directory, but it must normalize that

View file

@ -6848,12 +6848,25 @@ func (m *Manager) checkMetric(resourceID, resourceName, node, instance, resource
Msg("Checking metric threshold")
m.mu.Lock()
migratedAlertIdentity := false
defer func() {
if migratedAlertIdentity {
asyncSaveActiveAlerts("guest metric node move", m.SaveActiveAlerts)
}
}()
defer m.mu.Unlock()
existingAlert, exists := m.getActiveAlertNoLock(alertID)
if !exists && canonicalStateID != "" {
existingAlert, exists = m.getActiveAlertNoLock(canonicalStateID)
}
if !exists && canonicalStateID != "" {
if migrated := m.migrateGuestMetricAlertNoLock(canonicalStateID, canonicalSpecID, string(alertspecs.AlertSpecKindMetricThreshold), resourceID, resourceName, node, instance, resourceType); migrated != nil {
existingAlert = migrated
exists = true
migratedAlertIdentity = true
}
}
trackingKey := canonicalTrackingKeyOrFallback(existingAlert, canonicalStateID)
if trackingKey == "" {
trackingKey = canonicalStateID

View file

@ -85,9 +85,22 @@ func (m *Manager) evaluateCanonicalMetricAlert(spec alertspecs.ResourceAlertSpec
observedAt := time.Now()
m.mu.Lock()
migratedAlertIdentity := false
defer func() {
if migratedAlertIdentity {
asyncSaveActiveAlerts("canonical guest metric node move", m.SaveActiveAlerts)
}
}()
defer m.mu.Unlock()
existingAlert, exists := m.getActiveAlertNoLock(storageKey)
if !exists {
if migrated := m.migrateGuestMetricAlertNoLock(storageKey, spec.ID, string(spec.Kind), spec.ResourceID, resourceName, node, instance, resourceType); migrated != nil {
existingAlert = migrated
exists = true
migratedAlertIdentity = true
}
}
monitorOnly := opts != nil && opts.MonitorOnly
if suppressUntil, suppressed := m.suppressedUntil[trackingKey]; suppressed && time.Now().Before(suppressUntil) {

View file

@ -0,0 +1,219 @@
package alerts
import (
"strconv"
"strings"
"github.com/rs/zerolog/log"
)
type guestMetricIdentity struct {
instance string
node string
vmid int
resourceSuffix string
}
func isGuestMetricResourceType(resourceType string) bool {
switch strings.ToLower(strings.TrimSpace(resourceType)) {
case "guest", "vm", "container", "system-container":
return true
default:
return false
}
}
func parseGuestMetricIdentity(resourceID string) (guestMetricIdentity, bool) {
resourceID = strings.TrimSpace(resourceID)
if resourceID == "" {
return guestMetricIdentity{}, false
}
parts := strings.Split(resourceID, ":")
if len(parts) < 3 {
return guestMetricIdentity{}, false
}
last := strings.TrimSpace(parts[len(parts)-1])
digitCount := 0
for digitCount < len(last) && last[digitCount] >= '0' && last[digitCount] <= '9' {
digitCount++
}
if digitCount == 0 {
return guestMetricIdentity{}, false
}
vmid, err := strconv.Atoi(last[:digitCount])
if err != nil || vmid <= 0 {
return guestMetricIdentity{}, false
}
suffix := last[digitCount:]
if suffix != "" && !strings.HasPrefix(suffix, "-") {
return guestMetricIdentity{}, false
}
instance := strings.TrimSpace(strings.Join(parts[:len(parts)-2], ":"))
node := strings.TrimSpace(parts[len(parts)-2])
if instance == "" {
instance = node
}
if instance == "" || node == "" {
return guestMetricIdentity{}, false
}
return guestMetricIdentity{
instance: instance,
node: node,
vmid: vmid,
resourceSuffix: suffix,
}, true
}
func sameStableGuestMetricIdentity(left, right guestMetricIdentity) bool {
return left.instance == right.instance &&
left.vmid == right.vmid &&
left.resourceSuffix == right.resourceSuffix
}
func (m *Manager) migrateGuestMetricAlertNoLock(storageKey, specID, kind, resourceID, resourceName, node, instance, resourceType string) *Alert {
if !isGuestMetricResourceType(resourceType) {
return nil
}
currentIdentity, ok := parseGuestMetricIdentity(resourceID)
if !ok {
return nil
}
normalizedInstance := strings.TrimSpace(instance)
if normalizedInstance == "" {
normalizedInstance = currentIdentity.instance
}
normalizedNode := strings.TrimSpace(node)
if normalizedNode == "" {
normalizedNode = currentIdentity.node
}
var matchedStorageKey string
var matchedAlert *Alert
matchCount := 0
for existingStorageKey, alert := range m.activeAlerts {
if existingStorageKey == storageKey || alert == nil {
continue
}
backfillCanonicalIdentity(alert)
if alert.CanonicalSpecID != specID {
continue
}
existingIdentity, ok := parseGuestMetricIdentity(alert.ResourceID)
if !ok || !sameStableGuestMetricIdentity(existingIdentity, currentIdentity) {
continue
}
matchCount++
if matchedAlert == nil || alert.LastSeen.After(matchedAlert.LastSeen) {
matchedStorageKey = existingStorageKey
matchedAlert = alert
}
}
if matchedAlert == nil {
return nil
}
oldTrackingKey := canonicalTrackingKeyOrFallback(matchedAlert, matchedStorageKey)
delete(m.activeAlerts, matchedStorageKey)
m.unregisterActiveAlertAliasNoLock(matchedStorageKey, matchedAlert)
matchedAlert.ID = storageKey
matchedAlert.ResourceID = resourceID
matchedAlert.ResourceName = resourceName
matchedAlert.Node = normalizedNode
matchedAlert.Instance = normalizedInstance
if dn := m.resolveNodeDisplayName(normalizedNode); dn != "" {
matchedAlert.NodeDisplayName = dn
} else {
matchedAlert.NodeDisplayName = ""
}
applyCanonicalIdentity(matchedAlert, specID, kind)
m.setActiveAlertNoLock(storageKey, matchedAlert)
m.moveAlertTrackingStateNoLock(oldTrackingKey, storageKey, matchedAlert)
if matchCount > 1 {
log.Warn().
Str("resourceID", resourceID).
Str("metricSpecID", specID).
Int("matches", matchCount).
Msg("Multiple guest metric alerts matched node-move identity; migrated the most recently seen alert")
}
log.Info().
Str("oldTrackingKey", oldTrackingKey).
Str("newTrackingKey", storageKey).
Str("resourceID", resourceID).
Str("metricSpecID", specID).
Msg("Migrated guest metric alert to current node identity")
return matchedAlert
}
func (m *Manager) moveAlertTrackingStateNoLock(oldTrackingKey, newTrackingKey string, alert *Alert) {
if oldTrackingKey == "" || newTrackingKey == "" || oldTrackingKey == newTrackingKey {
return
}
if pending, exists := m.pendingAlerts[oldTrackingKey]; exists {
delete(m.pendingAlerts, oldTrackingKey)
m.pendingAlerts[newTrackingKey] = pending
}
if recent, exists := m.recentAlerts[oldTrackingKey]; exists {
delete(m.recentAlerts, oldTrackingKey)
if alert != nil {
m.recentAlerts[newTrackingKey] = alert
} else {
m.recentAlerts[newTrackingKey] = recent
}
} else if alert != nil {
m.recentAlerts[newTrackingKey] = alert
}
if until, exists := m.suppressedUntil[oldTrackingKey]; exists {
delete(m.suppressedUntil, oldTrackingKey)
m.suppressedUntil[newTrackingKey] = until
}
if rateLimit, exists := m.alertRateLimit[oldTrackingKey]; exists {
delete(m.alertRateLimit, oldTrackingKey)
m.alertRateLimit[newTrackingKey] = rateLimit
}
if record, exists := m.ackStateByCanonical[oldTrackingKey]; exists {
delete(m.ackStateByCanonical, oldTrackingKey)
m.ackStateByCanonical[newTrackingKey] = record
}
if record, exists := m.ackState[oldTrackingKey]; exists {
delete(m.ackState, oldTrackingKey)
m.ackState[newTrackingKey] = record
}
if flapping, exists := m.flappingHistory[oldTrackingKey]; exists {
delete(m.flappingHistory, oldTrackingKey)
m.flappingHistory[newTrackingKey] = flapping
}
if active, exists := m.flappingActive[oldTrackingKey]; exists {
delete(m.flappingActive, oldTrackingKey)
m.flappingActive[newTrackingKey] = active
}
if alert != nil {
m.historyManager.MigrateActiveAlert(oldTrackingKey, *alert)
}
}

View file

@ -188,6 +188,32 @@ func (hm *HistoryManager) UpdateAlertLastSeenForAlert(alert *Alert, lastSeen tim
}
}
// MigrateActiveAlert updates the most recent history entry for an in-flight
// alert when its canonical runtime identity changes, such as a guest metric
// alert moving from one node-scoped resource key to another after a VM move.
func (hm *HistoryManager) MigrateActiveAlert(oldTrackingKey string, updated Alert) {
if oldTrackingKey == "" {
return
}
updatedAlert := updated.Clone()
if updatedAlert == nil {
return
}
backfillCanonicalIdentity(updatedAlert)
hm.mu.Lock()
defer hm.mu.Unlock()
for i := len(hm.history) - 1; i >= 0; i-- {
entry := hm.history[i].Alert.Clone()
if historyIdentityKey(entry) == oldTrackingKey || hm.history[i].Alert.ID == oldTrackingKey {
hm.history[i].Alert = *updatedAlert
return
}
}
}
// GetHistory returns alert history within the specified time range
func (hm *HistoryManager) GetHistory(since time.Time, limit int) []Alert {
hm.mu.RLock()

View file

@ -195,6 +195,54 @@ func TestUpdateAlertLastSeenForAlertMatchesCanonicalState(t *testing.T) {
}
}
func TestMigrateActiveAlertMatchesCanonicalState(t *testing.T) {
hm := newTestHistoryManager(t)
start := time.Now().Add(-10 * time.Minute)
oldResourceID := BuildGuestKey("pve1", "node1", 101)
newResourceID := BuildGuestKey("pve1", "node2", 101)
specID := canonicalMetricSpecID(oldResourceID, "cpu")
oldState := buildCanonicalStateID(oldResourceID, specID)
newState := buildCanonicalStateID(newResourceID, specID)
hm.history = []HistoryEntry{
{
Alert: Alert{
ID: oldState,
Type: "cpu",
ResourceID: oldResourceID,
CanonicalSpecID: specID,
CanonicalState: oldState,
Node: "node1",
Instance: "pve1",
StartTime: start,
},
Timestamp: start,
},
}
hm.MigrateActiveAlert(oldState, Alert{
ID: newState,
Type: "cpu",
ResourceID: newResourceID,
CanonicalSpecID: specID,
CanonicalState: newState,
Node: "node2",
Instance: "pve1",
StartTime: start,
})
if hm.history[0].Alert.ID != newState {
t.Fatalf("history alert ID = %q, want %q", hm.history[0].Alert.ID, newState)
}
if hm.history[0].Alert.ResourceID != newResourceID {
t.Fatalf("history resource ID = %q, want %q", hm.history[0].Alert.ResourceID, newResourceID)
}
if hm.history[0].Alert.Node != "node2" {
t.Fatalf("history node = %q, want node2", hm.history[0].Alert.Node)
}
}
func TestOnAlert(t *testing.T) {
// t.Parallel()

View file

@ -549,6 +549,131 @@ func TestAlertCharacterizationResolvedLookupByCanonicalStateAlias(t *testing.T)
}
}
func TestAlertCharacterizationGuestMetricAlertMigratesAcrossNodeMove(t *testing.T) {
oldResourceID := BuildGuestKey("pve1", "node1", 101)
newResourceID := BuildGuestKey("pve1", "node2", 101)
oldState := canonicalMetricStateID(oldResourceID, "cpu")
newState := canonicalMetricStateID(newResourceID, "cpu")
m := newCharacterizationManager(t, characterizationBaseConfig())
m.CheckGuest(testVM(oldResourceID, 101, "app01", "node1", "pve1", "running", 0.95), "pve1")
if err := m.AcknowledgeAlert(oldState, "alice"); err != nil {
t.Fatalf("AcknowledgeAlert(%q) error = %v", oldState, err)
}
m.mu.Lock()
m.suppressedUntil[oldState] = time.Now().Add(time.Hour)
m.alertRateLimit[oldState] = []time.Time{time.Now()}
m.flappingHistory[oldState] = []time.Time{time.Now()}
m.flappingActive[oldState] = true
m.mu.Unlock()
m.CheckGuest(testVM(newResourceID, 101, "app01", "node2", "pve1", "running", 0.92), "pve1")
assertAlertMissing(t, m, oldState)
alert := activeAlert(t, m, newState)
if alert.ResourceID != newResourceID {
t.Fatalf("ResourceID = %q, want %q", alert.ResourceID, newResourceID)
}
if alert.Node != "node2" {
t.Fatalf("Node = %q, want node2", alert.Node)
}
if !alert.Acknowledged || alert.AckUser != "alice" {
t.Fatalf("expected acknowledgment to follow migrated alert, got acknowledged=%t user=%q", alert.Acknowledged, alert.AckUser)
}
m.mu.RLock()
_, oldAck := m.ackStateByCanonical[oldState]
record, newAck := m.ackStateByCanonical[newState]
_, oldSuppression := m.suppressedUntil[oldState]
_, newSuppression := m.suppressedUntil[newState]
_, oldRate := m.alertRateLimit[oldState]
_, newRate := m.alertRateLimit[newState]
_, oldFlapping := m.flappingHistory[oldState]
_, newFlapping := m.flappingHistory[newState]
m.mu.RUnlock()
if oldAck || !newAck || !record.acknowledged || record.user != "alice" {
t.Fatalf("expected canonical ack record to migrate, old=%t new=%t record=%+v", oldAck, newAck, record)
}
if oldSuppression || !newSuppression || oldRate || !newRate || oldFlapping || !newFlapping {
t.Fatalf(
"expected tracking state to migrate, suppression old=%t new=%t rate old=%t new=%t flapping old=%t new=%t",
oldSuppression,
newSuppression,
oldRate,
newRate,
oldFlapping,
newFlapping,
)
}
history := m.GetAlertHistory(5)
if len(history) != 1 || history[0].ID != newState {
t.Fatalf("expected migrated history entry under %q, got %#v", newState, history)
}
}
func TestAlertCharacterizationGuestMetricResolutionUsesCurrentNodeIdentityAfterMove(t *testing.T) {
oldResourceID := BuildGuestKey("pve1", "node1", 101)
newResourceID := BuildGuestKey("pve1", "node2", 101)
newState := canonicalMetricStateID(newResourceID, "cpu")
m := newCharacterizationManager(t, characterizationBaseConfig())
m.CheckGuest(testVM(oldResourceID, 101, "app01", "node1", "pve1", "running", 0.95), "pve1")
m.CheckGuest(testVM(newResourceID, 101, "app01", "node2", "pve1", "running", 0.70), "pve1")
assertAlertMissing(t, m, canonicalMetricStateID(oldResourceID, "cpu"))
assertAlertMissing(t, m, newState)
resolved := m.GetResolvedAlert(newState)
if resolved == nil || resolved.Alert == nil {
t.Fatalf("expected resolved alert lookup by migrated canonical state %q", newState)
}
if resolved.Alert.ResourceID != newResourceID {
t.Fatalf("resolved alert resource ID = %q, want %q", resolved.Alert.ResourceID, newResourceID)
}
history := m.GetAlertHistory(5)
if len(history) != 1 || history[0].ID != newState {
t.Fatalf("expected resolved history entry under %q, got %#v", newState, history)
}
}
func TestAlertCharacterizationGuestPerDiskMetricAlertMigratesAcrossNodeMove(t *testing.T) {
oldResourceID := BuildGuestKey("pve1", "node1", 101) + "-disk-root"
newResourceID := BuildGuestKey("pve1", "node2", 101) + "-disk-root"
threshold := &HysteresisThreshold{Trigger: 90, Clear: 85}
oldSpec, err := buildCanonicalMetricSpec(oldResourceID, "app01", unifiedresources.ResourceTypeVM, "disk", threshold)
if err != nil {
t.Fatalf("buildCanonicalMetricSpec(oldResourceID) error = %v", err)
}
newSpec, err := buildCanonicalMetricSpec(newResourceID, "app01", unifiedresources.ResourceTypeVM, "disk", threshold)
if err != nil {
t.Fatalf("buildCanonicalMetricSpec(newResourceID) error = %v", err)
}
oldState := buildCanonicalStateID(oldResourceID, oldSpec.ID)
newState := buildCanonicalStateID(newResourceID, newSpec.ID)
m := newCharacterizationManager(t, characterizationBaseConfig())
m.checkMetricWithCanonicalSpec(oldSpec, "app01", "node1", "pve1", "VM", 95, threshold, &metricOptions{Message: "VM disk (/root) at 95%"})
m.checkMetricWithCanonicalSpec(newSpec, "app01", "node2", "pve1", "VM", 95, threshold, &metricOptions{Message: "VM disk (/root) at 95%"})
assertAlertMissing(t, m, oldState)
alert := activeAlert(t, m, newState)
if alert.ResourceID != newResourceID {
t.Fatalf("ResourceID = %q, want %q", alert.ResourceID, newResourceID)
}
history := m.GetAlertHistory(5)
if len(history) != 1 || history[0].ID != newState {
t.Fatalf("expected per-disk history entry under %q, got %#v", newState, history)
}
}
func TestAlertCharacterizationManualClearRemovesCanonicalTrackingState(t *testing.T) {
resourceID := BuildGuestKey("pve1", "node1", 101)
specID := canonicalMetricSpecID(resourceID, "cpu")

View file

@ -214,6 +214,102 @@ func TestReevaluateActiveAlertsUsesStableClusterGuestOverrideAcrossNodeMove(t *t
}
}
func TestCheckMetricMigratesGuestAlertAcrossNodeMove(t *testing.T) {
manager := newTestManager(t)
manager.ClearActiveAlerts()
oldResourceID := BuildGuestKey("pve1", "node1", 101)
newResourceID := BuildGuestKey("pve1", "node2", 101)
oldState := canonicalMetricStateID(oldResourceID, "cpu")
newState := canonicalMetricStateID(newResourceID, "cpu")
start := time.Now().Add(-10 * time.Minute)
ackTime := start.Add(time.Minute)
alert := &Alert{
ID: oldState,
Type: "cpu",
Level: AlertLevelWarning,
ResourceID: oldResourceID,
CanonicalSpecID: canonicalMetricSpecID(oldResourceID, "cpu"),
CanonicalKind: "metric-threshold",
CanonicalState: oldState,
ResourceName: "vm101",
Node: "node1",
Instance: "pve1",
Message: "VM cpu at 95%",
Value: 95,
Threshold: 80,
StartTime: start,
LastSeen: start.Add(5 * time.Minute),
Acknowledged: true,
AckUser: "tester",
AckTime: &ackTime,
}
manager.mu.Lock()
manager.setActiveAlertNoLock(oldState, alert)
manager.recentAlerts[oldState] = alert
manager.suppressedUntil[oldState] = start.Add(2 * time.Minute)
manager.alertRateLimit[oldState] = []time.Time{start.Add(30 * time.Second)}
manager.ackStateByCanonical[oldState] = ackRecord{
acknowledged: true,
user: "tester",
time: ackTime,
}
manager.flappingHistory[oldState] = []time.Time{start.Add(45 * time.Second)}
manager.flappingActive[oldState] = true
manager.historyManager.AddAlert(*alert)
manager.mu.Unlock()
manager.checkMetric(newResourceID, "vm101", "node2", "pve1", "VM", "cpu", 92, &HysteresisThreshold{Trigger: 80, Clear: 70}, nil)
manager.mu.RLock()
migrated, exists := manager.activeAlerts[newState]
_, oldExists := manager.activeAlerts[oldState]
_, oldAckExists := manager.ackStateByCanonical[oldState]
newAck, newAckExists := manager.ackStateByCanonical[newState]
_, oldSuppressed := manager.suppressedUntil[oldState]
_, newSuppressed := manager.suppressedUntil[newState]
_, oldRateLimit := manager.alertRateLimit[oldState]
_, newRateLimit := manager.alertRateLimit[newState]
_, oldFlapping := manager.flappingHistory[oldState]
_, newFlapping := manager.flappingHistory[newState]
manager.mu.RUnlock()
if oldExists {
t.Fatal("expected old node-scoped alert to be removed")
}
if !exists {
t.Fatal("expected alert to migrate to the current node-scoped canonical state")
}
if migrated.Node != "node2" || migrated.ResourceID != newResourceID {
t.Fatalf("expected migrated alert to target node2, got node=%q resource=%q", migrated.Node, migrated.ResourceID)
}
if !migrated.Acknowledged || migrated.AckUser != "tester" {
t.Fatalf("expected migrated alert acknowledgment to be preserved, got %#v", migrated)
}
if migrated.StartTime != start {
t.Fatalf("expected migrated alert start time to be preserved, got %v want %v", migrated.StartTime, start)
}
if oldAckExists || !newAckExists || !newAck.acknowledged {
t.Fatalf("expected canonical ack state to move to new alert ID, old=%v new=%v", oldAckExists, newAckExists)
}
if oldSuppressed || !newSuppressed {
t.Fatalf("expected suppression window to move to new alert ID, old=%v new=%v", oldSuppressed, newSuppressed)
}
if oldRateLimit || !newRateLimit {
t.Fatalf("expected rate limit state to move to new alert ID, old=%v new=%v", oldRateLimit, newRateLimit)
}
if oldFlapping || !newFlapping {
t.Fatalf("expected flapping state to move to new alert ID, old=%v new=%v", oldFlapping, newFlapping)
}
history := manager.GetAlertHistory(1)
if len(history) != 1 || history[0].ID != newState {
t.Fatalf("expected history entry to follow migrated alert ID, got %#v", history)
}
}
// TestReevaluateActiveAlertsStillAboveThreshold tests that alerts stay active if still above threshold
func TestReevaluateActiveAlertsStillAboveThreshold(t *testing.T) {
manager := NewManager()