diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index b8ed3f012b..9b3e073f19 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -248,6 +248,18 @@ Without comparable identity (a SAS, SCSI or USB replacement with no WWN on one side) the row still matches. Report admission and the SMART wire contract are unchanged; the regression is `TestHostAgentSMARTRowForSwappedOutDiskDoesNotLendItsIdentity`. +A PVE disk that a still-reporting linked Agent's SMART row matches is marked +`AgentSMARTReported` (internal poll evidence on `models.PhysicalDisk`, never +serialized); disks the poller builds from that report when the Proxmox disk +query fails go through the same merge, and an Agent whose lease lapsed owns +none. The Agent then owns the disk's temperature alert +through `CheckHost`, and the PVE disk temperature check closes its own as +moved to the Agent, so one disk never alerts twice. Report admission, the SMART +wire contract and Agent authority are unchanged; +`TestMergeHostAgentSMARTIntoDisksMarksDisksTheAgentReports` in +`internal/monitoring/physical_disk_roundtrip_test.go` and +`TestPhysicalDiskAgentSMARTReportedStaysInternal` in +`internal/models/deepcopy_test.go` pin the marker. Assistant historical metric wiring uses the current monitor's retained store and registry metrics coordinates. Historical reads do not alter enrollment, diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index c7ab193445..67b6ad79ec 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -1876,6 +1876,38 @@ override. `TestNormalizeTrueNASDiskTemperatureKeepsOnlyChosenValues` pins the saved-config migration. +Proxmox physical disks alert on temperature under the same policy. +`CheckProxmoxDiskTemperature` (`internal/alerts/proxmox_disk_temperature.go`) +raises a `diskTemperature` metric alert under the disk's PVE alert reference +(`ProxmoxPhysicalDiskAlertResourceID`, resource type `proxmox-disk`), judged by +`proxmoxDiskTemperatureThresholdNoLock`: the disk type's `DiskTempByType` entry +under the agent Disk Temp default, which switches it off for every type. It +carries the disk's serial, WWN and `diskType` like the health and wearout +alerts, so `ProxmoxPhysicalDiskAlertIdentifiers` lists its +`::metric-threshold:diskTemperature` identifier for history +ownership, and `reevaluateActiveAlertsLocked` re-judges it per disk type (the +lowest per-type trigger when the type was never recorded) instead of falling +through to guest thresholds that have no disk temperature. A disk a +still-reporting linked agent lists in its SMART report is the agent's: its +alert closes with the `moved_to_agent` resolution and `CheckHost` raises the +agent's own. The handover happens on the next disk poll, so for that one +interval an agent newly linked to a disk with an open PVE alert can hold its +own alert beside it. A lapsed agent owns nothing, so the node's own readings +keep alerting while its agent is silent. An +excluded disk, or a policy switched off, closes the alert; a poll without a +current reading holds it. Like the health and wearout alerts, a disk that +leaves the inventory keeps its alert until the 24-hour stale sweep. The +`proxmox-disk` alert delay defaults to the factory 5 seconds, so a warning +waits out the same 300-second noisy-gauge stability window agent and TrueNAS +disk temperature warnings do, and a critical reading bypasses it. +`TestProxmoxDiskTemperatureAlertsFollowDiskTemperaturePolicy`, +`TestProxmoxDiskTemperatureAlertOwnershipAndHolds`, +`TestProxmoxDiskTemperatureWarningWaitsOutStabilityWindow`, +`TestProxmoxDiskTemperatureAlertsReevaluatePerDiskType` and +`TestCheckDiskHealthAlertsCarryHistoryOwnershipIdentity` in +`internal/alerts/alerts_test.go` pin the policy, ownership, holds, re-judging +and identifiers. + ### Agent disk temperature alerts clear when their disk leaves the report `CheckHost` also clears a disk temperature alert, and drops its pending diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 3c011447f5..bc30f02fc3 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -1825,6 +1825,29 @@ boundary around it: matches node names only, so it relies on fixture node names being unique across instances, which they are. +`checkPhysicalDiskAlerts` also hands each disk's temperature to +`CheckProxmoxDiskTemperature`, so a Proxmox disk that only the node's sensors +report alerts under the disk temperature policy, as its Running Hot verdict +already says it should. It passes the reading only when this poll collected it +(`collectedPhysicalDiskTemperature`, `diskinventory.TemperatureCollected`), so +a retained last-known value holds the alert rather than judging it, and it +passes `AgentSMARTReported`, which `mergeHostAgentSMARTIntoDisks` sets on a +disk a still-reporting linked agent lists in its SMART report (disks built +from that report when the Proxmox query fails go through the same merge). A +disk the agent lists is the agent's: `CheckHost` raises its temperature +alert, and the PVE disk check closes its own as moved to the agent, so no +disk alerts twice. Once the agent's lease lapses (`ExpireHostTelemetry` marks +the host offline) its retained rows still enrich the disk, but ownership +returns to the PVE check, which judges the node's own current readings; an +agent newly linked to a disk with an open PVE alert takes it over on the next +disk poll. An excluded device closes its temperature +alert with the health and wearout ones. Fixture disks carry no agent SMART +merge, so on a mock estate every hot disk on an online node alerts. +`TestMergeHostAgentSMARTIntoDisksMarksDisksTheAgentReports` and +`TestCheckPhysicalDiskAlertsRaisesProxmoxDiskTemperatureAlerts` in +`internal/monitoring/physical_disk_roundtrip_test.go` pin the marker, +the collected reading, agent ownership and exclusion. + Switching mock mode fences the alert evaluations that read mode-dependent data (`mockModeFence`, `internal/monitoring/mock_mode_fence.go`). `GetState`, the fixture graph, the unified read view, the recovery rollups and the connection diff --git a/docs/release-control/v6/internal/subsystems/unified-resources.md b/docs/release-control/v6/internal/subsystems/unified-resources.md index 63fd2077e1..aab859fa4e 100644 --- a/docs/release-control/v6/internal/subsystems/unified-resources.md +++ b/docs/release-control/v6/internal/subsystems/unified-resources.md @@ -5946,7 +5946,12 @@ the reference and is never retried. An owned row records its alert's reference as `alert_resource_id`. A read by the reference returns the rows journaled under it plus the rows owned away from it, matched through the indexed alert identifier (`ProxmoxPhysicalDiskAlertIdentifiers`) and that -recorded reference, and every count uses the same predicate. The Alerts +recorded reference, and every count uses the same predicate. Those +identifiers cover the health and wearout specs and the disk temperature +metric spec (`::metric-threshold:diskTemperature`), since PVE disks +also raise temperature alerts under the same reference +(`TestProxmoxDiskTemperatureAlertRowsFollowRecordedHardwareIdentity` in +`internal/unifiedresources/store_test.go`). The Alerts history Resource action therefore still lists every occurrence raised under the path, across the disks that held it, and alert-centric reads keep their occurrences (see the AI runtime contract's incident-history queries). Rows diff --git a/frontend-modern/browser-verification/d2df05cc61b44990.json b/frontend-modern/browser-verification/d2df05cc61b44990.json new file mode 100644 index 0000000000..6514b6055e --- /dev/null +++ b/frontend-modern/browser-verification/d2df05cc61b44990.json @@ -0,0 +1,39 @@ +{ + "version": 1, + "base_sha": "8e1a564d7e229bda51c021c7e6bde920e0ca0596", + "verified_at": "2026-10-07T14:35:42Z", + "result": "passed", + "changed_paths": [ + "frontend-modern/src/components/Alerts/ThresholdsTableAgentsResourcesSection.tsx" + ], + "content_sha256": { + "frontend-modern/src/components/Alerts/ThresholdsTableAgentsResourcesSection.tsx": "3052a190a76e885e98ed6601b3e83c8959e1c656d12270b87e074fffa95860ac" + }, + "routes": [ + "/alerts/thresholds/systems", + "/alerts/overview", + "/proxmox/storage?tab=disks" + ], + "viewports": [ + { + "width": 1366, + "height": 900 + }, + { + "width": 375, + "height": 812 + } + ], + "states": [ + "Private mock stack (8 PVE nodes) on this branch with a local-only generator overlay pinning PVE disks: pve1 NVMe 63C, pve2 SATA 56C, pve3 NVMe 72C, pve3 SAS 63C, pve3 SATA 66C", + "Alerts > Thresholds > Systems (Agents section): 'Disk temperature by type' card reads 'Alert trigger in \u00b0C for each disk type, for agent, TrueNAS and Proxmox disks. ... a TrueNAS Disks value replaces them for TrueNAS disks.' at 1366, and at 375 the card spans 34-341px with no horizontal overflow", + "Alerts overview: 'Samsung 870 EVO 1TB (/dev/nvme2) (Temperature) CRITICAL on West Production C \u2014 Temperature 66\u00b0C, above the 55\u00b0C alert level; Clears once it drops to 50\u00b0C or lower and stays there for 5 minutes', Alert level 55\u00b0C, Clear level 50\u00b0C", + "Warning-level PVE disk temperature alerts (SATA 56C trigger 55, NVMe 72C trigger 70) appear after the 300s stability window (first breach 15:26:32, firing by 15:31:46); the critical 66C fires at once; none for NVMe 63C or SAS 63C", + "Physical Disks filtered to West Production: exactly the three alerting disks read Running Hot (Crucial MX500 56\u00b0C, WD Black SN850X 72\u00b0C, Samsung 870 EVO 66\u00b0C); the 870 EVO drawer reads 'Running Hot \u2014 Disk temperature is 66\u00b0C, at or above its 55\u00b0C alert threshold.'" + ], + "interactions": [ + "Opened Alerts overview and read the PVE disk temperature alert card", + "Filtered Physical Disks by 'West Production C', expanded the Samsung 870 EVO row", + "Opened Thresholds > Systems, read the Disk temperature by type card at 1366 and at 375" + ] +} diff --git a/frontend-modern/src/components/Alerts/ThresholdsTableAgentsResourcesSection.tsx b/frontend-modern/src/components/Alerts/ThresholdsTableAgentsResourcesSection.tsx index 1fa712f1ee..0929cf7e5d 100644 --- a/frontend-modern/src/components/Alerts/ThresholdsTableAgentsResourcesSection.tsx +++ b/frontend-modern/src/components/Alerts/ThresholdsTableAgentsResourcesSection.tsx @@ -82,9 +82,10 @@ export function ThresholdsTableAgentsResourcesSection(props: ThresholdsTableSect

Disk temperature by type

- Alert trigger in °C for each disk type, for agent and TrueNAS disks. Warning colors - start 5°C below the trigger. A Disk Temp override on a host above replaces these for - all of that host's disks, and a TrueNAS Disks value replaces them for TrueNAS disks. + Alert trigger in °C for each disk type, for agent, TrueNAS and Proxmox disks. Warning + colors start 5°C below the trigger. A Disk Temp override on a host above replaces + these for all of that host's disks, and a TrueNAS Disks value replaces them for + TrueNAS disks.

diff --git a/frontend-modern/src/components/Alerts/__tests__/ThresholdsTable.test.tsx b/frontend-modern/src/components/Alerts/__tests__/ThresholdsTable.test.tsx index 06dd27a8c5..dd57de354b 100644 --- a/frontend-modern/src/components/Alerts/__tests__/ThresholdsTable.test.tsx +++ b/frontend-modern/src/components/Alerts/__tests__/ThresholdsTable.test.tsx @@ -457,6 +457,25 @@ describe('ThresholdsTable navigation and redirection', () => { }); }); + it('says Disk temperature by type covers agent, TrueNAS and Proxmox disks', async () => { + setPathname('/alerts/thresholds/systems'); + const host: Agent = { + id: 'legacy-h1', + hostname: 'legacy-host', + displayName: 'Legacy Host', + status: 'online', + lastSeen: 123, + memory: { total: 100, used: 50, free: 50, usage: 50 }, + }; + + render(() => ); + + const heading = await screen.findByText('Disk temperature by type'); + expect(heading.parentElement?.textContent).toContain( + 'for each disk type, for agent, TrueNAS and Proxmox disks', + ); + }); + it('navigates to correct route when tabs are clicked', () => { render(() => ); diff --git a/internal/alerts/alerts_test.go b/internal/alerts/alerts_test.go index 5dfafdca51..c5a8992366 100644 --- a/internal/alerts/alerts_test.go +++ b/internal/alerts/alerts_test.go @@ -22173,14 +22173,20 @@ func TestStaleCleanupKeepsThresholdAlertStillBeingEvaluated(t *testing.T) { // Resource history owns each PVE disk alert row by the hardware identity the // alert records, and reads by the path reference find those rows by alert -// identifier. Both must match what CheckDiskHealth produces. +// identifier. Both must match what CheckDiskHealth and +// CheckProxmoxDiskTemperature produce. func TestCheckDiskHealthAlertsCarryHistoryOwnershipIdentity(t *testing.T) { m := newTestManager(t) m.ClearActiveAlerts() + m.mu.Lock() + m.config.TimeThresholds = map[string]int{} + m.config.MetricTimeThresholds = nil + m.mu.Unlock() disk := proxmox.Disk{DevPath: "/dev/sdb", Model: "Crucial MX500", Serial: "2117E59AB123", WWN: "0x5002538f12345678", Type: "ssd", Health: "FAILED", Wearout: 3} for i := 0; i < 3; i++ { m.CheckDiskHealth("lab", "pve1", disk) + m.CheckProxmoxDiskTemperature("lab", "pve1", ProxmoxDiskTemperatureReading{Disk: disk, Celsius: 80}) } ref := unifiedresources.ProxmoxPhysicalDiskAlertResourceID("lab", "pve1", disk.DevPath) @@ -22201,3 +22207,187 @@ func TestCheckDiskHealthAlertsCarryHistoryOwnershipIdentity(t *testing.T) { } } } + +func proxmoxTemperatureDisk(devPath, diskType string) proxmox.Disk { + return proxmox.Disk{DevPath: devPath, Model: "Test Disk", Serial: "SER-" + devPath, Type: diskType, Health: "PASSED"} +} + +func proxmoxDiskTemperatureAlert(t *testing.T, m *Manager, devPath string) (*Alert, bool) { + t.Helper() + ref := unifiedresources.ProxmoxPhysicalDiskAlertResourceID("lab", "pve1", devPath) + return testLookupActiveAlert(t, m, canonicalMetricStateID(ref, proxmoxDiskTemperatureMetric)) +} + +func proxmoxDiskTemperatureResolution(m *Manager, devPath string) (*models.AlertResolution, bool) { + ref := unifiedresources.ProxmoxPhysicalDiskAlertResourceID("lab", "pve1", devPath) + id := canonicalMetricStateID(ref, proxmoxDiskTemperatureMetric) + for _, resolved := range m.GetRecentlyResolved() { + if resolved.ID == id { + return resolved.Resolution, true + } + } + return nil, false +} + +// TestProxmoxDiskTemperatureAlertsFollowDiskTemperaturePolicy pins Proxmox disk +// temperature alerts to the disk temperature policy that judges an agent's +// disks, TrueNAS disks and the Physical Disks verdict. +func TestProxmoxDiskTemperatureAlertsFollowDiskTemperaturePolicy(t *testing.T) { + m := newTestManager(t) + configureUnifiedEvalManager(t, m, unifiedEvalBaseConfig()) + + check := func(devPath, diskType string, celsius int) { + m.CheckProxmoxDiskTemperature("lab", "pve1", ProxmoxDiskTemperatureReading{Disk: proxmoxTemperatureDisk(devPath, diskType), Celsius: celsius}) + } + check("/dev/nvme0n1", "nvme", 63) + check("/dev/nvme1n1", "nvme", 72) + check("/dev/sda", "sata", 56) + check("/dev/sdb", "sas", 63) + + if _, firing := proxmoxDiskTemperatureAlert(t, m, "/dev/nvme0n1"); firing { + t.Fatalf("NVMe at 63C fired under its 70C trigger: %v", alertKeys(m)) + } + if _, firing := proxmoxDiskTemperatureAlert(t, m, "/dev/sdb"); firing { + t.Fatalf("SAS at 63C fired under its 65C trigger: %v", alertKeys(m)) + } + for devPath, trigger := range map[string]float64{"/dev/nvme1n1": 70, "/dev/sda": 55} { + alert, firing := proxmoxDiskTemperatureAlert(t, m, devPath) + if !firing { + t.Fatalf("%s did not fire: %v", devPath, alertKeys(m)) + } + if alert.Threshold != trigger { + t.Fatalf("%s trigger = %v, want %v", devPath, alert.Threshold, trigger) + } + if alert.Type != proxmoxDiskTemperatureMetric || alert.Metadata["resourceType"] != proxmoxDiskResourceType { + t.Fatalf("%s alert type %q, resourceType %v", devPath, alert.Type, alert.Metadata["resourceType"]) + } + if alert.Metadata["diskType"] == nil || alert.Metadata[unifiedresources.MetadataDiskSerial] != "SER-"+devPath { + t.Fatalf("%s metadata = %v", devPath, alert.Metadata) + } + } + if alert, _ := proxmoxDiskTemperatureAlert(t, m, "/dev/sda"); alert.Message != "Disk temperature at 56.0°C" { + t.Fatalf("message = %q", alert.Message) + } + + // A raised NVMe trigger moves NVMe disks with it. + m.mu.Lock() + m.config.DiskTempByType["nvme"] = HysteresisThreshold{Trigger: 75, Clear: 70} + m.mu.Unlock() + check("/dev/nvme2n1", "nvme", 72) + if _, firing := proxmoxDiskTemperatureAlert(t, m, "/dev/nvme2n1"); firing { + t.Fatalf("NVMe at 72C fired under a 75C trigger") + } +} + +// A disk the linked agent reports is the agent's: its open alert closes as +// moved to the agent, and the disk never alerts twice. An excluded disk, or a +// policy the agent Disk Temp default switched off, closes it as a plain clear. +// A disk with no current reading keeps its alert. +func TestProxmoxDiskTemperatureAlertOwnershipAndHolds(t *testing.T) { + m := newTestManager(t) + configureUnifiedEvalManager(t, m, unifiedEvalBaseConfig()) + disk := proxmoxTemperatureDisk("/dev/sda", "sata") + read := func(reading ProxmoxDiskTemperatureReading) { + reading.Disk = disk + m.CheckProxmoxDiskTemperature("lab", "pve1", reading) + } + firing := func() bool { + _, ok := proxmoxDiskTemperatureAlert(t, m, "/dev/sda") + return ok + } + + read(ProxmoxDiskTemperatureReading{Celsius: 60}) + if !firing() { + t.Fatalf("SATA at 60C did not fire: %v", alertKeys(m)) + } + read(ProxmoxDiskTemperatureReading{Celsius: 0}) + if !firing() { + t.Fatalf("a poll without a current reading resolved the alert") + } + + read(ProxmoxDiskTemperatureReading{Celsius: 60, AgentOwned: true}) + if firing() { + t.Fatalf("an agent-owned disk kept its Proxmox temperature alert") + } + if resolution, ok := proxmoxDiskTemperatureResolution(m, "/dev/sda"); !ok || resolution == nil || resolution.Reason != string(AlertResolutionMovedToAgent) { + t.Fatalf("handover resolution = %+v (resolved %v), want moved_to_agent", resolution, ok) + } + read(ProxmoxDiskTemperatureReading{Celsius: 80, AgentOwned: true}) + if firing() { + t.Fatalf("an agent-owned disk raised a Proxmox temperature alert") + } + + read(ProxmoxDiskTemperatureReading{Celsius: 60}) + if !firing() { + t.Fatalf("the disk did not alert again once the agent stopped listing it") + } + read(ProxmoxDiskTemperatureReading{Celsius: 60, Excluded: true}) + if firing() { + t.Fatalf("an excluded disk kept its temperature alert") + } + + read(ProxmoxDiskTemperatureReading{Celsius: 60}) + m.mu.Lock() + m.config.AgentDefaults.DiskTemperature = &HysteresisThreshold{Trigger: 0, Clear: 0} + m.mu.Unlock() + read(ProxmoxDiskTemperatureReading{Celsius: 60}) + if firing() { + t.Fatalf("a switched-off agent Disk Temp default kept the Proxmox disk alert") + } +} + +// A Proxmox disk temperature warning waits out the factory stability window for +// a noisy gauge, as agent and TrueNAS disk temperature warnings do; a critical +// reading bypasses it. +func TestProxmoxDiskTemperatureWarningWaitsOutStabilityWindow(t *testing.T) { + m := newTestManager(t) + cfg := unifiedEvalBaseConfig() + m.UpdateConfig(cfg) + if delay := m.GetConfig().TimeThresholds[proxmoxDiskResourceType]; delay != 5 { + t.Fatalf("proxmox-disk delay = %d, want the 5s factory default", delay) + } + m.mu.RLock() + grace := m.getTimeThreshold("lab:pve1:disk:dev-sda", proxmoxDiskResourceType, proxmoxDiskTemperatureMetric) + m.mu.RUnlock() + if grace != defaultNoisyGaugeStabilitySeconds { + t.Fatalf("Proxmox disk temperature delay = %ds, want the %ds stability window", grace, defaultNoisyGaugeStabilitySeconds) + } + + m.CheckProxmoxDiskTemperature("lab", "pve1", ProxmoxDiskTemperatureReading{Disk: proxmoxTemperatureDisk("/dev/sda", "sata"), Celsius: 57}) + if _, firing := proxmoxDiskTemperatureAlert(t, m, "/dev/sda"); firing { + t.Fatalf("a 57C SATA warning fired before its stability window") + } + m.CheckProxmoxDiskTemperature("lab", "pve1", ProxmoxDiskTemperatureReading{Disk: proxmoxTemperatureDisk("/dev/sdb", "sata"), Celsius: 70}) + if _, firing := proxmoxDiskTemperatureAlert(t, m, "/dev/sdb"); !firing { + t.Fatalf("a 70C SATA critical reading waited out the stability window") + } +} + +// A config save re-judges an open Proxmox disk temperature alert against its +// disk type's trigger, as the next poll does; one without a recorded disk type +// is held to the lowest per-type trigger. +func TestProxmoxDiskTemperatureAlertsReevaluatePerDiskType(t *testing.T) { + m := newTestManager(t) + cfg := unifiedEvalBaseConfig() + cfg.AgentDefaults.DiskTemperature = &HysteresisThreshold{Trigger: 70, Clear: 65} + configureUnifiedEvalManager(t, m, cfg) + m.CheckProxmoxDiskTemperature("lab", "pve1", ProxmoxDiskTemperatureReading{Disk: proxmoxTemperatureDisk("/dev/nvme0n1", "nvme"), Celsius: 72}) + m.CheckProxmoxDiskTemperature("lab", "pve1", ProxmoxDiskTemperatureReading{Disk: proxmoxTemperatureDisk("/dev/sda", "sata"), Celsius: 57}) + sata, ok := proxmoxDiskTemperatureAlert(t, m, "/dev/sda") + if !ok { + t.Fatalf("SATA at 57C did not fire: %v", alertKeys(m)) + } + + m.mu.Lock() + m.config.DiskTempByType["nvme"] = HysteresisThreshold{Trigger: 75, Clear: 70} + delete(sata.Metadata, "diskType") + m.reevaluateActiveAlertsLocked() + m.mu.Unlock() + + if _, firing := proxmoxDiskTemperatureAlert(t, m, "/dev/nvme0n1"); firing { + t.Fatalf("NVMe alert at 72C survived a save raising its trigger to 75C") + } + if _, firing := proxmoxDiskTemperatureAlert(t, m, "/dev/sda"); !firing { + t.Fatalf("a SATA alert without a disk type was resolved by the 70C agent default although its trigger is 55C") + } +} diff --git a/internal/alerts/config/normalize.go b/internal/alerts/config/normalize.go index 012cc63a33..5fd3214aa7 100644 --- a/internal/alerts/config/normalize.go +++ b/internal/alerts/config/normalize.go @@ -638,6 +638,10 @@ func NormalizeTimeThresholds(config *AlertConfig) { ensureDelay("truenas-pool") ensureDelay("truenas-dataset") ensureDelay("truenas-disk") + // Proxmox disk temperature alerts: the factory stability window for a + // noisy gauge applies to this type's unchanged default, as it does to + // agent and TrueNAS disk temperatures. + ensureDelay("proxmox-disk") ensureDelay("vmware-host") ensureDelay("vmware-vm") ensureDelay("vmware-datastore") diff --git a/internal/alerts/config_runtime.go b/internal/alerts/config_runtime.go index f0dbb81250..6af431cfbd 100644 --- a/internal/alerts/config_runtime.go +++ b/internal/alerts/config_runtime.go @@ -427,6 +427,19 @@ func (m *Manager) reevaluateActiveAlertsLocked() { threshold = getThresholdForMetric(thresholds, metricType) } + // A Proxmox disk temperature alert is judged by the disk temperature + // policy for its disk type, as CheckProxmoxDiskTemperature judges it. + // The node and guest policies below have no disk temperature, so a + // save would otherwise resolve an alert the next poll raises again. + if primaryResourceType == proxmoxDiskResourceType && metricType == proxmoxDiskTemperatureMetric { + diskType, known := alert.Metadata["diskType"].(string) + threshold = m.proxmoxDiskTemperatureThresholdNoLock(diskType, !known) + if threshold == nil || threshold.Trigger <= 0 { + alertsToResolve = append(alertsToResolve, alertID) + continue + } + } + isAgentResource := alertResourceTypeKeysContain(resourceTypeKeys, "agent") if !handledModernPlatformType && isAgentResource { if allDisabled, _ := m.alertPolicyTypeSwitchesNoLock("agent"); allDisabled { diff --git a/internal/alerts/default_config.go b/internal/alerts/default_config.go index 738e08a712..516756aaa8 100644 --- a/internal/alerts/default_config.go +++ b/internal/alerts/default_config.go @@ -150,6 +150,7 @@ func defaultAlertConfig() AlertConfig { "truenas-pool": 5, "truenas-dataset": 5, "truenas-disk": 5, + "proxmox-disk": 5, "vmware-host": 5, "vmware-vm": 5, "vmware-datastore": 5, diff --git a/internal/alerts/proxmox_disk_temperature.go b/internal/alerts/proxmox_disk_temperature.go new file mode 100644 index 0000000000..7aafd3e1c7 --- /dev/null +++ b/internal/alerts/proxmox_disk_temperature.go @@ -0,0 +1,110 @@ +package alerts + +import ( + "fmt" + "strings" + + "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" + "github.com/rcourtman/pulse-go-rewrite/pkg/proxmox" + "github.com/rs/zerolog/log" +) + +// proxmoxDiskTemperatureMetric is the metric of a Proxmox physical disk's +// temperature alert, the same metric CheckHost raises for an agent's disks. +const proxmoxDiskTemperatureMetric = "diskTemperature" + +// proxmoxDiskResourceType is the resource type PVE disk alerts carry in their +// spec and metadata (see CheckDiskHealth). +const proxmoxDiskResourceType = "proxmox-disk" + +// ProxmoxDiskTemperatureReading is one Proxmox physical disk's temperature as +// the PVE disk poller judged it. +type ProxmoxDiskTemperatureReading struct { + Disk proxmox.Disk + // Celsius is the reading collected this poll, or 0 when there is none: + // no sensor reports the disk, it is in standby, or normalization kept a + // last-known value it did not collect now. + Celsius int + // AgentOwned says a linked Pulse agent lists the disk in its SMART + // report, so the agent's CheckHost owns its temperature alert. + AgentOwned bool + // Excluded says the linked agent's --disk-exclude patterns match the + // device, which excludes it from disk alerts. + Excluded bool +} + +// CheckProxmoxDiskTemperature raises a Proxmox physical disk's temperature +// alert under the disk temperature policy (DiskTemperatureThreshold): the +// same per-type thresholds that judge an agent's disks, TrueNAS disks and the +// Physical Disks verdict. Without it a disk that only the node's sensors +// report could read Running Hot and never alert. +// +// A disk a linked agent lists is the agent's: its open alert here closes as +// moved to the agent rather than recovered, so the disk never alerts twice. +// An excluded disk, or a policy switched off by the agent Disk Temp default, +// closes its alert. A disk with no current reading keeps its alert as it is, +// since no reading shows it cooled. +func (m *Manager) CheckProxmoxDiskTemperature(instance, node string, reading ProxmoxDiskTemperatureReading) { + if m == nil { + return + } + disk := reading.Disk + resourceID := proxmoxDiskCanonicalResourceID(instance, node, disk.DevPath) + resourceName := fmt.Sprintf("%s (%s)", disk.Model, disk.DevPath) + diskType := strings.ToLower(strings.TrimSpace(disk.Type)) + + m.mu.RLock() + enabled := m.config.Enabled + threshold := m.proxmoxDiskTemperatureThresholdNoLock(diskType, false) + m.mu.RUnlock() + if !enabled { + return + } + + if reading.AgentOwned || reading.Excluded || threshold == nil || threshold.Trigger <= 0 { + spec, err := buildCanonicalMetricSpec(resourceID, resourceName, unifiedresources.ResourceType(proxmoxDiskResourceType), proxmoxDiskTemperatureMetric, nil) + if err != nil { + return + } + var resolution *AlertResolution + if reading.AgentOwned && !reading.Excluded { + resolution = &AlertResolution{Reason: AlertResolutionMovedToAgent} + } + m.releaseCanonicalMetricAlert(spec, resourceName, node, instance, proxmoxDiskResourceType, 0, resolution) + return + } + if reading.Celsius <= 0 { + return + } + + spec, err := buildCanonicalMetricSpec(resourceID, resourceName, unifiedresources.ResourceType(proxmoxDiskResourceType), proxmoxDiskTemperatureMetric, threshold) + if err != nil { + log.Warn(). + Err(err). + Str("resourceID", resourceID). + Str("node", node). + Str("disk", disk.DevPath). + Msg("Skipping invalid canonical Proxmox disk temperature metric spec") + return + } + metadata := proxmoxDiskAlertMetadata(disk) + metadata["metric"] = proxmoxDiskTemperatureMetric + // A config save re-judges the open alert against its disk type. + metadata["diskType"] = diskType + value := float64(reading.Celsius) + m.checkMetricWithCanonicalSpec(spec, resourceName, node, instance, proxmoxDiskResourceType, value, threshold, &metricOptions{ + Metadata: metadata, + Message: fmt.Sprintf("Disk temperature at %s", formatMetricValue(value, "°C")), + }) +} + +// proxmoxDiskTemperatureThresholdNoLock is the disk temperature policy for a +// Proxmox disk of diskType, or the lowest per-type trigger for an alert whose +// disk type was never recorded, so a config save never resolves an alert the +// next poll raises again. Callers must hold m.mu. +func (m *Manager) proxmoxDiskTemperatureThresholdNoLock(diskType string, typeUnknown bool) *HysteresisThreshold { + if typeUnknown { + return cloneThreshold(m.lowestHostDiskTemperatureThresholdNoLock(m.config.AgentDefaults.DiskTemperature)) + } + return cloneThreshold(m.hostDiskTemperatureThresholdNoLock(m.config.AgentDefaults.DiskTemperature, false, diskType)) +} diff --git a/internal/models/deepcopy_test.go b/internal/models/deepcopy_test.go index 964ae44a28..e412d86406 100644 --- a/internal/models/deepcopy_test.go +++ b/internal/models/deepcopy_test.go @@ -1,6 +1,8 @@ package models import ( + "encoding/json" + "strings" "testing" ) @@ -512,3 +514,22 @@ func TestCloneHostXCPNGInventoryIsolation(t *testing.T) { t.Fatal("cloned XCP-ng inventory aliases source VMs") } } + +// AgentSMARTReported is internal poll evidence: a state snapshot copy keeps it +// for the alert check, and it never reaches an API or websocket payload. +func TestPhysicalDiskAgentSMARTReportedStaysInternal(t *testing.T) { + disk := PhysicalDisk{ID: "lab-pve1-sda", DevPath: "/dev/sda", AgentSMARTReported: true} + if clone := clonePhysicalDisk(disk); !clone.AgentSMARTReported { + t.Fatalf("clone dropped AgentSMARTReported") + } + if clones := clonePhysicalDisks([]PhysicalDisk{disk}); !clones[0].AgentSMARTReported { + t.Fatalf("slice clone dropped AgentSMARTReported") + } + payload, err := json.Marshal(disk) + if err != nil { + t.Fatal(err) + } + if strings.Contains(strings.ToLower(string(payload)), "agentsmartreported") { + t.Fatalf("AgentSMARTReported leaked into JSON: %s", payload) + } +} diff --git a/internal/models/models.go b/internal/models/models.go index 57d5733980..941020da7d 100644 --- a/internal/models/models.go +++ b/internal/models/models.go @@ -2766,7 +2766,11 @@ type PhysicalDisk struct { IO *DiskIO `json:"io,omitempty"` Collection *diskinventory.CollectionStatus `json:"collection,omitempty"` ExpectedUpdateInterval time.Duration `json:"-"` // Collector schedule, independent of general node polling - LastChecked time.Time `json:"lastChecked"` + // AgentSMARTReported marks a disk a linked host agent lists in its SMART + // report, so the agent's CheckHost owns its temperature alert. Internal + // poll evidence, set by the PVE disk poller's agent SMART merge. + AgentSMARTReported bool `json:"-"` + LastChecked time.Time `json:"lastChecked"` } // PBSInstance represents a Proxmox Backup Server instance diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go index 25be288f63..c6b5171466 100644 --- a/internal/monitoring/monitor.go +++ b/internal/monitoring/monitor.go @@ -512,6 +512,7 @@ func mergeHostAgentSMARTIntoDisks(disks []models.PhysicalDisk, nodes []models.No // Build a map of node name to linked host's SMART data smartByNodeName := make(map[string][]models.HostDiskSMART) + reportingNodeNames := make(map[string]bool) for _, node := range nodes { if node.LinkedAgentID == "" { continue @@ -521,6 +522,11 @@ func mergeHostAgentSMARTIntoDisks(disks []models.PhysicalDisk, nodes []models.No continue } smartByNodeName[node.Name] = host.Sensors.SMART + // Only an agent still reporting owns its disks' temperature alerts; + // a silent one's retained rows still enrich the disk. + if !strings.EqualFold(strings.TrimSpace(host.Status), "offline") { + reportingNodeNames[node.Name] = true + } log.Debug(). Str("nodeName", node.Name). Str("hostAgentID", node.LinkedAgentID). @@ -607,6 +613,11 @@ func mergeHostAgentSMARTIntoDisks(disks []models.PhysicalDisk, nodes []models.No if matched == nil { continue } + // The linked agent reports this disk, so while it keeps reporting its + // CheckHost owns the disk's temperature alert and the PVE disk check + // stands aside. Once its lease lapses, CheckHost no longer alerts on + // the disk and the PVE check judges the node's own readings again. + updated[i].AgentSMARTReported = reportingNodeNames[updated[i].Node] if strings.TrimSpace(updated[i].Model) == "" && strings.TrimSpace(matched.Model) != "" { updated[i].Model = strings.TrimSpace(matched.Model) diff --git a/internal/monitoring/monitor_pve.go b/internal/monitoring/monitor_pve.go index 1e11c0b3d5..a063957263 100644 --- a/internal/monitoring/monitor_pve.go +++ b/internal/monitoring/monitor_pve.go @@ -10,6 +10,7 @@ import ( "sync" "time" + "github.com/rcourtman/pulse-go-rewrite/internal/alerts" "github.com/rcourtman/pulse-go-rewrite/internal/config" "github.com/rcourtman/pulse-go-rewrite/internal/logging" "github.com/rcourtman/pulse-go-rewrite/internal/models" @@ -1416,19 +1417,35 @@ func preserveUnavailablePhysicalDiskEvidence(current, previous models.PhysicalDi return current } -// checkPhysicalDiskAlerts evaluates one Proxmox physical disk's health and -// wearout alerts. A device matched by the linked host agent's --disk-exclude -// patterns is evaluated as healthy, so excluding a disk also resolves the -// alerts it already raised. +// checkPhysicalDiskAlerts evaluates one Proxmox physical disk's health, +// wearout and temperature alerts. A device matched by the linked host agent's +// --disk-exclude patterns is evaluated as healthy and without a temperature +// alert, so excluding a disk also resolves the alerts it already raised. func (m *Monitor) checkPhysicalDiskAlerts(instance string, disk models.PhysicalDisk, excludePatterns []string) { - if len(excludePatterns) > 0 && fsfilters.MatchesDeviceExclude(disk.DevPath, excludePatterns) { + excluded := len(excludePatterns) > 0 && fsfilters.MatchesDeviceExclude(disk.DevPath, excludePatterns) + if excluded { healthyDisk := proxmoxDiskFromPhysicalDisk(disk) healthyDisk.Health = "PASSED" healthyDisk.Wearout = 100 m.alertManager.CheckDiskHealth(instance, disk.Node, healthyDisk) - return + } else { + m.alertManager.CheckDiskHealth(instance, disk.Node, proxmoxDiskFromPhysicalDisk(disk)) } - m.alertManager.CheckDiskHealth(instance, disk.Node, proxmoxDiskFromPhysicalDisk(disk)) + m.alertManager.CheckProxmoxDiskTemperature(instance, disk.Node, alerts.ProxmoxDiskTemperatureReading{ + Disk: proxmoxDiskFromPhysicalDisk(disk), + Celsius: collectedPhysicalDiskTemperature(disk), + AgentOwned: disk.AgentSMARTReported, + Excluded: excluded, + }) +} + +// collectedPhysicalDiskTemperature is the disk's temperature when this poll +// collected it, or 0 for a last-known value normalization kept. +func collectedPhysicalDiskTemperature(disk models.PhysicalDisk) int { + if !diskinventory.TemperatureCollected(disk.Temperature, disk.Collection) { + return 0 + } + return disk.Temperature } func proxmoxDiskFromPhysicalDisk(disk models.PhysicalDisk) proxmox.Disk { diff --git a/internal/monitoring/physical_disk_roundtrip_test.go b/internal/monitoring/physical_disk_roundtrip_test.go index c5b4bb4f93..8e5b0f4d20 100644 --- a/internal/monitoring/physical_disk_roundtrip_test.go +++ b/internal/monitoring/physical_disk_roundtrip_test.go @@ -1121,3 +1121,96 @@ func TestTrueNASPlaceholderDiskSerialsStayPerApplianceAndShareOneHistoryKey(t *t t.Fatalf("canonical TrueNAS disks = %d, want %d (one per appliance and device)", disks, diskCount) } } + +// The agent SMART merge marks each Proxmox disk a reporting linked agent +// lists, so the disk's temperature alert is left to the agent's CheckHost. +// Disks the agent does not list, disks on nodes without an agent, and disks +// of an agent whose lease lapsed stay unmarked. Disks the poller builds from +// the agent's SMART report when the Proxmox query fails go through the same +// merge. +func TestMergeHostAgentSMARTIntoDisksMarksDisksTheAgentReports(t *testing.T) { + disks := []models.PhysicalDisk{ + {Instance: "pve1", Node: "node1", DevPath: "/dev/sda"}, + {Instance: "pve1", Node: "node1", DevPath: "/dev/sdb"}, + {Instance: "pve1", Node: "node2", DevPath: "/dev/sda"}, + {Instance: "pve1", Node: "node3", DevPath: "/dev/sda"}, + } + nodes := []models.Node{ + {Name: "node1", LinkedAgentID: "host-node1"}, + {Name: "node2"}, + {Name: "node3", LinkedAgentID: "host-node3"}, + } + smart := []models.HostDiskSMART{{Device: "sda", Serial: "SER-SDA", Type: "sata", Temperature: 40}} + hosts := []models.Host{ + {ID: "host-node1", Status: "online", Sensors: models.HostSensorSummary{SMART: smart}}, + {ID: "host-node3", Status: "offline", Sensors: models.HostSensorSummary{SMART: smart}}, + } + + merged := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts) + for i, want := range []bool{true, false, false, false} { + if merged[i].AgentSMARTReported != want { + t.Fatalf("%s/%s AgentSMARTReported = %v, want %v", merged[i].Node, merged[i].DevPath, merged[i].AgentSMARTReported, want) + } + } + if disks[0].AgentSMARTReported { + t.Fatalf("merge modified the input slice") + } + if merged[3].Serial != "SER-SDA" { + t.Fatalf("a silent agent's retained row no longer enriches the disk: %+v", merged[3]) + } + + fallback := mergeHostAgentSMARTIntoDisks(physicalDisksFromHostAgentSMART("pve1", "node1", smart), nodes, hosts) + if len(fallback) != 1 || !fallback[0].AgentSMARTReported { + t.Fatalf("a disk built from a reporting agent's SMART report is not the agent's: %+v", fallback) + } +} + +// checkPhysicalDiskAlerts raises a Proxmox disk's temperature alert from a +// reading collected this poll, under the disk temperature policy, and leaves +// alone a retained reading, a disk the linked agent reports and an excluded +// device. +func TestCheckPhysicalDiskAlertsRaisesProxmoxDiskTemperatureAlerts(t *testing.T) { + manager := alerts.NewManagerWithDataDir(t.TempDir()) + m := &Monitor{alertManager: manager} + const instance, node = "pve1", "node1" + alertID := func(devPath string) string { + return unifiedresources.ProxmoxPhysicalDiskAlertResourceID(instance, node, devPath) + "::metric-threshold:diskTemperature" + } + active := func(devPath string) bool { + for _, alert := range manager.GetActiveAlerts() { + if alert.ID == alertID(devPath) { + return true + } + } + return false + } + // 70C is critical for SATA (trigger 55, critical 65), which fires without + // the stability delay a warning waits out. + sata := func(devPath string) models.PhysicalDisk { + return models.PhysicalDisk{Instance: instance, Node: node, DevPath: devPath, Model: "Test SATA", Type: "sata", Health: "PASSED", Wearout: -1, Temperature: 70} + } + + m.checkPhysicalDiskAlerts(instance, sata("/dev/sda"), nil) + if !active("/dev/sda") { + t.Fatalf("a SATA disk at 70C raised no temperature alert: %+v", manager.GetActiveAlerts()) + } + + retained := sata("/dev/sdb") + retained.Collection = &diskinventory.CollectionStatus{Temperature: diskinventory.Unavailable("proxmox_node_smart", "disk is in standby")} + m.checkPhysicalDiskAlerts(instance, retained, nil) + if active("/dev/sdb") { + t.Fatalf("a retained 70C reading raised a temperature alert") + } + + agentOwned := sata("/dev/sdc") + agentOwned.AgentSMARTReported = true + m.checkPhysicalDiskAlerts(instance, agentOwned, nil) + if active("/dev/sdc") { + t.Fatalf("a disk the linked agent reports raised a second temperature alert") + } + + m.checkPhysicalDiskAlerts(instance, sata("/dev/sdd"), []string{"sdd"}) + if active("/dev/sdd") { + t.Fatalf("an excluded device raised a temperature alert") + } +} diff --git a/internal/unifiedresources/history_identity_test.go b/internal/unifiedresources/history_identity_test.go index 03758b80bf..39d0ff5ce7 100644 --- a/internal/unifiedresources/history_identity_test.go +++ b/internal/unifiedresources/history_identity_test.go @@ -848,7 +848,7 @@ func TestProxmoxPhysicalDiskAlertReferenceShape(t *testing.T) { ProxmoxPhysicalDiskAlertResourceID("lab", "pve1", "/"), } { require.True(t, isProxmoxPhysicalDiskAlertReference(ref), ref) - require.Len(t, ProxmoxPhysicalDiskAlertIdentifiers(ref), 2, ref) + require.Len(t, ProxmoxPhysicalDiskAlertIdentifiers(ref), 3, ref) } for _, ref := range []string{ ProxmoxPhysicalDiskAlertResourceID("lab", "pve1", ""), diff --git a/internal/unifiedresources/pve_disk_alert_history.go b/internal/unifiedresources/pve_disk_alert_history.go index e2040b2eaf..b15afaeceb 100644 --- a/internal/unifiedresources/pve_disk_alert_history.go +++ b/internal/unifiedresources/pve_disk_alert_history.go @@ -26,6 +26,11 @@ const ( // "::" (buildCanonicalStateID over the spec ID). var proxmoxDiskAlertSpecSuffixes = []string{"-health", "-wearout"} +// proxmoxDiskTemperatureAlertSpecID is the PVE disk temperature alert's spec, +// a metric threshold spec (canonicalMetricSpecID over "diskTemperature"), so +// its identifier is "::metric-threshold:diskTemperature". +const proxmoxDiskTemperatureAlertSpecID = "metric-threshold:diskTemperature" + // ProxmoxPhysicalDiskAlertIdentifiers lists the alert identifiers of the PVE // disk alerts raised under ref, or nil when ref is not shaped like a PVE disk // alert reference. @@ -33,11 +38,11 @@ func ProxmoxPhysicalDiskAlertIdentifiers(ref string) []string { if !isProxmoxPhysicalDiskAlertReference(ref) { return nil } - identifiers := make([]string, 0, len(proxmoxDiskAlertSpecSuffixes)) + identifiers := make([]string, 0, len(proxmoxDiskAlertSpecSuffixes)+1) for _, suffix := range proxmoxDiskAlertSpecSuffixes { identifiers = append(identifiers, ref+"::"+ref+suffix) } - return identifiers + return append(identifiers, ref+"::"+proxmoxDiskTemperatureAlertSpecID) } // isProxmoxPhysicalDiskAlertReference matches "::disk:" diff --git a/internal/unifiedresources/store_test.go b/internal/unifiedresources/store_test.go index 92ec4d333c..74649f8c83 100644 --- a/internal/unifiedresources/store_test.go +++ b/internal/unifiedresources/store_test.go @@ -14,6 +14,7 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/models" "github.com/rcourtman/pulse-go-rewrite/internal/operationreceipt" + "github.com/stretchr/testify/require" ) func TestSQLiteResourceStoreDoesNotPersistRawOperationReceiptProtocolOrReplayAuthority(t *testing.T) { @@ -3328,3 +3329,32 @@ func TestSuccessionMergesGuestChangeJournalEras(t *testing.T) { t.Fatalf("legacy-era journal row missing from new-ID read: %+v", changes) } } + +// A PVE disk temperature alert row is owned by the hardware identity it +// records, like the health and wearout rows, and a read by the path reference +// still finds it through ProxmoxPhysicalDiskAlertIdentifiers. +func TestProxmoxDiskTemperatureAlertRowsFollowRecordedHardwareIdentity(t *testing.T) { + store, err := NewSQLiteResourceStore(t.TempDir(), "default") + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, store.Close()) }) + now := time.Now().UTC().Truncate(time.Second) + ref := ProxmoxPhysicalDiskAlertResourceID("lab", "pve1", "/dev/sda") + snapshot := proxmoxHistoryIdentitySnapshot(now) + snapshot.PhysicalDisks = []models.PhysicalDisk{pveDiskHistoryTestDisk("pve1", "/dev/sda", "ZA1A2B3C", "", now)} + adapter := NewMonitorAdapter(NewRegistry(store)) + adapter.PopulateFromSnapshot(snapshot) + diskID, ok := adapter.currentRegistry().ResolveReferenceID(ref) + require.True(t, ok) + + change := BuildAlertTimelineChange(ref, ChangeAlertFired, now, "", AlertTimelineChange{ + AlertIdentifier: ref + "::metric-threshold:diskTemperature", AlertStartedAt: now, AlertType: "diskTemperature", + AlertMetadata: map[string]any{"disk_path": "/dev/sda", MetadataDiskSerial: "ZA1A2B3C", MetadataDiskWWN: ""}, + }) + change.ID = "temperature" + require.NoError(t, adapter.RecordChange(*change)) + + require.ElementsMatch(t, []string{"temperature"}, pveDiskHistoryIDs(pveDiskHistoryRows(t, store, diskID))) + byRef := pveDiskHistoryRows(t, store, ref) + require.ElementsMatch(t, []string{"temperature"}, pveDiskHistoryIDs(byRef)) + require.Equal(t, ref, OwnedAlertReference(byRef["temperature"])) +}