diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index a306e9a58a..a951911e5f 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -1802,20 +1802,34 @@ owns node metric and temperature projection, node offline lifecycle handling, host-agent deduplication bookkeeping, and instance-scoped node display-name cache updates; future Proxmox node alert behavior should extend that resource checker owner rather than expanding the central Manager file. -When a host agent with the node's hostname is registered, the agent resource -owns the machine's CPU, memory and disk usage alerts, and `CheckNode` releases -its own copies through the disabled-threshold path every cycle: the pending run -is dropped and any node alert still open from before the agent registered is -resolved, never left frozen until the agent goes offline. CPU temperature has no -host-agent metric, so the node keeps evaluating it whether or not an agent is -registered; the node poll already merges agent sensor readings into -`node.Temperature`, giving one temperature alert per machine. A missing CPU -reading is not recovery evidence: with a live trigger, `checkNodeTemperature` -skips evaluation instead of feeding 0°C, while a disabled threshold still clears. +When a reporting host agent is linked to a node (`LinkedNodeID`, the monitor's +automatic or operator-set identity decision), the agent resource owns that +machine's CPU, memory and disk usage alerts, and `CheckNode` releases its own +copies through the disabled-threshold path every cycle: the pending run is +dropped and any node alert still open from before the link is resolved, never +left frozen until the agent goes offline. Deduplication keys on that link, +never on a hostname match, so a same-named node in another instance keeps its +alerts, an agent reporting an FQDN still dedups its node, and an operator unlink +hands the alerts back. CPU temperature has no host-agent metric, so the node +keeps evaluating it whether or not an agent is linked; the node poll already +merges agent sensor readings into `node.Temperature`, giving one temperature +alert per machine. A missing CPU reading is not evidence: with a live trigger, +`checkNodeTemperature` neither feeds 0°C nor resolves the open alert, but +`interruptMetricRun` (reducer `InterruptMetricRun`) drops a pending activation +run and restarts a recovery run, so no sustained-for or recovery delay spans a +gap in readings; a disabled threshold still clears. Config-save reevaluation +classifies alerts whose metadata `resourceType` is `node` as node alerts and +judges them against node thresholds; the `Instance` heuristic only covers legacy +alerts without a resource type, because current node alerts keep the PVE +instance name in `Instance`. `TestCheckNodeKeepsTemperatureAlertWhenHostAgentMonitorsNode`, -`TestCheckNodeReleasesOpenMetricAlertWhenHostAgentRegisters` and -`TestCheckNodeMissingTemperatureDoesNotResolveOpenAlert` in -`internal/alerts/threshold_resolution_shared_test.go` pin these rules. +`TestCheckNodeReleasesOpenMetricAlertWhenHostAgentRegisters`, +`TestCheckNodeMissingTemperatureDoesNotResolveOpenAlert`, +`TestCheckNodeMissingTemperatureInterruptsTimingRuns` and +`TestConfigSaveKeepsNodeTemperatureAlertOverTrigger` in +`internal/alerts/threshold_resolution_shared_test.go`, and +`TestHostAgentDeduplicationFollowsNodeLink` in +`internal/alerts/host_dedup_test.go`, pin these rules. Host-agent alert evaluation now lives in `internal/alerts/host.go`. That file owns host identity, host-agent metric projection, host disk/SMART/RAID/Unraid health handling, host cleanup, and host offline lifecycle handling; future host diff --git a/internal/alerts/config_runtime.go b/internal/alerts/config_runtime.go index 17d0ecf11e..97260eca4e 100644 --- a/internal/alerts/config_runtime.go +++ b/internal/alerts/config_runtime.go @@ -487,9 +487,16 @@ func (m *Manager) reevaluateActiveAlertsLocked() { threshold = getThresholdForMetric(thresholds, metricType) } - isNodeResource := primaryResourceType == "" || primaryResourceType == "node" + // Node alerts carry resourceType "node" in metadata and keep the PVE + // instance name in Instance, so the metadata decides. The Instance + // heuristic only classifies legacy alerts without a resource type; + // applied to current node alerts it fell through to guest thresholds, + // which have no temperature threshold, and a config save resolved + // temperature alerts that were still over their trigger. + isNodeResource := primaryResourceType == "node" || + (primaryResourceType == "" && !strings.Contains(resourceID, ":") && (alert.Instance == "Node" || alert.Instance == alert.Node)) isStorageResource := alertResourceTypeKeysContain(resourceTypeKeys, "storage") - if threshold == nil && !handledModernPlatformType && isNodeResource && !strings.Contains(resourceID, ":") && (alert.Instance == "Node" || alert.Instance == alert.Node) { + if threshold == nil && !handledModernPlatformType && isNodeResource { if allDisabled, _ := m.alertPolicyTypeSwitchesNoLock("node"); allDisabled { alertsToResolve = append(alertsToResolve, alertID) continue diff --git a/internal/alerts/host.go b/internal/alerts/host.go index cc2969046f..8e0e8c5516 100644 --- a/internal/alerts/host.go +++ b/internal/alerts/host.go @@ -220,11 +220,9 @@ func (m *Manager) CheckHost(host models.Host) { return } - // Register this host agent hostname for deduplication with Proxmox nodes. - // This prevents duplicate alerts when both a Node and Host agent monitor the same machine. - if host.Hostname != "" { - m.RegisterHostAgentHostname(host.Hostname) - } + // Record the Proxmox node this agent is linked to, so the node releases the + // usage alerts the agent now owns instead of alerting twice for one machine. + m.registerHostAgentNodeLink(host) // Cache display name so host alerts show the user-configured name. m.UpdateNodeDisplayName("", host.Hostname, host.DisplayName) @@ -619,10 +617,8 @@ func (m *Manager) HandleHostRemoved(host models.Host) { return } - // Unregister the host agent hostname since it's being removed. - if host.Hostname != "" { - m.UnregisterHostAgentHostname(host.Hostname) - } + // The removed agent no longer owns its linked node's usage alerts. + m.unregisterHostAgentNodeLink(host.ID) m.HandleHostOnline(host) m.clearHostMetricAlerts(host.ID) @@ -729,11 +725,9 @@ func (m *Manager) HandleHostOfflineWithCorrelation(host models.Host, correlation return } - // Unregister the host agent hostname since it's no longer actively monitoring. - // This allows node alerts to resume if a Proxmox node with the same hostname exists. - if host.Hostname != "" { - m.UnregisterHostAgentHostname(host.Hostname) - } + // The agent is no longer actively monitoring, so its linked Proxmox node + // resumes evaluating its own usage alerts. + m.unregisterHostAgentNodeLink(host.ID) m.HandleHostTelemetryExpired(host) m.mu.RLock() diff --git a/internal/alerts/host_dedup_test.go b/internal/alerts/host_dedup_test.go index 66075acc32..a0b4ac3751 100644 --- a/internal/alerts/host_dedup_test.go +++ b/internal/alerts/host_dedup_test.go @@ -7,15 +7,17 @@ import ( ) func TestHostAgentDeduplicatesNodeAlerts(t *testing.T) { - // Test 1: Without a host agent registered, node metrics ARE checked + agent := models.Host{ID: "host-pi", Hostname: "pi", LinkedNodeID: "node/pi"} + + // Test 1: Without a linked host agent, node metrics ARE checked t.Run("node_metrics_checked_without_agent", func(t *testing.T) { m := NewManager() m.config.Enabled = true m.config.NodeDefaults.CPU = &HysteresisThreshold{Trigger: 80, Clear: 75} - // Verify no host agent is registered - if m.hasHostAgentForNode("pi") { - t.Error("Expected no host agent for 'pi' initially") + // Verify no host agent is linked + if m.hasHostAgentForNode("node/pi") { + t.Error("Expected no host agent for 'node/pi' initially") } node := models.Node{ @@ -38,18 +40,18 @@ func TestHostAgentDeduplicatesNodeAlerts(t *testing.T) { } }) - // Test 2: With host agent registered, node metrics are NOT checked + // Test 2: With a host agent linked to the node, node metrics are NOT checked t.Run("node_metrics_skipped_with_agent", func(t *testing.T) { m := NewManager() m.config.Enabled = true m.config.NodeDefaults.CPU = &HysteresisThreshold{Trigger: 80, Clear: 75} - // Register a host agent with the same hostname BEFORE checking the node - m.RegisterHostAgentHostname("pi") + // Link a host agent to the node BEFORE checking the node + m.registerHostAgentNodeLink(agent) - // Verify host agent IS registered - if !m.hasHostAgentForNode("pi") { - t.Error("Expected host agent for 'pi' to be registered") + // Verify host agent IS linked + if !m.hasHostAgentForNode("node/pi") { + t.Error("Expected host agent for 'node/pi' to be linked") } node := models.Node{ @@ -67,7 +69,7 @@ func TestHostAgentDeduplicatesNodeAlerts(t *testing.T) { m.mu.RUnlock() if hasPending { - t.Error("Expected NO pending alert for node CPU when host agent is registered") + t.Error("Expected NO pending alert for node CPU when host agent is linked") } }) @@ -78,12 +80,12 @@ func TestHostAgentDeduplicatesNodeAlerts(t *testing.T) { m.config.NodeDefaults.CPU = &HysteresisThreshold{Trigger: 80, Clear: 75} // Register and then unregister - m.RegisterHostAgentHostname("pi") - m.UnregisterHostAgentHostname("pi") + m.registerHostAgentNodeLink(agent) + m.unregisterHostAgentNodeLink(agent.ID) - // Verify host agent is NOT registered - if m.hasHostAgentForNode("pi") { - t.Error("Expected host agent for 'pi' to be unregistered") + // Verify host agent is NOT linked + if m.hasHostAgentForNode("node/pi") { + t.Error("Expected host agent for 'node/pi' to be unregistered") } node := models.Node{ @@ -106,67 +108,76 @@ func TestHostAgentDeduplicatesNodeAlerts(t *testing.T) { }) } -func TestHostAgentDeduplicationCaseInsensitive(t *testing.T) { +// Deduplication follows the agent's node link, never a hostname match: two +// instances can both have a node named "pve", and an agent can report an FQDN. +func TestHostAgentDeduplicationFollowsNodeLink(t *testing.T) { m := NewManager() - // Register with lowercase - m.RegisterHostAgentHostname("myhost") + m.registerHostAgentNodeLink(models.Host{ID: "host-a", Hostname: "pve.example.com", LinkedNodeID: "site-a-pve"}) + if !m.hasHostAgentForNode("site-a-pve") { + t.Error("Expected the linked node to dedup even though the agent hostname is an FQDN") + } + if m.hasHostAgentForNode("site-b-pve") { + t.Error("Expected a same-named node in another instance to keep its own alerts") + } - // Check should match regardless of case - if !m.hasHostAgentForNode("MYHOST") { - t.Error("Expected hasHostAgentForNode to match uppercase hostname") + m.registerHostAgentNodeLink(models.Host{ID: "host-b", Hostname: "pve"}) + if m.hasHostAgentForNode("site-b-pve") { + t.Error("Expected an unlinked agent with a matching hostname not to suppress node alerts") } - if !m.hasHostAgentForNode("MyHost") { - t.Error("Expected hasHostAgentForNode to match mixed-case hostname") - } - if !m.hasHostAgentForNode("myhost") { - t.Error("Expected hasHostAgentForNode to match lowercase hostname") + + // An operator unlinking the agent hands the node's alerts back. + m.registerHostAgentNodeLink(models.Host{ID: "host-a", Hostname: "pve.example.com"}) + if m.hasHostAgentForNode("site-a-pve") { + t.Error("Expected an unlinked agent to stop deduplicating its former node") } } -func TestCheckHostRegistersHostname(t *testing.T) { +func TestCheckHostRegistersNodeLink(t *testing.T) { m := NewManager() m.config.Enabled = true host := models.Host{ - ID: "host-test-123", - Hostname: "testhost", - CPUUsage: 50, + ID: "host-test-123", + Hostname: "testhost", + LinkedNodeID: "pve-testhost", + CPUUsage: 50, } - // Initially no hostname registered - if m.hasHostAgentForNode("testhost") { + // Initially no node link registered + if m.hasHostAgentForNode("pve-testhost") { t.Error("Expected no host agent registered initially") } - // CheckHost should register the hostname + // CheckHost should register the node link m.CheckHost(host) - if !m.hasHostAgentForNode("testhost") { - t.Error("Expected host agent hostname to be registered after CheckHost") + if !m.hasHostAgentForNode("pve-testhost") { + t.Error("Expected host agent node link to be registered after CheckHost") } } -func TestHandleHostOfflineUnregistersHostname(t *testing.T) { +func TestHandleHostOfflineUnregistersNodeLink(t *testing.T) { m := NewManager() m.config.Enabled = true host := models.Host{ - ID: "host-test-456", - Hostname: "offlinehost", + ID: "host-test-456", + Hostname: "offlinehost", + LinkedNodeID: "pve-offlinehost", } - // Register the hostname - m.RegisterHostAgentHostname(host.Hostname) + // Register the node link + m.registerHostAgentNodeLink(host) - if !m.hasHostAgentForNode("offlinehost") { + if !m.hasHostAgentForNode("pve-offlinehost") { t.Error("Expected host agent registered") } - // HandleHostOffline should unregister the hostname + // HandleHostOffline should unregister the node link m.HandleHostOffline(host) - if m.hasHostAgentForNode("offlinehost") { + if m.hasHostAgentForNode("pve-offlinehost") { t.Error("Expected host agent to be unregistered after HandleHostOffline") } } diff --git a/internal/alerts/manager.go b/internal/alerts/manager.go index ef7021c080..53aa04e84f 100644 --- a/internal/alerts/manager.go +++ b/internal/alerts/manager.go @@ -101,10 +101,10 @@ type Manager struct { flappingActive map[string]bool // Track which alerts are currently in flapping state // Cleanup control cleanupStop chan struct{} // Signal to stop cleanup goroutine - // Host agent deduplication: track hostnames of active host agents - // When a host agent is running on a Proxmox node, we prefer the host agent - // alerts and suppress the node alerts to avoid duplicate monitoring. - hostAgentHostnames map[string]struct{} // Normalized hostnames (lowercase) + // Host agent deduplication: the Proxmox node each reporting host agent is + // linked to. While a link holds, the agent resource owns that machine's + // CPU, memory and disk usage alerts and the node releases its own copies. + hostAgentNodeLinks map[string]string // Host agent ID -> linked node ID // Node display name caches. Proxmox nodes can share the same raw node name // across multiple configured instances, so keep instance-scoped entries in // addition to the legacy raw-name cache used by instance-less resources. @@ -245,7 +245,7 @@ func NewManagerWithDataDir(dataDir string, options ...ManagerOption) *Manager { flappingHistory: make(map[string][]time.Time), flappingActive: make(map[string]bool), cleanupStop: make(chan struct{}), - hostAgentHostnames: make(map[string]struct{}), + hostAgentNodeLinks: make(map[string]string), nodeDisplayNames: make(map[string]string), instanceNodeDisplayNames: make(map[string]string), now: time.Now, diff --git a/internal/alerts/node.go b/internal/alerts/node.go index 3cf82f3524..5f9e0500b2 100644 --- a/internal/alerts/node.go +++ b/internal/alerts/node.go @@ -93,10 +93,10 @@ func (m *Manager) CheckNode(node models.Node) { m.clearNodeOfflineAlert(node) // Check each metric (only if node is online and reachable) - // Check for host agent deduplication: if a host agent is running on this node, + // Check for host agent deduplication: if a host agent is linked to this node, // CPU, memory and disk usage alerts belong to the agent resource (CheckHost // evaluates them), so the node releases its own copies instead of alerting twice. - if m.hasHostAgentForNode(node.Name) { + if m.hasHostAgentForNode(node.ID) { m.releaseNodeMetricAlerts(node, "cpu", "memory", "disk") } else { var memoryMetric *UnifiedResourceMetric @@ -126,7 +126,8 @@ func (m *Manager) CheckNode(node models.Node) { // checkNodeTemperature evaluates the node CPU temperature alert. // A disabled threshold (nil or trigger 0) is always passed through so any // existing alert is cleared. Otherwise a missing CPU reading is not evidence: -// feeding 0°C would resolve an open alert without proving the node cooled down. +// feeding 0°C would resolve an open alert without proving the node cooled down, +// so the open alert is kept and only its timing runs are interrupted. func (m *Manager) checkNodeTemperature(node models.Node, threshold *HysteresisThreshold) { var temp float64 if node.Temperature != nil && node.Temperature.Available { @@ -136,9 +137,6 @@ func (m *Manager) checkNodeTemperature(node models.Node, threshold *HysteresisTh temp = node.Temperature.CPUMax } } - if temp <= 0 && threshold != nil && threshold.Trigger > 0 { - return - } spec, err := buildCanonicalMetricSpec(node.ID, node.Name, unifiedresources.ResourceType("node"), "temperature", threshold) if err != nil { log.Warn(). @@ -148,9 +146,29 @@ func (m *Manager) checkNodeTemperature(node models.Node, threshold *HysteresisTh Msg("Skipping invalid canonical node temperature metric spec") return } + if temp <= 0 && !spec.Disabled { + m.interruptMetricRun(spec) + return + } m.checkMetricWithCanonicalSpec(spec, node.Name, node.Name, node.Instance, "node", temp, threshold, nil) } +// interruptMetricRun records a missing observation for a metric spec. The +// incident and any open alert are kept, but a pending activation run is +// dropped and a recovery run restarts, so a sustained-for or recovery delay +// never completes across a gap in evidence. +func (m *Manager) interruptMetricRun(spec alertspecs.ResourceAlertSpec) { + m.mu.Lock() + for _, state := range m.mirrorStatesNoLock() { + state.InterruptMetricRun(spec.ResourceID, spec.ID) + } + intentChanged := m.clearIntentPendingNoLock(canonicalTrackingKeyForSpec(spec, spec.ID)) + m.mu.Unlock() + if intentChanged { + m.saveActiveAlertsAsync("canonical metric intent pending state") + } +} + // releaseNodeMetricAlerts stops node-side evaluation of the given metrics the // same way a disabled threshold does: any pending run is dropped and any open // node alert is resolved. Without this, a node alert that was open when a host @@ -171,50 +189,71 @@ func (m *Manager) releaseNodeMetricAlerts(node models.Node, metrics ...string) { } } -// RegisterHostAgentHostname registers a host agent hostname for deduplication. -// When a host agent is actively monitoring a machine, we prefer its alerts -// over Proxmox node alerts to avoid duplicate monitoring of the same machine. -func (m *Manager) RegisterHostAgentHostname(hostname string) { - normalized := strings.ToLower(strings.TrimSpace(hostname)) - if normalized == "" { +// registerHostAgentNodeLink records which Proxmox node a reporting host agent +// is linked to. While the link holds, the agent resource owns that machine's +// CPU, memory and disk usage alerts. The link is the monitor's identity decision +// (automatic or operator-set), so an agent whose hostname merely matches a node +// name in another instance, or one an operator unlinked, never makes a node +// release its alerts, and an agent reporting an FQDN still dedups its node. +func (m *Manager) registerHostAgentNodeLink(host models.Host) { + hostID := strings.TrimSpace(host.ID) + if hostID == "" { + return + } + linkedNodeID := strings.TrimSpace(host.LinkedNodeID) + m.mu.Lock() + previous, existed := m.hostAgentNodeLinks[hostID] + if linkedNodeID == "" { + delete(m.hostAgentNodeLinks, hostID) + } else { + m.hostAgentNodeLinks[hostID] = linkedNodeID + } + m.mu.Unlock() + + if !existed || previous != linkedNodeID { + log.Debug(). + Str("hostID", hostID). + Str("hostname", host.Hostname). + Str("linkedNodeID", linkedNodeID). + Msg("Updated host agent node link for deduplication") + } +} + +// unregisterHostAgentNodeLink removes a host agent from deduplication tracking, +// so its linked node evaluates its own usage alerts again. +func (m *Manager) unregisterHostAgentNodeLink(hostID string) { + hostID = strings.TrimSpace(hostID) + if hostID == "" { return } m.mu.Lock() - m.hostAgentHostnames[normalized] = struct{}{} + linkedNodeID, existed := m.hostAgentNodeLinks[hostID] + delete(m.hostAgentNodeLinks, hostID) m.mu.Unlock() - log.Debug(). - Str("hostname", hostname). - Msg("Registered host agent hostname for deduplication") -} - -// UnregisterHostAgentHostname removes a host agent hostname from deduplication tracking. -func (m *Manager) UnregisterHostAgentHostname(hostname string) { - normalized := strings.ToLower(strings.TrimSpace(hostname)) - if normalized == "" { - return + if existed { + log.Debug(). + Str("hostID", hostID). + Str("linkedNodeID", linkedNodeID). + Msg("Removed host agent node link from deduplication") } - m.mu.Lock() - delete(m.hostAgentHostnames, normalized) - m.mu.Unlock() - - log.Debug(). - Str("hostname", hostname). - Msg("Unregistered host agent hostname from deduplication") } -// hasHostAgentForNode checks if a host agent is monitoring a machine with the same -// hostname as the given Proxmox node. If so, we should suppress node alerts to -// avoid duplicate alerting. -func (m *Manager) hasHostAgentForNode(nodeName string) bool { - normalized := strings.ToLower(strings.TrimSpace(nodeName)) - if normalized == "" { +// hasHostAgentForNode reports whether a reporting host agent is linked to the +// given Proxmox node, in which case the node's usage alerts belong to the agent. +func (m *Manager) hasHostAgentForNode(nodeID string) bool { + nodeID = strings.TrimSpace(nodeID) + if nodeID == "" { return false } m.mu.RLock() - _, exists := m.hostAgentHostnames[normalized] - m.mu.RUnlock() - return exists + defer m.mu.RUnlock() + for _, linkedNodeID := range m.hostAgentNodeLinks { + if linkedNodeID == nodeID { + return true + } + } + return false } // UpdateNodeDisplayName caches the display name for a node/host so alerts diff --git a/internal/alerts/reducer/reducer.go b/internal/alerts/reducer/reducer.go index 35cb37059f..4f8e98aa6c 100644 --- a/internal/alerts/reducer/reducer.go +++ b/internal/alerts/reducer/reducer.go @@ -583,6 +583,29 @@ func (s *State) Forget(resourceID, subKey string, at time.Time) { } } +// InterruptMetricRun ends the timing evidence for one metric key without +// resolving anything: a pending activation run is dropped and a firing +// incident's recovery run restarts. Evaluators call it when an observation is +// missing, so neither a sustained-for delay nor a recovery delay spans a gap +// in evidence. It reports whether a run was interrupted. +func (s *State) InterruptMetricRun(resourceID, subKey string) bool { + key := incidentKey(resourceID, subKey) + incident, ok := s.incidents[key] + if !ok { + return false + } + switch incident.State { + case StatePending: + delete(s.incidents, key) + return true + case StateFiring: + interrupted := !incident.RecoverySince.IsZero() + resetMetricRecoveryRun(incident) + return interrupted + } + return false +} + // ShiftResolved moves every resolved-occurrence timestamp by delta // (negative = older). It exists for tests and simulations that age // resolved records past RefireRetention; production code has no reason to diff --git a/internal/alerts/threshold_resolution_shared_test.go b/internal/alerts/threshold_resolution_shared_test.go index 77436fee92..540ab6aedd 100644 --- a/internal/alerts/threshold_resolution_shared_test.go +++ b/internal/alerts/threshold_resolution_shared_test.go @@ -265,8 +265,8 @@ func TestCheckNodeKeepsTemperatureAlertWhenHostAgentMonitorsNode(t *testing.T) { node, host := testNodeWithHostAgent() m.CheckHost(host) - if !m.hasHostAgentForNode(node.Name) { - t.Fatalf("expected CheckHost to register %q for node deduplication", host.Hostname) + if !m.hasHostAgentForNode(node.ID) { + t.Fatalf("expected CheckHost to link %q to node %q for deduplication", host.ID, node.ID) } node.Temperature = &models.Temperature{Available: true, CPUPackage: 90} @@ -359,6 +359,104 @@ func TestCheckNodeMissingTemperatureDoesNotResolveOpenAlert(t *testing.T) { } } +// A missing reading keeps the incident but must not let a sustained-for or +// recovery delay complete across the gap in evidence. +func TestCheckNodeMissingTemperatureInterruptsTimingRuns(t *testing.T) { + m := newTestManager(t) + m.mu.Lock() + m.config.Enabled = true + m.config.TimeThresholds = map[string]int{} + m.config.MetricTimeThresholds = map[string]map[string]int{"node": {"temperature": 300}} + m.config.NodeDefaults.Temperature = &HysteresisThreshold{Trigger: 80, Clear: 75} + m.mu.Unlock() + + node, _ := testNodeWithHostAgent() + tempAlertID := canonicalMetricStateID(node.ID, "temperature") + specID := canonicalMetricSpecID(node.ID, "temperature") + hot := &models.Temperature{Available: true, CPUPackage: 85} + + node.Temperature = hot + m.CheckNode(node) + m.mu.Lock() + pending := testCoreIsPending(m, node.ID, specID) + m.core.ShiftPending(-10 * time.Minute) + m.mu.Unlock() + if !pending { + t.Fatalf("expected a pending temperature run before the reading went missing") + } + + node.Temperature = nil + m.CheckNode(node) + node.Temperature = hot + m.CheckNode(node) + if testHasActiveAlert(t, m, tempAlertID) { + t.Fatalf("expected the sustained-for delay to restart after a missing reading, not fire on the first sample back") + } + + m.mu.Lock() + m.core.ShiftPending(-10 * time.Minute) + m.mu.Unlock() + m.CheckNode(node) + if !testHasActiveAlert(t, m, tempAlertID) { + t.Fatalf("expected the temperature alert to fire after a sustained run") + } + + node.Temperature = &models.Temperature{Available: true, CPUPackage: 60} + m.CheckNode(node) + m.mu.RLock() + incident, _ := m.core.Incident(node.ID, specID) + m.mu.RUnlock() + if incident.RecoverySince.IsZero() { + t.Fatalf("expected a recovery run to start below the clear threshold") + } + + node.Temperature = nil + m.CheckNode(node) + m.mu.RLock() + incident, _ = m.core.Incident(node.ID, specID) + m.mu.RUnlock() + if !incident.RecoverySince.IsZero() { + t.Fatalf("expected a missing reading to restart the recovery run") + } + if !testHasActiveAlert(t, m, tempAlertID) { + t.Fatalf("expected the temperature alert to stay open without a reading") + } +} + +// Node alerts keep the PVE instance name in Instance. A config save must judge +// them against node thresholds, not fall through to guest thresholds, which +// have no temperature threshold and used to resolve a live temperature alert. +func TestConfigSaveKeepsNodeTemperatureAlertOverTrigger(t *testing.T) { + m := newTestManager(t) + m.mu.Lock() + m.config.Enabled = true + m.config.TimeThresholds = map[string]int{} + m.config.NodeDefaults.Temperature = &HysteresisThreshold{Trigger: 80, Clear: 75} + m.mu.Unlock() + + node, _ := testNodeWithHostAgent() + node.Temperature = &models.Temperature{Available: true, CPUPackage: 90} + m.CheckNode(node) + tempAlertID := canonicalMetricStateID(node.ID, "temperature") + if !testHasActiveAlert(t, m, tempAlertID) { + t.Fatalf("expected node temperature alert %q", tempAlertID) + } + + m.UpdateConfig(m.GetConfig()) + if !testHasActiveAlert(t, m, tempAlertID) { + t.Fatalf("expected a config save to keep a node temperature alert still over its trigger") + } + + config := m.GetConfig() + config.Overrides = map[string]ThresholdConfig{ + node.ID: {Temperature: &HysteresisThreshold{Trigger: 95, Clear: 92}}, + } + m.UpdateConfig(config) + if testHasActiveAlert(t, m, tempAlertID) { + t.Fatalf("expected a node override raising the trigger above the reading to resolve the alert") + } +} + func TestReevaluateActiveAlertsUsesSharedAgentOverrideResolution(t *testing.T) { m := newTestManager(t)