From 447807526fc07e4521f61b8730a6422e8f381ea3 Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Wed, 7 Oct 2026 13:58:33 +0100 Subject: [PATCH] Show the outdated sensor setup notice on every Proxmox tab The Proxmox page warns when a node's SSH temperature monitoring still runs the pre-rc.6 setup, which locks the key to `sensors -j` so SATA/SAS disk temperatures never arrive. The notice judged that from the disk rows of the active tab, and since each tab hydrates only its own resource families, Overview loads no disks and never showed it. The registry now derives the verdict on the node itself, after child counts resolve each disk's canonical parent: a node with an available, legacy-format temperature payload is flagged when a SMART-only disk under it has no reading collected now. The page lists flagged nodes from the rows every tab already loads, so Overview needs no disk inventory. Every Proxmox node carries an explicit true or false, because the browser merges a facet field by field and would keep an earlier true if a cleared verdict were omitted. Two rule fixes ride along with the move. Proxmox's own disk inventory types SATA/SAS disks as hdd or ssd, so the old sata/sas-only set never matched a node without a host agent. And a retained reading kept after a disk stopped being read no longer counts as present (diskinventory.TemperatureCollected). --- .../subsystems/frontend-primitives.md | 15 +- .../internal/subsystems/storage-recovery.md | 9 + .../internal/subsystems/unified-resources.md | 30 +++ .../15418f6d41666705.json | 46 ++++ .../sensorSetup.branchcov0712.test.ts | 208 --------------- .../features/platformPage/sensorSetup.test.ts | 147 ++--------- .../src/features/platformPage/sensorSetup.ts | 48 +--- .../features/proxmox/ProxmoxPageSurface.tsx | 7 +- .../ProxmoxPageSurface.contract.test.tsx | 38 +++ .../__tests__/useUnifiedResources.test.ts | 6 +- .../src/hooks/useUnifiedResources.ts | 1 + frontend-modern/src/types/resource.ts | 4 + .../__tests__/resourceStateAdapters.test.ts | 23 ++ internal/unifiedresources/clone.go | 1 + internal/unifiedresources/clone_test.go | 19 ++ .../unifiedresources/proxmox_sensor_setup.go | 79 ++++++ internal/unifiedresources/registry.go | 3 + internal/unifiedresources/registry_test.go | 245 ++++++++++++++++++ internal/unifiedresources/types.go | 8 + 19 files changed, 559 insertions(+), 378 deletions(-) create mode 100644 frontend-modern/browser-verification/15418f6d41666705.json delete mode 100644 frontend-modern/src/features/platformPage/__tests__/sensorSetup.branchcov0712.test.ts create mode 100644 internal/unifiedresources/proxmox_sensor_setup.go diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index d837f96e3d..6a3b9128bd 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -6661,6 +6661,11 @@ icon/content layout, and action-link chrome. The outdated-sensor notices, and the `platform-inline-notice-local-amber-shell` pattern guard blocks future `platformPage` files from reintroducing page-local amber notice shells. +A notice's render predicate reads what its page hydrates on every tab the +notice renders on. `collectOutdatedSensorSetupNodes` lists the PVE nodes that +carry the registry's `proxmox.sensorSetupOutdated` verdict and takes no disk +rows, because only the Proxmox Storage tab loads them; a predicate over them +left the notice silent on Overview. Alert incident-event filter containers, labels, and chips must now route through the shared presentation helpers in @@ -7896,10 +7901,12 @@ renders above the chart. TrueNAS disk rows follow it too: the drawer's Temperature row reads "(last known)" with the reason as title and no heat tone, and the one-line summary leaves a retained reading out because it has no room for the reason. -The Proxmox outdated sensor setup notice (`features/platformPage/sensorSetup.ts`) -counts a SATA or SAS disk as having a temperature only when the reading is -current, so a retained one cannot hide the notice for a disk Pulse cannot read -now. +The Proxmox outdated sensor setup notice reads the registry's per-node +`proxmox.sensorSetupOutdated` verdict, which counts a SATA or SAS disk as having +a temperature only when the reading was collected now +(`diskinventory.TemperatureCollected` in +`internal/unifiedresources/proxmox_sensor_setup.go`), so a retained one cannot +hide the notice for a disk Pulse cannot read now. The focused browser proofs are `frontend-modern/src/features/patrol/__tests__/patrolRunAcceptance.test.ts`, diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 66185fb88d..91e8334be9 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -944,6 +944,15 @@ Patrol coverage, schedule, finding, and approval state belongs on Patrol-owned surfaces or explicit Patrol affordances; Proxmox overview must not treat it as backup coverage, restore readiness, PBS verification/protection proof, or a replacement for the Proxmox Backups tab and workload Backup column. +The outdated sensor setup notice above the Proxmox tabs is temperature +guidance, not storage or recovery evidence. It reads the registry's per-node +`proxmox.sensorSetupOutdated` verdict from the node rows the active tab already +loads (Overview, Storage, Replication and Backups load nodes; Ceph and Mail do +not, so it does not render there), so the Overview query must not add +`physical_disk` rows to feed it; the derived `ProxmoxData` flag opens no storage +or recovery path. +`ProxmoxPageSurface.contract.test.tsx` pins the notice on Overview with no disk +rows and an Overview query without `physical_disk`. The Proxmox inline estate totals are likewise an adjacent frontend-primitives and unified-resource projection, not backup or recovery evidence. `ProxmoxPageSurface` must flow the already-loaded workload inventory into the diff --git a/docs/release-control/v6/internal/subsystems/unified-resources.md b/docs/release-control/v6/internal/subsystems/unified-resources.md index 25fb124809..1141ff711f 100644 --- a/docs/release-control/v6/internal/subsystems/unified-resources.md +++ b/docs/release-control/v6/internal/subsystems/unified-resources.md @@ -2997,6 +2997,36 @@ or force every provider type back into the active route payload. Docker, Kubernetes, TrueNAS, and VMware surface contract tests pin their route query maps, and the shared hook test pins facet normalization. +### Proxmox node verdicts over disks arrive on the node + +The Proxmox page hydrates one query per workflow, and only its Storage tab +loads `physical_disk` rows, so a node verdict that depends on the node's disks +is derived in the registry and carried on the node, never rebuilt in the page +from disk rows. `ProxmoxData.SensorSetupOutdated` (`proxmox.sensorSetupOutdated` +on REST and websocket rows) is that verdict for the outdated sensor setup +notice. `refreshProxmoxSensorSetupLocked` +(`internal/unifiedresources/proxmox_sensor_setup.go`) runs after +`buildChildCounts` on every ingest path and gives every Proxmox node an +explicit true or false (other resources omit it), because browser facet merges +read an omitted field as a partial snapshot and would keep an earlier true. It +is true when the node's temperature payload is available and in the legacy +`sensors -j` format (`models.Temperature.LegacySensorsFormat`) and a disk whose +canonical parent is the node has no reading collected now +(`diskinventory.TemperatureCollected`) and a type whose temperature a PVE node +gets only through SMART: `sata`, `sas`, or the `hdd` and `ssd` form factors +Proxmox's own inventory reports. Proxmox also types a non-rotational USB device +`ssd`, so a USB device without SMART counts. A retained reading does not count +as current, including one kept while a host agent reports the disk in standby; +a reading a linked host agent collects now clears the disk. NVMe disks never +set it, because kernel hwmon reports them even on a legacy setup, and a host +agent the registry has not linked to the node contributes no disks. The +verdict reflects the registry's node and disk state at each ingest. +`TestProxmoxNodeSensorSetupOutdatedFromItsDisks` and +`TestProxmoxNodeSensorSetupOutdatedFollowsWhereTheReadingArrives` in +`internal/unifiedresources/registry_test.go` pin the rule, the linked-agent +case, the explicit false on the wire and the presented JSON; +`resourceStateAdapters.test.ts` pins that a false clears a merged true. + ### Canonical REST facets preserve realtime workload evidence The frontend REST projection retains the complete source-authored Proxmox diff --git a/frontend-modern/browser-verification/15418f6d41666705.json b/frontend-modern/browser-verification/15418f6d41666705.json new file mode 100644 index 0000000000..2ea8e947d3 --- /dev/null +++ b/frontend-modern/browser-verification/15418f6d41666705.json @@ -0,0 +1,46 @@ +{ + "version": 1, + "base_sha": "e02edd9989c88fbe6e5459afdf488c73538ddd24", + "verified_at": "2026-10-07T12:56:24Z", + "result": "passed", + "changed_paths": [ + "frontend-modern/src/features/platformPage/sensorSetup.ts", + "frontend-modern/src/features/proxmox/ProxmoxPageSurface.tsx", + "frontend-modern/src/hooks/useUnifiedResources.ts", + "frontend-modern/src/types/resource.ts" + ], + "content_sha256": { + "frontend-modern/src/features/platformPage/sensorSetup.ts": "18e4da01eb9206bce83a2e7d2fbb9f97313685c5655d620092a9e0ca51804ef5", + "frontend-modern/src/features/proxmox/ProxmoxPageSurface.tsx": "db074960c69c59bb030814a04028a50ee5b347b7cf16b04b4c6edea708ffd782", + "frontend-modern/src/hooks/useUnifiedResources.ts": "77cc56a11213f951e051ceabd05bc2bf3161adedadc3586f2badee98bf1056c3", + "frontend-modern/src/types/resource.ts": "37e3ddb2e1334472a280e0ef93053292002567acabf84aace9fed1735a763b57" + }, + "routes": [ + "/proxmox", + "/proxmox/storage", + "/proxmox/replication", + "/proxmox/backups" + ], + "viewports": [ + { + "width": 1440, + "height": 900 + }, + { + "width": 390, + "height": 844 + } + ], + "states": [ + "Private mock stack (8-node estate) built from this tree with a local-only generator overlay: pve2, pve3 and pve5 report the legacy sensors -j payload; pve2 has a Proxmox-inventory hdd disk with no temperature, pve3 a SATA disk whose reading is retained (host agent stopped reporting), pve5 a SATA disk with no temperature.", + "Overview, Storage, Replication and Backups each show the warning notice naming pve2, pve3 and pve5 at both viewports. The Overview query is type=agent,vm,system-container,oci-container and no physical_disk request is made on Overview, Replication or Backups; only Storage requests disk rows.", + "Before control on the same stack with origin/main's ProxmoxPageSurface.tsx and sensorSetup.ts swapped in: no notice on Overview, Replication or Backups, and Storage names only pve5 (hdd type and retained reading missed).", + "Websocket frames carry sensorSetupOutdated true on every tab. Fresh desktop load: no console errors and no 5xx responses. Notice text fits the 390px viewport without overflow; the node table stays below it in reading order.", + "Mock data only; not a real pre-rc.6 SSH node, installed upgrade or release acceptance." + ], + "interactions": [ + "Navigated each tab by URL at 1440x900 and 390x844 and waited 20s on Overview to outlast websocket row merges; the notice stayed.", + "Live clearing without reload: switching pve5's disk to a current 37C reading dropped pve5 from the notice (3 nodes to pve2, pve3) on the next websocket update. Control with the earlier omitempty backend kept pve5 for 90s.", + "Open Infrastructure settings link targets /settings/infrastructure (contract test)." + ] +} diff --git a/frontend-modern/src/features/platformPage/__tests__/sensorSetup.branchcov0712.test.ts b/frontend-modern/src/features/platformPage/__tests__/sensorSetup.branchcov0712.test.ts deleted file mode 100644 index ce1e3a6d06..0000000000 --- a/frontend-modern/src/features/platformPage/__tests__/sensorSetup.branchcov0712.test.ts +++ /dev/null @@ -1,208 +0,0 @@ -import { describe, expect, it } from 'vitest'; -import type { Resource } from '@/types/resource'; -import { collectOutdatedSensorSetupNodes } from '../sensorSetup'; - -// Branch-coverage tests for the two UNEXPORTED helpers inside sensorSetup.ts: -// - getNodeName (3-way `||` fallback chain + `.trim()`) -// - isSMARTOnlyDiskWithoutTemperature (no-meta guard, diskType normalize, -// SMART-set membership, `temperature ?? 0`) -// Both helpers are private, so every case drives them through the public -// `collectOutdatedSensorSetupNodes` and asserts the concrete observable shape. -// -// Conventions mirror the sibling `sensorSetup.test.ts` fixtures. Disks are -// linked to nodes either by `platformData.proxmox.nodeName` (the name-matching -// path, used to prove `getNodeName` resolution) or by `parentId` (which makes -// `matchesPhysicalDiskNode` short-circuit true, used to isolate the SMART-only -// predicate from name resolution). - -type LegacyNodeOver = { - id?: string; - name?: string; - proxmox?: Partial; -}; - -const legacyNode = (over: LegacyNodeOver = {}): Resource => - ({ - id: over.id ?? 'node-1', - name: over.name ?? '', - displayName: over.name ?? '', - type: 'node', - proxmox: { - temperatureDetails: { available: true, legacySensorsFormat: true }, - ...(over.proxmox ?? {}), - }, - }) as unknown as Resource; - -type DiskOver = { - id?: string; - parentId?: string; - nodeName?: string; - diskType?: string; - temperature?: number; - noMeta?: boolean; -}; - -const disk = (over: DiskOver = {}): Resource => - ({ - id: over.id ?? 'disk-1', - name: 'disk', - displayName: 'disk', - type: 'physical_disk', - parentId: over.parentId, - physicalDisk: over.noMeta - ? undefined - : { - diskType: over.diskType, - temperature: over.temperature, - }, - platformData: over.nodeName ? { proxmox: { nodeName: over.nodeName } } : {}, - }) as unknown as Resource; - -describe('getNodeName (via collectOutdatedSensorSetupNodes)', () => { - it('falls back to proxmox.nodeName when proxmox.node is absent', () => { - const node = legacyNode({ id: 'n-nodeName', proxmox: { nodeName: 'bravo' } }); - const result = collectOutdatedSensorSetupNodes( - [node], - // Disk resolves to the node by the nodeName-derived identity, proving - // getNodeName returned 'bravo' (not '' and not node.name). - [disk({ id: 'd1', nodeName: 'bravo', diskType: 'sata', temperature: 0 })], - ); - expect(result).toStrictEqual([{ id: 'n-nodeName', name: 'bravo' }]); - }); - - it('falls back to the resource name when both proxmox.node and nodeName are absent', () => { - const node = legacyNode({ id: 'n-name', name: 'charlie' }); - const result = collectOutdatedSensorSetupNodes( - [node], - [disk({ id: 'd2', nodeName: 'charlie', diskType: 'sata', temperature: 0 })], - ); - expect(result).toStrictEqual([{ id: 'n-name', name: 'charlie' }]); - }); - - it('trims surrounding whitespace from proxmox.node before using it as the node identity', () => { - const node = legacyNode({ id: 'n-trim', proxmox: { node: ' delta ' } }); - const result = collectOutdatedSensorSetupNodes( - [node], - // matchesPhysicalDiskNode lowercases/trims both sides, so 'delta' must be - // the trimmed getNodeName output for the disk to match. - [disk({ id: 'd3', nodeName: 'delta', diskType: 'sata', temperature: 0 })], - ); - expect(result).toStrictEqual([{ id: 'n-trim', name: 'delta' }]); - }); - - it('returns the empty string when every name source is blank, observable via a parentId-linked disk', () => { - // No proxmox.node, no proxmox.nodeName, name is whitespace -> getNodeName - // hits the final `|| ''` arm and trims to ''. The disk is matched by - // parentId so matchesPhysicalDiskNode short-circuits true regardless of the - // (empty) name, letting the flagged node surface with name: ''. - const node = legacyNode({ id: 'n-empty', name: ' ' }); - const result = collectOutdatedSensorSetupNodes( - [node], - [disk({ id: 'd4', parentId: 'n-empty', diskType: 'sata', temperature: 0 })], - ); - expect(result).toStrictEqual([{ id: 'n-empty', name: '' }]); - }); -}); - -describe('isSMARTOnlyDiskWithoutTemperature (via collectOutdatedSensorSetupNodes)', () => { - it('returns false for a disk with no physicalDisk metadata, so the node is not flagged', () => { - // Disk is linked to the node by parentId (match guaranteed true); the only - // reason the node drops out is the `if (!meta) return false` guard. - const node = legacyNode({ id: 'n-nometa', proxmox: { node: 'pve' } }); - const result = collectOutdatedSensorSetupNodes( - [node], - [disk({ id: 'd-nometa', parentId: 'n-nometa', noMeta: true })], - ); - expect(result).toStrictEqual([]); - }); - - it('returns false when diskType is missing, so a matched disk does not flag the node', () => { - // diskType undefined -> `(meta.diskType || '')` -> '' not in the SMART-only - // set. parentId link isolates the cause to the diskType check. - const node = legacyNode({ id: 'n-notype', proxmox: { node: 'pve' } }); - const result = collectOutdatedSensorSetupNodes( - [node], - [disk({ id: 'd-notype', parentId: 'n-notype', temperature: 0 })], - ); - expect(result).toStrictEqual([]); - }); - - it('normalizes a whitespace-padded uppercase SAS diskType and flags the node', () => { - // Exercises `diskType.trim().toLowerCase()` -> 'sas' (in the SMART-only - // set) and temperature 0 -> `(0 ?? 0) <= 0` true. - const node = legacyNode({ id: 'n-sas', proxmox: { node: 'pve' } }); - const result = collectOutdatedSensorSetupNodes( - [node], - [ - disk({ - id: 'd-sas', - parentId: 'n-sas', - diskType: ' SAS ', - temperature: 0, - }), - ], - ); - expect(result).toStrictEqual([{ id: 'n-sas', name: 'pve' }]); - }); - - it('uses 0 in place of an undefined temperature and flags the node', () => { - // temperature is omitted entirely -> `(meta.temperature ?? 0)` takes the - // nullish right-hand side (0) -> `<= 0` true. - const node = legacyNode({ id: 'n-undef-temp', proxmox: { node: 'pve' } }); - const result = collectOutdatedSensorSetupNodes( - [node], - [disk({ id: 'd-undef-temp', parentId: 'n-undef-temp', diskType: 'sata' })], - ); - expect(result).toStrictEqual([{ id: 'n-undef-temp', name: 'pve' }]); - }); - - it('uses 0 in place of an explicit null temperature and flags the node', () => { - // Deliberately-malformed payload: temperature typed as number | undefined - // but carrying null. The `?? 0` coalesces it to 0 -> `<= 0` true. - const node = legacyNode({ id: 'n-null-temp', proxmox: { node: 'pve' } }); - const nullTempDisk = { - id: 'd-null-temp', - name: 'disk', - displayName: 'disk', - type: 'physical_disk', - parentId: 'n-null-temp', - physicalDisk: { diskType: 'sata', temperature: null }, - } as unknown as Resource; - const result = collectOutdatedSensorSetupNodes([node], [nullTempDisk]); - expect(result).toStrictEqual([{ id: 'n-null-temp', name: 'pve' }]); - }); - - it('flags the node for a SATA disk reporting a negative temperature', () => { - // Concrete negative value exercises the `<= 0` true arm (not the ?? arm). - const node = legacyNode({ id: 'n-neg-temp', proxmox: { node: 'pve' } }); - const result = collectOutdatedSensorSetupNodes( - [node], - [disk({ id: 'd-neg-temp', parentId: 'n-neg-temp', diskType: 'sata', temperature: -5 })], - ); - expect(result).toStrictEqual([{ id: 'n-neg-temp', name: 'pve' }]); - }); - - it('does not flag the node when a SMART-only disk reports a positive temperature', () => { - // SAS at 41 degrees -> `(41 ?? 0) <= 0` is false. - const node = legacyNode({ id: 'n-warm', proxmox: { node: 'pve' } }); - const result = collectOutdatedSensorSetupNodes( - [node], - [disk({ id: 'd-warm', parentId: 'n-warm', diskType: 'sas', temperature: 41 })], - ); - expect(result).toStrictEqual([]); - }); - - it('evaluates the no-meta guard on the first disk but still flags the node via a second matching disk', () => { - // Drives the `!meta` branch on d-bad, then `.some()` advances to d-good - // which is SMART-only without a temperature -> node flagged. - const node = legacyNode({ id: 'n-mixed', proxmox: { node: 'pve' } }); - const result = collectOutdatedSensorSetupNodes( - [node], - [ - disk({ id: 'd-bad', parentId: 'n-mixed', noMeta: true }), - disk({ id: 'd-good', parentId: 'n-mixed', diskType: 'sata', temperature: 0 }), - ], - ); - expect(result).toStrictEqual([{ id: 'n-mixed', name: 'pve' }]); - }); -}); diff --git a/frontend-modern/src/features/platformPage/sensorSetup.test.ts b/frontend-modern/src/features/platformPage/sensorSetup.test.ts index 3b2ada61d9..949c649186 100644 --- a/frontend-modern/src/features/platformPage/sensorSetup.test.ts +++ b/frontend-modern/src/features/platformPage/sensorSetup.test.ts @@ -1,150 +1,57 @@ import { describe, expect, it } from 'vitest'; -import type { PhysicalDiskCollectionStatus, Resource } from '@/types/resource'; +import type { Resource } from '@/types/resource'; import { collectOutdatedSensorSetupNodes } from './sensorSetup'; const node = (over: Partial & { proxmox?: Resource['proxmox'] }): Resource => - ({ id: 'node-1', name: 'pve1', type: 'node', ...over }) as Resource; + ({ id: 'node-1', name: 'pve1', type: 'agent', ...over }) as Resource; -const disk = (over: { - id?: string; - parentId?: string; - nodeName?: string; - diskType?: string; - temperature?: number; - collection?: PhysicalDiskCollectionStatus; -}): Resource => - ({ - id: over.id ?? 'disk-1', - name: 'disk', - type: 'physical_disk', - parentId: over.parentId, - physicalDisk: { - diskType: over.diskType ?? 'sata', - temperature: over.temperature ?? 0, - collection: over.collection, - }, - platformData: over.nodeName ? { proxmox: { nodeName: over.nodeName } } : {}, - }) as unknown as Resource; - -const legacyNode = (over: Partial = {}): Resource => +const flaggedNode = (over: Partial = {}): Resource => node({ proxmox: { node: 'pve1', temperatureDetails: { available: true, legacySensorsFormat: true }, + sensorSetupOutdated: true, }, ...over, }); describe('collectOutdatedSensorSetupNodes', () => { - it('flags a legacy-format node with a SATA disk missing a temperature', () => { - const result = collectOutdatedSensorSetupNodes( - [legacyNode()], - [disk({ nodeName: 'pve1', diskType: 'sata' })], - ); - expect(result).toEqual([{ id: 'node-1', name: 'pve1' }]); + it('lists a node carrying the registry verdict without any disk rows', () => { + expect(collectOutdatedSensorSetupNodes([flaggedNode()])).toEqual([ + { id: 'node-1', name: 'pve1' }, + ]); }); - it('matches disks to nodes by parentId as well as node name', () => { - const result = collectOutdatedSensorSetupNodes( - [legacyNode()], - [disk({ parentId: 'node-1', diskType: 'sas' })], - ); - expect(result).toEqual([{ id: 'node-1', name: 'pve1' }]); - }); - - it('ignores nodes whose payload is the wrapper format', () => { - const wrapperNode = node({ - proxmox: { node: 'pve1', temperatureDetails: { available: true } }, - }); - expect(collectOutdatedSensorSetupNodes([wrapperNode], [disk({ nodeName: 'pve1' })])).toEqual( - [], - ); - }); - - it('ignores legacy nodes when temperature collection did not succeed', () => { - const unavailableNode = node({ + it('leaves a legacy-format node the registry did not flag', () => { + // The registry clears the verdict when every SATA/SAS disk under the node + // has a current temperature, for example from a linked host agent. + const reading = node({ proxmox: { node: 'pve1', - temperatureDetails: { available: false, legacySensorsFormat: true }, + temperatureDetails: { available: true, legacySensorsFormat: true }, }, }); - expect( - collectOutdatedSensorSetupNodes([unavailableNode], [disk({ nodeName: 'pve1' })]), - ).toEqual([]); + expect(collectOutdatedSensorSetupNodes([reading])).toEqual([]); }); - it('ignores legacy nodes whose SATA/SAS disks already have temperatures', () => { - expect( - collectOutdatedSensorSetupNodes( - [legacyNode()], - [disk({ nodeName: 'pve1', diskType: 'sata', temperature: 34 })], - ), - ).toEqual([]); - }); - - it('treats a retained temperature that is not current as missing', () => { - // Normalization keeps a disk's last known temperature when the current - // observation did not collect one, such as after a linked host agent - // stopped reporting. The disk cannot be read now, so the notice stays. - const retained = (state: 'available' | 'unavailable' | 'missing') => - disk({ - nodeName: 'pve1', - diskType: 'sata', - temperature: 34, - collection: { - temperature: { state, source: 'agent', reason: 'host agent stopped reporting' }, - }, - }); - expect(collectOutdatedSensorSetupNodes([legacyNode()], [retained('unavailable')])).toEqual([ - { id: 'node-1', name: 'pve1' }, + it('names a node by its Proxmox node, then node name, then resource name, trimmed', () => { + const nodes = [ + node({ id: 'by-node', proxmox: { node: ' alpha ', sensorSetupOutdated: true } }), + node({ id: 'by-node-name', proxmox: { nodeName: 'bravo', sensorSetupOutdated: true } }), + node({ id: 'by-name', name: 'charlie', proxmox: { sensorSetupOutdated: true } }), + ]; + expect(collectOutdatedSensorSetupNodes(nodes)).toEqual([ + { id: 'by-node', name: 'alpha' }, + { id: 'by-node-name', name: 'bravo' }, + { id: 'by-name', name: 'charlie' }, ]); - expect(collectOutdatedSensorSetupNodes([legacyNode()], [retained('missing')])).toEqual([ - { id: 'node-1', name: 'pve1' }, - ]); - expect(collectOutdatedSensorSetupNodes([legacyNode()], [retained('available')])).toEqual([]); - }); - - it('ignores legacy nodes with only NVMe disks missing temperatures', () => { - expect( - collectOutdatedSensorSetupNodes( - [legacyNode()], - [disk({ nodeName: 'pve1', diskType: 'nvme' })], - ), - ).toEqual([]); - }); - - it('ignores disks that belong to a different node', () => { - expect(collectOutdatedSensorSetupNodes([legacyNode()], [disk({ nodeName: 'pve2' })])).toEqual( - [], - ); }); it('sorts multiple affected nodes by name', () => { const nodes = [ - node({ - id: 'node-b', - name: 'pve-b', - proxmox: { - node: 'pve-b', - temperatureDetails: { available: true, legacySensorsFormat: true }, - }, - }), - node({ - id: 'node-a', - name: 'pve-a', - proxmox: { - node: 'pve-a', - temperatureDetails: { available: true, legacySensorsFormat: true }, - }, - }), + flaggedNode({ id: 'node-b', proxmox: { node: 'pve-b', sensorSetupOutdated: true } }), + flaggedNode({ id: 'node-a', proxmox: { node: 'pve-a', sensorSetupOutdated: true } }), ]; - const disks = [ - disk({ id: 'disk-a', nodeName: 'pve-a' }), - disk({ id: 'disk-b', nodeName: 'pve-b' }), - ]; - expect(collectOutdatedSensorSetupNodes(nodes, disks).map((n) => n.name)).toEqual([ - 'pve-a', - 'pve-b', - ]); + expect(collectOutdatedSensorSetupNodes(nodes).map((n) => n.name)).toEqual(['pve-a', 'pve-b']); }); }); diff --git a/frontend-modern/src/features/platformPage/sensorSetup.ts b/frontend-modern/src/features/platformPage/sensorSetup.ts index af8ff285b2..2e27d70d5a 100644 --- a/frontend-modern/src/features/platformPage/sensorSetup.ts +++ b/frontend-modern/src/features/platformPage/sensorSetup.ts @@ -1,10 +1,4 @@ import type { Resource } from '@/types/resource'; -import { matchesPhysicalDiskNode } from '@/components/Storage/diskResourceUtils'; -import { isPhysicalDiskTemperatureCurrent } from '@/features/storageBackups/diskTemperaturePresentation'; - -// Disk transports whose temperatures only arrive via SMART (the pulse-sensors -// wrapper). NVMe temps come from kernel hwmon and work even on legacy setups. -const SMART_ONLY_DISK_TYPES = new Set(['sata', 'sas']); export type OutdatedSensorSetupNode = { id: string; @@ -14,44 +8,16 @@ export type OutdatedSensorSetupNode = { const getNodeName = (node: Resource): string => (node.proxmox?.node || node.proxmox?.nodeName || node.name || '').trim(); -const isSMARTOnlyDiskWithoutTemperature = (disk: Resource): boolean => { - const meta = disk.physicalDisk; - if (!meta) return false; - const diskType = (meta.diskType || '').trim().toLowerCase(); - if (!SMART_ONLY_DISK_TYPES.has(diskType)) return false; - // A last known temperature kept for a disk that could not be read now, such - // as after a linked host agent stopped reporting, is not one arriving. - return (meta.temperature ?? 0) <= 0 || !isPhysicalDiskTemperatureCurrent(meta.collection); -}; - -// Flags PVE nodes whose SSH temperature monitoring still runs the pre-rc.6 +// Lists PVE nodes whose SSH temperature monitoring still runs the pre-rc.6 // setup (authorized_keys locked to `sensors -j`). Such a payload parses fine // and delivers CPU/NVMe temps, but SMART (SATA/SAS) disk temperatures can -// never arrive, so they silently stay blank. Data-gated three ways: the last -// collection succeeded, the payload was legacy-format, and the node actually -// has SATA/SAS disks sitting without a temperature. Nodes whose disk temps -// arrive elsewhere (e.g. a linked host agent) fall out via the last check. -export function collectOutdatedSensorSetupNodes( - nodes: Resource[], - physicalDisks: Resource[], -): OutdatedSensorSetupNode[] { - if (!nodes.length || !physicalDisks.length) return []; - +// never arrive, so they silently stay blank. The registry derives the verdict +// on the node (`proxmox.sensorSetupOutdated`) from the node's last collection, +// its legacy payload and the disks under it, so every Proxmox tab that lists +// nodes can show the notice without loading the disk inventory. +export function collectOutdatedSensorSetupNodes(nodes: Resource[]): OutdatedSensorSetupNode[] { return nodes - .filter((node) => { - const details = node.proxmox?.temperatureDetails; - if (!details?.available || !details.legacySensorsFormat) return false; - const nodeName = getNodeName(node); - return physicalDisks.some( - (disk) => - isSMARTOnlyDiskWithoutTemperature(disk) && - matchesPhysicalDiskNode(disk, { - id: node.id, - name: nodeName, - instance: node.proxmox?.instance, - }), - ); - }) + .filter((node) => node.proxmox?.sensorSetupOutdated === true) .map((node) => ({ id: node.id, name: getNodeName(node) })) .sort((a, b) => a.name.localeCompare(b.name)); } diff --git a/frontend-modern/src/features/proxmox/ProxmoxPageSurface.tsx b/frontend-modern/src/features/proxmox/ProxmoxPageSurface.tsx index e34418e333..fa0a193e29 100644 --- a/frontend-modern/src/features/proxmox/ProxmoxPageSurface.tsx +++ b/frontend-modern/src/features/proxmox/ProxmoxPageSurface.tsx @@ -286,11 +286,10 @@ export function ProxmoxPageSurface() { const serverVersionDisplay = createMemo(() => formatAgentVersionDisplay(agentUpdateTargetVersion()), ); + // The node carries the registry's verdict, so the notice reads the same on + // every tab that lists nodes; no tab hydrates disks for it. const outdatedSensorSetupNodes = createMemo(() => - collectOutdatedSensorSetupNodes( - model().pveNodes, - model().resources.filter((resource) => resource.type === 'physical_disk'), - ), + collectOutdatedSensorSetupNodes(model().pveNodes), ); // The hosts table at the top and the embedded WorkloadsSurface below share diff --git a/frontend-modern/src/features/proxmox/__tests__/ProxmoxPageSurface.contract.test.tsx b/frontend-modern/src/features/proxmox/__tests__/ProxmoxPageSurface.contract.test.tsx index 198c6b273c..63964ff67a 100644 --- a/frontend-modern/src/features/proxmox/__tests__/ProxmoxPageSurface.contract.test.tsx +++ b/frontend-modern/src/features/proxmox/__tests__/ProxmoxPageSurface.contract.test.tsx @@ -375,6 +375,44 @@ describe('ProxmoxPageSurface contract', () => { expect(screen.queryByTestId('platform-outdated-agent-notice')).not.toBeInTheDocument(); }); + it('shows the outdated sensor setup notice on Overview without hydrating disks', () => { + const node = makeResource({ + id: 'agent:pve3', + name: 'pve3', + displayName: 'pve3', + type: 'agent', + proxmox: { + nodeName: 'pve3', + temperatureDetails: { available: true, legacySensorsFormat: true }, + sensorSetupOutdated: true, + }, + }); + // Like production, the Overview query returns nodes and guests only: the + // verdict has to arrive on the node, because no physical_disk rows do. + mockUseUnifiedResources.mockImplementation((options: { cacheKey?: string }) => ({ + resources: () => (options.cacheKey === 'proxmox-overview' ? [node] : []), + loading: () => false, + error: () => null, + refetch: vi.fn(), + })); + + renderSurface(); + + const overviewOptions = mockUseUnifiedResources.mock.calls + .map(([options]) => options as { cacheKey?: string; query?: string }) + .find((options) => options.cacheKey === 'proxmox-overview'); + expect(overviewOptions?.query).toBeDefined(); + expect(overviewOptions?.query).not.toContain('physical_disk'); + expect(screen.getByTestId('platform-section-tabs')).toHaveAttribute('data-active', 'overview'); + expect(screen.getByTestId('platform-outdated-sensor-setup-notice')).toHaveTextContent( + 'pve3 is using an older temperature monitoring setup that cannot read SATA/SAS disk temperatures.', + ); + expect(screen.getByRole('link', { name: 'Open Infrastructure settings' })).toHaveAttribute( + 'href', + '/settings/infrastructure', + ); + }); + it('folds estate topology into the existing nodes table header contract', () => { setResources([ makeResource({ diff --git a/frontend-modern/src/hooks/__tests__/useUnifiedResources.test.ts b/frontend-modern/src/hooks/__tests__/useUnifiedResources.test.ts index 3f62f9fa63..3d1f06a4ba 100644 --- a/frontend-modern/src/hooks/__tests__/useUnifiedResources.test.ts +++ b/frontend-modern/src/hooks/__tests__/useUnifiedResources.test.ts @@ -1943,7 +1943,7 @@ describe('useUnifiedResources', () => { dispose(); }); - it('passes proxmox temperatureDetails through for the outdated-sensor-setup gate', async () => { + it('passes the proxmox temperature payload and sensor setup verdict through', async () => { apiFetchMock.mockResolvedValueOnce({ ok: true, json: async () => ({ @@ -1956,6 +1956,7 @@ describe('useUnifiedResources', () => { nodeName: 'pve1', temperature: 58.4, temperatureDetails: { available: true, legacySensorsFormat: true }, + sensorSetupOutdated: true, }, }, ], @@ -1974,6 +1975,9 @@ describe('useUnifiedResources', () => { available: true, legacySensorsFormat: true, }); + // The outdated sensor setup notice reads the registry's verdict on the + // node, so it must survive the projection that rows on every tab go through. + expect(result!.resources()[0].proxmox?.sensorSetupOutdated).toBe(true); dispose(); }); diff --git a/frontend-modern/src/hooks/useUnifiedResources.ts b/frontend-modern/src/hooks/useUnifiedResources.ts index 489881c89c..6c6547e550 100644 --- a/frontend-modern/src/hooks/useUnifiedResources.ts +++ b/frontend-modern/src/hooks/useUnifiedResources.ts @@ -261,6 +261,7 @@ type APIResource = { available?: boolean; legacySensorsFormat?: boolean; }; + sensorSetupOutdated?: boolean; template?: boolean; containerType?: string; isOci?: boolean; diff --git a/frontend-modern/src/types/resource.ts b/frontend-modern/src/types/resource.ts index 0bc9b1826b..a80952ef36 100644 --- a/frontend-modern/src/types/resource.ts +++ b/frontend-modern/src/types/resource.ts @@ -773,6 +773,10 @@ export interface ResourceProxmoxMeta { available?: boolean; legacySensorsFormat?: boolean; }; + // Registry-derived on Proxmox nodes: the legacy setup above is in use and a + // SATA/SAS disk under the node has no current temperature. It arrives on the + // node so the outdated sensor setup notice never needs the disk inventory. + sensorSetupOutdated?: boolean; } export interface ResourceVirtualMachineMeta { diff --git a/frontend-modern/src/utils/__tests__/resourceStateAdapters.test.ts b/frontend-modern/src/utils/__tests__/resourceStateAdapters.test.ts index fa946df7c7..9673cfd24b 100644 --- a/frontend-modern/src/utils/__tests__/resourceStateAdapters.test.ts +++ b/frontend-modern/src/utils/__tests__/resourceStateAdapters.test.ts @@ -1379,6 +1379,29 @@ describe('resourceStateAdapters unavailable memory contract', () => { }); describe('incremental canonical resource snapshots', () => { + it('clears a node sensor setup verdict when the next snapshot sends false', () => { + // Facets merge field by field and an omitted field is read as a partial + // snapshot, so the registry sends every Proxmox node an explicit verdict. + const node = (sensorSetupOutdated?: boolean) => + ({ + id: 'agent-pve3', + type: 'agent', + name: 'pve3', + status: 'online', + sources: ['proxmox'], + proxmox: { nodeName: 'pve3', sensorSetupOutdated }, + }) as unknown as Resource; + + const [cleared] = mergeCanonicalResourceSnapshot([node(false)], [node(true)]); + expect(cleared?.proxmox?.sensorSetupOutdated).toBe(false); + + const [omitted] = mergeCanonicalResourceSnapshot( + [{ ...node(), proxmox: { nodeName: 'pve3' } } as unknown as Resource], + [node(true)], + ); + expect(omitted?.proxmox?.sensorSetupOutdated).toBe(true); + }); + it('preserves untouched row identity while refreshing changed resources', () => { const unchanged = { id: 'vm-unchanged', diff --git a/internal/unifiedresources/clone.go b/internal/unifiedresources/clone.go index 104c712f9f..0beca03f35 100644 --- a/internal/unifiedresources/clone.go +++ b/internal/unifiedresources/clone.go @@ -198,6 +198,7 @@ func cloneProxmoxData(in *ProxmoxData) *ProxmoxData { out.DockerCheckedAt = cloneTimePtr(in.DockerCheckedAt) out.TemperatureMonitoringEnabled = cloneBoolPtr(in.TemperatureMonitoringEnabled) out.PendingUpdatesCheckedAt = cloneTimePtr(in.PendingUpdatesCheckedAt) + out.SensorSetupOutdated = cloneBoolPtr(in.SensorSetupOutdated) return &out } diff --git a/internal/unifiedresources/clone_test.go b/internal/unifiedresources/clone_test.go index e46dc22840..63b87b54a1 100644 --- a/internal/unifiedresources/clone_test.go +++ b/internal/unifiedresources/clone_test.go @@ -518,6 +518,25 @@ func TestCloneProxmoxData_TemperatureIsolation(t *testing.T) { } } +// List and presentation readers get detached clones of a node, so the +// registry-derived sensor setup verdict travels with the clone and stays put +// when the live node's verdict changes afterwards. +func TestCloneProxmoxDataKeepsDerivedSensorSetupVerdict(t *testing.T) { + outdated := true + original := &ProxmoxData{NodeName: "pve", SensorSetupOutdated: &outdated} + cloned := cloneProxmoxData(original) + if cloned.SensorSetupOutdated == nil || !*cloned.SensorSetupOutdated { + t.Fatal("clone dropped the derived sensor setup verdict") + } + if cloned.SensorSetupOutdated == original.SensorSetupOutdated { + t.Fatal("clone shares the live node's verdict pointer") + } + *original.SensorSetupOutdated = false + if !*cloned.SensorSetupOutdated { + t.Fatal("changing the live node's verdict changed a clone already handed out") + } +} + func TestCloneProxmoxData_NetworkInterfaceIsolation(t *testing.T) { original := &ProxmoxData{ NetworkInterfaces: []NetworkInterface{ diff --git a/internal/unifiedresources/proxmox_sensor_setup.go b/internal/unifiedresources/proxmox_sensor_setup.go new file mode 100644 index 0000000000..4ff3c9c1b9 --- /dev/null +++ b/internal/unifiedresources/proxmox_sensor_setup.go @@ -0,0 +1,79 @@ +package unifiedresources + +import ( + "strings" + + "github.com/rcourtman/pulse-go-rewrite/pkg/diskinventory" +) + +// smartTemperatureDiskTypes are the disk types whose temperature reaches a +// Proxmox node only through SMART: the transports a host agent or the +// pulse-sensors wrapper reports (sata, sas) and the form factors Proxmox's own +// disk inventory reports for those same disks (hdd, ssd). Proxmox also types +// a non-rotational USB device ssd. NVMe temperatures come from kernel hwmon +// and arrive even on a legacy setup. +var smartTemperatureDiskTypes = map[string]struct{}{ + "sata": {}, + "sas": {}, + "hdd": {}, + "ssd": {}, +} + +// refreshProxmoxSensorSetupLocked derives ProxmoxData.SensorSetupOutdated on +// every Proxmox node, as an explicit true or false. SSH temperature monitoring +// set up before v6.0.0-rc.6 locks the node's key to `sensors -j`: the payload +// still parses and carries CPU and NVMe temperatures, but SMART disk +// temperatures can never arrive, so they stay blank with no error anywhere. +// The flag is data-gated three ways: the node's last temperature collection +// succeeded, its payload was the legacy format, and a disk under it waits on a +// SMART temperature it does not have now. A last known temperature kept for a +// disk that could not be read now (standby, a host agent past its lease) is not +// a current one (diskinventory.TemperatureCollected), while a disk whose +// temperature arrives another way, such as a linked host agent, falls out. +// Disks count through their canonical parent, so an agent the registry has not +// linked to the node contributes none. Deriving the flag here lets every +// surface that lists nodes read it without loading the node's disk inventory. +// It must run after buildChildCounts, which resolves each disk's canonical +// parent node. +func (rr *ResourceRegistry) refreshProxmoxSensorSetupLocked() { + waiting := make(map[string]struct{}) + for _, resource := range rr.resources { + if resource == nil { + continue + } + if resource.Proxmox != nil { + // A fresh pointer every time: a facet copied by an earlier merge or + // clone must not see this generation's verdict. + resource.Proxmox.SensorSetupOutdated = nil + if CanonicalResourceType(resource.Type) == ResourceTypeAgent { + resource.Proxmox.SensorSetupOutdated = sensorSetupVerdict(false) + } + } + if resource.ParentID != nil && physicalDiskWaitsOnSMARTTemperature(resource) { + waiting[*resource.ParentID] = struct{}{} + } + } + for parentID := range waiting { + node := rr.resources[parentID] + if node == nil || node.Proxmox == nil || CanonicalResourceType(node.Type) != ResourceTypeAgent { + continue + } + details := node.Proxmox.TemperatureDetails + node.Proxmox.SensorSetupOutdated = sensorSetupVerdict(details != nil && details.Available && details.LegacySensorsFormat) + } +} + +func sensorSetupVerdict(outdated bool) *bool { + return &outdated +} + +func physicalDiskWaitsOnSMARTTemperature(resource *Resource) bool { + if CanonicalResourceType(resource.Type) != ResourceTypePhysicalDisk || resource.PhysicalDisk == nil { + return false + } + disk := resource.PhysicalDisk + if _, ok := smartTemperatureDiskTypes[strings.ToLower(strings.TrimSpace(disk.DiskType))]; !ok { + return false + } + return !diskinventory.TemperatureCollected(disk.Temperature, disk.Collection) +} diff --git a/internal/unifiedresources/registry.go b/internal/unifiedresources/registry.go index 6c9a786cad..827d8dfc43 100644 --- a/internal/unifiedresources/registry.go +++ b/internal/unifiedresources/registry.go @@ -597,6 +597,7 @@ func (rr *ResourceRegistry) ingestSnapshot(snapshot models.StateSnapshot, thresh rr.refreshStoragePostureLocked() rr.refreshIncidentRollupsLocked() rr.buildChildCounts() + rr.refreshProxmoxSensorSetupLocked() rr.refreshCanonicalIdentitiesLocked() rr.markStaleLocked(time.Now().UTC(), thresholds) rr.invalidateViewsLocked() @@ -648,6 +649,7 @@ func (rr *ResourceRegistry) ingestRecords(source DataSource, records []IngestRec rr.refreshStoragePostureLocked() rr.refreshIncidentRollupsLocked() rr.buildChildCounts() + rr.refreshProxmoxSensorSetupLocked() rr.refreshCanonicalIdentitiesLocked() rr.invalidateViewsLocked() rr.mu.Unlock() @@ -843,6 +845,7 @@ func (rr *ResourceRegistry) ingestResources(resources []Resource, thresholds map rr.refreshStoragePostureLocked() rr.refreshIncidentRollupsLocked() rr.buildChildCounts() + rr.refreshProxmoxSensorSetupLocked() rr.refreshCanonicalIdentitiesLocked() rr.markStaleLocked(time.Now().UTC(), thresholds) rr.invalidateViewsLocked() diff --git a/internal/unifiedresources/registry_test.go b/internal/unifiedresources/registry_test.go index 7c98c19bca..f619ec2cb3 100644 --- a/internal/unifiedresources/registry_test.go +++ b/internal/unifiedresources/registry_test.go @@ -7144,3 +7144,248 @@ func TestPhysicalDiskRiskNeverJudgesTemperature(t *testing.T) { t.Fatalf("meta recompute risk = %+v, want none for a collected 72C", risk) } } + +// The Proxmox page's Overview lists nodes without their disk inventory, so the +// outdated sensor setup verdict has to arrive on the node itself. The registry +// derives it from the node's legacy-format temperature payload and the disks +// it parents, with the same reading rule every disk temperature consumer uses. +func TestProxmoxNodeSensorSetupOutdatedFromItsDisks(t *testing.T) { + now := time.Now() + legacy := &models.Temperature{Available: true, LegacySensorsFormat: true} + inventoryOnly := &diskinventory.CollectionStatus{ + Temperature: diskinventory.Unsupported("proxmox_disks", "Proxmox disk inventory does not expose temperature"), + } + node := func(name string, temperature *models.Temperature) models.Node { + return models.Node{ + ID: "homelab-" + name, + Name: name, + Instance: "homelab", + Status: "online", + LastSeen: now, + Temperature: temperature, + } + } + disk := func(nodeName, devPath, diskType string, temperature int, collection *diskinventory.CollectionStatus) models.PhysicalDisk { + return models.PhysicalDisk{ + ID: ProxmoxPhysicalDiskSourceID("homelab", nodeName, devPath, "", ""), + Node: nodeName, + Instance: "homelab", + DevPath: devPath, + Model: "Disk " + devPath, + Serial: "SERIAL-" + nodeName + devPath, + Type: diskType, + Health: "PASSED", + Wearout: -1, + Temperature: temperature, + Collection: collection, + LastChecked: now, + } + } + + cases := []struct { + name string + nodes []models.Node + disks []models.PhysicalDisk + want bool + }{ + { + name: "SATA disk without a temperature", + nodes: []models.Node{node("pve", legacy)}, + disks: []models.PhysicalDisk{disk("pve", "/dev/sda", "sata", 0, inventoryOnly)}, + want: true, + }, + { + // Proxmox's own disk inventory types these disks by form factor. + name: "Proxmox hdd and ssd inventory types", + nodes: []models.Node{node("pve", legacy)}, + disks: []models.PhysicalDisk{disk("pve", "/dev/sda", "hdd", 0, inventoryOnly), disk("pve", "/dev/sdb", "ssd", 0, inventoryOnly)}, + want: true, + }, + { + name: "SAS disk whose reading is retained, not current", + nodes: []models.Node{node("pve", legacy)}, + disks: []models.PhysicalDisk{disk("pve", "/dev/sda", "sas", 41, &diskinventory.CollectionStatus{ + Temperature: diskinventory.Unavailable("host_agent", "host agent stopped reporting"), + })}, + want: true, + }, + { + name: "SATA disk whose reading the source marks missing", + nodes: []models.Node{node("pve", legacy)}, + disks: []models.PhysicalDisk{disk("pve", "/dev/sda", "sata", 38, &diskinventory.CollectionStatus{ + Temperature: diskinventory.Missing("proxmox_node_smart", "temperature was not reported"), + })}, + want: true, + }, + { + name: "SATA disk with a current reading", + nodes: []models.Node{node("pve", legacy)}, + disks: []models.PhysicalDisk{disk("pve", "/dev/sda", "sata", 36, &diskinventory.CollectionStatus{ + Temperature: diskinventory.Available("proxmox_node_smart"), + })}, + want: false, + }, + { + name: "reading from a source that predates collection state", + nodes: []models.Node{node("pve", legacy)}, + disks: []models.PhysicalDisk{disk("pve", "/dev/sda", "sata", 36, nil)}, + want: false, + }, + { + // NVMe temperatures arrive through kernel hwmon even on a legacy setup. + name: "NVMe disk without a temperature", + nodes: []models.Node{node("pve", legacy)}, + disks: []models.PhysicalDisk{disk("pve", "/dev/nvme0n1", "nvme", 0, inventoryOnly)}, + want: false, + }, + { + name: "current setup payload", + nodes: []models.Node{node("pve", &models.Temperature{Available: true})}, + disks: []models.PhysicalDisk{disk("pve", "/dev/sda", "sata", 0, inventoryOnly)}, + want: false, + }, + { + name: "failed temperature collection", + nodes: []models.Node{node("pve", &models.Temperature{Available: false, LegacySensorsFormat: true})}, + disks: []models.PhysicalDisk{disk("pve", "/dev/sda", "sata", 0, inventoryOnly)}, + want: false, + }, + { + name: "waiting disk belongs to another node", + nodes: []models.Node{node("pve", legacy), node("pve2", &models.Temperature{Available: true})}, + disks: []models.PhysicalDisk{disk("pve2", "/dev/sda", "sata", 0, inventoryOnly)}, + want: false, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + registry := NewRegistry(nil) + registry.IngestSnapshot(models.StateSnapshot{Nodes: tc.nodes, PhysicalDisks: tc.disks}) + got := proxmoxNodeSensorSetupOutdated(t, registry.List(), "pve") + if got != tc.want { + t.Fatalf("pve sensorSetupOutdated = %v, want %v", got, tc.want) + } + }) + } +} + +// A linked host agent that reads the disk's SMART temperature itself answers +// what the legacy SSH setup cannot, so the node is not flagged; the verdict is +// derived on every ingest, so a later snapshot clears an earlier flag. +func TestProxmoxNodeSensorSetupOutdatedFollowsWhereTheReadingArrives(t *testing.T) { + now := time.Now() + nodes := []models.Node{{ + ID: "homelab-pve", + Name: "pve", + Instance: "homelab", + LinkedAgentID: "host-pve", + Status: "online", + LastSeen: now, + Temperature: &models.Temperature{Available: true, LegacySensorsFormat: true}, + }} + inventory := []models.PhysicalDisk{{ + ID: ProxmoxPhysicalDiskSourceID("homelab", "pve", "/dev/sda", "", ""), + Node: "pve", + Instance: "homelab", + DevPath: "/dev/sda", + Model: "WDC WD80EFAX", + Serial: "SERIAL-SDA", + Type: "hdd", + Health: "PASSED", + Wearout: -1, + Collection: &diskinventory.CollectionStatus{Temperature: diskinventory.Unsupported("proxmox_disks", "Proxmox disk inventory does not expose temperature")}, + LastChecked: now, + }} + agent := func(collection *diskinventory.CollectionStatus) []models.Host { + return []models.Host{{ + ID: "host-pve", + Hostname: "pve", + LinkedNodeID: "homelab-pve", + Status: "online", + LastSeen: now, + Sensors: models.HostSensorSummary{SMART: []models.HostDiskSMART{{ + Device: "/dev/sda", + Model: "WDC WD80EFAX", + Serial: "SERIAL-SDA", + Type: "sata", + Temperature: 34, + Health: "PASSED", + Collection: collection, + }}}, + }} + } + + registry := NewRegistry(nil) + registry.IngestSnapshot(models.StateSnapshot{Nodes: nodes, PhysicalDisks: inventory}) + if !proxmoxNodeSensorSetupOutdated(t, registry.List(), "pve") { + t.Fatal("legacy node with an unread hdd: sensorSetupOutdated = false, want true") + } + + registry.IngestSnapshot(models.StateSnapshot{ + Nodes: nodes, + PhysicalDisks: inventory, + Hosts: agent(&diskinventory.CollectionStatus{Temperature: diskinventory.Available("host_agent")}), + }) + if proxmoxNodeSensorSetupOutdated(t, registry.List(), "pve") { + t.Fatal("linked agent reads the disk now: sensorSetupOutdated = true, want the earlier flag cleared") + } + // Clients merge a facet field by field, so the cleared verdict must be on + // the wire as false, not omitted. + assertProxmoxNodeSensorSetupJSON(t, registry.ListForPresentation(), "pve", `"sensorSetupOutdated":false`) + + registry.IngestSnapshot(models.StateSnapshot{ + Nodes: nodes, + PhysicalDisks: inventory, + Hosts: agent(&diskinventory.CollectionStatus{Temperature: diskinventory.Unavailable("host_agent", "host agent stopped reporting")}), + }) + presented := registry.ListForPresentation() + if !proxmoxNodeSensorSetupOutdated(t, presented, "pve") { + t.Fatal("agent reading retained, not current: sensorSetupOutdated = false, want true") + } + assertProxmoxNodeSensorSetupJSON(t, presented, "pve", `"sensorSetupOutdated":true`) +} + +func assertProxmoxNodeSensorSetupJSON(t *testing.T, resources []Resource, nodeName, want string) { + t.Helper() + for _, resource := range resources { + if resource.Proxmox == nil || resource.Proxmox.NodeName != nodeName || CanonicalResourceType(resource.Type) != ResourceTypeAgent { + continue + } + payload, err := json.Marshal(resource.Proxmox) + if err != nil { + t.Fatalf("marshal node proxmox facet: %v", err) + } + if !strings.Contains(string(payload), want) { + t.Fatalf("node proxmox payload = %s, want %s for list readers", payload, want) + } + return + } + t.Fatalf("node %q not presented", nodeName) +} + +func proxmoxNodeSensorSetupOutdated(t *testing.T, resources []Resource, nodeName string) bool { + t.Helper() + var found *Resource + for i := range resources { + resource := &resources[i] + if CanonicalResourceType(resource.Type) != ResourceTypeAgent || resource.Proxmox == nil || resource.Proxmox.NodeName != nodeName { + continue + } + if found != nil { + t.Fatalf("node %q presented twice: %s and %s", nodeName, found.ID, resource.ID) + } + found = resource + } + if found == nil { + t.Fatalf("node %q not found among %d resources", nodeName, len(resources)) + } + for _, resource := range resources { + if CanonicalResourceType(resource.Type) != ResourceTypeAgent && resource.Proxmox != nil && resource.Proxmox.SensorSetupOutdated != nil { + t.Fatalf("%s %s carries the node verdict", resource.Type, resource.ID) + } + } + if found.Proxmox.SensorSetupOutdated == nil { + t.Fatalf("node %q has no explicit sensor setup verdict", nodeName) + } + return *found.Proxmox.SensorSetupOutdated +} diff --git a/internal/unifiedresources/types.go b/internal/unifiedresources/types.go index fe8acee7c7..d54c5e831b 100644 --- a/internal/unifiedresources/types.go +++ b/internal/unifiedresources/types.go @@ -414,6 +414,14 @@ type ProxmoxData struct { MemoryCache int64 `json:"memoryCache,omitempty"` Memory *models.Memory `json:"memory,omitempty"` Lock string `json:"lock,omitempty"` // Proxmox lock state (e.g. "backup", "migrate", "snapshot") + // SensorSetupOutdated is derived by the registry, never ingested: the + // node's SSH temperature monitoring still runs the pre-rc.6 setup and a + // disk under it waits on a SMART temperature that setup cannot deliver + // (refreshProxmoxSensorSetupLocked). Every Proxmox node carries an + // explicit true or false, so clients that merge facets field by field + // replace a cleared verdict instead of keeping an earlier true; other + // resources omit it. + SensorSetupOutdated *bool `json:"sensorSetupOutdated,omitempty"` // Internal link hint to a host agent resource. LinkedAgentID string `json:"-"` }