From 73a2d68cf91d011597ba9630539746c0cafd2dd4 Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Wed, 7 Oct 2026 12:08:03 +0100 Subject: [PATCH] Pin disk heat parity for canonical and linked-guest overrides A second review of the per-host Disk Temp change found two gaps. The parity test did not cover an override stored under the canonical resource an agent ID resolves to, or one inherited from a linked guest. The documented browser limits also listed only two cases where the browser cannot see CheckHost's chain. The parity test now checks both overrides, and a disabled linked guest, against what CheckHost fires. storage-recovery.md lists every case where the browser's keys can differ from alerts and Patrol: manually merged agent and VM disks on the Storage page, overrides inherited through unpublished link hints or stored under a linked node's canonical ID, and guest keys for an agent merged into a guest it is not automatically linked to. --- .../v6/internal/subsystems/alerts.md | 6 ++- .../internal/subsystems/storage-recovery.md | 18 ++++++--- .../threshold_resolution_shared_test.go | 38 +++++++++++++++++-- 3 files changed, 51 insertions(+), 11 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index 887fbc1521..d7d2e973e5 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -1780,14 +1780,16 @@ override on that host, or one inherited from its linked node or guest, replaces the per-type entry, the first override in that chain decides even when it sets no Disk Temp, and an override that switches the host's alerts off leaves no threshold. `TestHostDiskTemperatureThresholdMatchesCheckHostOverrides` pins each -case against what `CheckHost` fires. `Manager.DiskTemperatureThreshold(diskType)` +case against what `CheckHost` fires, including an override under the canonical +resource the agent ID resolves to and one inherited from a linked guest. `Manager.DiskTemperatureThreshold(diskType)` is the hostless case. Patrol reads the policy through `AlertThresholdAdapter.GetDiskTemperatureThreshold`. `DefaultDiskTemperatureThreshold` serves callers with no manager. Disk risk (`internal/storagehealth`) judges no temperature. The Physical Disks Temp cell and Health verdict mirror the policy in `resolveDiskTemperatureDisplayThresholds` under the override keys of the machine whose agent reports the disk, in the -order `CheckHost` reads them. That resolver returns null when the agent Disk +order `CheckHost` reads them as far as the browser can see that chain +(`storage-recovery.md` lists where it cannot). That resolver returns null when the agent Disk Temp default is off or an override or agent default switches the host's alerts off. The global alerts switch and the agent alert-type switch stop `CheckHost` without changing the policy. diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index ebe1c40537..7ad1e5341b 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -6799,12 +6799,18 @@ judges the current reading, with no memory of an alert. A disk temperature alert held between its clear value and trigger shows an amber Temp cell, not `Running Hot`. A switched-off agent Disk Temp default resolves to null, which leaves the reading uncoloured and never hot, as alerting is off for it. -Two cases stay on the per-type trigger in the browser while alerts and Patrol -apply the override: on the Storage page, a disk whose agent an operator -manually linked into a VM (the page loads agents and pools, not guests; the -guest drawer applies it), and an override an agent inherits from a guest it is -linked to without a merge, because agent link hints are not published to the -browser. PDF performance +The browser rebuilds that chain from what it loads, so it can differ from +alerts and Patrol, which read the agent's own link hints and resolve every key +to its canonical resource. On the Storage page, a disk whose agent an operator +manually linked into a VM keeps the per-type trigger, because the page loads +agents and pools, not guests. An override the agent inherits from a node or +guest it is linked to without a merge, or one stored under that node's own +canonical resource ID, does not reach the browser, because agent link hints +are not published. The guest drawer applies the guest's own override keys, +which `CheckHost` reads only when the agent is also linked to that guest +automatically. Overrides set on the reporting agent's own row, or on the node +or guest it is merged with, reach the browser through the keys above. PDF +performance reports colour a disk reading by the same per-type thresholds, which `reporting.DiskInfo` carries. `frontend-modern/src/features/storageBackups/__tests__/diskPresentation.test.ts` diff --git a/internal/alerts/threshold_resolution_shared_test.go b/internal/alerts/threshold_resolution_shared_test.go index f459cd3cef..cf040367e4 100644 --- a/internal/alerts/threshold_resolution_shared_test.go +++ b/internal/alerts/threshold_resolution_shared_test.go @@ -1448,18 +1448,23 @@ func TestDiskTemperatureThresholdMatchesCheckHostPolicy(t *testing.T) { // HostDiskTemperatureThreshold is the disk heat policy for a disk a host agent // reports, so it must agree with CheckHost on every host-level override: an -// explicit Disk Temp override on the host, or inherited from its linked node, -// replaces the per-type entry; the first override in the chain decides even -// when it sets no Disk Temp; and a host whose alerts are off judges no heat. +// explicit Disk Temp override on the host (or the canonical resource its ID +// resolves to), or inherited from its linked node or guest, replaces the +// per-type entry; the first override in the chain decides even when it sets no +// Disk Temp; and a host whose alerts are off judges no heat. func TestHostDiskTemperatureThresholdMatchesCheckHostOverrides(t *testing.T) { const hostID = "host-temp-policy" const nodeID = "pve1-node1" + const vmID = "pve1-node1-105" + const canonicalID = "agent-canonical-temp-policy" perType := &HysteresisThreshold{Trigger: 70, Clear: 65} for _, tc := range []struct { name string overrides map[string]ThresholdConfig linkNode bool + linkVM bool + canonical bool want *HysteresisThreshold off bool }{ @@ -1489,6 +1494,24 @@ func TestHostDiskTemperatureThresholdMatchesCheckHostOverrides(t *testing.T) { linkNode: true, want: perType, }, + { + // The agent ID resolves to the machine's canonical resource ID. + name: "canonical machine disk temp override", + overrides: map[string]ThresholdConfig{canonicalID: {DiskTemperature: &HysteresisThreshold{Trigger: 82, Clear: 77}}}, + canonical: true, + want: &HysteresisThreshold{Trigger: 82, Clear: 77}, + }, + { + name: "linked guest disk temp override", + overrides: map[string]ThresholdConfig{vmID: {DiskTemperature: &HysteresisThreshold{Trigger: 76, Clear: 71}}}, + linkVM: true, + want: &HysteresisThreshold{Trigger: 76, Clear: 71}, + }, + { + name: "linked guest alerts disabled", + overrides: map[string]ThresholdConfig{vmID: {Disabled: true}}, + linkVM: true, + }, { name: "host disk temp switched off", overrides: map[string]ThresholdConfig{hostID: {DiskTemperature: &HysteresisThreshold{}}}, @@ -1511,11 +1534,19 @@ func TestHostDiskTemperatureThresholdMatchesCheckHostOverrides(t *testing.T) { m.config.Overrides[id] = override } m.mu.Unlock() + if tc.canonical { + m.SetResourceIntentIdentityResolver(func(resourceID string) (string, bool) { + return canonicalID, resourceID == hostID + }) + } owner := DiskTemperatureHost{ID: hostID} if tc.linkNode { owner.LinkedNodeID = nodeID } + if tc.linkVM { + owner.LinkedVMID = vmID + } got := m.HostDiskTemperatureThreshold(owner, "nvme") switch { case tc.off: @@ -1529,6 +1560,7 @@ func TestHostDiskTemperatureThresholdMatchesCheckHostOverrides(t *testing.T) { check := func(temperature int) bool { host := hostWithSMARTDiskTemp(hostID, "nvme", temperature) host.LinkedNodeID = owner.LinkedNodeID + host.LinkedVMID = owner.LinkedVMID m.CheckHost(host) _, exists := testLookupActiveAlert(t, m, hostDiskTempAlertID(host)) return exists