Fix guest suppression for posture alerts

Refs #1545
This commit is contained in:
rcourtman
2026-07-09 20:28:52 +01:00
parent 910418c3b2
commit c59af9a501
10 changed files with 302 additions and 82 deletions
@@ -247,6 +247,12 @@ posture alerts. Snapshot age, backup age, powered-off state, and
configuration-change reevaluation must all construct a canonical lightweight
guest snapshot and route threshold resolution through the shared
guest-defaults → filter-driven custom rules → guest-override chain.
That canonical guest context must preserve the live guest name and tags for
snapshot and backup posture evaluation. Ignored prefixes, `pulse-no-alerts`,
configured ignored tags, and required-tag filtering must resolve through the
same guest alert policy before any guest-derived alert is created; posture
pollers may not downgrade that context to a name-only lookup that bypasses the
operator's suppression policy.
Passing `nil` guest context or resolving only overrides/defaults is forbidden
because it silently bypasses custom guest rules and makes guest lifecycle
alerting diverge from running-guest metric truth.
@@ -239,6 +239,13 @@ resource health.
guest slices as the primary source of truth. Clustered PVE snapshot polling
must therefore see guests collected earlier in the same cycle before calling
the Proxmox guest snapshot APIs.
Guest lookups handed from backup/snapshot polling to alert evaluation must
preserve the canonical instance, node, VMID, type, live display name, and
live guest tags from read-state. Monitoring must key snapshot lookups with
the shared alert guest identity and must not downgrade the handoff to a
name-only map, because ignored-name prefixes, `pulse-no-alerts`, ignored
tags, and required-tag filtering are alerts-owned policies that require the
same guest context as ordinary threshold evaluation.
PBS backup snapshot refreshes in that same file must stay bounded by the
package worker-pool constant and stream requests through workers instead of
creating one goroutine per backup group; per-group API failures may reuse
+105 -14
View File
@@ -1066,11 +1066,11 @@ func TestCheckSnapshotsForInstanceCreatesAndClearsAlerts(t *testing.T) {
SizeBytes: 60 << 30,
},
}
guestNames := map[string]string{
"inst:node:100": "app-server",
guestLookups := map[string]GuestLookup{
"inst:node:100": {Name: "app-server"},
}
m.CheckSnapshotsForInstance("inst", snapshots, guestNames)
m.CheckSnapshotsForInstance("inst", snapshots, guestLookups)
m.mu.RLock()
snapshotSpecID := "inst:node:100/snapshot:inst-node-100-weekly"
@@ -1092,7 +1092,29 @@ func TestCheckSnapshotsForInstanceCreatesAndClearsAlerts(t *testing.T) {
t.Fatalf("canonicalSpecID = %v, want inst:node:100/snapshot:inst-node-100-weekly", got)
}
m.CheckSnapshotsForInstance("inst", nil, guestNames)
lookup := guestLookups["inst:node:100"]
lookup.Tags = []string{"pulse-no-alerts"}
guestLookups["inst:node:100"] = lookup
m.CheckSnapshotsForInstance("inst", snapshots, guestLookups)
m.mu.RLock()
_, exists = testLookupActiveAlert(t, m, buildCanonicalStateID("inst:node:100", snapshotSpecID))
m.mu.RUnlock()
if exists {
t.Fatalf("expected pulse-no-alerts to clear the snapshot alert")
}
lookup.Tags = nil
guestLookups["inst:node:100"] = lookup
m.CheckSnapshotsForInstance("inst", snapshots, guestLookups)
m.mu.RLock()
_, exists = testLookupActiveAlert(t, m, buildCanonicalStateID("inst:node:100", snapshotSpecID))
m.mu.RUnlock()
if !exists {
t.Fatalf("expected snapshot alert to return after suppression is removed")
}
m.CheckSnapshotsForInstance("inst", nil, guestLookups)
m.mu.RLock()
_, exists = m.activeAlerts[buildCanonicalStateID("inst:node:100", snapshotSpecID)]
@@ -1102,6 +1124,51 @@ func TestCheckSnapshotsForInstanceCreatesAndClearsAlerts(t *testing.T) {
}
}
func TestCheckSnapshotsRespectsConfiguredGuestTagBlacklist(t *testing.T) {
m := newTestManager(t)
m.ClearActiveAlerts()
cfg := AlertConfig{
Enabled: true,
SnapshotDefaults: SnapshotAlertConfig{
Enabled: true,
WarningDays: 7,
CriticalDays: 14,
},
GuestTagBlacklist: []string{"maintenance"},
}
m.UpdateConfig(cfg)
snapshot := models.GuestSnapshot{
ID: "inst-node-100-weekly",
Name: "weekly",
Node: "node",
Instance: "inst",
Type: "qemu",
VMID: 100,
Time: time.Now().Add(-10 * 24 * time.Hour),
}
guestLookups := map[string]GuestLookup{
"inst:node:100": {
Name: "app-server",
Instance: "inst",
Node: "node",
Type: "qemu",
VMID: 100,
Tags: []string{"production", "maintenance"},
},
}
m.CheckSnapshotsForInstance("inst", []models.GuestSnapshot{snapshot}, guestLookups)
m.mu.RLock()
_, exists := testLookupActiveAlert(t, m, buildCanonicalStateID("inst:node:100", "inst:node:100/snapshot:inst-node-100-weekly"))
m.mu.RUnlock()
if exists {
t.Fatalf("expected configured ignored tag to suppress snapshot alert")
}
}
func TestCheckSnapshotsRespectsOverrides(t *testing.T) {
m := newTestManager(t)
m.ClearActiveAlerts()
@@ -1131,12 +1198,12 @@ func TestCheckSnapshotsRespectsOverrides(t *testing.T) {
},
}
resourceKey := "inst:node:100"
guestNames := map[string]string{
resourceKey: "app-server",
guestLookups := map[string]GuestLookup{
resourceKey: {Name: "app-server"},
}
// 1. Verify warning alert is created
m.CheckSnapshotsForInstance("inst", snapshots, guestNames)
m.CheckSnapshotsForInstance("inst", snapshots, guestLookups)
m.mu.RLock()
alert, exists := testLookupActiveAlert(t, m, "snapshot-age-inst:node:100:weekly")
m.mu.RUnlock()
@@ -1155,7 +1222,7 @@ func TestCheckSnapshotsRespectsOverrides(t *testing.T) {
},
}
m.UpdateConfig(cfg)
m.CheckSnapshotsForInstance("inst", snapshots, guestNames)
m.CheckSnapshotsForInstance("inst", snapshots, guestLookups)
m.mu.RLock()
_, exists = m.activeAlerts["snapshot-age-inst:node:100:weekly"]
m.mu.RUnlock()
@@ -1198,11 +1265,11 @@ func TestCheckSnapshotsForInstanceTriggersOnSnapshotSize(t *testing.T) {
SizeBytes: int64(120) << 30,
},
}
guestNames := map[string]string{
"inst:node:200": "db-server",
guestLookups := map[string]GuestLookup{
"inst:node:200": {Name: "db-server"},
}
m.CheckSnapshotsForInstance("inst", snapshots, guestNames)
m.CheckSnapshotsForInstance("inst", snapshots, guestLookups)
m.mu.RLock()
alert, exists := testLookupActiveAlert(t, m, "snapshot-age-inst-node-200-sizey")
@@ -1278,11 +1345,11 @@ func TestCheckSnapshotsForInstanceIncludesAgeAndSizeReasons(t *testing.T) {
SizeBytes: int64(90) << 30,
},
}
guestNames := map[string]string{
"inst:node:300": "app-server",
guestLookups := map[string]GuestLookup{
"inst:node:300": {Name: "app-server"},
}
m.CheckSnapshotsForInstance("inst", snapshots, guestNames)
m.CheckSnapshotsForInstance("inst", snapshots, guestLookups)
m.mu.RLock()
alert, exists := testLookupActiveAlert(t, m, "snapshot-age-inst-node-300-combined")
@@ -1375,6 +1442,30 @@ func TestCheckBackupsCreatesAndClearsAlerts(t *testing.T) {
t.Fatalf("canonicalSpecID = %v, want inst:node:100-backup-age", got)
}
lookup := guestsByKey[key]
lookup.Tags = []string{"pulse-no-alerts"}
guestsByKey[key] = lookup
guestsByVMID["100"] = []GuestLookup{lookup}
m.CheckBackups(rollups, guestsByKey, guestsByVMID)
m.mu.RLock()
_, exists = testLookupActiveAlert(t, m, buildCanonicalStateID("inst:node:100", backupSpecID))
m.mu.RUnlock()
if exists {
t.Fatalf("expected pulse-no-alerts to clear the backup alert")
}
lookup.Tags = nil
guestsByKey[key] = lookup
guestsByVMID["100"] = []GuestLookup{lookup}
m.CheckBackups(rollups, guestsByKey, guestsByVMID)
m.mu.RLock()
_, exists = testLookupActiveAlert(t, m, buildCanonicalStateID("inst:node:100", backupSpecID))
m.mu.RUnlock()
if !exists {
t.Fatalf("expected backup alert to return after suppression is removed")
}
// Recent backup clears alert
rollups[0].LastSuccessAt = ptrTime(now)
m.CheckBackups(rollups, guestsByKey, guestsByVMID)
+34 -4
View File
@@ -280,7 +280,7 @@ func canonicalBackupSubjectResourceID(alertKey string, record backupRecord) stri
}
// CheckSnapshotsForInstance evaluates guest snapshots for age-based alerts.
func (m *Manager) CheckSnapshotsForInstance(instanceName string, snapshots []models.GuestSnapshot, guestNames map[string]string) {
func (m *Manager) CheckSnapshotsForInstance(instanceName string, snapshots []models.GuestSnapshot, guestsByKey map[string]GuestLookup) {
m.mu.RLock()
enabled := m.config.Enabled
snapshotCfg := m.config.SnapshotDefaults
@@ -319,9 +319,32 @@ func (m *Manager) CheckSnapshotsForInstance(instanceName string, snapshots []mod
}
// Determine thresholds for this snapshot
resourceID := fmt.Sprintf("%s:%s:%d", snapshot.Instance, snapshot.Node, snapshot.VMID)
guestName := strings.TrimSpace(guestNames[BuildGuestKey(snapshot.Instance, snapshot.Node, snapshot.VMID)])
guestContext := guestSnapshotFromIdentity(resourceID, guestName, snapshot.Node, snapshot.Instance, snapshot.Type, "")
resourceID := BuildGuestKey(snapshot.Instance, snapshot.Node, snapshot.VMID)
lookup := guestsByKey[resourceID]
if lookup.ResourceID == "" {
lookup.ResourceID = resourceID
}
if lookup.Instance == "" {
lookup.Instance = snapshot.Instance
}
if lookup.Node == "" {
lookup.Node = snapshot.Node
}
if lookup.Type == "" {
lookup.Type = snapshot.Type
}
if lookup.VMID <= 0 {
lookup.VMID = snapshot.VMID
}
guestContext := guestSnapshotFromLookup(lookup, "")
guestName := guestContext.Name
policy := m.resolveGuestAlertPolicy(guestContext)
if policy.SuppressionReason != "" {
if cleared := m.suppressGuestAlerts(resourceID); cleared {
m.saveActiveAlertsAsync(policy.SuppressionReason)
}
continue
}
m.mu.RLock()
gh := m.getGuestThresholds(guestContext, resourceID)
m.mu.RUnlock()
@@ -756,6 +779,13 @@ func (m *Manager) CheckBackupsWithInventory(
guestResourceID = guestContext.ID
}
if guestResourceID != "" {
policy := m.resolveGuestAlertPolicy(guestContext)
if policy.SuppressionReason != "" {
if cleared := m.suppressGuestAlerts(guestResourceID); cleared {
m.saveActiveAlertsAsync(policy.SuppressionReason)
}
continue
}
m.mu.RLock()
gh := m.getGuestThresholds(guestContext, guestResourceID)
m.mu.RUnlock()
+1
View File
@@ -209,6 +209,7 @@ type GuestLookup struct {
Node string
Type string
VMID int
Tags []string
}
// AlertConfig represents the complete alert configuration
+60 -56
View File
@@ -97,9 +97,6 @@ func (m *Manager) CheckGuest(guest any, instanceName string) {
enabled := m.config.Enabled
disableAllGuests := m.config.DisableAllGuests
disableAllGuestsOffline := m.config.DisableAllGuestsOffline
ignoredGuestPrefixes := m.config.IgnoredGuestPrefixes
guestTagWhitelist := m.config.GuestTagWhitelist
guestTagBlacklist := m.config.GuestTagBlacklist
m.mu.RUnlock()
if !enabled {
@@ -132,7 +129,6 @@ func (m *Manager) CheckGuest(guest any, instanceName string) {
netIn := snapshot.NetworkIn
netOut := snapshot.NetworkOut
disks := snapshot.Disks
tags := snapshot.Tags
// Debug logging for high memory VMs
if snapshot.Kind == guestKindVM && memUsage > 85 {
@@ -143,65 +139,19 @@ func (m *Manager) CheckGuest(guest any, instanceName string) {
Msg("VM with high memory detected in CheckGuest")
}
// Check ignored prefixes
for _, prefix := range ignoredGuestPrefixes {
if prefix != "" && strings.HasPrefix(name, prefix) {
if cleared := m.suppressGuestAlerts(guestID); cleared {
m.saveActiveAlertsAsync("ignored-prefix")
}
return
}
}
settings := parsePulseTags(tags)
if settings.Suppress {
policy := m.resolveGuestAlertPolicy(snapshot)
if policy.SuppressionReason != "" {
if cleared := m.suppressGuestAlerts(guestID); cleared {
m.saveActiveAlertsAsync("pulse-no-alerts")
m.saveActiveAlertsAsync(policy.SuppressionReason)
}
log.Debug().
Str("guestID", guestID).
Msg("Pulse no-alerts tag active; suppressing guest alerts")
Str("reason", policy.SuppressionReason).
Msg("Guest alert policy suppressed alerts")
return
}
// Custom Tag Filtering
if len(guestTagBlacklist) > 0 || len(guestTagWhitelist) > 0 {
// Normalize tags once for checking
normalizedTags := make(map[string]bool)
for _, tag := range tags {
normalizedTags[strings.ToLower(strings.TrimSpace(tag))] = true
}
// Check Blacklist
for _, block := range guestTagBlacklist {
if normalizedTags[strings.ToLower(strings.TrimSpace(block))] {
if cleared := m.suppressGuestAlerts(guestID); cleared {
m.saveActiveAlertsAsync("tag-blacklist")
}
log.Debug().Str("guestID", guestID).Msg("guest suppressed by tag blacklist")
return
}
}
// Check Whitelist
if len(guestTagWhitelist) > 0 {
found := false
for _, allow := range guestTagWhitelist {
if normalizedTags[strings.ToLower(strings.TrimSpace(allow))] {
found = true
break
}
}
if !found {
if cleared := m.suppressGuestAlerts(guestID); cleared {
m.saveActiveAlertsAsync("tag-whitelist")
}
log.Debug().Str("guestID", guestID).Msg("guest suppressed by tag whitelist (required tag not found)")
return
}
}
}
settings := policy.TagSettings
monitorOnly := settings.MonitorOnly
if monitorOnly || m.guestHasMonitorOnlyAlerts(guestID) {
log.Debug().
@@ -501,6 +451,60 @@ type pulseTagSettings struct {
Relaxed bool
}
type guestAlertPolicy struct {
TagSettings pulseTagSettings
SuppressionReason string
}
func (m *Manager) resolveGuestAlertPolicy(snapshot guestSnapshot) guestAlertPolicy {
m.mu.RLock()
ignoredPrefixes := append([]string(nil), m.config.IgnoredGuestPrefixes...)
tagWhitelist := append([]string(nil), m.config.GuestTagWhitelist...)
tagBlacklist := append([]string(nil), m.config.GuestTagBlacklist...)
m.mu.RUnlock()
policy := guestAlertPolicy{TagSettings: parsePulseTags(snapshot.Tags)}
for _, prefix := range ignoredPrefixes {
if prefix != "" && strings.HasPrefix(snapshot.Name, prefix) {
policy.SuppressionReason = "ignored-prefix"
return policy
}
}
if policy.TagSettings.Suppress {
policy.SuppressionReason = "pulse-no-alerts"
return policy
}
if len(tagBlacklist) == 0 && len(tagWhitelist) == 0 {
return policy
}
normalizedTags := make(map[string]struct{}, len(snapshot.Tags))
for _, tag := range snapshot.Tags {
normalizedTags[strings.ToLower(strings.TrimSpace(tag))] = struct{}{}
}
for _, blockedTag := range tagBlacklist {
if _, blocked := normalizedTags[strings.ToLower(strings.TrimSpace(blockedTag))]; blocked {
policy.SuppressionReason = "tag-blacklist"
return policy
}
}
if len(tagWhitelist) == 0 {
return policy
}
for _, allowedTag := range tagWhitelist {
if _, allowed := normalizedTags[strings.ToLower(strings.TrimSpace(allowedTag))]; allowed {
return policy
}
}
policy.SuppressionReason = "tag-whitelist"
return policy
}
func parsePulseTags(tags []string) pulseTagSettings {
settings := pulseTagSettings{}
for _, raw := range tags {
+1
View File
@@ -188,6 +188,7 @@ func guestSnapshotFromLookup(lookup GuestLookup, fallbackName string) guestSnaps
if snapshot.VMID <= 0 && lookup.VMID > 0 {
snapshot.VMID = lookup.VMID
}
snapshot.Tags = append([]string(nil), lookup.Tags...)
return snapshot.normalizeCollections()
}
@@ -437,12 +437,12 @@ func TestCheckSnapshotsUsesGuestContextForCustomRules(t *testing.T) {
Time: now.Add(-10 * 24 * time.Hour),
},
}
guestNames := map[string]string{
BuildGuestKey("inst", "node", 100): "db-server",
BuildGuestKey("inst", "node", 101): "web-server",
guestLookups := map[string]GuestLookup{
BuildGuestKey("inst", "node", 100): {Name: "db-server"},
BuildGuestKey("inst", "node", 101): {Name: "web-server"},
}
m.CheckSnapshotsForInstance("inst", snapshots, guestNames)
m.CheckSnapshotsForInstance("inst", snapshots, guestLookups)
m.mu.RLock()
_, dbExists := testLookupActiveAlert(t, m, "snapshot-age-inst-node-100-weekly")
+24 -4
View File
@@ -584,6 +584,7 @@ func buildGuestLookupsFromReadState(readState unifiedresources.ReadState, metada
Node: vm.Node(),
Type: "qemu",
VMID: vm.VMID(),
Tags: vm.Tags(),
}
key := alerts.BuildGuestKey(vm.Instance(), vm.Node(), vm.VMID())
byKey[key] = info
@@ -609,6 +610,7 @@ func buildGuestLookupsFromReadState(readState unifiedresources.ReadState, metada
Node: ct.Node(),
Type: guestType,
VMID: ct.VMID(),
Tags: ct.Tags(),
}
key := alerts.BuildGuestKey(ct.Instance(), ct.Node(), ct.VMID())
if _, exists := byKey[key]; !exists {
@@ -836,12 +838,30 @@ func (m *Monitor) pollGuestSnapshots(ctx context.Context, instanceName string, c
return fmt.Sprintf("%s-%s-%d", instance, node, vmid)
}
guestNames := make(map[string]string, len(vms)+len(containers))
guestLookups := make(map[string]alerts.GuestLookup, len(vms)+len(containers))
for _, vm := range vms {
guestNames[guestKey(instanceName, vm.Node, vm.VMID)] = vm.Name
key := alerts.BuildGuestKey(vm.Instance, vm.Node, vm.VMID)
guestLookups[key] = alerts.GuestLookup{
ResourceID: key,
Name: vm.Name,
Instance: vm.Instance,
Node: vm.Node,
Type: "qemu",
VMID: vm.VMID,
Tags: append([]string(nil), vm.Tags...),
}
}
for _, ct := range containers {
guestNames[guestKey(instanceName, ct.Node, ct.VMID)] = ct.Name
key := alerts.BuildGuestKey(ct.Instance, ct.Node, ct.VMID)
guestLookups[key] = alerts.GuestLookup{
ResourceID: key,
Name: ct.Name,
Instance: ct.Instance,
Node: ct.Node,
Type: firstNonEmptyString(ct.Type, "lxc"),
VMID: ct.VMID,
Tags: append([]string(nil), ct.Tags...),
}
}
activeGuests := 0
@@ -1086,7 +1106,7 @@ func (m *Monitor) pollGuestSnapshots(ctx context.Context, instanceName string, c
m.ingestRecoveryPointsAsync(proxmoxrecoverymapper.FromPVEGuestSnapshots(allSnapshots, guestInfo))
if m.alertManager != nil {
m.alertManager.CheckSnapshotsForInstance(instanceName, allSnapshots, guestNames)
m.alertManager.CheckSnapshotsForInstance(instanceName, allSnapshots, guestLookups)
}
log.Debug().
@@ -11,6 +11,7 @@ import (
"testing"
"time"
"github.com/rcourtman/pulse-go-rewrite/internal/alerts"
"github.com/rcourtman/pulse-go-rewrite/internal/models"
proxmoxmapper "github.com/rcourtman/pulse-go-rewrite/internal/recovery/mapper/proxmox"
"github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources"
@@ -87,6 +88,65 @@ func TestPopulateGuestNodeMapFromReadState_UsesCanonicalWorkloads(t *testing.T)
}
}
func TestBuildGuestLookupsFromReadState_PreservesCanonicalIdentityAndTags(t *testing.T) {
readState := backupReadState([]unifiedresources.Resource{
{
ID: "vm-100",
Type: unifiedresources.ResourceTypeVM,
Name: "database",
Status: unifiedresources.StatusOnline,
Tags: []string{"production", "pulse-no-alerts"},
Proxmox: &unifiedresources.ProxmoxData{
Instance: "pve1",
NodeName: "node1",
VMID: 100,
},
},
{
ID: "ct-200",
Type: unifiedresources.ResourceTypeSystemContainer,
Name: "worker",
Status: unifiedresources.StatusOnline,
Tags: []string{"maintenance"},
Proxmox: &unifiedresources.ProxmoxData{
Instance: "pve1",
NodeName: "node2",
VMID: 200,
},
},
})
byKey, byVMID := buildGuestLookupsFromReadState(readState, nil)
vmKey := alerts.BuildGuestKey("pve1", "node1", 100)
vm, ok := byKey[vmKey]
if !ok {
t.Fatalf("expected canonical VM lookup %q", vmKey)
}
if vm.Name != "database" || vm.Instance != "pve1" || vm.Node != "node1" || vm.VMID != 100 || vm.Type != "qemu" {
t.Fatalf("unexpected VM identity: %+v", vm)
}
if len(vm.Tags) != 2 || vm.Tags[0] != "production" || vm.Tags[1] != "pulse-no-alerts" {
t.Fatalf("expected VM tags to survive alert handoff, got %v", vm.Tags)
}
ctKey := alerts.BuildGuestKey("pve1", "node2", 200)
ct, ok := byKey[ctKey]
if !ok {
t.Fatalf("expected canonical container lookup %q", ctKey)
}
if ct.Name != "worker" || ct.Type != "lxc" || len(ct.Tags) != 1 || ct.Tags[0] != "maintenance" {
t.Fatalf("unexpected container lookup: %+v", ct)
}
if candidates := byVMID["100"]; len(candidates) != 1 || len(candidates[0].Tags) != 2 {
t.Fatalf("expected VMID lookup to preserve VM tags, got %+v", candidates)
}
if candidates := byVMID["200"]; len(candidates) != 1 || len(candidates[0].Tags) != 1 {
t.Fatalf("expected VMID lookup to preserve container tags, got %+v", candidates)
}
}
func TestStorageNamesForNode_UsesCanonicalStoragePools(t *testing.T) {
readState := backupReadState([]unifiedresources.Resource{
{