diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index e55ec55063..9be46e6839 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -1452,7 +1452,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 @@ -1462,6 +1463,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 diff --git a/internal/alerts/alert_policy.go b/internal/alerts/alert_policy.go index c15752a6fa..cf91caab4c 100644 --- a/internal/alerts/alert_policy.go +++ b/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. @@ -47,7 +50,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 } @@ -120,6 +124,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: diff --git a/internal/alerts/alerts_test.go b/internal/alerts/alerts_test.go index dea4577c90..ea06410c53 100644 --- a/internal/alerts/alerts_test.go +++ b/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{ diff --git a/internal/alerts/guest.go b/internal/alerts/guest.go index 6a0c694529..819dc77911 100644 --- a/internal/alerts/guest.go +++ b/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 diff --git a/internal/alerts/guest_snapshot.go b/internal/alerts/guest_snapshot.go index 9cbcc805e0..ac5393fe37 100644 --- a/internal/alerts/guest_snapshot.go +++ b/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() } diff --git a/internal/alerts/read_model.go b/internal/alerts/read_model.go index 68382bc6ea..d45f09e84b 100644 --- a/internal/alerts/read_model.go +++ b/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 diff --git a/internal/alerts/storage_override_identity.go b/internal/alerts/storage_override_identity.go index 00c19a86ad..80cb619917 100644 --- a/internal/alerts/storage_override_identity.go +++ b/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 diff --git a/internal/alerts/unified_eval.go b/internal/alerts/unified_eval.go index c7120cf079..82d4f1972b 100644 --- a/internal/alerts/unified_eval.go +++ b/internal/alerts/unified_eval.go @@ -305,15 +305,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 diff --git a/internal/alerts/unified_eval_test.go b/internal/alerts/unified_eval_test.go index bdb0b7080a..eb717931d5 100644 --- a/internal/alerts/unified_eval_test.go +++ b/internal/alerts/unified_eval_test.go @@ -901,6 +901,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())