Judge pulse-relaxed guests by one threshold on poll and save (#2705)

Stacked on #2700 (which carries #2696 and #2692). Merge order is
enforced by the stack: this change must not reach main before #2700's
per-filesystem reevaluation (see "Why stacked" below).

## Problem

`pulse-relaxed` lifts a guest's CPU, memory and disk triggers to a
95/92/95 floor. Only `CheckGuest` applied it. Config-save reevaluation
rebuilt the guest from alert metadata without its tags
(`guestSnapshotFromAlert`), so a save judged a relaxed guest's alert by
its unrelaxed thresholds:

- A relaxed guest with an unset memory threshold alerts at 92 while
polling. On save that reads as no threshold, so the save resolved the
alert and sent a recovery notification, and the next poll raised it
again.
- The floor also lifted an Off threshold (trigger 0, or -1 from the
editors) like any low trigger, so tagging a guest relaxed turned an Off
alert on, with no recovery band (`{95, 0}`). A save then re-judged it
Off and resolved it, and the next poll raised it again.

## Change

- The relaxed step moves into `effectiveAlertPolicyNoLock`'s guest
branch, so `CheckGuest`, config-save reevaluation (including #2700's
`resolveGuestAlertThresholdsNoLock`) and posture checks share one
resolver.
- `guestSnapshotFromAlert` reads the recorded `tags` back, as `[]string`
live or `[]interface{}` after a restart, through a new
`metadataStringsValue` reader. `storagePolicyAliases` now uses the same
reader.
- An Off threshold (trigger <= 0) stays Off under `pulse-relaxed`; an
unset one still gets the floor, as the existing tests pin. The tag is
documented (issue #863) as relaxing thresholds, so it must never make a
guest more sensitive than its config.
- `metricOptionsWithTags` drops the `tags` key from an open alert once
its resource has no tags. Reevaluation now reads that key, so a removed
`pulse-relaxed` left behind would judge the alert by the relaxed trigger
while polling uses the guest's own, and the alert would flap.
Notification tag routing reads the same key, so it also stops routing on
removed tags.

## Why stacked

A filesystem's own override replaces the relaxed guest disk threshold in
`CheckGuest`. On main, reevaluation misses filesystem overrides (fixed
by #2700). With this change alone, a save would judge a relaxed guest's
filesystem alert by the 95 floor instead of, say, an 85 override, and
flap it. `TestConfigSaveKeepsRelaxedGuestFilesystemOverride` fails on
main plus this change and passes on this stack.

## Proof

New tests in `internal/alerts/alerts_test.go` and
`internal/alerts/unified_eval_test.go`. With `go test -overlay` mapping
the six runtime files back to #2700's head, these fail:

- `TestConfigSaveJudgesRelaxedGuestAlertByRelaxedThresholds` (the save
resolved the alert)
- `TestPulseRelaxedKeepsOffGuestThresholdOff` (a disk alert was raised
with disk Off)
- `TestApplyRelaxedGuestThresholds/off_thresholds_stay_off` (Off became
`{95, 0}` / `{92, 0}`)
- `TestCheckGuestDropsRemovedTagsFromOpenAlerts` (stale tags; with only
the metadata assertion relaxed, the save resolves the alert)

`go test ./internal/alerts/... ./internal/notifications/` and the
monitoring alert tests pass.

docs/release-control/v6/internal/subsystems/alerts.md

@@ -1473,7 +1473,8 @@ That same guest-threshold owner also governs guest-derived lifecycle and
 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.
+guest-defaults → filter-driven custom rules → guest-override → `pulse-relaxed`
+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
@@ -1483,6 +1484,27 @@ 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.
+The `pulse-relaxed` step lives in `effectiveAlertPolicyNoLock`, not in
+`CheckGuest`, so polling and config-save reevaluation judge a relaxed guest's
+alert by one threshold. Reevaluation rebuilds the guest from alert metadata,
+so `guestSnapshotFromAlert` reads back the `tags` the evaluator recorded
+(`[]string` live, `[]interface{}` after a restart). Without them a save judged
+the alert by unrelaxed thresholds, resolved one raised under an unset
+threshold's relaxed floor with a recovery notification, and the next poll
+raised it again. Those tags must stay current: `metricOptionsWithTags` drops
+the key from an open alert once its resource has no tags, or a removed
+`pulse-relaxed` would keep judging the alert by the relaxed trigger. The tag
+lifts CPU, memory and disk triggers to a 95/92/95 floor and gives an unset
+threshold that floor, but an Off threshold (trigger <= 0) stays Off: relaxing
+a guest must never raise an alert its config turned off. A filesystem's own
+override still replaces the relaxed guest disk threshold for that filesystem,
+on save as while polling. Proof:
+`TestConfigSaveJudgesRelaxedGuestAlertByRelaxedThresholds`,
+`TestConfigSaveKeepsRelaxedGuestFilesystemOverride`,
+`TestPulseRelaxedKeepsOffGuestThresholdOff` and the Off case of
+`TestApplyRelaxedGuestThresholds` in `internal/alerts/alerts_test.go`, and
+`TestCheckGuestDropsRemovedTagsFromOpenAlerts` in
+`internal/alerts/unified_eval_test.go`.
 That same guest-alert owner also has to retire per-disk guest alerts when the
 guest stops, disk alerting is disabled, or the reported disk set changes.
 Canonical guest disk identity is only valid while the guest still exposes that

internal/alerts/alert_policy.go

@@ -19,6 +19,8 @@ package alerts
 //	4. the per-resource override, through the identity-aware lookup for
 //	   the kind (guest stable keys, storage aliases, canonical registry
 //	   translation)
+//	5. a guest's pulse-relaxed tag, which lifts its CPU/memory/disk
+//	   triggers to a 95/92/95 floor
 
 // alertPolicyQuery identifies one resource asking for its effective
 // alert policy.
@@ -28,9 +30,10 @@ type alertPolicyQuery struct {
 	TypeKey    string
 	ResourceID string
 	// Guest carries the live guest model when the query is for a guest
-	// kind: filter-scoped custom rules and the clustered override
-	// identity need it. Nil is valid — filter rules then never match and
-	// override lookup falls back to the raw resource ID.
+	// kind: filter-scoped custom rules, the clustered override identity and
+	// the pulse-relaxed tag need it. Nil is valid — filter rules then never
+	// match, override lookup falls back to the raw resource ID, and no tag
+	// relaxes the thresholds.
 	Guest any
 	// StorageAliases are the storage resource's alias IDs, honored by the
 	// storage override lookup.
@@ -53,7 +56,8 @@ type EffectiveAlertPolicy struct {
 	// connectivity opt-outs are Thresholds.DisableConnectivity.
 	OfflineDisabled bool
 	// Thresholds is the folded threshold block: type defaults, then
-	// custom rules, then the per-resource override.
+	// custom rules, then the per-resource override, then a guest's
+	// pulse-relaxed floor.
 	Thresholds ThresholdConfig
 }
 
@@ -129,6 +133,12 @@ func (m *Manager) effectiveAlertPolicyNoLock(q alertPolicyQuery) EffectiveAlertP
 	case isGuestThresholdResourceType(q.TypeKey):
 		thresholds = m.customRuleThresholdsNoLock(thresholds, q.Guest)
 		thresholds = m.resolveGuestThresholdOverride(thresholds, q.Guest, q.ResourceID)
+		// Polling and config-save reevaluation both resolve through here,
+		// so a relaxed guest's alert is judged by one threshold. Applied
+		// only by CheckGuest, a save re-judged it by the unrelaxed one.
+		if guest, ok := extractGuestSnapshot(q.Guest); ok && parsePulseTags(guest.Tags).Relaxed {
+			thresholds = applyRelaxedGuestThresholds(thresholds)
+		}
 	case q.TypeKey == "storage":
 		thresholds = m.resolveStorageThresholdOverride(thresholds, q.ResourceID, q.StorageAliases)
 	default:

internal/alerts/alerts_test.go

@@ -784,6 +784,137 @@ func TestPulseRelaxedThresholdsIncreaseCpuTrigger(t *testing.T) {
 	}
 }
 
+// pulse-relaxed lifted an Off trigger to its floor like any low trigger, so a
+// relaxed guest with disk alerts off raised one at 96%. A config save then
+// re-judged it by the Off threshold and resolved it, and the next poll raised
+// it again. Relaxing a guest must not raise an alert its config turned off.
+func TestPulseRelaxedKeepsOffGuestThresholdOff(t *testing.T) {
+	m := newTestManager(t)
+	m.mu.Lock()
+	m.config.Enabled = true
+	m.config.TimeThresholds = map[string]int{}
+	m.config.GuestDefaults = ThresholdConfig{
+		CPU:    &HysteresisThreshold{Trigger: 80, Clear: 75},
+		Memory: &HysteresisThreshold{Trigger: 85, Clear: 80},
+		Disk:   &HysteresisThreshold{Trigger: 0, Clear: 0},
+	}
+	m.mu.Unlock()
+
+	vm := models.VM{
+		ID:       "inst/qemu/105",
+		Name:     "relaxed-vm",
+		Node:     "node1",
+		Instance: "inst",
+		Status:   "running",
+		Memory:   models.Memory{Usage: 40},
+		Disk:     models.Disk{Usage: 96},
+		Tags:     []string{"pulse-relaxed"},
+	}
+
+	m.CheckGuest(vm, "inst")
+	if testHasActiveAlert(t, m, canonicalMetricStateID(vm.ID, "disk")) {
+		t.Fatalf("expected no disk alert on a relaxed guest whose disk threshold is off")
+	}
+}
+
+// A config save re-judges a relaxed guest's alert by the thresholds CheckGuest
+// raised it under. Reevaluation rebuilt the guest from alert metadata without
+// its tags, so an unset memory threshold, which pulse-relaxed sets to 92, read
+// as no threshold there: the save resolved the alert with a recovery
+// notification and the next poll raised it again. The tags are read back in
+// the shape a restart restores them.
+func TestConfigSaveJudgesRelaxedGuestAlertByRelaxedThresholds(t *testing.T) {
+	m := newTestManager(t)
+	m.mu.Lock()
+	m.config.Enabled = true
+	m.config.TimeThresholds = map[string]int{}
+	m.config.GuestDefaults = ThresholdConfig{}
+	m.mu.Unlock()
+
+	vm := models.VM{
+		ID:       "inst/qemu/106",
+		Name:     "relaxed-vm",
+		Node:     "node1",
+		Instance: "inst",
+		Status:   "running",
+		Memory:   models.Memory{Usage: 95},
+		Tags:     []string{"pulse-relaxed"},
+	}
+	memoryAlertID := canonicalMetricStateID(vm.ID, "memory")
+
+	m.CheckGuest(vm, "inst")
+	alert := testRequireActiveAlert(t, m, memoryAlertID)
+	if alert.Threshold != 92 {
+		t.Fatalf("memory alert threshold = %v, want the relaxed 92", alert.Threshold)
+	}
+
+	m.mu.Lock()
+	raw, err := json.Marshal(alert.Metadata)
+	if err != nil {
+		m.mu.Unlock()
+		t.Fatalf("marshal metadata: %v", err)
+	}
+	var restored map[string]interface{}
+	if err := json.Unmarshal(raw, &restored); err != nil {
+		m.mu.Unlock()
+		t.Fatalf("unmarshal metadata: %v", err)
+	}
+	alert.Metadata = restored
+	m.mu.Unlock()
+	if _, ok := restored["tags"].([]interface{}); !ok {
+		t.Fatalf("restored tags = %T, want []interface{}", restored["tags"])
+	}
+
+	m.UpdateConfig(m.GetConfig())
+	if !testHasActiveAlert(t, m, memoryAlertID) {
+		t.Fatalf("expected a config save to keep a relaxed guest's memory alert over its relaxed trigger")
+	}
+}
+
+// pulse-relaxed lifts a guest's disk threshold, but a filesystem's own
+// override still decides that filesystem, while polling and on a config save.
+// Judged by the guest's relaxed floor on save, a filesystem alert over its
+// lower override resolved and the next poll raised it again.
+func TestConfigSaveKeepsRelaxedGuestFilesystemOverride(t *testing.T) {
+	m := newTestManager(t)
+	guestID := BuildGuestKey("pve1", "node1", 108)
+	m.mu.Lock()
+	m.config.Enabled = true
+	m.config.TimeThresholds = map[string]int{}
+	m.config.GuestDefaults = ThresholdConfig{
+		Disk: &HysteresisThreshold{Trigger: 90, Clear: 85},
+	}
+	m.config.Overrides = map[string]ThresholdConfig{
+		"guest-disk:guest:pve1:108/disk:boot-dev-vda1": {
+			Disk: &HysteresisThreshold{Trigger: 85, Clear: 80},
+		},
+	}
+	m.mu.Unlock()
+
+	vm := models.VM{
+		ID:       guestID,
+		VMID:     108,
+		Name:     "relaxed-fs",
+		Node:     "node1",
+		Instance: "pve1",
+		Status:   "running",
+		Disks: []models.Disk{
+			{Mountpoint: "/boot", Device: "/dev/vda1", Usage: 92, Total: 100, Used: 92, Free: 8},
+		},
+		Tags: []string{"pulse-relaxed"},
+	}
+	bootAlertID := canonicalMetricStateID(guestID+"-disk-boot-dev-vda1", "disk")
+
+	m.CheckGuest(vm, "pve1")
+	if !testHasActiveAlert(t, m, bootAlertID) {
+		t.Fatalf("expected a /boot alert over its 85%% override on a relaxed guest")
+	}
+	m.UpdateConfig(m.GetConfig())
+	if !testHasActiveAlert(t, m, bootAlertID) {
+		t.Fatalf("expected a config save to keep a relaxed guest's /boot alert over its own 85%% override")
+	}
+}
+
 func TestClearAlertMarksResolutionAndReturnsStatus(t *testing.T) {
 	m := newTestManager(t)
 	m.ClearActiveAlerts()
@@ -6349,6 +6480,22 @@ func TestApplyRelaxedGuestThresholds(t *testing.T) {
 		}
 	})
 
+	t.Run("off thresholds stay off", func(t *testing.T) {
+		cfg := ThresholdConfig{
+			CPU:    &HysteresisThreshold{Trigger: 0, Clear: 0},
+			Memory: &HysteresisThreshold{Trigger: -1, Clear: -1},
+			Disk:   &HysteresisThreshold{Trigger: 0, Clear: 0},
+		}
+
+		result := applyRelaxedGuestThresholds(cfg)
+
+		for name, th := range map[string]*HysteresisThreshold{"CPU": result.CPU, "Memory": result.Memory, "Disk": result.Disk} {
+			if th == nil || th.Trigger > 0 {
+				t.Errorf("%s = %+v, want it to stay off", name, th)
+			}
+		}
+	})
+
 	t.Run("original config unchanged", func(t *testing.T) {
 		// t.Parallel()
 		original := ThresholdConfig{

internal/alerts/guest.go

@@ -141,8 +141,7 @@ func (m *Manager) CheckGuest(guest any, instanceName string) {
 		return
 	}
 
-	settings := policy.TagSettings
-	monitorOnly := settings.MonitorOnly
+	monitorOnly := policy.TagSettings.MonitorOnly
 	if monitorOnly || m.guestHasMonitorOnlyAlerts(guestID) {
 		log.Debug().
 			Str("guest", name).
@@ -215,19 +214,12 @@ func (m *Manager) CheckGuest(guest any, instanceName string) {
 	// If guest is running, clear any powered-off alert
 	m.clearGuestPoweredOffAlert(guestID, name)
 
-	// Get thresholds (check custom rules, then overrides, then defaults)
+	// Get thresholds (defaults, custom rules, overrides, then the
+	// pulse-relaxed floor)
 	m.mu.RLock()
 	thresholds := m.getGuestThresholds(guest, guestID)
 	m.mu.RUnlock()
 
-	if settings.Relaxed {
-		thresholds = applyRelaxedGuestThresholds(thresholds)
-		log.Info().
-			Str("guest", name).
-			Float64("trigger", thresholds.CPU.Trigger).
-			Msg("Applied relaxed thresholds for pulse-relaxed tag")
-	}
-
 	// If alerts are disabled for this guest, clear any existing alerts and return
 	if thresholds.Disabled {
 		if alertsCleared := m.clearGuestMetricAlerts(guestID); alertsCleared > 0 {
@@ -688,6 +680,10 @@ func parsePulseTags(tags []string) pulseTagSettings {
 	return settings
 }
 
+// applyRelaxedGuestThresholds lifts a pulse-relaxed guest's CPU, memory and
+// disk triggers to a 95/92/95 floor. An unset threshold gets the floor; one
+// switched Off (trigger <= 0) stays Off, since relaxing a guest must never
+// raise an alert its config turned off.
 func applyRelaxedGuestThresholds(cfg ThresholdConfig) ThresholdConfig {
 	relaxed := cloneThresholdConfig(cfg)
 
@@ -696,6 +692,9 @@ func applyRelaxedGuestThresholds(cfg ThresholdConfig) ThresholdConfig {
 			*th = &HysteresisThreshold{Trigger: minTrigger, Clear: minTrigger - 5}
 			return
 		}
+		if (*th).Trigger <= 0 {
+			return
+		}
 		ensureHysteresisThreshold(*th)
 		if (*th).Trigger < minTrigger {
 			(*th).Trigger = minTrigger

internal/alerts/guest_snapshot.go

@@ -251,6 +251,9 @@ func guestSnapshotFromAlert(alert *Alert, resourceID string) guestSnapshot {
 	if snapshot.VMID <= 0 {
 		snapshot.VMID = metadataIntValue(alert.Metadata["guestVmid"])
 	}
+	// Tags carry pulse-relaxed, so a config save re-judges the alert by the
+	// thresholds CheckGuest raised it under.
+	snapshot.Tags, _ = metadataStringsValue(alert.Metadata, "tags")
 	return snapshot.normalizeCollections()
 }
 

internal/alerts/read_model.go

@@ -196,6 +196,24 @@ func metadataStringValue(metadata map[string]interface{}, key string) string {
 	}
 }
 
+// metadataStringsValue reads a string list from alert metadata. A live alert
+// holds a []string; one reloaded from disk holds a []interface{}.
+func metadataStringsValue(metadata map[string]interface{}, key string) ([]string, bool) {
+	switch values := metadata[key].(type) {
+	case []string:
+		return append([]string(nil), values...), true
+	case []interface{}:
+		result := make([]string, 0, len(values))
+		for _, value := range values {
+			if s, ok := value.(string); ok {
+				result = append(result, s)
+			}
+		}
+		return result, true
+	}
+	return nil, false
+}
+
 func metadataBoolValue(metadata map[string]interface{}, key string) bool {
 	if metadata == nil {
 		return false

internal/alerts/storage_override_identity.go

@@ -76,17 +76,8 @@ func storagePolicyAliases(alert *Alert) []string {
 	if alert == nil {
 		return nil
 	}
-	switch aliases := alert.Metadata[storagePolicyAliasesKey].(type) {
-	case []string:
-		return append([]string(nil), aliases...)
-	case []interface{}:
-		result := make([]string, 0, len(aliases))
-		for _, value := range aliases {
-			if alias, ok := value.(string); ok {
-				result = append(result, alias)
-			}
-		}
-		return result
+	if aliases, ok := metadataStringsValue(alert.Metadata, storagePolicyAliasesKey); ok {
+		return aliases
 	}
 	// Older PBS snapshots predate alias metadata. The poller records the
 	// instance and datastore name separately: only reconstruct an alias when

internal/alerts/unified_eval.go

@@ -314,15 +314,19 @@ func (m *Manager) evaluateUnifiedMetrics(input *UnifiedResourceInput, thresholds
 	}
 }
 
+// metricOptionsWithTags records a resource's tags on its metric alerts. With
+// no tags it drops any an open alert still carries: config-save reevaluation
+// reads pulse-relaxed back from them, and a removed tag left behind judged
+// the alert by thresholds the evaluator no longer applies.
 func metricOptionsWithTags(opts *metricOptions, tags []string) *metricOptions {
-	if len(tags) == 0 {
-		return opts
-	}
-
 	merged := metricOptions{}
 	if opts != nil {
 		merged = *opts
 	}
+	if len(tags) == 0 {
+		merged.RemoveMetadata = append(append([]string(nil), merged.RemoveMetadata...), "tags")
+		return &merged
+	}
 	metadata := make(map[string]interface{}, len(merged.Metadata)+1)
 	for key, value := range merged.Metadata {
 		metadata[key] = value

internal/alerts/unified_eval_test.go

@@ -902,6 +902,51 @@ func TestCheckGuestPerDiskAnnotatesCanonicalSpecMetadata(t *testing.T) {
 	}
 }
 
+// A guest that drops its last tag leaves none on its open alerts. Config-save
+// reevaluation reads pulse-relaxed back from alert metadata, so a removed tag
+// left behind judged the alert by the relaxed trigger: the save resolved a
+// reading still over the guest's own trigger, and the next poll raised it
+// again.
+func TestCheckGuestDropsRemovedTagsFromOpenAlerts(t *testing.T) {
+	m := newTestManager(t)
+	configureUnifiedEvalManager(t, m, unifiedEvalBaseConfig())
+
+	guestID := BuildGuestKey("pve1", "node1", 102)
+	vm := models.VM{
+		ID:       guestID,
+		VMID:     102,
+		Name:     "app02",
+		Node:     "node1",
+		Instance: "pve1",
+		Status:   "running",
+		CPU:      0.97,
+		Memory:   models.Memory{Usage: 40},
+		Disk:     models.Disk{Usage: 40},
+		Tags:     []string{"pulse-relaxed"},
+	}
+	alertID := canonicalMetricStateID(guestID, "cpu")
+	m.CheckGuest(vm, "pve1")
+	if !testHasActiveAlert(t, m, alertID) {
+		t.Fatalf("expected CPU alert %q over the relaxed trigger", alertID)
+	}
+
+	vm.Tags = nil
+	vm.CPU = 0.90
+	m.CheckGuest(vm, "pve1")
+	alert := testRequireActiveAlert(t, m, alertID)
+	m.mu.RLock()
+	tags, hasTags := alert.Metadata["tags"]
+	m.mu.RUnlock()
+	if hasTags {
+		t.Fatalf("alert metadata tags = %v, want none once the guest dropped them", tags)
+	}
+
+	m.UpdateConfig(m.GetConfig())
+	if !testHasActiveAlert(t, m, alertID) {
+		t.Fatalf("expected a config save to keep a CPU alert over the guest's own trigger after pulse-relaxed is removed")
+	}
+}
+
 func TestCheckGuestPerDiskCleansUpRemovedDiskAlerts(t *testing.T) {
 	m := newTestManager(t)
 	configureUnifiedEvalManager(t, m, unifiedEvalBaseConfig())