diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index caf6712d64..87ad98cc0a 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -8833,6 +8833,25 @@ in `internal/monitoring/monitor_host_agents_test.go` `internal/monitoring/issue1595_collection_trust_test.go` (`TestAgentDiskHistoryFollowsTheSerialItsUnraidRowReports`). +### Unraid temperatures chart agent disk history + +`internal/monitoring/monitor_agents.go` changed only so the SMART history +writer charts the temperature a host agent's disk shows. A SMART row without +its own reading shows the one the host's Unraid inventory reports for the disk, +when that inventory row describes the disk (`HostSMARTDiskTemperature`), and a +disk only the Unraid inventory reports, +such as a member the agent's SMART collection skips under `--disk-exclude`, +shows that row's reading (`HostUnraidDiskTemperature`). Both are now written +as `smart_temp` under the key the disk's metrics target reads +(`HostSMARTDiskMetricID`, `HostUnraidDiskMetricID`); a reading the disk does +not show as collected, a spun-down disk's or an expired host's, is not. Agent +registration, enrolment, install, update, removal, report identity and disk +source IDs are unchanged. Focused proof lives in +`internal/monitoring/monitor_host_agents_test.go` +(`TestApplyHostReportChartsUnraidTemperatureOfDiskWithoutSMART`) and +`internal/monitoring/issue1595_collection_trust_test.go` +(`TestAgentDiskChartsTheUnraidTemperatureItShows`). + ### Windows braced MachineGuid does not abort agent startup The Windows unified agent resolves host information through gopsutil's combined diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index e1086c8b3f..513b44c27d 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -2034,6 +2034,61 @@ alert. `shouldPreserveAlertOutsideNodeCleanup` now keeps any alert carrying `internal/alerts/alerts_test.go` pins both signals, and its sibling pins that a guest alert on a removed Proxmox node is still removed. +### Agent disk temperature alerts judge the reading the disk shows + +`CheckHost` judges each agent disk on the temperature, collection state and +disk type the disk shows, read by `unifiedresources.HostDiskTemperatureReadings` +from the same adapters the registry builds the disks with. A SMART row shows its +own reading or, when the agent's probe returned none, the one the host's Unraid +inventory reports for that disk (`HostSMARTDiskTemperature`), and keeps its +`agent:/disk_temp:` resource. An Unraid inventory row whose disk +key no SMART row carries, a disk only the inventory lists, shows that row's +reading (`HostUnraidDiskTemperature`) and its own transport as disk type, under +its kernel block device token, the resource a plain Linux SMART row for the +same device uses. A SMART row and an Unraid row with one key are one registry +disk even under different device labels, such as a controller member +(`0 [megaraid,0]`) and its Unraid device (`sda`), and alert once, under the +SMART row's resource. When smartctl and Unraid name +one device with different serials the registry shows two disks on it, and the +device's one alert judges the reading that stands highest against its own disk +type's threshold (`judgedHostDiskTemperature`): over its trigger first, then +inside its recovery band, then the larger margin over the trigger, with a tie +keeping the reading listed first. +Before this `CheckHost` read only the SMART row's own field, while the Physical +Disks Temp cell, the Running Hot verdict ("at or above its alert threshold") +and Patrol judged the Unraid reading against the same trigger, so such a disk +could read as at its alert threshold with no alert. Unraid's own warning and +critical disk temperature notifications do not make this redundant: Pulse +already alerts on the smartctl readings and array state of the same hosts, +which Unraid also notifies on, and a user who routes alerts through Pulse sets +these thresholds here. + +Only a reading collected now fires or resolves +(`HostDiskTemperatureReading.Collected`: no standby row, and +`diskinventory.TemperatureCollected`), so a spun-down disk holds the alert +like a listed disk without a temperature. A disk only the +inventory lists whose device matches the agent's `--disk-exclude` patterns +(`fsfilters.MatchesDeviceExclude`) raises nothing, and an alert it raised before +the exclusion resolves on the next report (`clearHostDiskTemperatureResources`), +whatever the SMART list holds, unless another listed reading uses the same +resource: `docs/UNIFIED_AGENT.md` documents exclusion as +removing the disk from monitoring, and linked Proxmox hosts already judge an +excluded disk healthy. A disk only the inventory lists is also judged in a +report whose SMART list is empty, because the disk shows the inventory's +reading in that report. The absence cleanup above still runs only on a +non-empty SMART list, and such a report neither counts toward nor restarts a +disk's three, so a disk that leaves the inventory while every report's SMART +list stays empty keeps its alert until the 24-hour stale-alert sweep. +`TestCheckHostJudgesTheUnraidTemperatureADiskShows` and +`TestCheckHostHoldsUnraidDiskTemperatureAlertUntilAReadingOrDeparture` in +`internal/alerts/host_unraid_lifecycle_test.go` pin the fallback, the +inventory-only disk, the empty SMART list, two disks on one device (also of +different types), a controller member under another device label, standby, +exclusion, the disk type +and host override thresholds, the holds, the handover to a SMART reading, +departure, and the exclusion clear with and without SMART rows and beside a +listed SMART row. `TestJudgedHostDiskTemperatureOutranks` pins the ranking. + ### Configured flapping thresholds remain reachable Every accepted positive `FlappingThreshold`, including values above ten, must diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 84faf8f01b..7402fa6a1d 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -4830,8 +4830,10 @@ timeline instead of creating a second drawer-only or mock-only disk history path. The SMART-resolved id is the serial the disk resource carries: when a SMART row reports none, `HostSMARTDiskMetricID` takes the one the host's Unraid inventory -reports for that disk, as the unified-resources adapter does, unless several -SMART rows share the disk's device path, and `hostDiskIOMetricResourceID` keys +reports for that disk, as the unified-resources adapter does, unless the Unraid +row does not describe the disk (`unraidDiskDescribesSMARTRow`: a controller +member, or several SMART rows on the disk's device path), and +`hostDiskIOMetricResourceID` keys a device with no non-standby SMART row by its Unraid serial (`HostUnraidDeviceMetricID`, which refuses a device several Unraid rows name) before the linked Proxmox node's disks and the `:` fallback. @@ -4842,6 +4844,21 @@ their counter under no member. Proof: `TestAgentDiskHistoryFollowsTheSerialItsUnraidRowReports` and `TestHostDiskIOMetricResourceIDFallbacks`. +The agent's `smart_temp` series holds the reading the disk resource shows, +not only the SMART row's own. `writeHostSMARTMetrics` writes +`HostSMARTDiskTemperature`, which falls back to the host's Unraid inventory +reading when the row has none, and `writeHostUnraidDiskTemperatures` writes +`HostUnraidDiskTemperature` under `HostUnraidDiskMetricID` for each Unraid row +whose key is not one of the host's SMART rows' keys, such as a `--disk-exclude` +member, so an Unraid reading never overwrites a SMART row's own at the same +timestamp. +Unraid rows sharing a key are one disk in the registry, which shows the latest +of their readings, so the last row with a reading decides that key's single +sample. `diskinventory.TemperatureCollected` keeps a spun-down disk's or an +expired host's leftover reading out of history. +Proof: +`TestAgentDiskChartsTheUnraidTemperatureItShows` and +`TestApplyHostReportChartsUnraidTemperatureOfDiskWithoutSMART`. That same monitoring-owned disk-health boundary also includes shared storage risk assessment in `internal/storagehealth/`. When providers or host agents emit structured storage topology such as Unraid per-disk state, the shared diff --git a/docs/release-control/v6/internal/subsystems/unified-resources.md b/docs/release-control/v6/internal/subsystems/unified-resources.md index 8dc22a7805..d040831d74 100644 --- a/docs/release-control/v6/internal/subsystems/unified-resources.md +++ b/docs/release-control/v6/internal/subsystems/unified-resources.md @@ -5474,6 +5474,43 @@ A row's own serial is kept even when it is a placeholder, as the adapter keeps it, and the Unraid row then stays a disk of its own. Proof: `TestHostSMARTDiskMetricIDTakesTheSerialItsUnraidRowReports`, `TestAgentDiskHistoryFollowsTheSerialItsUnraidRowReports`. +The adapters' disk temperature rule is shared with the agent disk history +writer. `HostSMARTDiskTemperature` returns the temperature and collection state +a SMART row's disk shows: the row's own reading, or, when it has none, its +Unraid row's under `unraidDiskTemperatureStatus`. `matchUnraidDisk` finds that +Unraid row by the SMART row's usable serial anywhere in the inventory before +any device path, and a placeholder serial matches nothing. The fallback, like +the serial a row without one takes (`hostSMARTDiskSerial`), comes only from an +Unraid row that describes the row's disk (`unraidDiskDescribesSMARTRow`): one +carrying the row's usable serial, or one naming the row's device path when the +row is that whole kernel block device. A controller member, even the only one +reported, and a row sharing its path with other SMART rows are not, because +the Unraid row describes the block device as a whole. `HostUnraidDiskTemperature` +returns what a disk built from an Unraid row alone shows, and +`HostUnraidDiskMetricID` returns the key that disk's metrics target reads: its +usable serial, else its `HostUnraidDiskSourceID`, and nothing for a row without +a device, which is not ingested. The writer used to take only the SMART row's +own reading, so a temperature shown from the Unraid inventory, for a disk whose +SMART probe returned none or one excluded from SMART collection, was never +charted. Proof: `TestAgentDiskChartsTheUnraidTemperatureItShows` and +`TestHostUnraidDiskMetricIDMatchesItsDiskMetricsTarget`. +`HostDiskTemperatureReadings` lists the reading each disk of a host agent shows +for consumers that judge disk heat, the agent disk temperature alerts in +`CheckHost`: one per SMART row the registry ingests (no virtual block device), +then one per Unraid row whose disk key (`HostUnraidDiskMetricID`) no SMART row's +`HostSMARTDiskMetricID` equals, the history writer's rule, with a row without a +device skipped as the registry skips it. Rows with one key are one registry +disk whatever device labels they carry, such as a controller member +(`0 [megaraid,0]`) and its Unraid device (`sda`). When smartctl and Unraid +report different usable serials for one device the registry shows both disks, +and both are listed. Each reading is taken from the disk resource +`resourceFromHostSMARTDisk` or `resourceFromHostUnraidPhysicalDisk` builds, so +its temperature, collection state and disk type are the ones that row's disk +shows. Unraid rows on different devices that share a key and no SMART row are +one registry disk but keep a reading each, since alerts are keyed by device. +Proof: `TestHostDiskTemperatureReadingsMatchTheDisksTheRegistryShows` compares +each reading with the ingested disk, including two disks on one device and a +controller member Unraid lists under its block device. That same canonical physical-disk view must also expose source-independent host context. When a disk is API-backed rather than node-backed, typed views should fall back to canonical host identity such as `identity.hostnames` instead of diff --git a/internal/alerts/host.go b/internal/alerts/host.go index db47b5bdf6..be3d9aa092 100644 --- a/internal/alerts/host.go +++ b/internal/alerts/host.go @@ -10,6 +10,7 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/models" "github.com/rcourtman/pulse-go-rewrite/internal/storagehealth" "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" + "github.com/rcourtman/pulse-go-rewrite/pkg/fsfilters" "github.com/rs/zerolog/log" ) @@ -419,43 +420,79 @@ func (m *Manager) CheckHost(host models.Host) { } if thresholds.DiskTemperature != nil && thresholds.DiskTemperature.Trigger > 0 { + // Each disk is judged on the temperature it shows: its SMART row's + // reading, or the one the host's Unraid inventory reports for it when + // the agent's probe returned none, and the inventory's reading for a + // disk only the inventory lists. + readings := unifiedresources.HostDiskTemperatureReadings(host) + seenDiskTemps := make(map[string]struct{}, len(readings)) + excludedDiskTemps := make(map[string]struct{}) + judgedDiskTemps := make(map[string]judgedHostDiskTemperature, len(readings)) + judgedOrder := make([]string, 0, len(readings)) + for _, disk := range readings { + tempResourceID := hostDiskTemperatureResourceID(host.ID, disk.Device) + // A disk the agent's --disk-exclude patterns match is out of + // monitoring, so the Unraid inventory's reading raises nothing + // for it either. + if disk.UnraidOnly && fsfilters.MatchesDeviceExclude(disk.Device, host.DiskExclude) { + excludedDiskTemps[tempResourceID] = struct{}{} + continue + } + // A listed disk in standby, or without a current temperature after + // a failed probe, is still present. Its alert holds until a fresh + // reading resolves it instead of clearing and re-raising. + seenDiskTemps[tempResourceID] = struct{}{} + if !disk.Collected() { + continue + } + m.mu.RLock() + threshold := m.hostDiskTemperatureThresholdNoLock(thresholds.DiskTemperature, diskTempOverridden, disk.DiskType) + m.mu.RUnlock() + // Sources that disagree on a disk's identity can show two disks + // on one device. Its alert judges the reading that stands highest + // against its own disk type's threshold. + candidate := judgedHostDiskTemperature{reading: disk, threshold: threshold} + if judged, ok := judgedDiskTemps[tempResourceID]; !ok { + judgedOrder = append(judgedOrder, tempResourceID) + } else if !candidate.outranks(judged) { + continue + } + judgedDiskTemps[tempResourceID] = candidate + } + for _, tempResourceID := range judgedOrder { + disk := judgedDiskTemps[tempResourceID].reading + effectiveTempThreshold := judgedDiskTemps[tempResourceID].threshold + + tempResourceName := fmt.Sprintf("%s (%s Temp)", hostDisplayName(host), disk.Device) + + diskTempMetadata := hostChildAlertMetadata(baseMetadata, hostDiskAlertResourceType) + diskTempMetadata["metric"] = "diskTemperature" + diskTempMetadata["device"] = disk.Device + diskTempMetadata["temperature"] = disk.Temperature + diskTempMetadata["model"] = disk.Model + diskTempMetadata["diskType"] = disk.DiskType + spec, err := buildCanonicalMetricSpec(tempResourceID, tempResourceName, unifiedresources.ResourceType("agent-disk"), "diskTemperature", effectiveTempThreshold) + if err != nil { + log.Warn(). + Err(err). + Str("resourceID", tempResourceID). + Str("host", resourceName). + Str("device", disk.Device). + Msg("Skipping invalid canonical host disk temperature metric spec") + continue + } + + m.checkMetricWithCanonicalSpec(spec, tempResourceName, nodeName, disk.Device, "agent", float64(disk.Temperature), effectiveTempThreshold, &metricOptions{Metadata: diskTempMetadata}) + } + // An excluded disk's alert from before the exclusion resolves now, + // whatever the SMART list holds, unless another listed disk shares it. + for tempResourceID := range seenDiskTemps { + delete(excludedDiskTemps, tempResourceID) + } + m.clearHostDiskTemperatureResources(excludedDiskTemps) // An empty SMART list means collection failed or is unsupported for // this report, not that every disk left, so existing alerts are held. if len(host.Sensors.SMART) > 0 { - seenDiskTemps := make(map[string]struct{}, len(host.Sensors.SMART)) - for _, disk := range host.Sensors.SMART { - // A listed disk in standby, or without a temperature after a - // failed probe, is still present. Its alert holds until a fresh - // reading resolves it instead of clearing and re-raising. - tempResourceID := hostDiskTemperatureResourceID(host.ID, disk.Device) - seenDiskTemps[tempResourceID] = struct{}{} - if disk.Temperature > 0 && !disk.Standby { - m.mu.RLock() - effectiveTempThreshold := m.hostDiskTemperatureThresholdNoLock(thresholds.DiskTemperature, diskTempOverridden, disk.Type) - m.mu.RUnlock() - - tempResourceName := fmt.Sprintf("%s (%s Temp)", hostDisplayName(host), disk.Device) - - diskTempMetadata := hostChildAlertMetadata(baseMetadata, hostDiskAlertResourceType) - diskTempMetadata["metric"] = "diskTemperature" - diskTempMetadata["device"] = disk.Device - diskTempMetadata["temperature"] = disk.Temperature - diskTempMetadata["model"] = disk.Model - diskTempMetadata["diskType"] = disk.Type - spec, err := buildCanonicalMetricSpec(tempResourceID, tempResourceName, unifiedresources.ResourceType("agent-disk"), "diskTemperature", effectiveTempThreshold) - if err != nil { - log.Warn(). - Err(err). - Str("resourceID", tempResourceID). - Str("host", resourceName). - Str("device", disk.Device). - Msg("Skipping invalid canonical host disk temperature metric spec") - continue - } - - m.checkMetricWithCanonicalSpec(spec, tempResourceName, nodeName, disk.Device, "agent", float64(disk.Temperature), effectiveTempThreshold, &metricOptions{Metadata: diskTempMetadata}) - } - } // A disk missing from consecutive non-empty reports was removed, // replaced or renamed, so no later reading will resolve its alert. m.cleanupHostDiskTemperatureAlerts(host.ID, seenDiskTemps) diff --git a/internal/alerts/host_disk_temperature_readings.go b/internal/alerts/host_disk_temperature_readings.go new file mode 100644 index 0000000000..f1e997d8fc --- /dev/null +++ b/internal/alerts/host_disk_temperature_readings.go @@ -0,0 +1,72 @@ +package alerts + +import "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" + +// clearHostDiskTemperatureResources resolves the disk temperature alerts +// raised under the given resource IDs and drops their pending threshold runs. +// CheckHost calls it for disks its host still lists but the agent's +// --disk-exclude patterns took out of monitoring, so no reading will resolve +// them. +func (m *Manager) clearHostDiskTemperatureResources(resourceIDs map[string]struct{}) { + if len(resourceIDs) == 0 { + return + } + + m.mu.Lock() + defer m.mu.Unlock() + + var storageKeys []string + for storageKey, alert := range m.activeAlerts { + if alert == nil { + continue + } + if _, ok := resourceIDs[alert.ResourceID]; ok { + storageKeys = append(storageKeys, storageKey) + } + } + for resourceID := range resourceIDs { + m.core.DropPendingForResource(resourceID) + } + for _, storageKey := range storageKeys { + m.clearAlertNoLock(storageKey) + } +} + +// judgedHostDiskTemperature is a disk temperature reading with the threshold +// its disk type resolves to under the host's policy. +type judgedHostDiskTemperature struct { + reading unifiedresources.HostDiskTemperatureReading + threshold *HysteresisThreshold +} + +// standing ranks the reading against its own threshold: 2 at or above the +// trigger, 1 inside the recovery band, where an open alert holds, 0 below it, +// and -1 when the threshold is off. margin is how far the reading sits above +// the trigger. +func (j judgedHostDiskTemperature) standing() (rank int, margin float64) { + threshold := j.threshold + if threshold == nil || threshold.Trigger <= 0 { + return -1, 0 + } + temperature := float64(j.reading.Temperature) + margin = temperature - threshold.Trigger + switch { + case temperature >= threshold.Trigger: + return 2, margin + case threshold.Clear > 0 && threshold.Clear < threshold.Trigger && temperature > threshold.Clear: + return 1, margin + default: + return 0, margin + } +} + +// outranks reports whether j should be judged instead of other for their +// shared alert. A tie keeps the reading listed first. +func (j judgedHostDiskTemperature) outranks(other judgedHostDiskTemperature) bool { + rank, margin := j.standing() + otherRank, otherMargin := other.standing() + if rank != otherRank { + return rank > otherRank + } + return margin > otherMargin +} diff --git a/internal/alerts/host_unraid_lifecycle_test.go b/internal/alerts/host_unraid_lifecycle_test.go index 1dd52d59b6..a3c217fb20 100644 --- a/internal/alerts/host_unraid_lifecycle_test.go +++ b/internal/alerts/host_unraid_lifecycle_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/rcourtman/pulse-go-rewrite/internal/models" + "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" ) func TestHostCustomSensorAlertLifecycle(t *testing.T) { @@ -546,6 +547,303 @@ func TestConfigSaveKeepsAlertTheEvaluatorKeepsWithoutRecoveryBand(t *testing.T) } } +const unraidTempHostID = "host-unraid-temp" + +// unraidTempHost is an Unraid agent report with the given SMART rows and +// Unraid inventory rows. +func unraidTempHost(smart []models.HostDiskSMART, unraid ...models.HostUnraidDisk) models.Host { + host := hostWithSMARTDiskTemp(unraidTempHostID, "sata", 40) + host.Sensors.SMART = smart + host.Unraid = &models.HostUnraidStorage{ArrayStarted: true, ArrayState: "STARTED", Disks: unraid} + return host +} + +func unraidTempAlertID(device string) string { + return canonicalMetricStateID(hostDiskTemperatureResourceID(unraidTempHostID, device), "diskTemperature") +} + +func TestCheckHostJudgesTheUnraidTemperatureADiskShows(t *testing.T) { + cool := models.HostDiskSMART{Device: "sda", Model: "cool-disk", Serial: "COOL-1", Type: "sata", Temperature: 40} + coolUnraid := models.HostUnraidDisk{Name: "disk1", Device: "/dev/sda", Serial: "COOL-1", Transport: "sata", Temperature: 40} + hot := models.HostUnraidDisk{Name: "disk2", Device: "/dev/sdc", Model: "hot-disk", Serial: "HOT-1", Transport: "sata", Temperature: 60} + + cases := []struct { + name string + host func() models.Host + configure func(m *Manager) + device string + wantAlert bool + // quiet lists devices that must raise nothing. + quiet []string + }{ + { + // smartctl returned no temperature, and the disk shows Unraid's. + name: "SMART row without a reading", + host: func() models.Host { + return unraidTempHost([]models.HostDiskSMART{ + {Device: "sdc", Model: "hot-disk", Serial: "HOT-1", Type: "sata"}, + }, hot) + }, + device: "sdc", + wantAlert: true, + }, + { + name: "disk only the Unraid inventory lists", + host: func() models.Host { + return unraidTempHost([]models.HostDiskSMART{cool}, coolUnraid, hot) + }, + device: "sdc", + wantAlert: true, + }, + { + name: "Unraid disk while the SMART list is empty", + host: func() models.Host { + return unraidTempHost(nil, coolUnraid, hot) + }, + device: "sdc", + wantAlert: true, + }, + { + // smartctl and Unraid name the device with different serials, so + // two disks show on it; its alert judges the hotter reading. + name: "two disks shown on one device", + host: func() models.Host { + return unraidTempHost([]models.HostDiskSMART{ + cool, + {Device: "sdc", Model: "bridge", Serial: "BRIDGE-9", Type: "sata", Temperature: 41}, + }, coolUnraid, hot) + }, + device: "sdc", + wantAlert: true, + }, + { + // 64C is under the sas trigger (65); 60C is over the sata one (55). + name: "two disks of different types on one device", + host: func() models.Host { + return unraidTempHost([]models.HostDiskSMART{ + cool, + {Device: "sdc", Model: "bridge", Serial: "BRIDGE-9", Type: "sas", Temperature: 64}, + }, coolUnraid, hot) + }, + device: "sdc", + wantAlert: true, + }, + { + // A controller member and its Unraid device are one registry + // disk, alerted once under the member's own resource. + name: "controller member under another device label", + host: func() models.Host { + member := models.HostDiskSMART{Device: "0 [megaraid,0]", Model: "hot-disk", Serial: "HOT-1", Type: "sata", Controller: "0", Target: "megaraid,0", Temperature: 60} + return unraidTempHost([]models.HostDiskSMART{cool, member}, coolUnraid, hot) + }, + device: "0 [megaraid,0]", + wantAlert: true, + quiet: []string{"sdc"}, + }, + { + name: "spun-down Unraid disk", + host: func() models.Host { + spunDown := hot + spunDown.SpunDown = true + return unraidTempHost([]models.HostDiskSMART{cool}, coolUnraid, spunDown) + }, + device: "sdc", + }, + { + name: "Unraid disk the agent excludes", + host: func() models.Host { + host := unraidTempHost([]models.HostDiskSMART{cool}, coolUnraid, hot) + host.DiskExclude = []string{"/dev/sdc"} + return host + }, + device: "sdc", + }, + { + // 65C is under the nvme trigger (70) and over the agent default (55). + name: "Unraid disk judged by its disk type", + host: func() models.Host { + nvme := hot + nvme.Transport = "nvme" + nvme.Temperature = 65 + return unraidTempHost([]models.HostDiskSMART{cool}, coolUnraid, nvme) + }, + device: "sdc", + }, + { + name: "Unraid disk under a host override", + host: func() models.Host { + return unraidTempHost([]models.HostDiskSMART{cool}, coolUnraid, hot) + }, + configure: func(m *Manager) { + m.config.Overrides[unraidTempHostID] = ThresholdConfig{DiskTemperature: &HysteresisThreshold{Trigger: 65, Clear: 60}} + }, + device: "sdc", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + m := configureDiskTempTypeHostManager(t) + if tc.configure != nil { + m.mu.Lock() + tc.configure(m) + m.mu.Unlock() + } + m.CheckHost(tc.host()) + + alert, exists := testLookupActiveAlert(t, m, unraidTempAlertID(tc.device)) + if exists != tc.wantAlert { + t.Fatalf("disk temperature alert for %s active = %v, want %v; active: %v", tc.device, exists, tc.wantAlert, alertKeys(m)) + } + for _, device := range append([]string{"sda"}, tc.quiet...) { + if _, exists := testLookupActiveAlert(t, m, unraidTempAlertID(device)); exists { + t.Fatalf("%s raised a temperature alert, active: %v", device, alertKeys(m)) + } + } + if !tc.wantAlert { + return + } + if alert.Value != 60 || alert.Metadata["temperature"] != 60 || alert.Metadata["diskType"] != "sata" || alert.Metadata["model"] != "hot-disk" { + t.Fatalf("alert value/metadata = %v/%v, want the 60C sata reading of hot-disk", alert.Value, alert.Metadata) + } + }) + } +} + +func TestCheckHostHoldsUnraidDiskTemperatureAlertUntilAReadingOrDeparture(t *testing.T) { + cool := models.HostDiskSMART{Device: "sda", Model: "cool-disk", Serial: "COOL-1", Type: "sata", Temperature: 40} + hot := models.HostUnraidDisk{Name: "disk2", Device: "/dev/sdc", Model: "hot-disk", Serial: "HOT-1", Transport: "sata", Temperature: 60} + alertID := unraidTempAlertID("sdc") + // CheckHost clears a disk's alert once three non-empty reports in a row + // omit it. + const absentReports = 3 + active := func(m *Manager) bool { + _, exists := testLookupActiveAlert(t, m, alertID) + return exists + } + hotHost := func(t *testing.T) *Manager { + m := configureDiskTempTypeHostManager(t) + m.CheckHost(unraidTempHost([]models.HostDiskSMART{cool}, hot)) + if !active(m) { + t.Fatalf("expected the Unraid reading to raise a disk temperature alert, active: %v", alertKeys(m)) + } + return m + } + + t.Run("listed without a reading", func(t *testing.T) { + m := hotHost(t) + spunDown := hot + spunDown.SpunDown = true + for i := 0; i < 4; i++ { + m.CheckHost(unraidTempHost([]models.HostDiskSMART{cool}, spunDown)) + } + if !active(m) { + t.Fatalf("a spun-down disk still in the inventory lost its alert, active: %v", alertKeys(m)) + } + }) + + t.Run("cooled reading resolves it", func(t *testing.T) { + m := hotHost(t) + cooled := hot + cooled.Temperature = 45 + m.CheckHost(unraidTempHost([]models.HostDiskSMART{cool}, cooled)) + if active(m) { + t.Fatalf("a 45C Unraid reading (clear 50) left the alert active: %v", alertKeys(m)) + } + }) + + t.Run("smartctl reading takes over the same alert", func(t *testing.T) { + m := hotHost(t) + withSMART := []models.HostDiskSMART{cool, {Device: "sdc", Model: "hot-disk", Serial: "HOT-1", Type: "sata", Temperature: 62}} + m.CheckHost(unraidTempHost(withSMART, hot)) + if !active(m) { + t.Fatalf("the disk's SMART row did not carry its Unraid-raised alert, active: %v", alertKeys(m)) + } + withSMART[1].Temperature = 45 + m.CheckHost(unraidTempHost(withSMART, hot)) + if active(m) { + t.Fatalf("a 45C SMART reading left the alert active: %v", alertKeys(m)) + } + }) + + t.Run("left the inventory", func(t *testing.T) { + m := hotHost(t) + for i := 1; i < absentReports; i++ { + m.CheckHost(unraidTempHost([]models.HostDiskSMART{cool})) + if !active(m) { + t.Fatalf("alert cleared after %d report(s) without the disk, want %d", i, absentReports) + } + } + m.CheckHost(unraidTempHost([]models.HostDiskSMART{cool})) + if active(m) { + t.Fatalf("alert outlived a disk gone from %d reports, active: %v", absentReports, alertKeys(m)) + } + }) + + t.Run("excluded row on a device a SMART row lists", func(t *testing.T) { + m := hotHost(t) + excluded := unraidTempHost([]models.HostDiskSMART{ + cool, + {Device: "sdc", Model: "bridge", Serial: "BRIDGE-9", Type: "sata", Temperature: 62}, + }, hot) + excluded.DiskExclude = []string{"sdc"} + m.CheckHost(excluded) + if !active(m) { + t.Fatalf("the exclusion cleared an alert the device's SMART row still raises, active: %v", alertKeys(m)) + } + }) + + // The disk is still listed but out of monitoring, so its alert resolves + // on the first report, even one with an empty SMART list. + for name, smart := range map[string][]models.HostDiskSMART{ + "excluded after it fired": {cool}, + "excluded after it fired, SMART list empty": nil, + } { + t.Run(name, func(t *testing.T) { + m := hotHost(t) + excluded := unraidTempHost(smart, hot) + excluded.DiskExclude = []string{"sdc"} + m.CheckHost(excluded) + if active(m) { + t.Fatalf("alert outlived the disk's exclusion from monitoring, active: %v", alertKeys(m)) + } + }) + } +} + +func TestJudgedHostDiskTemperatureOutranks(t *testing.T) { + sata := &HysteresisThreshold{Trigger: 55, Clear: 50} + sas := &HysteresisThreshold{Trigger: 65, Clear: 60} + noBand := &HysteresisThreshold{Trigger: 55, Clear: 55} + off := &HysteresisThreshold{Trigger: 0} + judged := func(temperature int, threshold *HysteresisThreshold) judgedHostDiskTemperature { + return judgedHostDiskTemperature{ + reading: unifiedresources.HostDiskTemperatureReading{Temperature: temperature}, + threshold: threshold, + } + } + + cases := []struct { + name string + first, second judgedHostDiskTemperature + secondWins bool + }{ + {name: "over its trigger beats a hotter reading under its own", first: judged(64, sas), second: judged(56, sata), secondWins: true}, + {name: "in its recovery band beats one below its clear", first: judged(49, sata), second: judged(62, sas), secondWins: true}, + {name: "no band means under the trigger is below it", first: judged(54, noBand), second: judged(51, sata), secondWins: true}, + {name: "further over its trigger wins", first: judged(56, sata), second: judged(70, sas), secondWins: true}, + {name: "a threshold that is off never wins", first: judged(40, sata), second: judged(90, off)}, + {name: "a tie keeps the first", first: judged(60, sata), second: judged(60, sata)}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := tc.second.outranks(tc.first); got != tc.secondWins { + t.Fatalf("second outranks first = %v, want %v", got, tc.secondWins) + } + }) + } +} + func hasAlertType(alerts []Alert, alertType string) bool { for _, alert := range alerts { if alert.Type == alertType { diff --git a/internal/monitoring/issue1595_collection_trust_test.go b/internal/monitoring/issue1595_collection_trust_test.go index 6a47558c65..e3de00161e 100644 --- a/internal/monitoring/issue1595_collection_trust_test.go +++ b/internal/monitoring/issue1595_collection_trust_test.go @@ -681,6 +681,167 @@ func TestAgentDiskHistoryFollowsTheSerialItsUnraidRowReports(t *testing.T) { } } +// An Unraid host's disk can show a temperature only its Unraid inventory +// reports: a member the agent's SMART collection skips (--disk-exclude), or +// one Unraid reads with its own per-disk SMART settings while the agent's +// probe returns no temperature. The chart must hold the reading the disk +// shows, under the key its metrics target reads, and nothing for a reading the +// disk does not show as collected (a spun-down disk, a host past its lease) or +// does not show at all (a controller member behind the device Unraid reads). +func TestAgentDiskChartsTheUnraidTemperatureItShows(t *testing.T) { + storeConfig := metrics.DefaultConfig(t.TempDir()) + storeConfig.FlushInterval = time.Hour + store, err := metrics.NewStore(storeConfig) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = store.Close() }) + m := &Monitor{metricsStore: store, rateTracker: NewRateTracker()} + + now := time.Now().UTC().Truncate(time.Second) + host := models.Host{ID: "host-tower", Hostname: "tower", MachineID: "machine-tower", Status: "online", LastSeen: now} + host.Sensors.SMART = []models.HostDiskSMART{ + // The agent's identity-only row for a disk its probe got nothing from. + {Device: "sdb", Serial: "SER-B", Health: "UNKNOWN", Collection: &diskinventory.CollectionStatus{ + Temperature: diskinventory.Unavailable("smartctl", "SMART probe returned no usable temperature data")}}, + // A row with its own reading keeps it; the Unraid one is not written too. + {Device: "sdc", WWN: "0x5000c500aaaa0003", Health: "PASSED", Temperature: 41, Collection: &diskinventory.CollectionStatus{ + Temperature: diskinventory.Available("smartctl")}}, + // The agent's native standby row for a disk Unraid reports spun down. + {Device: "sdd", Serial: "SER-D", Health: "UNKNOWN", Standby: true, Collection: &diskinventory.CollectionStatus{ + Temperature: diskinventory.Unavailable("unraid", "disk is reported spun down")}}, + // Controller members behind one kernel block device, which the Unraid + // row describes as a whole: neither member takes its temperature. + {Device: "sdh", Controller: "sdh", Target: "megaraid,0", WWN: "0x5000c500bbbb0001", Health: "PASSED", Collection: &diskinventory.CollectionStatus{ + Temperature: diskinventory.Unavailable("smartctl", "SMART probe returned no usable temperature data")}}, + {Device: "sdh", Controller: "sdh", Target: "megaraid,1", WWN: "0x5000c500bbbb0002", Health: "PASSED", Collection: &diskinventory.CollectionStatus{ + Temperature: diskinventory.Unavailable("smartctl", "SMART probe returned no usable temperature data")}}, + // A member whose own serial is the Unraid row's is the disk that row + // describes, though its path is shared. + {Device: "sdi", Controller: "sdi", Target: "megaraid,0", Serial: "SER-I", Health: "UNKNOWN", Collection: &diskinventory.CollectionStatus{ + Temperature: diskinventory.Unavailable("smartctl", "SMART probe returned no usable temperature data")}}, + {Device: "sdi", Controller: "sdi", Target: "megaraid,1", Serial: "SER-I2", Health: "UNKNOWN", Collection: &diskinventory.CollectionStatus{ + Temperature: diskinventory.Unavailable("smartctl", "SMART probe returned no usable temperature data")}}, + // A placeholder serial is no evidence of which member the row is. + {Device: "sdn", Controller: "sdn", Target: "megaraid,0", Serial: "UNKNOWN", WWN: "0x5000c500cccc0001", Health: "UNKNOWN", Collection: &diskinventory.CollectionStatus{ + Temperature: diskinventory.Unavailable("smartctl", "SMART probe returned no usable temperature data")}}, + {Device: "sdn", Controller: "sdn", Target: "megaraid,1", Serial: "UNKNOWN", WWN: "0x5000c500cccc0002", Health: "UNKNOWN", Collection: &diskinventory.CollectionStatus{ + Temperature: diskinventory.Unavailable("smartctl", "SMART probe returned no usable temperature data")}}, + // A placeholder serial matches no Unraid row; the row's path finds its own. + {Device: "sdo", Serial: "0123456789", WWN: "0x5000c500eeee0001", Health: "UNKNOWN", Collection: &diskinventory.CollectionStatus{ + Temperature: diskinventory.Unavailable("smartctl", "SMART probe returned no usable temperature data")}}, + // The only reported member behind a controller path is still a member. + {Device: "sdq", Controller: "sdq", Target: "megaraid,0", WWN: "0x5000c500ffff0001", Health: "UNKNOWN", Collection: &diskinventory.CollectionStatus{ + Temperature: diskinventory.Unavailable("smartctl", "SMART probe returned no usable temperature data")}}, + // Without a serial or WWN, a member charts its own reading under its + // topology key. + {Device: "sdr", Controller: "sdr", Target: "megaraid,0", Health: "PASSED", Temperature: 35, Collection: &diskinventory.CollectionStatus{ + Temperature: diskinventory.Available("smartctl")}}, + // A SMART row's key is its own: the merged disk shows the row's reading, + // here not collected, and the Unraid one is not charted over it. + {Device: "sdm", WWN: "0x5000c500dddd0001", Health: "PASSED", Temperature: 40, Collection: &diskinventory.CollectionStatus{ + Temperature: diskinventory.Unavailable("smartctl", "disk is in standby")}}, + } + host.Unraid = &models.HostUnraidStorage{ArrayStarted: true, Disks: []models.HostUnraidDisk{ + {Device: "sdb", Serial: "SER-B", Name: "disk1", Role: "data", Status: "online", Temperature: 38}, + {Device: "sdc", Serial: "SER-C", Name: "disk2", Role: "data", Status: "online", Temperature: 39}, + {Device: "sdd", Serial: "SER-D", Name: "disk3", Role: "data", Status: "online", Temperature: 35, SpunDown: true}, + // Members with no SMART row at all. + {Device: "sde", Serial: "SER-E", Name: "disk4", Role: "data", Status: "online", Temperature: 36}, + {Device: "sdf", Serial: "SER-F", Name: "disk5", Role: "data", Status: "online", Temperature: 33, SpunDown: true}, + {Device: "sdg", Name: "disk6", Role: "data", Status: "online", Temperature: 37}, + {Device: "sdh", Serial: "SER-H", Name: "disk7", Role: "data", Status: "online", Temperature: 42}, + {Device: "sdi", Serial: "SER-I", Name: "disk8", Role: "data", Status: "online", Temperature: 44}, + // Rows sharing one history key are one disk, showing the latest reading. + {Device: "sdj", Name: "disk9", Role: "data", Status: "online"}, + {Device: "sdj", Name: "disk10", Role: "data", Status: "online", Temperature: 43}, + {Device: "sdk", Name: "disk11", Role: "data", Status: "online", Temperature: 38}, + {Device: "sdk", Name: "disk12", Role: "data", Status: "online", Temperature: 39}, + {Device: "sdl", Name: "disk13", Role: "data", Status: "online", Temperature: 40}, + {Device: "sdl", Name: "disk14", Role: "data", Status: "online", Temperature: 32, SpunDown: true}, + {Device: "sdn", Serial: "UNKNOWN", Name: "disk15", Role: "data", Status: "online", Temperature: 45}, + {Device: "sdm", Serial: "SER-M", Name: "disk16", Role: "data", Status: "online", Temperature: 37}, + {Device: "sdp", Serial: "0123456789", Name: "disk17", Role: "data", Status: "online", Temperature: 30}, + {Device: "sdo", Serial: "0123456789", Name: "disk18", Role: "data", Status: "online", Temperature: 46}, + {Device: "sdq", Serial: "SER-Q", Name: "disk19", Role: "data", Status: "online", Temperature: 47}, + {Device: "sdr", Serial: "SER-R", Name: "disk20", Role: "data", Status: "online", Temperature: 36}, + }} + // Disks by usable serial, else WWN, else device path, and the reading each + // shows as collected. + want := map[string]float64{ + "SER-B": 38, "SER-C": 41, "SER-D": 0, "SER-E": 36, "SER-F": 0, "sdg": 37, + "SER-H": 42, "0x5000c500bbbb0001": 0, "0x5000c500bbbb0002": 0, + "SER-I": 44, "SER-I2": 0, "sdj": 43, "sdk": 39, "sdl": 0, + "sdn": 45, "0x5000c500cccc0001": 0, "0x5000c500cccc0002": 0, "SER-M": 0, + "0x5000c500eeee0001": 46, "sdo": 46, "sdp": 30, "0x5000c500ffff0001": 0, "SER-Q": 47, + "sdr": 35, "SER-R": 36, + } + + m.writeHostSMARTMetrics(host, now) + // Once the host's lease expires its last report is kept, offline, as + // context; nothing it carries is a current reading. + reported := host + reported.Sensors.SMART = append([]models.HostDiskSMART(nil), host.Sensors.SMART...) + state := &models.State{Hosts: []models.Host{reported}} + offline, expired := state.ExpireHostTelemetry(host.ID, host.LastSeen) + if !expired { + t.Fatal("host telemetry did not expire") + } + m.writeHostSMARTMetrics(offline, now.Add(30*time.Second)) + store.Flush() + + live := unifiedresources.NewRegistry(nil) + live.IngestSnapshot(models.StateSnapshot{Hosts: []models.Host{host}}) + rehydrated := unifiedresources.NewRegistry(nil) + rehydrated.IngestResources(live.List()) + for _, tc := range []struct { + name string + registry *unifiedresources.ResourceRegistry + }{{"live", live}, {"rehydrated", rehydrated}} { + disks := tc.registry.ListByType(unifiedresources.ResourceTypePhysicalDisk) + if len(disks) != len(want) { + t.Fatalf("%s: disks = %d, want %d", tc.name, len(disks), len(want)) + } + for _, disk := range disks { + device := disk.PhysicalDisk.Serial + if !diskinventory.IsUsableHardwareID(device) { + device = "" + } + if device == "" { + device = disk.PhysicalDisk.WWN + } + if device == "" { + device = disk.PhysicalDisk.DevPath + } + wantTemp, ok := want[device] + if !ok { + t.Fatalf("%s: unexpected disk %s at %q", tc.name, disk.ID, device) + } + shown := 0.0 + if diskinventory.TemperatureCollected(disk.PhysicalDisk.Temperature, disk.PhysicalDisk.Collection) { + shown = float64(disk.PhysicalDisk.Temperature) + } + if shown != wantTemp { + t.Fatalf("%s: disk %s shows %v°C as collected, want %v°C", tc.name, device, shown, wantTemp) + } + target := tc.registry.MetricsTarget(disk.ID) + if target == nil { + t.Fatalf("%s: disk %s has no metrics target", tc.name, device) + } + points, err := store.Query(target.ResourceType, target.ResourceID, "smart_temp", now.Add(-time.Minute), now.Add(time.Minute), 0) + if err != nil { + t.Fatal(err) + } + switch { + case wantTemp == 0 && len(points) != 0: + t.Errorf("%s: disk %s shows no collected temperature but target %+v charts %+v", tc.name, device, *target, points) + case wantTemp > 0 && (len(points) != 1 || points[0].Value != wantTemp || !points[0].Timestamp.Equal(now)): + t.Errorf("%s: disk %s shows %v°C but target %+v charts %+v, want one sample at the online report", tc.name, device, wantTemp, *target, points) + } + } + } +} + // A SMART row found only by device path may describe the slot's previous // occupant. It is refused when its identity contradicts the Proxmox disk's, // judged only on what both producers report like for like: WWNs for every diff --git a/internal/monitoring/monitor_agents.go b/internal/monitoring/monitor_agents.go index 85599d39e7..598c6b3bd5 100644 --- a/internal/monitoring/monitor_agents.go +++ b/internal/monitoring/monitor_agents.go @@ -4081,14 +4081,18 @@ func (m *Monitor) writeHostSMARTMetrics(host models.Host, now time.Time) { return } + smartMetricIDs := make(map[string]struct{}, len(host.Sensors.SMART)) for _, disk := range host.Sensors.SMART { resourceID := unifiedresources.HostSMARTDiskMetricID(host, disk) if resourceID == "" { continue } + smartMetricIDs[resourceID] = struct{}{} - if diskinventory.TemperatureCollected(disk.Temperature, disk.Collection) { - m.metricsStore.Write("disk", resourceID, "smart_temp", float64(disk.Temperature), now) + // Chart the reading the disk shows: a row without its own carries the + // host's Unraid reading for the disk. + if temperature, collection := unifiedresources.HostSMARTDiskTemperature(host, disk); diskinventory.TemperatureCollected(temperature, collection) { + m.metricsStore.Write("disk", resourceID, "smart_temp", float64(temperature), now) } attrs := disk.Attributes @@ -4127,6 +4131,50 @@ func (m *Monitor) writeHostSMARTMetrics(host models.Host, now time.Time) { m.metricsStore.Write("disk", resourceID, "smart_unsafe_shutdowns", float64(*attrs.UnsafeShutdowns), now) } } + + m.writeHostUnraidDiskTemperatures(host, smartMetricIDs, now) +} + +// writeHostUnraidDiskTemperatures charts the temperature of each disk only the +// host's Unraid inventory reports, such as a member the agent's SMART +// collection skips under --disk-exclude. smartMetricIDs holds the history keys +// of the host's SMART rows. A disk under one of those keys is that row's to +// chart, and the row already carries the Unraid reading when it has none of +// its own. Unraid rows sharing a key are one disk in the registry, which keeps +// the latest of their readings, so the last row with a reading decides that +// key's sample. +func (m *Monitor) writeHostUnraidDiskTemperatures(host models.Host, smartMetricIDs map[string]struct{}, now time.Time) { + if host.Unraid == nil { + return + } + type reading struct { + temperature int + collection *diskinventory.CollectionStatus + } + var keys []string + readings := make(map[string]reading, len(host.Unraid.Disks)) + for _, disk := range host.Unraid.Disks { + resourceID := unifiedresources.HostUnraidDiskMetricID(host, disk) + if resourceID == "" { + continue + } + if _, ok := smartMetricIDs[resourceID]; ok { + continue + } + temperature, collection := unifiedresources.HostUnraidDiskTemperature(host, disk) + if temperature <= 0 { + continue + } + if _, ok := readings[resourceID]; !ok { + keys = append(keys, resourceID) + } + readings[resourceID] = reading{temperature: temperature, collection: collection} + } + for _, resourceID := range keys { + if r := readings[resourceID]; diskinventory.TemperatureCollected(r.temperature, r.collection) { + m.metricsStore.Write("disk", resourceID, "smart_temp", float64(r.temperature), now) + } + } } func normalizeHostDiskDevice(device string) string { diff --git a/internal/monitoring/monitor_host_agents_test.go b/internal/monitoring/monitor_host_agents_test.go index 2f99b2aada..f5abd19370 100644 --- a/internal/monitoring/monitor_host_agents_test.go +++ b/internal/monitoring/monitor_host_agents_test.go @@ -3017,6 +3017,71 @@ func TestApplyHostReportPersistsSMARTMetricsForAgentDisks(t *testing.T) { } } +// An Unraid member the agent sends no SMART row for (a --disk-exclude match) +// still shows the temperature its Unraid inventory row reports, so the report +// charts it under the disk's serial. A disk with its own SMART reading keeps +// it, and a spun-down member's leftover reading is not charted. +func TestApplyHostReportChartsUnraidTemperatureOfDiskWithoutSMART(t *testing.T) { + storeCfg := metrics.DefaultConfig(t.TempDir()) + storeCfg.WriteBufferSize = 1 + store, err := metrics.NewStore(storeCfg) + if err != nil { + t.Fatalf("NewStore: %v", err) + } + defer store.Close() + + monitor := &Monitor{ + state: models.NewState(), + alertManager: alerts.NewManager(), + hostTokenBindings: make(map[string]string), + config: &config.Config{}, + rateTracker: NewRateTracker(), + metricsStore: store, + } + t.Cleanup(func() { monitor.alertManager.Stop() }) + + report := agentshost.Report{ + Agent: agentshost.AgentInfo{ID: "agent-tower", Version: "1.0.0", IntervalSeconds: 30}, + Host: agentshost.HostInfo{ID: "machine-tower", Hostname: "tower", MachineID: "machine-tower"}, + Metrics: agentshost.Metrics{ + Memory: agentshost.MemoryMetric{TotalBytes: 1024, UsedBytes: 512, FreeBytes: 512, Usage: 50}, + }, + Sensors: agentshost.Sensors{ + SMART: []agentshost.DiskSMART{ + {Device: "/dev/sda", Model: "IronWolf", Serial: "SERIAL-TOWER-1", Temperature: 41}, + }, + }, + Unraid: &agentshost.UnraidStorage{ArrayStarted: true, Disks: []agentshost.UnraidDisk{ + {Name: "disk1", Device: "sda", Role: "data", Status: "online", Serial: "SERIAL-TOWER-1", Temperature: 40}, + {Name: "disk2", Device: "sdb", Role: "data", Status: "online", Serial: "SERIAL-TOWER-2", Temperature: 36}, + {Name: "disk3", Device: "sdc", Role: "data", Status: "online", Serial: "SERIAL-TOWER-3", Temperature: 34, SpunDown: true}, + }}, + Timestamp: time.Now().UTC(), + } + + if _, err := monitor.ApplyHostReport(report, nil); err != nil { + t.Fatalf("ApplyHostReport: %v", err) + } + store.Flush() + + points := waitForStoredDiskMetric(t, store, "SERIAL-TOWER-2", "smart_temp") + if len(points) != 1 || points[0].Value != 36 { + t.Fatalf("SERIAL-TOWER-2 smart_temp = %+v, want the Unraid reading 36", points) + } + points = waitForStoredDiskMetric(t, store, "SERIAL-TOWER-1", "smart_temp") + if len(points) != 1 || points[0].Value != 41 { + t.Fatalf("SERIAL-TOWER-1 smart_temp = %+v, want its own SMART reading 41", points) + } + now := time.Now().UTC() + points, err = store.Query("disk", "SERIAL-TOWER-3", "smart_temp", now.Add(-time.Hour), now.Add(time.Hour), 60) + if err != nil { + t.Fatalf("Query smart_temp: %v", err) + } + if len(points) != 0 { + t.Fatalf("spun-down SERIAL-TOWER-3 smart_temp = %+v, want none", points) + } +} + func TestApplyHostReportPersistsAgentTemperatureMetric(t *testing.T) { t.Helper() diff --git a/internal/unifiedresources/adapters.go b/internal/unifiedresources/adapters.go index 167af4c6b1..e5220c3800 100644 --- a/internal/unifiedresources/adapters.go +++ b/internal/unifiedresources/adapters.go @@ -806,10 +806,7 @@ func resourceFromHostUnraidPhysicalDisk(host models.Host, disk models.HostUnraid health := unraidPhysicalDiskHealth(disk) assessment := assessUnraidPhysicalDisk(disk) sizeBytes := unraidDiskSizeBytes(host, disk) - var collection *diskinventory.CollectionStatus - if status := unraidDiskTemperatureStatus(host, disk); status.State != "" { - collection = &diskinventory.CollectionStatus{Temperature: status} - } + temperature, collection := HostUnraidDiskTemperature(host, disk) resource := Resource{ Type: ResourceTypePhysicalDisk, Name: name, @@ -824,7 +821,7 @@ func resourceFromHostUnraidPhysicalDisk(host models.Host, disk models.HostUnraid SizeBytes: sizeBytes, Health: health, Wearout: -1, - Temperature: disk.Temperature, + Temperature: temperature, Used: unraidDiskMountPath(disk), StorageRole: unraidDiskRole(&disk), StorageGroup: unraidDiskGroup(&disk), @@ -869,6 +866,46 @@ func unraidDiskTemperatureStatus(host models.Host, disk models.HostUnraidDisk) d } } +// HostUnraidDiskTemperature returns the temperature the disk resource built +// from a host's Unraid inventory row shows, with its collection state. The +// agent disk history writer charts this reading. +func HostUnraidDiskTemperature(host models.Host, disk models.HostUnraidDisk) (int, *diskinventory.CollectionStatus) { + status := unraidDiskTemperatureStatus(host, disk) + if status.State == "" { + return disk.Temperature, nil + } + return disk.Temperature, &diskinventory.CollectionStatus{Temperature: status} +} + +// HostSMARTDiskTemperature returns the temperature the disk resource built +// from a host agent's SMART row shows, with its collection state. The agent +// disk history writer charts this reading. +func HostSMARTDiskTemperature(host models.Host, disk models.HostDiskSMART) (int, *diskinventory.CollectionStatus) { + return hostSMARTDiskTemperature(host, disk, matchUnraidDisk(host.Unraid, disk)) +} + +// hostSMARTDiskTemperature returns the temperature the disk resource built +// from a host agent's SMART row shows, with the row's collection state: the +// row's own reading, or, when the row has none, the one the host's Unraid +// inventory reports for the disk. Unraid reads its members' temperatures with +// its own per-disk SMART settings, so it can have one when the agent's probe +// returned none. The reading is taken only from an Unraid row that describes +// the row's disk (unraidDiskDescribesSMARTRow). +func hostSMARTDiskTemperature(host models.Host, disk models.HostDiskSMART, unraidDisk *models.HostUnraidDisk) (int, *diskinventory.CollectionStatus) { + collection := diskinventory.CloneStatus(disk.Collection) + if disk.Temperature > 0 || !unraidDiskDescribesSMARTRow(host, disk, unraidDisk) { + return disk.Temperature, collection + } + // The reading is now the Unraid inventory's, and so is its state. + if status := unraidDiskTemperatureStatus(host, *unraidDisk); status.State != "" { + if collection == nil { + collection = &diskinventory.CollectionStatus{} + } + collection.Temperature = status + } + return unraidDisk.Temperature, collection +} + func resourceFromHostSMARTDisk(host models.Host, disk models.HostDiskSMART) (Resource, ResourceIdentity) { name := strings.TrimSpace(disk.Model) if name == "" { @@ -907,8 +944,7 @@ func resourceFromHostSMARTDisk(host models.Host, disk models.HostDiskSMART) (Res model := strings.TrimSpace(disk.Model) serial := hostSMARTDiskSerial(host, disk, unraidDisk) diskType := strings.TrimSpace(disk.Type) - temperature := disk.Temperature - collection := diskinventory.CloneStatus(disk.Collection) + temperature, collection := hostSMARTDiskTemperature(host, disk, unraidDisk) health := strings.TrimSpace(disk.Health) if unraidDisk != nil { if model == "" { @@ -920,16 +956,6 @@ func resourceFromHostSMARTDisk(host models.Host, disk models.HostDiskSMART) (Res if sizeBytes <= 0 { sizeBytes = unraidDiskSizeBytes(host, *unraidDisk) } - if temperature <= 0 { - temperature = unraidDisk.Temperature - // The reading is now the Unraid inventory's, and so is its state. - if status := unraidDiskTemperatureStatus(host, *unraidDisk); status.State != "" { - if collection == nil { - collection = &diskinventory.CollectionStatus{} - } - collection.Temperature = status - } - } if health == "" || strings.EqualFold(health, "UNKNOWN") { health = unraidPhysicalDiskHealth(*unraidDisk) } @@ -1219,21 +1245,30 @@ func maxInt64() int64 { return int64(^uint64(0) >> 1) } +// matchUnraidDisk returns the host's Unraid row for a SMART row: the one +// carrying the row's usable serial, else the one at its device path. A serial +// names the drive and a path only where the kernel put it, so a serial match +// anywhere in the inventory outranks a path match. A placeholder serial, which +// several bridges report alike, is no match at all. func matchUnraidDisk(unraid *models.HostUnraidStorage, disk models.HostDiskSMART) *models.HostUnraidDisk { if unraid == nil || len(unraid.Disks) == 0 { return nil } - normalizedDevice := strings.ToLower(normalizePhysicalDiskDeviceToken(disk.Device)) - normalizedSerial := strings.TrimSpace(strings.ToLower(disk.Serial)) - for i := range unraid.Disks { - candidate := &unraid.Disks[i] - if normalizedSerial != "" && strings.EqualFold(strings.TrimSpace(candidate.Serial), normalizedSerial) { - return candidate + if serial := strings.TrimSpace(disk.Serial); diskinventory.IsUsableHardwareID(serial) { + for i := range unraid.Disks { + if strings.EqualFold(strings.TrimSpace(unraid.Disks[i].Serial), serial) { + return &unraid.Disks[i] + } } - candidateDevice := strings.ToLower(normalizePhysicalDiskDeviceToken(candidate.Device)) - if normalizedDevice != "" && candidateDevice != "" && candidateDevice == normalizedDevice { - return candidate + } + normalizedDevice := strings.ToLower(normalizePhysicalDiskDeviceToken(disk.Device)) + if normalizedDevice == "" { + return nil + } + for i := range unraid.Disks { + if strings.ToLower(normalizePhysicalDiskDeviceToken(unraid.Disks[i].Device)) == normalizedDevice { + return &unraid.Disks[i] } } return nil diff --git a/internal/unifiedresources/canonical_ids_types_test.go b/internal/unifiedresources/canonical_ids_types_test.go index 8dbb9b6580..293d664ca8 100644 --- a/internal/unifiedresources/canonical_ids_types_test.go +++ b/internal/unifiedresources/canonical_ids_types_test.go @@ -503,6 +503,50 @@ func TestHostUnraidDiskSourceIDNormalizesDeviceAndPrefersSerial(t *testing.T) { } } +// An Unraid inventory row's history key is the one the metrics target of the +// disk resource built from it reads: its usable serial, unscoped like every +// physical-disk history key, or else its host/device source ID. A row without +// a device is not ingested as a disk and has no key. +func TestHostUnraidDiskMetricIDMatchesItsDiskMetricsTarget(t *testing.T) { + host := models.Host{ID: "host-tower", Hostname: "tower"} + tests := []struct { + name string + disk models.HostUnraidDisk + want string + }{ + {"serial", models.HostUnraidDisk{Device: "/dev/sdb", Serial: "SERIAL-DATA"}, "SERIAL-DATA"}, + {"placeholder serial", models.HostUnraidDisk{Device: "sdc", Serial: "N/A"}, "host-tower:sdc"}, + {"no serial", models.HostUnraidDisk{Device: "sdd [sat]"}, "host-tower:sdd"}, + {"no device", models.HostUnraidDisk{Name: "disk9", Serial: "SERIAL-MISSING"}, ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := HostUnraidDiskMetricID(host, tt.disk) + if got != tt.want { + t.Fatalf("HostUnraidDiskMetricID(%+v) = %q, want %q", tt.disk, got, tt.want) + } + if tt.want == "" { + return + } + disk := tt.disk + disk.Name, disk.Role, disk.Status = "disk1", "data", "online" + registry := NewRegistry(nil) + registry.IngestSnapshot(models.StateSnapshot{Hosts: []models.Host{{ + ID: host.ID, Hostname: host.Hostname, Status: "online", + Unraid: &models.HostUnraidStorage{ArrayStarted: true, Disks: []models.HostUnraidDisk{disk}}, + }}}) + disks := registry.ListByType(ResourceTypePhysicalDisk) + if len(disks) != 1 { + t.Fatalf("disks = %d, want 1", len(disks)) + } + target := registry.MetricsTarget(disks[0].ID) + if target == nil || target.ResourceType != "disk" || target.ResourceID != got { + t.Fatalf("metrics target = %+v, want disk %q", target, got) + } + }) + } +} + // An agent disk's source ID carries its host, because a usable serial or WWN // names the drive but not the machine; its metrics key is the hardware ID // alone. Without hardware identity the two keep one host/device/topology key. diff --git a/internal/unifiedresources/host_disk_temperatures.go b/internal/unifiedresources/host_disk_temperatures.go new file mode 100644 index 0000000000..12c34d802a --- /dev/null +++ b/internal/unifiedresources/host_disk_temperatures.go @@ -0,0 +1,97 @@ +package unifiedresources + +import ( + "github.com/rcourtman/pulse-go-rewrite/internal/models" + "github.com/rcourtman/pulse-go-rewrite/pkg/diskinventory" + "github.com/rcourtman/pulse-go-rewrite/pkg/fsfilters" +) + +// HostDiskTemperatureReading is the temperature one physical disk of a host +// agent shows, taken from the disk resource the registry builds for it. +type HostDiskTemperatureReading struct { + // Device names the disk on the host: a SMART row's device label, or the + // kernel block device token of an Unraid inventory row. + Device string + // MetricID is the disk's history key (HostSMARTDiskMetricID or + // HostUnraidDiskMetricID). + MetricID string + Model string + DiskType string + Temperature int + Collection *diskinventory.CollectionStatus + // Standby reports a SMART row the agent marked as in standby. + Standby bool + // UnraidOnly reports a disk built from the host's Unraid inventory row + // alone, because no SMART row of the host carries its disk key. + UnraidOnly bool +} + +// Collected reports whether the reading is a temperature collected now: not +// a disk in standby, and not a reading its collection state marks as kept +// from earlier (diskinventory.TemperatureCollected). +func (r HostDiskTemperatureReading) Collected() bool { + return !r.Standby && diskinventory.TemperatureCollected(r.Temperature, r.Collection) +} + +// HostDiskTemperatureReadings returns the temperature each physical disk of a +// host agent shows, in report order: one reading per SMART row the registry +// ingests, then one per Unraid inventory row whose disk key no SMART row of +// the host carries. The registry merges a SMART row and an Unraid row with +// one key into one disk whatever device labels they use, as a controller +// member ("0 [megaraid,0]") and its Unraid device ("sda") do. A SMART row +// without a reading of its own shows the one its Unraid row reports +// (HostSMARTDiskTemperature), and a disk only the Unraid inventory lists, such +// as a --disk-exclude match, shows that row's (HostUnraidDiskTemperature). +// When smartctl and Unraid report different usable serials for one device the +// registry shows both disks, so both are listed. The readings come from the +// adapters the registry builds the disks with, so a consumer judging them +// judges the temperature, collection state and disk type each row's disk +// shows. +func HostDiskTemperatureReadings(host models.Host) []HostDiskTemperatureReading { + readings := make([]HostDiskTemperatureReading, 0, len(host.Sensors.SMART)) + smartKeys := make(map[string]struct{}, len(host.Sensors.SMART)) + for _, disk := range host.Sensors.SMART { + // The registry ingests no disk for a virtual block device. + if fsfilters.IsVirtualBlockDevice(disk.Device) { + continue + } + key := HostSMARTDiskMetricID(host, disk) + if key != "" { + smartKeys[key] = struct{}{} + } + resource, _ := resourceFromHostSMARTDisk(host, disk) + reading := hostDiskTemperatureReading(disk.Device, key, resource) + reading.Standby = disk.Standby + readings = append(readings, reading) + } + if host.Unraid == nil { + return readings + } + for _, disk := range host.Unraid.Disks { + // HostUnraidDiskMetricID is empty for a row without a device, which + // the registry ingests no disk for. + key := HostUnraidDiskMetricID(host, disk) + if key == "" { + continue + } + if _, ok := smartKeys[key]; ok { + continue + } + resource, _ := resourceFromHostUnraidPhysicalDisk(host, disk) + reading := hostDiskTemperatureReading(normalizePhysicalDiskDeviceToken(disk.Device), key, resource) + reading.UnraidOnly = true + readings = append(readings, reading) + } + return readings +} + +func hostDiskTemperatureReading(device, metricID string, resource Resource) HostDiskTemperatureReading { + reading := HostDiskTemperatureReading{Device: device, MetricID: metricID} + if disk := resource.PhysicalDisk; disk != nil { + reading.Model = disk.Model + reading.DiskType = disk.DiskType + reading.Temperature = disk.Temperature + reading.Collection = diskinventory.CloneStatus(disk.Collection) + } + return reading +} diff --git a/internal/unifiedresources/physical_disk_ids.go b/internal/unifiedresources/physical_disk_ids.go index 4ba7cefa98..9d210e9e8c 100644 --- a/internal/unifiedresources/physical_disk_ids.go +++ b/internal/unifiedresources/physical_disk_ids.go @@ -61,17 +61,37 @@ func hostSMARTDiskKey(host models.Host, disk models.HostDiskSMART, serial, wwn s // row's own, or, when the row reports none, the one the host's Unraid // inventory reports for the disk. smartctl reports no serial for a disk in // standby, and the Unraid row still names it. A row without a serial matches -// its Unraid row by device path alone, so it takes no serial from a path -// several of the host's rows share: controller members behind one kernel block -// device, which the Unraid row describes as a whole. +// its Unraid row by device path alone, so it takes the serial only where that +// row describes its disk (unraidDiskDescribesSMARTRow). func hostSMARTDiskSerial(host models.Host, disk models.HostDiskSMART, unraidDisk *models.HostUnraidDisk) string { serial := strings.TrimSpace(disk.Serial) - if serial != "" || unraidDisk == nil || hostSMARTDevicePathShared(host, disk) { + if serial != "" || !unraidDiskDescribesSMARTRow(host, disk, unraidDisk) { return serial } return strings.TrimSpace(unraidDisk.Serial) } +// unraidDiskDescribesSMARTRow reports whether the Unraid row matched to a SMART +// row (matchUnraidDisk) describes that row's disk. An Unraid row describes a +// kernel block device as a whole, so it describes the row when it carries the +// row's usable serial, or when it names the row's device path and the row is +// that whole device: not a controller member behind it, even the only one +// reported, nor one of several rows sharing the path. +func unraidDiskDescribesSMARTRow(host models.Host, disk models.HostDiskSMART, unraidDisk *models.HostUnraidDisk) bool { + if unraidDisk == nil { + return false + } + if serial := strings.TrimSpace(disk.Serial); diskinventory.IsUsableHardwareID(serial) && + strings.EqualFold(serial, strings.TrimSpace(unraidDisk.Serial)) { + return true + } + device := strings.ToLower(normalizePhysicalDiskDeviceToken(disk.Device)) + if device == "" || device != strings.ToLower(normalizePhysicalDiskDeviceToken(unraidDisk.Device)) { + return false + } + return !diskinventory.IsControllerMemberTarget(disk.Target) && !hostSMARTDevicePathShared(host, disk) +} + // hostSMARTDevicePathShared reports whether more than one of the host's SMART // rows, the row itself among them, names the row's device path. func hostSMARTDevicePathShared(host models.Host, disk models.HostDiskSMART) bool { @@ -130,6 +150,17 @@ func HostUnraidDiskSourceID(host models.Host, disk models.HostUnraidDisk) string return "" } +// HostUnraidDiskMetricID returns the history key of the disk resource built +// from a host's Unraid inventory row: the key its metrics target reads, the +// row's usable serial or else its source ID. It returns "" for a row the +// registry does not ingest as a disk, one without a device. +func HostUnraidDiskMetricID(host models.Host, disk models.HostUnraidDisk) string { + if strings.TrimSpace(disk.Device) == "" { + return "" + } + return PreferredPhysicalDiskMetricID(disk.Serial, "", HostUnraidDiskSourceID(host, disk)) +} + const hostPhysicalDiskSourceIDMarker = "/physical-disk:" func hostPhysicalDiskSourceID(hostID, hardwareID string) string { diff --git a/internal/unifiedresources/registry_test.go b/internal/unifiedresources/registry_test.go index 879a4efea6..271c2c4a36 100644 --- a/internal/unifiedresources/registry_test.go +++ b/internal/unifiedresources/registry_test.go @@ -2857,6 +2857,101 @@ func TestResourceRegistry_IngestSnapshotCreatesUnraidDisksWithoutSMART(t *testin } } +func TestHostDiskTemperatureReadingsMatchTheDisksTheRegistryShows(t *testing.T) { + now := time.Date(2026, 10, 7, 12, 0, 0, 0, time.UTC) + host := models.Host{ + ID: "host-tower", + Hostname: "tower", + Status: "online", + LastSeen: now, + Sensors: models.HostSensorSummary{ + SMART: []models.HostDiskSMART{ + // smartctl read this disk itself. + {Device: "sda", Model: "WDC A", Serial: "SER-A", Type: "sata", Temperature: 38}, + // smartctl returned no temperature; Unraid read one. + {Device: "sdb", Model: "WDC B", Serial: "SER-B", Type: "sata"}, + // smartctl and Unraid name this device with different serials. + {Device: "sde", Model: "Bridge E", Serial: "SER-E1", Type: "sata", Temperature: 41}, + // The registry ingests no virtual device. + {Device: "zram0", Temperature: 70}, + // A controller member Unraid lists under its block device. + {Device: "0 [megaraid,0]", Model: "Seagate F", Serial: "SER-F", Type: "sata", Controller: "0", Target: "megaraid,0", Temperature: 44}, + }, + }, + Unraid: &models.HostUnraidStorage{ + ArrayStarted: true, + ArrayState: "STARTED", + Disks: []models.HostUnraidDisk{ + {Name: "disk1", Device: "/dev/sda", Role: "data", Status: "online", Serial: "SER-A", Transport: "sata", Temperature: 36}, + {Name: "disk2", Device: "/dev/sdb", Role: "data", Status: "online", Serial: "SER-B", Transport: "sata", Temperature: 47}, + // No SMART row, as for a --disk-exclude match. + {Name: "disk3", Device: "/dev/nvme0n1", Role: "data", Status: "online", Model: "Samsung C", Serial: "SER-C", Transport: "nvme", Temperature: 52}, + {Name: "disk4", Device: "/dev/sdd", Role: "data", Status: "online", Model: "WDC D", Serial: "SER-D", Transport: "sata", Temperature: 30, SpunDown: true}, + {Name: "disk5", Device: "/dev/sde", Role: "data", Status: "online", Model: "WDC E", Serial: "SER-E2", Transport: "sata", Temperature: 43}, + {Name: "disk7", Device: "/dev/sdf", Role: "data", Status: "online", Model: "Seagate F", Serial: "SER-F", Transport: "sata", Temperature: 44}, + // An empty slot: the registry ingests no disk for it. + {Name: "disk6", Role: "data", Status: "missing"}, + }, + }, + } + + type want struct { + device string + diskType string + temperature int + collected bool + unraidOnly bool + } + wants := map[string]want{ + "SER-A": {device: "sda", diskType: "sata", temperature: 38, collected: true}, + "SER-B": {device: "sdb", diskType: "sata", temperature: 47, collected: true}, + "SER-E1": {device: "sde", diskType: "sata", temperature: 41, collected: true}, + "SER-F": {device: "0 [megaraid,0]", diskType: "sata", temperature: 44, collected: true}, + "SER-C": {device: "nvme0n1", diskType: "nvme", temperature: 52, collected: true, unraidOnly: true}, + "SER-D": {device: "sdd", diskType: "sata", temperature: 30, unraidOnly: true}, + "SER-E2": {device: "sde", diskType: "sata", temperature: 43, collected: true, unraidOnly: true}, + } + + readings := HostDiskTemperatureReadings(host) + if len(readings) != len(wants) { + t.Fatalf("expected %d readings, got %d: %+v", len(wants), len(readings), readings) + } + byKey := make(map[string]HostDiskTemperatureReading, len(readings)) + for _, reading := range readings { + w, ok := wants[reading.MetricID] + if !ok { + t.Fatalf("unexpected reading %+v", reading) + } + if reading.Device != w.device || reading.DiskType != w.diskType || reading.Temperature != w.temperature || + reading.Collected() != w.collected || reading.UnraidOnly != w.unraidOnly { + t.Fatalf("reading for %s = %+v (collected %v), want %+v", reading.MetricID, reading, reading.Collected(), w) + } + byKey[reading.MetricID] = reading + } + + // The readings are the ones the disks the registry builds show, one each. + rr := NewRegistry(nil) + rr.IngestSnapshot(models.StateSnapshot{Hosts: []models.Host{host}}) + disks := rr.ListByType(ResourceTypePhysicalDisk) + if len(disks) != len(wants) { + t.Fatalf("expected %d physical disks, got %d: %+v", len(wants), len(disks), disks) + } + for _, disk := range disks { + meta := disk.PhysicalDisk + if meta == nil { + t.Fatalf("physical disk without meta: %+v", disk) + } + reading, ok := byKey[meta.Serial] + if !ok { + t.Fatalf("registry disk %q has no reading", meta.Serial) + } + if meta.Temperature != reading.Temperature || meta.DiskType != reading.DiskType || + diskinventory.TemperatureCollected(meta.Temperature, meta.Collection) != diskinventory.TemperatureCollected(reading.Temperature, reading.Collection) { + t.Fatalf("registry disk %q shows %dC %q (collection %+v), reading %+v", meta.Serial, meta.Temperature, meta.DiskType, meta.Collection, reading) + } + } +} + func TestResourceRegistry_IngestSnapshotCreatesUnraidStorageResource(t *testing.T) { rr := NewRegistry(nil) now := time.Date(2026, 3, 7, 12, 0, 0, 0, time.UTC)