mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-24 04:33:17 +00:00
Base node usage ownership on agent config, all links, summary disk
A review of the previous commit found three ways the node and its linked agent could still disagree about who alerts. Memory ownership required a known reading on every report. When an agent's memory reading went missing while its alert was open, the agent kept that alert (a missing reading is not a recovery) but stopped owning memory, so the node raised a second alert. It resolved again as soon as the reading returned, with history and notifications for each flip. Ownership now follows the agent's configured thresholds, not one report's data. With several agents linked to one node, only the lowest agent ID's coverage counted. If that agent had CPU off and another linked agent evaluated CPU, the node alerted on CPU as well. A metric is now owned when any linked agent evaluates it. Disk ownership needed any evaluated filesystem, but a linked node's disk metric is the agent's summary filesystem (root when reported, see models.SummaryDisk). With root disabled by a per-disk override and /data still alerting, the node gave up its root disk alert and a full root filesystem raised nothing. The agent now owns the node's disk metric only while it evaluates that same filesystem.
This commit is contained in:
@@ -1808,10 +1808,15 @@ CPU, memory or disk usage metric it actually evaluates, and `CheckNode` releases
|
||||
its own copy of those metrics 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. `CheckHost` registers
|
||||
the link only after evaluation, recording which metrics had a live threshold
|
||||
(memory also needs a known reading, disk at least one evaluated filesystem), and
|
||||
removes it while agent alerts are disabled; a metric the agent does not evaluate
|
||||
stays with the node, so deduplication never leaves a machine unmonitored.
|
||||
the link only after evaluation, recording which metrics the agent is configured
|
||||
to evaluate (a live CPU or memory threshold; for disk, a live threshold on the
|
||||
summary filesystem `models.SummaryDisk` picks, which is also what the linked
|
||||
node's disk metric reports), and removes it while agent alerts are disabled.
|
||||
Ownership follows configuration, not one report's data, so a missing agent
|
||||
reading keeps the agent's alert open instead of handing the metric back to the
|
||||
node for a cycle. With several agents linked to one node, a metric is owned when
|
||||
any of them evaluates it. A metric no agent evaluates stays with the node, so
|
||||
deduplication never leaves a machine unmonitored.
|
||||
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
|
||||
@@ -1832,7 +1837,8 @@ instance name in `Instance`.
|
||||
`TestCheckNodeMissingTemperatureDoesNotResolveOpenAlert`,
|
||||
`TestCheckNodeMissingTemperatureInterruptsTimingRuns` and
|
||||
`TestConfigSaveKeepsNodeTemperatureAlertOverTrigger` and
|
||||
`TestCheckNodeKeepsUsageMetricsTheAgentDoesNotEvaluate` in
|
||||
`TestCheckNodeKeepsUsageMetricsTheAgentDoesNotEvaluate`,
|
||||
`TestCheckNodeUsageOwnershipFollowsWhatAgentsEvaluate` in
|
||||
`internal/alerts/threshold_resolution_shared_test.go`, and
|
||||
`TestHostAgentDeduplicationFollowsNodeLink` in
|
||||
`internal/alerts/host_dedup_test.go`, pin these rules.
|
||||
|
||||
+15
-6
@@ -387,7 +387,13 @@ func (m *Manager) CheckHost(host models.Host) {
|
||||
}
|
||||
|
||||
seenDisks := make(map[string]struct{}, len(host.Disks))
|
||||
evaluatesDiskUsage := false
|
||||
// A linked node's disk metric is this agent's summary filesystem (root when
|
||||
// reported), so the agent owns it only while it evaluates that filesystem.
|
||||
summaryDiskResourceID := ""
|
||||
if summary, ok := models.SummaryDisk(host.Disks); ok {
|
||||
summaryDiskResourceID, _ = hostDiskResourceID(host, summary)
|
||||
}
|
||||
evaluatesSummaryDisk := false
|
||||
if len(host.Sensors.SMART) > 0 {
|
||||
for _, disk := range host.Sensors.SMART {
|
||||
diskResourceID, diskName := hostSMARTDiskResourceID(host, disk)
|
||||
@@ -445,7 +451,7 @@ func (m *Manager) CheckHost(host models.Host) {
|
||||
if effectiveDiskThreshold == nil {
|
||||
continue
|
||||
}
|
||||
evaluatesDiskUsage = evaluatesDiskUsage || effectiveDiskThreshold.Trigger > 0
|
||||
evaluatesSummaryDisk = evaluatesSummaryDisk || (diskResourceID == summaryDiskResourceID && effectiveDiskThreshold.Trigger > 0)
|
||||
|
||||
diskMetadata := cloneMetadata(baseMetadata)
|
||||
diskMetadata["metric"] = "disk"
|
||||
@@ -493,15 +499,18 @@ func (m *Manager) CheckHost(host models.Host) {
|
||||
|
||||
m.cleanupHostDiskAlerts(host, seenDisks)
|
||||
|
||||
// The linked node releases exactly the usage metrics evaluated above and
|
||||
// keeps the rest, so deduplication never leaves the machine unmonitored.
|
||||
// The linked node releases exactly the usage metrics this agent is set up
|
||||
// to evaluate and keeps the rest, so deduplication never leaves the machine
|
||||
// unmonitored. Ownership follows configuration, not one report's data: a
|
||||
// missing memory reading keeps the agent's alert open rather than handing
|
||||
// memory back to the node for a cycle.
|
||||
m.registerHostAgentNodeLink(hostAgentNodeLink{
|
||||
agentID: host.ID,
|
||||
agentName: resourceName,
|
||||
nodeID: host.LinkedNodeID,
|
||||
cpu: thresholds.CPU != nil && thresholds.CPU.Trigger > 0,
|
||||
memory: thresholds.Memory != nil && thresholds.Memory.Trigger > 0 && host.Memory.HasKnownUsage(),
|
||||
disk: evaluatesDiskUsage,
|
||||
memory: thresholds.Memory != nil && thresholds.Memory.Trigger > 0,
|
||||
disk: evaluatesSummaryDisk,
|
||||
})
|
||||
|
||||
if host.Unraid != nil {
|
||||
|
||||
+12
-5
@@ -263,9 +263,11 @@ func (m *Manager) unregisterHostAgentNodeLink(hostID string) {
|
||||
}
|
||||
}
|
||||
|
||||
// linkedHostAgentForNode returns the host agent link that owns usage metrics
|
||||
// for the given Proxmox node. If several agents are linked to one node, the
|
||||
// lowest agent ID wins so the choice is stable.
|
||||
// linkedHostAgentForNode returns the host agent coverage for the given Proxmox
|
||||
// node. A usage metric counts as owned when any agent linked to the node
|
||||
// evaluates it, so a second linked agent covering a metric the first does not
|
||||
// still keeps the node from alerting twice. The agent identity reported is the
|
||||
// lowest agent ID among the linked agents, so the choice is stable.
|
||||
func (m *Manager) linkedHostAgentForNode(nodeID string) (hostAgentNodeLink, bool) {
|
||||
nodeID = strings.TrimSpace(nodeID)
|
||||
if nodeID == "" {
|
||||
@@ -280,9 +282,14 @@ func (m *Manager) linkedHostAgentForNode(nodeID string) (hostAgentNodeLink, bool
|
||||
continue
|
||||
}
|
||||
if !ok || link.agentID < found.agentID {
|
||||
found = link
|
||||
ok = true
|
||||
found.agentID = link.agentID
|
||||
found.agentName = link.agentName
|
||||
}
|
||||
found.nodeID = nodeID
|
||||
found.cpu = found.cpu || link.cpu
|
||||
found.memory = found.memory || link.memory
|
||||
found.disk = found.disk || link.disk
|
||||
ok = true
|
||||
}
|
||||
return found, ok
|
||||
}
|
||||
|
||||
@@ -420,6 +420,97 @@ func TestCheckNodeKeepsUsageMetricsTheAgentDoesNotEvaluate(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
// Ownership follows what the linked agents are configured to evaluate, not one
|
||||
// report's data: a missing agent memory reading must not hand memory back to
|
||||
// the node, a second linked agent's metrics count, and the node's disk metric
|
||||
// belongs to the agent only when the agent evaluates that same filesystem.
|
||||
func TestCheckNodeUsageOwnershipFollowsWhatAgentsEvaluate(t *testing.T) {
|
||||
setup := func(t *testing.T) (*Manager, models.Node, models.Host) {
|
||||
m := newTestManager(t)
|
||||
m.mu.Lock()
|
||||
m.config.Enabled = true
|
||||
m.config.TimeThresholds = map[string]int{}
|
||||
m.config.NodeDefaults.CPU = &HysteresisThreshold{Trigger: 80, Clear: 75}
|
||||
m.config.NodeDefaults.Memory = &HysteresisThreshold{Trigger: 85, Clear: 80}
|
||||
m.config.NodeDefaults.Disk = &HysteresisThreshold{Trigger: 90, Clear: 85}
|
||||
m.config.AgentDefaults.CPU = &HysteresisThreshold{Trigger: 80, Clear: 75}
|
||||
m.config.AgentDefaults.Memory = &HysteresisThreshold{Trigger: 85, Clear: 80}
|
||||
m.config.AgentDefaults.Disk = &HysteresisThreshold{Trigger: 90, Clear: 85}
|
||||
m.mu.Unlock()
|
||||
node, host := testNodeWithHostAgent()
|
||||
return m, node, host
|
||||
}
|
||||
|
||||
t.Run("missing_agent_memory_reading", func(t *testing.T) {
|
||||
m, node, host := setup(t)
|
||||
node.Memory = models.Memory{Total: 100, Used: 95, Free: 5, Usage: 95}
|
||||
host.Memory = node.Memory
|
||||
m.CheckHost(host)
|
||||
m.CheckNode(node)
|
||||
|
||||
host.Memory = models.Memory{Total: 100, UsageUnavailable: true}
|
||||
m.CheckHost(host)
|
||||
m.CheckNode(node)
|
||||
agentAlertID := canonicalMetricStateID(hostResourceID(host.ID), "memory")
|
||||
if got := testActiveAlertIDsOfType(m, "memory"); len(got) != 1 || got[0] != agentAlertID {
|
||||
t.Fatalf("expected only the agent memory alert while its reading is missing, got %v", got)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("second_linked_agent", func(t *testing.T) {
|
||||
m, node, host := setup(t)
|
||||
node.CPU = 0.95
|
||||
m.mu.Lock()
|
||||
m.config.Overrides = map[string]ThresholdConfig{
|
||||
"agent-a": {CPU: &HysteresisThreshold{Trigger: 0, Clear: 0}},
|
||||
}
|
||||
m.mu.Unlock()
|
||||
agentA := host
|
||||
agentA.ID = "agent-a"
|
||||
agentB := host
|
||||
agentB.ID = "agent-b"
|
||||
agentB.CPUUsage = 95
|
||||
m.CheckHost(agentA)
|
||||
m.CheckHost(agentB)
|
||||
m.CheckNode(node)
|
||||
agentBAlertID := canonicalMetricStateID(hostResourceID(agentB.ID), "cpu")
|
||||
if got := testActiveAlertIDsOfType(m, "cpu"); len(got) != 1 || got[0] != agentBAlertID {
|
||||
t.Fatalf("expected only agent-b's CPU alert when it covers CPU for the node, got %v", got)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("summary_disk", func(t *testing.T) {
|
||||
m, node, host := setup(t)
|
||||
root := models.Disk{Mountpoint: "/", Device: "/dev/sda1", Total: 100, Used: 95, Free: 5, Usage: 95}
|
||||
data := models.Disk{Mountpoint: "/data", Device: "/dev/sdb1", Total: 100, Used: 10, Free: 90, Usage: 10}
|
||||
host.Disks = []models.Disk{root, data}
|
||||
node.Disk = models.Disk{Total: 100, Used: 95, Free: 5, Usage: 95}
|
||||
rootResourceID, _ := hostDiskResourceID(host, root)
|
||||
nodeDiskAlertID := canonicalMetricStateID(node.ID, "disk")
|
||||
|
||||
m.mu.Lock()
|
||||
m.config.Overrides = map[string]ThresholdConfig{rootResourceID: {Disabled: true}}
|
||||
m.mu.Unlock()
|
||||
m.CheckHost(host)
|
||||
m.CheckNode(node)
|
||||
if !testHasActiveAlert(t, m, nodeDiskAlertID) {
|
||||
t.Fatalf("expected the node to keep its root disk alert while the agent skips that filesystem")
|
||||
}
|
||||
|
||||
m.mu.Lock()
|
||||
m.config.Overrides = nil
|
||||
m.mu.Unlock()
|
||||
m.CheckHost(host)
|
||||
m.CheckNode(node)
|
||||
if testHasActiveAlert(t, m, nodeDiskAlertID) {
|
||||
t.Fatalf("expected the node to release its disk alert once the agent evaluates the same filesystem")
|
||||
}
|
||||
if !testHasActiveAlert(t, m, canonicalMetricStateID(rootResourceID, "disk")) {
|
||||
t.Fatalf("expected the agent's root filesystem alert")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// 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) {
|
||||
|
||||
Reference in New Issue
Block a user