diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 9ae9a5d83a..bad4963674 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -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 diff --git a/docs/release-control/v6/internal/subsystems/unified-resources.md b/docs/release-control/v6/internal/subsystems/unified-resources.md index 0f0b86147c..0df9a747ce 100644 --- a/docs/release-control/v6/internal/subsystems/unified-resources.md +++ b/docs/release-control/v6/internal/subsystems/unified-resources.md @@ -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 diff --git a/internal/monitoring/host_agent_temps.go b/internal/monitoring/host_agent_temps.go index 5d10cabf3c..661caeaf6e 100644 --- a/internal/monitoring/host_agent_temps.go +++ b/internal/monitoring/host_agent_temps.go @@ -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 diff --git a/internal/monitoring/host_agent_temps_test.go b/internal/monitoring/host_agent_temps_test.go index b03dd61496..5885b47ea8 100644 --- a/internal/monitoring/host_agent_temps_test.go +++ b/internal/monitoring/host_agent_temps_test.go @@ -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) diff --git a/internal/monitoring/monitor_alert_handling_test.go b/internal/monitoring/monitor_alert_handling_test.go index 498bd35b8d..1bbef67f02 100644 --- a/internal/monitoring/monitor_alert_handling_test.go +++ b/internal/monitoring/monitor_alert_handling_test.go @@ -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") +} diff --git a/internal/monitoring/monitor_polling_node_helpers.go b/internal/monitoring/monitor_polling_node_helpers.go index 03156560d1..ced65961a2 100644 --- a/internal/monitoring/monitor_polling_node_helpers.go +++ b/internal/monitoring/monitor_polling_node_helpers.go @@ -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 diff --git a/internal/unifiedresources/views.go b/internal/unifiedresources/views.go index 4219d0469e..81667e27e1 100644 --- a/internal/unifiedresources/views.go +++ b/internal/unifiedresources/views.go @@ -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 diff --git a/internal/unifiedresources/views_test.go b/internal/unifiedresources/views_test.go index 4840036535..8c8bcc98cc 100644 --- a/internal/unifiedresources/views_test.go +++ b/internal/unifiedresources/views_test.go @@ -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",