mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-24 04:33:17 +00:00
Dedup node alerts by agent link and keep node thresholds on save (#2577)
A review of #2569 (node temperature alerts with a host agent) found three more ways node alerts went wrong. Each was reproduced with a test that fails on `main`. ## Changes - **Dedup follows the agent's node link, not its hostname.** Node deduplication matched an agent's hostname against node names across every PVE instance. With two instances that each have a node called `pve` and an agent on only one, both nodes released their CPU, memory and disk alerts, so the second machine went unmonitored. An agent reporting an FQDN never matched its node, so that machine alerted twice. An operator unlinking an agent didn't hand the alerts back either. The manager now tracks each reporting agent's `LinkedNodeID`, which the monitor already sets automatically or from an operator link. On pulse-prod the delly2 agent is linked to `homelab-delly2`, the same ID as the node alert it should dedup. - **A config save keeps node thresholds.** Saving the alert config resolved node temperature alerts that were still over their trigger. Reevaluation only treated an alert as a node alert when its `Instance` equalled its node name. Current node alerts keep the PVE instance name there (`homelab`), so they fell through to guest thresholds, and guests have no temperature threshold. Metadata `resourceType: node` now decides, and the `Instance` heuristic only covers legacy alerts that have no resource type. - **Missing readings interrupt timing runs.** Since #2569, a missing temperature reading no longer resolves an open alert, but a pending run still spanned the gap. One hot sample after ten minutes without readings fired at once, and a recovery run could complete the same way. The reducer gains `InterruptMetricRun`, which drops a pending run or restarts a recovery run without resolving anything. The node temperature check calls it, along with the explicit-intent pending clear, when the reading is missing. ## Not in this PR Two other review findings are filed separately: - When collection fails, the poller re-presents a stale temperature as current with no time limit, and alerting treats it as live. The fix touches the poller, mock timestamps and what the UI shows. - At handover, the released node alert closes with "now healthy" wording. That needs a typed resolution reason in notifications. ## Tests - `TestHostAgentDeduplicationFollowsNodeLink`: same-named nodes in two instances, an FQDN hostname, an unlinked agent, and an operator unlink. - `TestConfigSaveKeepsNodeTemperatureAlertOverTrigger`: fails on `main`. - `TestCheckNodeMissingTemperatureInterruptsTimingRuns`: fails on `main` (one sample back after the gap fires at once). - The existing dedup tests now use link semantics. `go test ./internal/alerts/...` passes, and so do the monitoring alert, temperature, host-agent and node tests.
This commit is contained in:
rcourtman
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
+8
-14
@@ -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()
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
+78
-39
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user