From 7b884bc98afc0c97c74d5dd3776043a5b746d8f1 Mon Sep 17 00:00:00 2001 From: Richard Courtman Date: Wed, 7 Oct 2026 17:25:26 +0100 Subject: [PATCH 1/2] Chart the Unraid temperature an agent disk shows A host agent's disk can show a temperature only its Unraid inventory reports. The adapter takes the Unraid row's reading when a SMART row has none, which happens when the agent's smartctl probe returns no temperature for a disk Unraid reads with its own per-disk SMART settings, and a disk the agent sends no SMART row for, such as a --disk-exclude match, is built from its Unraid row alone. The only agent disk temperature writer took the SMART row's own reading, and the unified sync skips agent disks as natively written, so these temperatures were shown live but never charted. writeHostSMARTMetrics now writes the reading the disk shows (HostSMARTDiskTemperature, the adapter's own rule), and a new pass writes each Unraid row's reading under HostUnraidDiskMetricID, the key its disk's metrics target reads, unless that key is one of the host's SMART rows' keys. An Unraid reading therefore 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 the sample. TemperatureCollected still keeps spun-down and expired-host readings out of history. Charting the fallback makes its row matching matter, so the fallback and the serial a row without one takes now need an Unraid row that describes the disk: one carrying the row's usable serial, or one naming the row's device path when the row is that whole block device, not a controller member (even the only one reported) or one of several rows on the path. matchUnraidDisk now tries usable serials across the whole inventory before device paths, and a placeholder serial matches nothing, so a row no longer takes another disk's Unraid row through a shared placeholder serial or an earlier path match. A lone member without a serial or WWN now charts under its own topology key, which its metrics target reads as written. --- .../v6/internal/subsystems/agent-lifecycle.md | 19 +++ .../v6/internal/subsystems/monitoring.md | 21 ++- .../internal/subsystems/unified-resources.md | 20 +++ .../issue1595_collection_trust_test.go | 161 ++++++++++++++++++ internal/monitoring/monitor_agents.go | 52 +++++- .../monitoring/monitor_host_agents_test.go | 65 +++++++ internal/unifiedresources/adapters.go | 87 +++++++--- .../canonical_ids_types_test.go | 44 +++++ .../unifiedresources/physical_disk_ids.go | 39 ++++- 9 files changed, 474 insertions(+), 34 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index faec9ec737..1e47117003 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -8806,6 +8806,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/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 796fc46ca0..1285052caa 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -4800,8 +4800,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. @@ -4812,6 +4814,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 84269fbcce..826a91d7c7 100644 --- a/docs/release-control/v6/internal/subsystems/unified-resources.md +++ b/docs/release-control/v6/internal/subsystems/unified-resources.md @@ -5432,6 +5432,26 @@ 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`. 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/monitoring/issue1595_collection_trust_test.go b/internal/monitoring/issue1595_collection_trust_test.go index 7486e493af..0da88b9821 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 baef427792..600be586b6 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 c8ed1bedb5..edd8be0863 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/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 { From ac631eadb87f0783953b45cc303a887c851504f3 Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Wed, 7 Oct 2026 17:32:08 +0100 Subject: [PATCH 2/2] Alert on the Unraid temperature an agent disk shows A host agent's disk can show a temperature only its Unraid inventory reports: a SMART row whose smartctl probe returned none takes its Unraid row's reading, and a disk only the inventory lists is built from that row alone. The Physical Disks Temp cell, the Running Hot verdict and Patrol judge that reading against the alert disk temperature policy, but CheckHost read only the SMART row's own field, so a disk could read as at its alert threshold while no alert fired. unifiedresources.HostDiskTemperatureReadings now lists the reading each of a host's disks shows, built with the registry's own adapters: one per SMART row, then one per Unraid row whose disk key no SMART row carries (the history writer's rule). CheckHost judges those. SMART rows keep their alert resource IDs, an inventory-only disk uses its block device token, and per-type thresholds follow the disk type the disk shows. Only a reading collected now fires or resolves, so spun-down disks hold their alerts. When conflicting serials put two disks on one device, its alert judges the reading that stands highest against its own type's threshold. An inventory-only disk the agent's --disk-exclude patterns match raises nothing, and an alert it raised before the exclusion resolves on the next report, as the documented exclusion from monitoring already suppresses linked Proxmox health and wear alerts. Absence cleanup still runs only on a non-empty SMART list. Unraid's own 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. --- .../v6/internal/subsystems/alerts.md | 55 ++++ .../internal/subsystems/unified-resources.md | 17 + internal/alerts/host.go | 105 ++++-- .../alerts/host_disk_temperature_readings.go | 72 +++++ internal/alerts/host_unraid_lifecycle_test.go | 298 ++++++++++++++++++ .../host_disk_temperatures.go | 97 ++++++ internal/unifiedresources/registry_test.go | 95 ++++++ 7 files changed, 705 insertions(+), 34 deletions(-) create mode 100644 internal/alerts/host_disk_temperature_readings.go create mode 100644 internal/unifiedresources/host_disk_temperatures.go diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index 9d7946179e..a07724fb6b 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -1867,6 +1867,61 @@ well as its alerts. `internal/alerts/host_unraid_lifecycle_test.go` pins the clear, the holds, the restarted count and the pending-run and count cleanup. +### 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/unified-resources.md b/docs/release-control/v6/internal/subsystems/unified-resources.md index 826a91d7c7..ecd57dc3dc 100644 --- a/docs/release-control/v6/internal/subsystems/unified-resources.md +++ b/docs/release-control/v6/internal/subsystems/unified-resources.md @@ -5452,6 +5452,23 @@ 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 65ae1cbd40..00d3b3ed9d 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" ) @@ -378,43 +379,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/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/registry_test.go b/internal/unifiedresources/registry_test.go index 53214cd891..bc0f2e3b1b 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)