mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-24 20:52:51 +00:00
rcourtman
27051deb27
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.