Stop presenting a silent agent's node temperature as live (#2582)

A host agent linked to a Proxmox node merges with it into one read-state
row, and every PVE poll refreshes that row's `LastSeen`. The agent's
last sensors stay in state after it stops reporting. The node
temperature lookup (`getHostAgentTemperatureByID`) checked neither, so
each poll merged the silent agent's last reading into the node as a
fresh measurement, stamped with the row's fresh `LastSeen`:

- an open node temperature alert was re-observed every poll, so its
`LastSeen` kept moving and the 24h stale-alert cleanup never reached it;
a stale hot reading could keep activating
- node history recorded the frozen value as new samples, and the UI and
reports showed it as current
- SSH collection was skipped as if a recent agent payload existed

Since #2569 evaluates node temperature for agent-backed nodes, this
holds their temperature alerts open.

## Change

- `HostView.SourceStatus(source)` exposes the per-source freshness the
registry already records (same accessor the VM, container and
physical-disk views have).
- The linked-agent lookup reads the agent source's own sighting, uses
the agent's sensors only inside the reporting lease that keeps the agent
online (`hostAgentHealthWindow`, as `evaluateHostAgents` uses), and
stamps the reading with the agent's report time. Past the lease the node
has no agent reading (the cluster sensor cache is still consulted), and
alert evaluation treats that as a missing reading, not as evidence the
node cooled down.
- The poller's carry-over of a previous reading after a failed
collection is bounded to max(5 min, 2 × PVE polling interval) and keeps
its original timestamp. This branch does not engage today: the
previous-node projection carries the unified resource id and no
temperature, so it never matches the polled node. That is filed
separately; the bound keeps the carry honest once it's repaired.

The owner is the poller rather than `checkNodeTemperature`: the same
stale value also reaches history, reports and the UI. An alerts-side
`LastUpdate` check would also have missed this case, because the reading
was stamped with the merged row's fresh `LastSeen`. Mock fixtures are
untouched.

## Tests

- `TestSilentLinkedAgentStopsRefreshingNodeTemperatureAlert` (registered
monitoring proof): registry-backed merged agent+node row through the
production previous-node projection and `CheckNode`. A reporting agent
opens the alert. A silent agent leaves the alert open but stops
refreshing `LastSeen`. A resumed agent is evaluated again. It fails on
origin/main.
- `TestView_HostViewSourceStatusSeparatesAgentFromMergedRow`: a merged
row's `LastSeen` follows PVE, while `SourceStatus(SourceAgent)` keeps
the agent's own report time.
- `TestCollectNodeTemperatureCarryIsBoundedByCarryWindow`,
`TestHostAgentReportCurrent`,
`TestGetHostAgentTemperatureByID_IgnoresAgentPastReportingLease`.

Out of scope, filed separately: `mergeHostAgentSMARTIntoDisks` copies
silent agents' retained SMART temperatures into physical disks.
This commit is contained in:
rcourtman
2026-10-06 17:31:31 +01:00
committed by GitHub
8 changed files with 313 additions and 3 deletions
@@ -3695,6 +3695,23 @@ exists or the agent payload has no usable positive reading. Identity-only or
zero-temperature SMART rows do not count as usable by themselves, but the
runtime must not keep probing legacy SSH solely to augment an otherwise healthy
agent temperature payload with SMART data.
A node temperature presented as available must describe the node now, because
node alert evaluation, node history writes, reporting, and the UI all read it as
a live measurement. A host agent linked to a Proxmox node merges into one host
row whose `LastSeen` every PVE poll keeps fresh, while the agent's last sensors
stay in state after it stops reporting, so the row's `LastSeen` cannot decide
whether those sensors are current. `internal/monitoring/host_agent_temps.go`
reads the agent's own sighting from the row's `SourceStatus(SourceAgent)`, feeds
the agent's sensors into the node only while that sighting is inside the
reporting lease that keeps the agent online (`hostAgentReportCurrent`, the same
`hostAgentHealthWindow` that `evaluateHostAgents` applies, so a slow-interval
agent's readings count exactly while the agent is shown online), and stamps the
reading with that report time. Past the lease the lookup falls back to the
cluster sensor cache, which keeps its own recency check. When every source
returns nothing, `internal/monitoring/monitor_polling_node_helpers.go` may carry
a previous reading, with its original `LastUpdate`, only inside the carry window
(twice the PVE polling interval, never under five minutes); an older reading is
dropped rather than re-presented as current.
Legacy SSH temperature collection must also use the Pulse sensor-wrapper
contract before falling back to raw lm-sensors output. `internal/monitoring/temperature.go`
must request `/usr/local/sbin/pulse-sensors` when it exists, parse the wrapper
@@ -485,6 +485,18 @@ layer consumes it only as a fallback replacement for Proxmox LXC memory, so
platform metric priority for a healthy agent is unchanged.
`TestContainerViewLinkedAgentMemory` pins those boundaries.
**Host-row agent source freshness (6 October 2026)**
`HostView.SourceStatus(source)` exposes the per-source delivery freshness the
registry already records, matching the VM, container, and physical-disk views.
A host agent linked to a Proxmox node merges into one row whose `LastSeen`
follows the freshest source, so consumers of agent-owned samples (monitoring's
linked-agent node temperature lookup) must read the agent source's own sighting
here instead of the row's `LastSeen`. The accessor returns a copy of the recorded
status and adds no freshness policy of its own.
`TestView_HostViewSourceStatusSeparatesAgentFromMergedRow` pins the merged-row
split.
### Bounded incident-history selection
Canonical history queries filter exact alert identifiers and observation windows
+20 -2
View File
@@ -138,8 +138,19 @@ func (m *Monitor) getHostAgentTemperatureByID(nodeID, nodeName string) *models.T
return m.getClusterSensorTemperature(nodeName)
}
// Convert host agent sensor data to Temperature model
return convertUnifiedHostSensorsToTemperature(sensors, matchedHost.LastSeen())
// An agent that stopped reporting keeps its last sensors in state. Those
// describe the machine as it was, so they must not keep feeding the node
// (and through it alerts and history) as a live reading. The row's own
// LastSeen cannot tell: a row merged with the Proxmox node stays fresh from
// every PVE poll, so read the agent source's sighting.
agentStatus, ok := matchedHost.SourceStatus(unifiedresources.SourceAgent)
if !ok || !hostAgentReportCurrent(agentStatus.LastSeen, matchedHost.IntervalSeconds(), time.Now()) {
return m.getClusterSensorTemperature(nodeName)
}
// Convert host agent sensor data to Temperature model, stamped with the
// agent's report time rather than the merged row's.
return convertUnifiedHostSensorsToTemperature(sensors, agentStatus.LastSeen)
}
func convertUnifiedHostSensorsToTemperature(sensors *unifiedresources.HostSensorMeta, lastSeen time.Time) *models.Temperature {
@@ -411,6 +422,13 @@ func canonicalSMARTDevicePath(device string) string {
// isHostAgentTemperatureRecent checks if the host agent temperature data is recent enough to use.
// We consider data stale if the host hasn't reported in more than 2 minutes.
// hostAgentReportCurrent reports whether an agent's last report is still inside
// the reporting lease that keeps the agent online (evaluateHostAgents uses the
// same window), so its sensor readings still describe the machine.
func hostAgentReportCurrent(lastSeen time.Time, intervalSeconds int, now time.Time) bool {
return !lastSeen.IsZero() && now.Sub(lastSeen) <= hostAgentHealthWindow(intervalSeconds)
}
func isHostAgentTemperatureRecent(lastSeen time.Time) bool {
const staleDuration = 2 * time.Minute
return time.Since(lastSeen) < staleDuration
@@ -1,11 +1,14 @@
package monitoring
import (
"context"
"testing"
"time"
"github.com/rcourtman/pulse-go-rewrite/internal/config"
"github.com/rcourtman/pulse-go-rewrite/internal/models"
unifiedresources "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources"
"github.com/rcourtman/pulse-go-rewrite/pkg/proxmox"
"github.com/stretchr/testify/assert"
)
@@ -459,6 +462,7 @@ func TestGetHostAgentTemperature(t *testing.T) {
Sensors: models.HostSensorSummary{
TemperatureCelsius: map[string]float64{"cpu_package": 60.0},
},
LastSeen: time.Now(),
}
m.state.UpsertHost(host)
@@ -497,6 +501,7 @@ func TestGetHostAgentTemperature(t *testing.T) {
Sensors: models.HostSensorSummary{
TemperatureCelsius: map[string]float64{"cpu_package": 65.0},
},
LastSeen: time.Now(),
}
m.state.UpsertHost(host)
@@ -663,6 +668,7 @@ func TestGetHostAgentTemperatureByID_LocalAgentTakesPriority(t *testing.T) {
"cpu_package": 70.0, // local agent reports 70
},
},
LastSeen: time.Now(),
})
m.clusterSensorsCache["shared-node"] = clusterSensorsCacheEntry{
sensors: models.HostSensorSummary{
@@ -678,6 +684,98 @@ func TestGetHostAgentTemperatureByID_LocalAgentTakesPriority(t *testing.T) {
assert.Equal(t, 70.0, result.CPUPackage, "local agent data should take priority over cluster cache")
}
// An agent that stops reporting keeps its last sensors in state. Past the
// reporting lease that marks it offline, those sensors must not keep feeding the
// linked node as a live reading; the cluster cache (with its own recency) is
// still consulted.
func TestGetHostAgentTemperatureByID_IgnoresAgentPastReportingLease(t *testing.T) {
m := &Monitor{
state: models.NewState(),
clusterSensorsCache: make(map[string]clusterSensorsCacheEntry),
}
m.state.UpsertHost(models.Host{
ID: "host-silent",
Hostname: "silent-node",
LinkedNodeID: "node-silent",
IntervalSeconds: 30,
Sensors: models.HostSensorSummary{
TemperatureCelsius: map[string]float64{"cpu_package": 95.0},
},
LastSeen: time.Now().Add(-hostAgentHealthWindow(30) - time.Minute),
})
assert.Nil(t, m.getHostAgentTemperatureByID("node-silent", "silent-node"),
"a silent agent's retained sensors must not be presented as a current reading")
m.clusterSensorsCache["silent-node"] = clusterSensorsCacheEntry{
sensors: models.HostSensorSummary{
TemperatureCelsius: map[string]float64{"cpu_package": 58.0},
},
updatedAt: time.Now(),
}
if result := m.getHostAgentTemperatureByID("node-silent", "silent-node"); assert.NotNil(t, result) {
assert.Equal(t, 58.0, result.CPUPackage, "a recent cluster-cache reading still serves the node")
}
m.state.UpsertHost(models.Host{
ID: "host-silent",
Hostname: "silent-node",
LinkedNodeID: "node-silent",
IntervalSeconds: 30,
Sensors: models.HostSensorSummary{
TemperatureCelsius: map[string]float64{"cpu_package": 71.0},
},
LastSeen: time.Now(),
})
if result := m.getHostAgentTemperatureByID("node-silent", "silent-node"); assert.NotNil(t, result) {
assert.Equal(t, 71.0, result.CPUPackage, "a reporting agent's reading is used again")
}
}
// When every temperature source returns nothing, a previous reading may be
// carried only inside the carry window and keeps its original timestamp; an
// older one is dropped rather than re-presented as current.
func TestCollectNodeTemperatureCarryIsBoundedByCarryWindow(t *testing.T) {
m := &Monitor{
config: &config.Config{TemperatureMonitoringEnabled: true, PVEPollingInterval: 10 * time.Second},
state: models.NewState(),
}
collectWithPrevious := func(prev *models.Temperature) *models.Temperature {
node := models.Node{ID: "pve1-node1", Name: "node1", Instance: "pve1"}
m.collectNodeTemperatureData(
context.Background(), "pve1", &config.PVEInstance{Name: "pve1"}, proxmox.Node{Node: "node1"},
&node, []models.Node{{ID: node.ID, Temperature: prev}}, "online",
)
return node.Temperature
}
assert.Equal(t, 5*time.Minute, m.nodeTemperatureCarryWindow(), "the floor applies at the default polling interval")
m.config.PVEPollingInterval = 10 * time.Minute
assert.Equal(t, 20*time.Minute, m.nodeTemperatureCarryWindow(), "the window scales with a slow polling interval")
m.config.PVEPollingInterval = 10 * time.Second
recent := time.Now().Add(-time.Minute)
if carried := collectWithPrevious(&models.Temperature{Available: true, CPUPackage: 92, LastUpdate: recent}); assert.NotNil(t, carried) {
assert.True(t, carried.Available)
assert.True(t, carried.LastUpdate.Equal(recent), "a carried reading keeps its original timestamp")
}
stale := time.Now().Add(-m.nodeTemperatureCarryWindow() - time.Minute)
assert.Nil(t, collectWithPrevious(&models.Temperature{Available: true, CPUPackage: 92, LastUpdate: stale}),
"a reading older than the carry window must not be presented as current")
assert.Nil(t, collectWithPrevious(&models.Temperature{Available: true, CPUPackage: 92}),
"a reading with no timestamp cannot be shown to be recent")
}
func TestHostAgentReportCurrent(t *testing.T) {
now := time.Now()
assert.False(t, hostAgentReportCurrent(time.Time{}, 30, now), "an agent that never reported has no current reading")
assert.True(t, hostAgentReportCurrent(now.Add(-hostAgentHealthWindow(30)), 30, now))
assert.False(t, hostAgentReportCurrent(now.Add(-hostAgentHealthWindow(30)-time.Second), 30, now))
// A slow agent's lease scales with its declared interval.
assert.True(t, hostAgentReportCurrent(now.Add(-8*time.Minute), 120, now))
}
func TestGetHostAgentTemperatureByID_UsesUnifiedReadState(t *testing.T) {
now := time.Now().UTC()
registry := unifiedresources.NewRegistry(nil)
@@ -1,6 +1,7 @@
package monitoring
import (
"context"
"encoding/json"
"io"
"net/http"
@@ -13,10 +14,12 @@ import (
"github.com/rcourtman/pulse-go-rewrite/internal/ai/memory"
"github.com/rcourtman/pulse-go-rewrite/internal/alerts"
"github.com/rcourtman/pulse-go-rewrite/internal/alerts/eventlog"
"github.com/rcourtman/pulse-go-rewrite/internal/config"
"github.com/rcourtman/pulse-go-rewrite/internal/models"
"github.com/rcourtman/pulse-go-rewrite/internal/notifications"
unifiedresources "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources"
"github.com/rcourtman/pulse-go-rewrite/internal/websocket"
"github.com/rcourtman/pulse-go-rewrite/pkg/proxmox"
"github.com/stretchr/testify/require"
)
@@ -1466,3 +1469,92 @@ func TestMonitorDelayedPartialResolutionKeepsDestinationRecovery(t *testing.T) {
t.Fatal("unannounced destination received a recovery")
}
}
// A host agent linked to a Proxmox node merges into one read-state row that
// every PVE poll keeps fresh. When the agent stops reporting, its retained
// sensors must stop feeding the node's temperature: otherwise the frozen
// reading is re-observed every poll, an open temperature alert's LastSeen keeps
// moving, and stale-alert cleanup never reaches it.
func TestSilentLinkedAgentStopsRefreshingNodeTemperatureAlert(t *testing.T) {
manager := alerts.NewManagerWithDataDir(t.TempDir(), alerts.WithoutPersistedAlertRestore())
t.Cleanup(manager.Stop)
alertConfig := manager.GetConfig()
alertConfig.Enabled = true
alertConfig.TimeThresholds = map[string]int{}
alertConfig.SuppressionWindow = 0
alertConfig.NodeDefaults.Temperature = &alerts.HysteresisThreshold{Trigger: 80, Clear: 75}
manager.UpdateConfig(alertConfig)
registry := unifiedresources.NewRegistry(nil)
monitor := &Monitor{
config: &config.Config{TemperatureMonitoringEnabled: true},
state: models.NewState(),
resourceStore: unifiedresources.NewMonitorAdapter(registry),
}
node := models.Node{
ID: "pve1-node1", Name: "node1", Instance: "pve1", Status: "online",
ConnectionHealth: "healthy", LinkedAgentID: "agent-1",
}
// ingest records a PVE poll that just saw the node, plus the agent as of its
// last report.
ingest := func(agentLastReport time.Time, agentStatus string) {
polledNode := node
polledNode.LastSeen = time.Now()
registry.IngestSnapshot(models.StateSnapshot{
Nodes: []models.Node{polledNode},
Hosts: []models.Host{{
ID: "agent-1", Hostname: "node1", Status: agentStatus, LinkedNodeID: node.ID,
IntervalSeconds: 30, LastSeen: agentLastReport,
Sensors: models.HostSensorSummary{TemperatureCelsius: map[string]float64{"cpu_package": 92}},
}},
})
}
// poll runs the node temperature step of a PVE poll (no SSH collector) and
// evaluates the node's alerts with the result.
poll := func() *models.Temperature {
_, prevNodes := monitor.snapshotPrevNodes("pve1")
polled := node
monitor.collectNodeTemperatureData(
context.Background(), "pve1", &config.PVEInstance{Name: "pve1"}, proxmox.Node{Node: node.Name},
&polled, prevNodes, "online",
)
manager.CheckNode(polled)
return polled.Temperature
}
temperatureAlert := func() alerts.Alert {
t.Helper()
for _, alert := range manager.GetActiveAlerts() {
if alert.Type == "temperature" && alert.ResourceID == node.ID {
return alert
}
}
t.Fatalf("expected an open temperature alert for %s", node.ID)
return alerts.Alert{}
}
// While the agent reports, its hot reading feeds the node, stamped with the
// agent's report time, and opens the alert.
agentReport := time.Now().Add(-10 * time.Second)
ingest(agentReport, "online")
reading := poll()
require.NotNil(t, reading)
require.Equal(t, 92.0, reading.CPUPackage)
require.True(t, reading.LastUpdate.Equal(agentReport), "the reading carries the agent's own report time")
opened := temperatureAlert()
// The agent goes silent while PVE polling continues: the merged row stays
// fresh, but the agent's retained sensors no longer count as a reading.
ingest(time.Now().Add(-hostAgentHealthWindow(30)-time.Minute), "offline")
time.Sleep(5 * time.Millisecond)
require.Nil(t, poll(), "a silent agent's retained sensors must not be presented as a current reading")
held := temperatureAlert()
require.True(t, held.LastSeen.Equal(opened.LastSeen), "a frozen agent reading must not keep the alert fresh")
require.Nil(t, poll())
require.True(t, temperatureAlert().LastSeen.Equal(opened.LastSeen))
// The agent reports again and its reading is evaluated.
ingest(time.Now(), "online")
time.Sleep(5 * time.Millisecond)
require.NotNil(t, poll())
require.True(t, temperatureAlert().LastSeen.After(held.LastSeen), "a fresh agent reading is evaluated")
}
@@ -129,6 +129,18 @@ func (m *Monitor) pveNodeUsesProviderScopedIdentity(instanceName string, instanc
)
}
// nodeTemperatureCarryFloor is the shortest time a node may keep showing its
// last temperature after collection stops returning one, sized to ride out the
// SSH collector's early failure backoff steps (30s, 1m, 2m).
const nodeTemperatureCarryFloor = 5 * time.Minute
// nodeTemperatureCarryWindow bounds how old a carried-over node temperature may
// be. Beyond it the reading no longer describes the node, so it stops being
// presented as available.
func (m *Monitor) nodeTemperatureCarryWindow() time.Duration {
return resourceStaleThresholdForPollInterval(effectivePVEPollingIntervalForConfig(m.config), nodeTemperatureCarryFloor)
}
func (m *Monitor) collectNodeTemperatureData(
ctx context.Context,
instanceName string,
@@ -319,7 +331,16 @@ func (m *Monitor) collectNodeTemperatureData(
}
}
if prevTemp != nil {
if prevTemp != nil && time.Since(prevTemp.LastUpdate) > m.nodeTemperatureCarryWindow() {
// Collection has stayed broken past the carry window. Re-presenting
// the old reading would let alerts, history and the UI treat it as
// a live measurement forever, so the node has no reading now.
log.Debug().
Str("node", node.Node).
Bool("isCluster", modelNode.IsClusterMember).
Time("lastUpdate", prevTemp.LastUpdate).
Msg("Dropped stale temperature data (collection has not returned a reading within the carry window)")
} else if prevTemp != nil {
// Clone the previous temperature to avoid modifying historical data
preserved := *prevTemp
preserved.LastUpdate = prevTemp.LastUpdate // Keep original update time to indicate staleness
+12
View File
@@ -1372,6 +1372,18 @@ func (v HostView) LastSeen() time.Time {
return v.r.LastSeen
}
// SourceStatus returns the canonical delivery freshness recorded for one
// source. On a merged row (a host agent linked to a Proxmox node) LastSeen
// follows the freshest source, so consumers of agent-owned samples such as
// sensors must read the agent source's own sighting here instead.
func (v HostView) SourceStatus(source DataSource) (SourceStatus, bool) {
if v.r == nil {
return SourceStatus{}, false
}
status, ok := v.r.SourceStatus[source]
return status, ok
}
func (v HostView) CPUPercent() float64 {
if v.r == nil {
return 0
+40
View File
@@ -932,6 +932,46 @@ func TestView_HostViewIntegrationSource(t *testing.T) {
}
}
// A host agent linked to a Proxmox node merges into one row whose LastSeen
// follows the freshest source. Agent-owned samples (sensors) need the agent's
// own sighting, which SourceStatus exposes.
func TestView_HostViewSourceStatusSeparatesAgentFromMergedRow(t *testing.T) {
now := time.Now().UTC()
agentLastReport := now.Add(-30 * time.Minute)
registry := NewRegistry(nil)
registry.IngestSnapshot(models.StateSnapshot{
Nodes: []models.Node{{
ID: "pve1-node1", Name: "node1", Instance: "pve1", Status: "online",
LastSeen: now, LinkedAgentID: "agent-1",
}},
Hosts: []models.Host{{
ID: "agent-1", Hostname: "node1", Status: "offline", LinkedNodeID: "pve1-node1",
IntervalSeconds: 30, LastSeen: agentLastReport,
}},
})
hosts := registry.Hosts()
if len(hosts) != 1 {
t.Fatalf("expected the agent and node to merge into one host row, got %d", len(hosts))
}
host := hosts[0]
if !host.LastSeen().Equal(now) {
t.Fatalf("merged row LastSeen = %v, want the fresher Proxmox sighting %v", host.LastSeen(), now)
}
agent, ok := host.SourceStatus(SourceAgent)
if !ok || !agent.LastSeen.Equal(agentLastReport) {
t.Fatalf("agent source status = %+v (ok=%v), want the agent's own report time %v", agent, ok, agentLastReport)
}
if _, ok := host.SourceStatus(SourceTrueNAS); ok {
t.Fatal("a source that never reported must not have a status")
}
var nilView HostView
if _, ok := nilView.SourceStatus(SourceAgent); ok {
t.Fatal("nil-backed view must report no source status")
}
}
func TestView_HostViewMetricsTargetAccessor(t *testing.T) {
r := &Resource{
ID: "host-metrics",