Stop serving retained disk temperatures as current readings

Normalization may keep a disk temperature it did not collect (a disk in
standby, a host agent past its reporting lease) as long as its collection
state says so. Two consumers still presented that value as current.

The metrics-history handler answered an empty disk range with the disk's
temperature as a "live" point at now, regardless of its state. Since the
history writers stopped recording retained temperatures, a short-range
chart for a silent agent's disk or a sleeping disk showed the retained
value as a fresh reading. Mock mode also seeded synthetic series from it
and padded sparse stored series out to now. All of these now require a
temperature collected now (diskinventory.TemperatureCollected, the
shared form of the rule the history writers apply).

Unraid array-inventory rows carry no per-field provenance, so a
temperature taken from one looked collected after the agent went silent.
The adapter now derives its state from the host lease that
ExpireHostTelemetry already records, and from the disk's spin state:
unavailable once the host is offline or the disk is spun down. A SMART
row that borrows the inventory reading takes that state, including
available. The native row records only the withdrawing states, because
the registry merge picks temperature and state separately and an
available claim from it would mask the SMART row's own state. A
withdrawn inventory temperature no longer counts as heat in disk risk.
This commit is contained in:
rcourtman
2026-10-06 20:09:04 +01:00
parent 9aaac4d3e5
commit 95b29286dd
14 changed files with 483 additions and 10 deletions
@@ -734,6 +734,11 @@ agent's SMART temperature and I/O counters as last-known values but marks them
report replaces them. Expiry is compare-and-set on the report time the offline
sweep judged stale, so a report admitted between that judgement and the expiry
is never expired.
Unraid array-inventory temperatures follow the same lease: while the host is
offline the unified-resources adapter reports them `unavailable` with the same
reason (`models.HostAgentStoppedReportingReason`) and leaves them out of the
disk's risk, and the metrics-history API does not return any such retained
disk temperature as a live point.
An enabled availability target assigned to a host agent creates an
agent-lifecycle lease for that exact target/agent pairing. First assignment
@@ -6308,6 +6308,17 @@ windows. `/api/metrics-store/history` must accept `resourceType=disk`, keep
`MetricsTarget.ResourceID` that unified resources already expose, instead of
leaving storage drawers or other callers to fork a disk-local history route or
invent an alternate disk identity.
When a disk range has no samples, the `smart_temp` live fallback (single-metric
and all-metrics) is served only for a temperature that is collected
(`diskinventory.TemperatureCollected`). A temperature that normalization kept
under a non-available collection state, such as a disk in standby or a host
agent past its reporting lease, is omitted, so the range stays empty instead of
returning that value at `now` with `source: "live"`. Mock mode derives a
synthetic disk temperature series from the disk's reading, or pads a sparse
stored one out to `now`, only from a temperature collected now; without one the
stored samples are returned as they are. An empty mock range can still fall
through to the monitor's generic demo chart series for that disk ID, which is
not derived from the disk's reading.
That same metrics-history contract also owns Kubernetes pod identity
normalization. `/api/metrics-store/history` must accept legacy bare pod IDs
such as `cluster-1:pod:pod-1`, canonicalize them onto the unified pod metrics
@@ -3778,6 +3778,12 @@ predates collection state), so a retained reading has to keep its non-available
state to stay out of history; a path that relabels a carried reading as
available, such as a node-temperature carry stamped `proxmox_node_smart`, is
not covered by this rule.
`diskinventory.TemperatureCollected` is the shared form of that test for
consumers outside the history writers: the metrics-history API serves a disk
`smart_temp` live point, and mock mode derives a synthetic disk temperature
series from the disk's reading or pads a stored one, only from a collected
temperature, so a retained standby or silent-agent reading never reappears as a
point at the current time.
Unified-resource physical-disk round trips must retain named
`StorageGroup` membership rather than degrading it to the generic `Used`
filesystem label.
@@ -3065,3 +3065,11 @@ paused-delivery banner strings in English, German and Spanish. This is alert
presentation copy only: no API token name, scope preset, privacy disclosure,
telemetry statement or security term changes, and no request, storage or
authorization path moves.
### Retained disk temperatures are not served as live history
`/api/metrics-store/history` now omits a disk's `smart_temp` live point, and
the mock-mode synthetic series derived from that reading, when the temperature
was not collected by the current observation. This only withholds a value the
caller was already authorized to read; the route, its authentication and scope
checks, tenant resolution and response shape are unchanged.
@@ -6739,3 +6739,12 @@ blank header is removed on the next save. Export and import pass through the
same normalization. No new file, retention window, backup, migration or
recovery authority is introduced; `TestWebhookMaskedValuesResolvePerKey` in
`internal/api/alerting/notifications_test.go` covers the write path.
### Retained disk temperatures open no storage or recovery path
The metrics-history handler in `internal/api/router.go` serves a disk
temperature live point only when `diskinventory.TemperatureCollected` holds,
and the Unraid physical-disk adapter marks an inventory temperature kept past
the host agent's lease as no longer collected. Both change presentation of
values Pulse already holds: no metrics write, retention window, backup,
migration or recovery authority is added or moved.
@@ -3576,6 +3576,20 @@ state the Proxmox row still carries from the same source
past its reporting lease must not stay "collected" through the Proxmox row's
copy of the agent's own state, including when its host is down and no disk
poll refreshes that copy.
Unraid array-inventory rows carry no per-field provenance, so the adapter
derives the state of a temperature taken from one: `unavailable` from `unraid`
with "host agent stopped reporting" once `State.ExpireHostTelemetry` has marked
the host offline, or with "disk is reported spun down" for a spun-down disk,
and otherwise `available`. The value is kept as last-known context; it is
collected again only when a reporting host sends a positive reading for a disk
that is no longer spun down. A SMART row that falls back to
the inventory reading because it has none of its own takes that state with it.
The Unraid-native disk row records only the withdrawing states, never
`available`: the registry chooses a merged disk's temperature and its state
separately, and an `available` claim from the provenance-less row would
outrank the SMART row's explicit state for the same disk when the SMART
reading is the one shown. A withdrawn inventory temperature also stops
counting as heat in the disk's risk assessment.
Cross-source correlation compares normalized serial and WWN values across
fields without truncation, allowing a PVE bare-hex array-volume serial to join
+124
View File
@@ -65,6 +65,7 @@ import (
"github.com/rcourtman/pulse-go-rewrite/pkg/audit"
authpkg "github.com/rcourtman/pulse-go-rewrite/pkg/auth"
"github.com/rcourtman/pulse-go-rewrite/pkg/cloudauth"
"github.com/rcourtman/pulse-go-rewrite/pkg/diskinventory"
"github.com/rcourtman/pulse-go-rewrite/pkg/edition"
pkglicensing "github.com/rcourtman/pulse-go-rewrite/pkg/licensing"
licensetestsupport "github.com/rcourtman/pulse-go-rewrite/pkg/licensing/testsupport"
@@ -12546,6 +12547,129 @@ func TestContract_MetricsHistoryAgentTemperatureLiveFallback(t *testing.T) {
}
}
// Normalization may keep a disk temperature it did not collect this time: a
// host agent past its reporting lease, or a disk in standby. The history
// endpoint must not return that retained value as a live point at "now", and
// mock mode must not grow a synthetic series that ends on it. A collected
// temperature still answers an empty range with a live point.
func TestContract_MetricsHistoryDiskLivePointOnlyForCollectedTemperature(t *testing.T) {
state := models.NewState()
lastReport := time.Now().Add(-time.Hour)
state.UpsertHost(models.Host{
ID: "agent-silent", Hostname: "silent", Status: "online", IntervalSeconds: 30, LastSeen: lastReport,
Sensors: models.HostSensorSummary{SMART: []models.HostDiskSMART{{
Device: "/dev/sdb", Serial: "WD-SILENT-LIVE", Type: "sata", Health: "PASSED", Temperature: 41,
Collection: &diskinventory.CollectionStatus{Temperature: diskinventory.Available("smartctl")},
}}},
})
if _, changed := state.ExpireHostTelemetry("agent-silent", lastReport); !changed {
t.Fatal("the silent agent's lease expiry did not mark its SMART readings")
}
registry := unifiedresources.NewRegistry(nil)
registry.IngestSnapshot(state.GetSnapshot())
var resources []unifiedresources.Resource
for _, resource := range unifiedresources.NewMonitorAdapter(registry).GetAll() {
if resource.Type == unifiedresources.ResourceTypePhysicalDisk {
resources = append(resources, resource)
}
}
if len(resources) != 1 || resources[0].PhysicalDisk.Temperature != 41 {
t.Fatalf("silent agent disk resources = %+v, want one disk retaining 41", resources)
}
staticDisk := func(serial string, temperature int, status diskinventory.FieldStatus) unifiedresources.Resource {
return unifiedresources.Resource{
ID: "physical-disk:" + serial,
Type: unifiedresources.ResourceTypePhysicalDisk,
PhysicalDisk: &unifiedresources.PhysicalDiskMeta{
Serial: serial,
Temperature: temperature,
Collection: &diskinventory.CollectionStatus{Temperature: status},
},
MetricsTarget: &unifiedresources.MetricsTarget{ResourceType: "disk", ResourceID: serial},
}
}
resources = append(resources,
staticDisk("WD-STANDBY-LIVE", 38, diskinventory.Unavailable("smartctl", "disk is in standby")),
staticDisk("WD-AWAKE-LIVE", 36, diskinventory.Available("smartctl")),
staticDisk("WD-ASLEEP-LIVE", 0, diskinventory.Unavailable("smartctl", "disk is in standby")),
)
store, err := metrics.NewStore(metrics.DefaultConfig(t.TempDir()))
if err != nil {
t.Fatal(err)
}
t.Cleanup(func() { _ = store.Close() })
monitor := &monitoring.Monitor{}
monitor.SetResourceStore(staticPhysicalDiskResourceStore{resources: resources})
setUnexportedField(t, monitor, "metricsHistory", monitoring.NewMetricsHistory(10, time.Hour))
setUnexportedField(t, monitor, "metricsStore", store)
router := &Router{monitor: monitor}
history := func(query string) (metricsHistoryResponse, map[string]json.RawMessage) {
t.Helper()
rec := httptest.NewRecorder()
router.handleMetricsHistory(rec, httptest.NewRequest(http.MethodGet, "/api/metrics-store/history?resourceType=disk&"+query, nil))
if rec.Code != http.StatusOK {
t.Fatalf("%s: status = %d, body=%s", query, rec.Code, rec.Body.String())
}
var single metricsHistoryResponse
var all struct {
Metrics map[string]json.RawMessage `json:"metrics"`
}
if err := json.Unmarshal(rec.Body.Bytes(), &single); err != nil {
t.Fatalf("%s: decode: %v", query, err)
}
if err := json.Unmarshal(rec.Body.Bytes(), &all); err != nil {
t.Fatalf("%s: decode metrics: %v", query, err)
}
return single, all.Metrics
}
for _, serial := range []string{"WD-SILENT-LIVE", "WD-STANDBY-LIVE"} {
resp, _ := history("resourceId=" + serial + "&metric=smart_temp&range=5m")
if resp.Source == "live" || len(resp.Points) != 0 {
t.Fatalf("%s: retained temperature served as current: source=%q points=%+v", serial, resp.Source, resp.Points)
}
if _, all := history("resourceId=" + serial + "&range=5m"); all["smart_temp"] != nil {
t.Fatalf("%s: retained temperature served in the all-metrics response: %s", serial, all["smart_temp"])
}
}
resp, _ := history("resourceId=WD-AWAKE-LIVE&metric=smart_temp&range=5m")
if resp.Source != "live" || len(resp.Points) != 1 || resp.Points[0].Value != 36 {
t.Fatalf("collected temperature not served as a live point: source=%q points=%+v", resp.Source, resp.Points)
}
// Mock mode pads a sparse stored series out to now from the disk's current
// temperature. A retained temperature is not current, so the stored
// samples stay as they are.
// Without a current reading (retained, absent, or no such disk) there is
// nothing to pad with, so the last stored sample is not extended either.
setMockModeForTest(t, true)
now := time.Now()
for _, serial := range []string{"WD-STANDBY-LIVE", "WD-ASLEEP-LIVE", "WD-REMOVED-LIVE"} {
store.Write("disk", serial, "smart_temp", 38, now.Add(-50*time.Minute))
store.Write("disk", serial, "smart_temp", 38, now.Add(-40*time.Minute))
}
store.Flush()
for _, serial := range []string{"WD-STANDBY-LIVE", "WD-ASLEEP-LIVE", "WD-REMOVED-LIVE"} {
resp, _ = history("resourceId=" + serial + "&metric=smart_temp&range=1h")
if resp.Source != "store" || len(resp.Points) != 2 {
t.Fatalf("%s: mock mode padded stored temperature history to now without a current reading: source=%q points=%d", serial, resp.Source, len(resp.Points))
}
}
// With nothing stored, mock mode must not seed its synthetic series from a
// retained temperature either. (The monitor's generic demo series for the
// disk ID is not derived from the disk's reading.)
resp, _ = history("resourceId=WD-SILENT-LIVE&metric=smart_temp&range=1h")
if resp.Source == "mock_synthetic" {
t.Fatalf("mock mode synthesized history from a silent agent's retained temperature: points=%+v", resp.Points)
}
resp, _ = history("resourceId=WD-AWAKE-LIVE&metric=smart_temp&range=1h")
if resp.Source != "mock_synthetic" || len(resp.Points) < 2 || resp.Points[len(resp.Points)-1].Value != 36 {
t.Fatalf("mock mode stopped synthesizing a collected temperature: source=%q points=%+v", resp.Source, resp.Points)
}
}
func TestContract_MetricsHistoryCanonicalizesLegacyKubernetesPodIDs(t *testing.T) {
mh := monitoring.NewMetricsHistory(1000, time.Hour)
now := time.Now().UTC().Truncate(time.Second)
+11 -3
View File
@@ -6723,7 +6723,9 @@ func (r *Router) handleMetricsHistory(w http.ResponseWriter, req *http.Request)
return points
}
pd := disk.PhysicalDisk
if pd.Temperature > 0 {
// A retained temperature (standby, a silent host agent) is not a
// reading taken now.
if diskinventory.TemperatureCollected(pd.Temperature, pd.Collection) {
points["smart_temp"] = monitoring.MetricPoint{Timestamp: now, Value: float64(pd.Temperature)}
}
if pd.SMART != nil {
@@ -6802,7 +6804,8 @@ func (r *Router) handleMetricsHistory(w http.ResponseWriter, req *http.Request)
if mock.IsMockEnabled() && runtimeResourceType == "disk" {
current := 0.0
if disk := findDisk(resourceID); disk != nil && disk.PhysicalDisk != nil && metricType == "smart_temp" {
if disk := findDisk(resourceID); disk != nil && disk.PhysicalDisk != nil && metricType == "smart_temp" &&
diskinventory.TemperatureCollected(disk.PhysicalDisk.Temperature, disk.PhysicalDisk.Collection) {
current = float64(disk.PhysicalDisk.Temperature)
}
if current > 0 || metricType == "disk" || metricType == "diskread" || metricType == "diskwrite" {
@@ -7189,7 +7192,12 @@ func (r *Router) handleMetricsHistory(w http.ResponseWriter, req *http.Request)
if len(points) > 0 && len(points) < targetPoints {
current := points[len(points)-1].Value
if metricType == "smart_temp" {
if disk := findDisk(resourceID); disk != nil && disk.PhysicalDisk != nil && disk.PhysicalDisk.Temperature > 0 {
// Pad a temperature series to now only with a reading
// collected now. Without one (retained, absent, or no
// such disk) the stored samples stay as they are.
current = 0
if disk := findDisk(resourceID); disk != nil && disk.PhysicalDisk != nil &&
diskinventory.TemperatureCollected(disk.PhysicalDisk.Temperature, disk.PhysicalDisk.Collection) {
current = float64(disk.PhysicalDisk.Temperature)
}
}
+4 -2
View File
@@ -5695,7 +5695,9 @@ func (s *State) ExpireHostTelemetry(hostID string, lastSeen time.Time) (Host, bo
return Host{}, false
}
const hostAgentStoppedReportingReason = "host agent stopped reporting"
// HostAgentStoppedReportingReason is the collection-state reason recorded on
// readings a host agent supplied once its reporting lease has expired.
const HostAgentStoppedReportingReason = "host agent stopped reporting"
// expireHostSMARTReadings marks the temperature and I/O counters of one SMART
// row as no longer collected, keeping their values as last-known evidence.
@@ -5725,7 +5727,7 @@ func expireCollectedReading(status *diskinventory.FieldStatus, hasValue bool) bo
default:
return false
}
*status = diskinventory.Unavailable(status.Source, hostAgentStoppedReportingReason)
*status = diskinventory.Unavailable(status.Source, HostAgentStoppedReportingReason)
return true
}
@@ -633,3 +633,88 @@ func TestStandbyDiskRetainedTemperatureIsNotRecordedAsHistory(t *testing.T) {
}
}
}
// An Unraid agent reports array disk temperatures in its Unraid inventory,
// which carries no per-field provenance, and a SMART row without a temperature
// falls back to that inventory reading. Once the agent's reporting lease
// expires both rows stay as last-known context, so the canonical disks must say
// the retained temperature is no longer collected. The next report collects
// it again.
func TestSilentUnraidAgentDiskTemperatureIsRetainedButNotCollected(t *testing.T) {
now := time.Now()
state := models.NewState()
report := func(seen time.Time, temperature int) {
state.UpsertHost(models.Host{
ID: "agent-tower", Hostname: "tower", Status: "online", IntervalSeconds: 30, LastSeen: seen,
Unraid: &models.HostUnraidStorage{ArrayStarted: true, Disks: []models.HostUnraidDisk{
{Name: "disk1", Device: "sdc", Role: "data", Status: "online", Serial: "UNRAID-ONLY1", Temperature: temperature},
{Name: "disk2", Device: "sdd", Role: "data", Status: "online", Serial: "UNRAID-SMART2", Temperature: temperature + 2},
{Name: "disk3", Device: "sde", Role: "data", Status: "online", Serial: "UNRAID-SAS3", Temperature: temperature + 4},
}},
// The SMART rows for disk2 and disk3 carry no temperature of their
// own; disk3's smartctl could not read one at all.
Sensors: models.HostSensorSummary{SMART: []models.HostDiskSMART{
{Device: "/dev/sdd", Serial: "UNRAID-SMART2", Type: "sata", Health: "PASSED"},
{Device: "/dev/sde", Serial: "UNRAID-SAS3", Type: "sas", Health: "PASSED", Collection: &diskinventory.CollectionStatus{
Temperature: diskinventory.Unsupported("smartctl", "no temperature attribute"),
}},
}},
})
}
adapter := unifiedresources.NewMonitorAdapter(unifiedresources.NewRegistry(nil))
m := &Monitor{state: state, resourceStore: adapter, startTime: now.Add(-time.Hour)}
temperatures := func() map[string]diskinventory.FieldStatus {
t.Helper()
adapter.PopulateFromSnapshot(state.GetSnapshot())
got := make(map[string]diskinventory.FieldStatus)
for _, view := range adapter.PhysicalDisks() {
status := diskinventory.FieldStatus{}
if collection := view.Collection(); collection != nil {
status = collection.Temperature
}
if _, seen := got[view.Serial()]; seen {
t.Fatalf("disk %s surfaced twice", view.Serial())
}
got[view.Serial()] = status
wantTemperature := map[string]int{"UNRAID-ONLY1": 37, "UNRAID-SMART2": 39, "UNRAID-SAS3": 41}[view.Serial()]
if view.Temperature() != wantTemperature {
t.Fatalf("disk %s temperature = %d, want %d", view.Serial(), view.Temperature(), wantTemperature)
}
}
if len(got) != 3 {
t.Fatalf("canonical disks = %v, want the three Unraid disks", got)
}
return got
}
// A reporting agent's readings are collected. The inventory-only disk
// keeps no state of its own (it counts as collected); the SMART rows that
// borrow the inventory reading take its provenance.
collected := map[string]diskinventory.FieldStatus{
"UNRAID-ONLY1": {},
"UNRAID-SMART2": diskinventory.Available("unraid"),
"UNRAID-SAS3": diskinventory.Available("unraid"),
}
report(now, 37)
for serial, status := range temperatures() {
if status != collected[serial] {
t.Fatalf("reporting agent: disk %s temperature status = %+v, want %+v", serial, status, collected[serial])
}
}
state.TouchHost("agent-tower", now.Add(-hostAgentHealthWindow(30)-time.Minute))
m.evaluateHostAgents(now)
for serial, status := range temperatures() {
if status != diskinventory.Unavailable("unraid", "host agent stopped reporting") {
t.Fatalf("silent agent: disk %s retained temperature presented as collected: %+v", serial, status)
}
}
report(time.Now(), 37)
m.evaluateHostAgents(time.Now())
for serial, status := range temperatures() {
if status != collected[serial] {
t.Fatalf("resumed agent: disk %s temperature status = %+v, want collected again (%+v)", serial, status, collected[serial])
}
}
}
@@ -7,6 +7,7 @@ import (
"github.com/rcourtman/pulse-go-rewrite/internal/models"
"github.com/rcourtman/pulse-go-rewrite/internal/storagehealth"
"github.com/rcourtman/pulse-go-rewrite/pkg/diskinventory"
)
func strPointer(v string) *string {
@@ -1045,3 +1046,119 @@ func TestMetricPercent(t *testing.T) {
})
}
}
// Unraid inventory temperatures have no agent provenance, so the adapter
// derives their state from the host's reporting lease and the disk's spin
// state. A native row records only a state that withdraws its reading, a SMART
// row that borrows the inventory reading takes the inventory's state, and a
// SMART row with its own reading keeps the agent's state. A withdrawn reading
// never counts as heat in the disk's risk.
func TestUnraidDiskTemperatureFollowsTheHostLease(t *testing.T) {
smartWithout := models.HostDiskSMART{Device: "/dev/sdc", Serial: "UNRAID-1", Collection: &diskinventory.CollectionStatus{
Serial: diskinventory.Available("smartctl"),
Temperature: diskinventory.Unsupported("smartctl", "no temperature attribute"),
}}
smartWith := models.HostDiskSMART{Device: "/dev/sdc", Serial: "UNRAID-1", Temperature: 40, Collection: &diskinventory.CollectionStatus{
Temperature: diskinventory.Unavailable("smartctl", models.HostAgentStoppedReportingReason),
}}
stopped := diskinventory.Unavailable("unraid", models.HostAgentStoppedReportingReason)
spunDown := diskinventory.Unavailable("unraid", "disk is reported spun down")
hotRisk := func(risk *PhysicalDiskRisk) bool {
if risk == nil {
return false
}
for _, reason := range risk.Reasons {
if reason.Code == "temperature_high" {
return true
}
}
return false
}
for _, tc := range []struct {
name string
status string
spunDown bool
native diskinventory.FieldStatus
borrowed diskinventory.FieldStatus
}{
{"reporting", "online", false, diskinventory.FieldStatus{}, diskinventory.Available("unraid")},
{"silent agent", "offline", false, stopped, stopped},
{"spun down", "online", true, spunDown, spunDown},
} {
inventory := models.HostUnraidDisk{Name: "disk1", Device: "sdc", Role: "data", Status: "online", Serial: "UNRAID-1",
Temperature: 72, SpunDown: tc.spunDown}
host := models.Host{ID: "agent-tower", Hostname: "tower", Status: tc.status,
Unraid: &models.HostUnraidStorage{Disks: []models.HostUnraidDisk{inventory}}}
collected := tc.borrowed.State == diskinventory.FieldAvailable
native, _ := resourceFromHostUnraidPhysicalDisk(host, inventory)
got := native.PhysicalDisk
var nativeStatus diskinventory.FieldStatus
if got.Collection != nil {
nativeStatus = got.Collection.Temperature
}
if got.Temperature != 72 || nativeStatus != tc.native {
t.Fatalf("%s: Unraid row temperature=%d state=%+v, want 72 with %+v", tc.name, got.Temperature, nativeStatus, tc.native)
}
if hotRisk(got.Risk) != collected {
t.Fatalf("%s: Unraid row temperature risk = %v, want %v: %+v", tc.name, hotRisk(got.Risk), collected, got.Risk)
}
borrowed, _ := resourceFromHostSMARTDisk(host, smartWithout)
got = borrowed.PhysicalDisk
if got.Temperature != 72 || got.Collection.Temperature != tc.borrowed ||
got.Collection.Serial != diskinventory.Available("smartctl") {
t.Fatalf("%s: SMART row borrowing the inventory reading: temperature=%d collection=%+v", tc.name, got.Temperature, got.Collection)
}
if hotRisk(got.Risk) != collected {
t.Fatalf("%s: borrowed temperature risk = %v, want %v: %+v", tc.name, hotRisk(got.Risk), collected, got.Risk)
}
if smartWithout.Collection.Temperature.State != diskinventory.FieldUnsupported {
t.Fatal("the adapter mutated the SMART row's own collection state")
}
own, _ := resourceFromHostSMARTDisk(host, smartWith)
if got := own.PhysicalDisk; got.Temperature != 40 || got.Collection.Temperature != smartWith.Collection.Temperature {
t.Fatalf("%s: SMART row with its own reading lost the agent's state: temperature=%d collection=%+v", tc.name, got.Temperature, got.Collection)
}
}
cold := models.HostUnraidDisk{Name: "disk2", Device: "sdd", Serial: "UNRAID-2", SpunDown: true}
native, _ := resourceFromHostUnraidPhysicalDisk(models.Host{ID: "agent-tower", Status: "online"}, cold)
if native.PhysicalDisk.Collection != nil {
t.Fatalf("an Unraid row without a temperature claimed temperature state: %+v", native.PhysicalDisk.Collection)
}
}
// The registry merges an Unraid inventory row and the agent's SMART row for the
// same disk, choosing the temperature and the collection state separately. A
// reporting host's inventory row must not lend the merged disk an "available"
// state when the reading it shows is the SMART row's, and that row says it was
// not collected.
func TestUnraidInventoryRowDoesNotMaskTheSMARTRowState(t *testing.T) {
state := models.NewState()
state.UpsertHost(models.Host{
ID: "agent-tower", Hostname: "tower", Status: "online",
Unraid: &models.HostUnraidStorage{Disks: []models.HostUnraidDisk{
{Name: "disk1", Device: "sdc", Role: "data", Status: "online", Serial: "UNRAID-MERGE1", Temperature: 37},
}},
Sensors: models.HostSensorSummary{SMART: []models.HostDiskSMART{{
Device: "/dev/sdc", Serial: "UNRAID-MERGE1", Temperature: 40,
Collection: &diskinventory.CollectionStatus{Temperature: diskinventory.Unavailable("smartctl", "disk is in standby")},
}}},
})
registry := NewRegistry(nil)
registry.IngestSnapshot(state.GetSnapshot())
var disks []*PhysicalDiskMeta
for _, resource := range registry.List() {
if resource.Type == ResourceTypePhysicalDisk {
disks = append(disks, resource.PhysicalDisk)
}
}
if len(disks) != 1 {
t.Fatalf("physical disks = %d, want the inventory and SMART rows merged into one", len(disks))
}
if got := disks[0]; got.Temperature != 40 || diskinventory.TemperatureCollected(got.Temperature, got.Collection) {
t.Fatalf("merged disk presents the SMART row's uncollected reading as collected: temperature=%d collection=%+v", got.Temperature, got.Collection)
}
}
+50 -5
View File
@@ -804,8 +804,16 @@ func resourceFromHostUnraidPhysicalDisk(host models.Host, disk models.HostUnraid
model := strings.TrimSpace(disk.Model)
name := firstNonEmpty(model, disk.Name, disk.Device, host.Hostname)
health := unraidPhysicalDiskHealth(disk)
assessment := assessUnraidPhysicalDisk(disk)
assessment := assessUnraidPhysicalDisk(host, disk)
sizeBytes := unraidDiskSizeBytes(host, disk)
// The row records only a state that withdraws its reading. A positive
// claim from this provenance-less row would outrank another source's
// explicit state for the same disk when the registry merges them, even
// where that source's temperature is the one shown.
var collection *diskinventory.CollectionStatus
if status := unraidDiskTemperatureStatus(host, disk); status.State == diskinventory.FieldUnavailable {
collection = &diskinventory.CollectionStatus{Temperature: status}
}
resource := Resource{
Type: ResourceTypePhysicalDisk,
Name: name,
@@ -829,6 +837,7 @@ func resourceFromHostUnraidPhysicalDisk(host models.Host, disk models.HostUnraid
ReadCount: disk.ReadCount,
WriteCount: disk.WriteCount,
ErrorCount: disk.ErrorCount,
Collection: collection,
Risk: physicalDiskRiskFromAssessment(assessment),
},
}
@@ -841,6 +850,29 @@ func resourceFromHostUnraidPhysicalDisk(host models.Host, disk models.HostUnraid
return resource, identity
}
// unraidTemperatureSource is the provenance of a temperature read from an
// Unraid agent's array inventory; the agent's standby SMART rows use it too.
const unraidTemperatureSource = "unraid"
// unraidDiskTemperatureStatus returns the collection state of the temperature
// an Unraid inventory row carries. The inventory has no per-field provenance:
// each accepted report replaces it, and once the host's reporting lease
// expires State.ExpireHostTelemetry keeps it as last-known context and marks
// the host offline. A temperature kept past that point, or one a spun-down
// disk still carries, is not collected.
func unraidDiskTemperatureStatus(host models.Host, disk models.HostUnraidDisk) diskinventory.FieldStatus {
switch {
case disk.Temperature <= 0:
return diskinventory.FieldStatus{}
case strings.EqualFold(strings.TrimSpace(host.Status), "offline"):
return diskinventory.Unavailable(unraidTemperatureSource, models.HostAgentStoppedReportingReason)
case disk.SpunDown:
return diskinventory.Unavailable(unraidTemperatureSource, "disk is reported spun down")
default:
return diskinventory.Available(unraidTemperatureSource)
}
}
func resourceFromHostSMARTDisk(host models.Host, disk models.HostDiskSMART) (Resource, ResourceIdentity) {
name := strings.TrimSpace(disk.Model)
if name == "" {
@@ -880,6 +912,7 @@ func resourceFromHostSMARTDisk(host models.Host, disk models.HostDiskSMART) (Res
serial := strings.TrimSpace(disk.Serial)
diskType := strings.TrimSpace(disk.Type)
temperature := disk.Temperature
collection := diskinventory.CloneStatus(disk.Collection)
health := strings.TrimSpace(disk.Health)
if unraidDisk != nil {
if model == "" {
@@ -896,6 +929,13 @@ func resourceFromHostSMARTDisk(host models.Host, disk models.HostDiskSMART) (Res
}
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)
@@ -903,7 +943,7 @@ func resourceFromHostSMARTDisk(host models.Host, disk models.HostDiskSMART) (Res
}
assessment := storagehealth.AssessHostSMARTDisk(disk)
if unraidDisk != nil {
assessment = storagehealth.SummarizeAssessments(assessment, assessUnraidPhysicalDisk(*unraidDisk))
assessment = storagehealth.SummarizeAssessments(assessment, assessUnraidPhysicalDisk(host, *unraidDisk))
}
wearout := physicalDiskWearoutFromSMARTAttributes(disk.Attributes)
@@ -939,7 +979,7 @@ func resourceFromHostSMARTDisk(host models.Host, disk models.HostDiskSMART) (Res
WriteCount: unraidDiskCounter(unraidDisk, "write"),
ErrorCount: unraidDiskCounter(unraidDisk, "error"),
IO: physicalDiskIOToMeta(disk.IO),
Collection: diskinventory.CloneStatus(disk.Collection),
Collection: collection,
SMART: convertSMARTAttributes(disk.Attributes),
Risk: physicalDiskRiskFromAssessment(assessment),
},
@@ -1288,11 +1328,16 @@ func unraidPhysicalDiskHealth(disk models.HostUnraidDisk) string {
}
}
func assessUnraidPhysicalDisk(disk models.HostUnraidDisk) storagehealth.Assessment {
func assessUnraidPhysicalDisk(host models.Host, disk models.HostUnraidDisk) storagehealth.Assessment {
// A retained inventory temperature is not current evidence of heat.
temperature := 0
if status := unraidDiskTemperatureStatus(host, disk); status.State == diskinventory.FieldAvailable {
temperature = disk.Temperature
}
assessment := storagehealth.AssessSample(storagehealth.Sample{
Model: disk.Model,
Health: unraidPhysicalDiskHealth(disk),
Temperature: disk.Temperature,
Temperature: temperature,
Wearout: -1,
})
addReason := func(code string, severity storagehealth.RiskLevel, summary string) {
+17
View File
@@ -71,6 +71,23 @@ func CloneStatus(status *CollectionStatus) *CollectionStatus {
return &clone
}
// TemperatureCollected reports whether a disk temperature was collected by the
// observation that carries it. Normalization may keep a last-known temperature
// it did not collect (a disk in standby, a host agent past its reporting
// lease) under a non-available state; that value must not be recorded or
// presented as a current reading. A temperature without collection state
// predates this contract and counts as collected.
func TemperatureCollected(temperature int, status *CollectionStatus) bool {
if temperature <= 0 {
return false
}
if status == nil {
return true
}
state := status.Temperature.State
return state == "" || state == FieldAvailable
}
// MergeStatus keeps an available observation over a weaker state while still
// allowing a current available observation to replace older provenance.
func MergeStatus(existing, incoming *CollectionStatus) *CollectionStatus {
+22
View File
@@ -65,3 +65,25 @@ func TestMergeReportedStatusLetsTheSourceWithdrawItsOwnAvailability(t *testing.T
t.Fatal("MergeReportedStatus mutated its input")
}
}
func TestTemperatureCollectedOnlyForAReadingTakenByThisObservation(t *testing.T) {
for _, tc := range []struct {
name string
temperature int
status *CollectionStatus
want bool
}{
{"no reading", 0, &CollectionStatus{Temperature: Available("smartctl")}, false},
{"collected", 41, &CollectionStatus{Temperature: Available("smartctl")}, true},
{"predates collection state", 41, nil, true},
{"no temperature state", 41, &CollectionStatus{Serial: Available("smartctl")}, true},
{"retained in standby", 41, &CollectionStatus{Temperature: Unavailable("smartctl", "disk is in standby")}, false},
{"retained past the agent lease", 41, &CollectionStatus{Temperature: Unavailable("unraid", "host agent stopped reporting")}, false},
{"unsupported", 41, &CollectionStatus{Temperature: Unsupported("proxmox_disks", "not exposed")}, false},
{"missing", 41, &CollectionStatus{Temperature: Missing("smartctl", "attribute absent")}, false},
} {
if got := TemperatureCollected(tc.temperature, tc.status); got != tc.want {
t.Errorf("%s: TemperatureCollected = %v, want %v", tc.name, got, tc.want)
}
}
}