Keep a split PVE disk off the agent's SMART row

Splitting a merged physical disk (the drawer's "Split merged resource",
POST /api/resources/{id}/report-merge) recorded its exclusions and the
registry kept two rows, but the PVE disk poller had already paired the
linked agent's SMART row with the Proxmox disk by WWN, serial or path
before the registry saw either, and read no exclusions. The split Proxmox
row still carried the agent's attributes, I/O, health, any serial, WWN,
type or temperature it filled, and AgentSMARTReported, which hands the
disk's temperature alert to the agent's CheckHost. #2748 listed this as
the part of the split it did not reach.

mergeHostAgentSMARTIntoDisks now asks the resource store
(MonitorAdapter.ProxmoxDiskAgentSMARTSplit) once per matched row. The
answer is physicalDiskSplitLocked's, applied to the agent's disk as the
registry generation holds it and to the Proxmox observation under its own
ID, so the exclusions that keep the registry's rows apart keep the
poller's pair apart: an exclusion naming only the agent's candidate
splits nothing, a replacement in the slot inherits no split recorded
against the disk it replaced, and a manual link that leaves the
generation holding both observations as one disk pairs them again.

A split disk takes nothing from the row. That includes the node-sensor
temperature the agent supplied: the node poll takes its SMART list from a
reporting agent whose report carries a usable reading, so that reading is
the agent's row of the other disk; an SSH-supplied reading stays and the
PVE disk check judges it. The record is marked AgentSMARTSplit, so the
poll's evidence retention restores nothing into it, and the
Proxmox-query-failure fallback gives it no stand-in row.

The split applies on the first full disk poll that collects the node from
Proxmox and matches the agent's row; the unified-resources contract names
that lag and the other limits.
This commit is contained in:
rcourtman
2026-10-10 10:02:34 +01:00
parent cd3904c89f
commit ec50758974
14 changed files with 1165 additions and 59 deletions
@@ -401,6 +401,20 @@ wire contract and Agent authority are unchanged;
`internal/monitoring/physical_disk_roundtrip_test.go` and
`TestPhysicalDiskAgentSMARTReportedStaysInternal` in
`internal/models/deepcopy_test.go` pin the marker.
A PVE disk the operator split from the Agent's disk (report-merge on the merged
disk) is paired with no SMART row of the linked Agent and, from the first full
disk poll that matches the Agent's row, shows none of the Agent's temperature,
including the node sensor reading the node poll took from that Agent's report
while it is reporting, so the Agent's `CheckHost` owns the temperature alert of
the Agent's own disk only and the PVE disk check has no Agent-supplied reading
of the Proxmox disk to alert on beside it: the split disk is marked `AgentSMARTSplit` (internal poll
evidence on `models.PhysicalDisk`, never serialized) instead of
`AgentSMARTReported`. Report admission, the SMART wire contract and Agent
authority are unchanged: the Agent still reports the row, and the split only
decides whose disk it is on the server;
`TestOperatorSplitKeepsAgentSMARTOffTheProxmoxDisk` in
`internal/monitoring/physical_disk_roundtrip_test.go` and
`TestPhysicalDiskAgentSMARTReportedStaysInternal` pin it.
Assistant historical metric wiring uses the current monitor's retained store
and registry metrics coordinates. Historical reads do not alter enrollment,
@@ -2434,6 +2434,48 @@ merge, so on a mock estate every hot disk on an online node alerts.
`internal/monitoring/physical_disk_roundtrip_test.go` pin the marker,
the collected reading, agent ownership and exclusion.
A disk the operator split from the agent's disk (report-merge on the merged
disk) is not paired with the agent's row. `mergeHostAgentSMARTIntoDisks` asks
the resource store, through `diskAgentSplitStore`
(`unifiedresources.MonitorAdapter.ProxmoxDiskAgentSMARTSplit`), once per disk
it matches a row to, on the full poll and on the skipped polls between, and a
split disk takes nothing from the row: no attributes, I/O, health, identity,
type or temperature, and no `AgentSMARTReported`. It takes no agent-supplied
node-sensor temperature either: the node poll takes the node's SMART list from
the linked agent when the agent's report carries a usable temperature
(`mergeTemperatureData`; `hostSuppliesNodeSMARTTemperatures` mirrors that
rule), and its legacy NVMe list is matched by order, so
`dropNodeSensorTemperature` removes a reading `mergeNVMeTempsIntoDisks` took
from either, leaving the PVE disk check no such reading to judge beside the
agent's own alert, while a reading from the SSH collector's list stays (the
rule needs the agent still reporting). The
split disk is still evaluated by `checkPhysicalDiskAlerts` whenever its node
is polled. It is marked
`AgentSMARTSplit` (internal poll evidence, never serialized), so the poll's
retention of unavailable evidence restores nothing into it. The decider is
asked about the observation under its own ID, as the registry holds it. The
Proxmox-query-failure fallback omits the rows the operator split from the disk
in their slot, so while the query fails that disk's Proxmox record is not
listed (a node every row of which is split keeps its last records, and is not
evaluated by `checkPhysicalDiskAlerts`, as any unqueried node). A store that holds no
decisions, or none for the pair, leaves the merge as it was. The decision
comes from the registry generation the monitor last built, so a split applies
from the first full disk poll that collects the node from Proxmox and matches
the agent's row after the rebuild that loaded it; the skipped polls before then
mark the record and drop its node-sensor temperature but leave what an earlier
poll merged into it.
`TestMergeHostAgentSMARTIntoDisksLeavesDisksTheOperatorSplitAlone`,
`TestHostAgentSMARTFallbackSkipsRowsTheOperatorSplit`,
`TestFailedDiskQueryListsOnlyTheRowsTheOperatorDidNotSplit`,
`TestMergeHostAgentSMARTIntoDisksKeepsSSHNodeSensorReadingOfSplitDisk`,
`TestDropNodeSensorTemperatureRemovesOnlyAgentSuppliedReadings`,
`TestMergeNVMeTempsLabelsReadingsWithTheNodeSensorSources`,
`TestHostSuppliesNodeSMARTTemperaturesFollowsUsableReadings` and
`TestOperatorSplitKeepsAgentSMARTOffTheProxmoxDisk` in
`internal/monitoring/physical_disk_roundtrip_test.go` pin the merge, the
fallback and the poller end to end, and `TestProxmoxDiskAlertsRunOnMergedDiskState`
in `canonical_guardrails_test.go` pins the signature and call.
Switching mock mode fences the alert evaluations that read mode-dependent data
(`mockModeFence`, `internal/monitoring/mock_mode_fence.go`). `GetState`, the
fixture graph, the unified read view, the recovery rollups and the connection
@@ -7252,13 +7252,68 @@ every merged source. The monitor rebuild and a resources API registry built from
snapshot apply the split on their next build; the registry seeded from the
monitor's listing follows once the monitor has rebuilt.
What the split does not reach. The PVE disk poller's
`mergeHostAgentSMARTIntoDisks` pairs the linked agent's SMART rows with
Proxmox disks by WWN, serial or path before the registry sees them and reads
no exclusions, so the split Proxmox row still carries the agent's SMART
attributes and I/O, its health where it reports a failure or Proxmox none,
and any serial, WWN, type or temperature it filled, and the agent keeps the
disk's temperature alert while it reports (`AgentSMARTReported`). Two split
The PVE disk poller honours the split too. `mergeHostAgentSMARTIntoDisks`
pairs the linked agent's SMART rows with Proxmox disks by WWN, serial or path
before the registry sees either, and asks
`MonitorAdapter.ProxmoxDiskAgentSMARTSplit`
(`internal/unifiedresources/physical_disk_agent_split.go`) about each pair it
is about to make. The answer is `physicalDiskSplitLocked`'s, applied to the
agent's disk as the registry generation holds it (as its row would key it
when the generation does not hold it) and to the Proxmox observation by its
slot's source-specific candidate ID, so the exclusions that keep the
registry's rows apart keep the poller's pair apart, in the same forms: one
naming only the agent's candidate splits nothing, a replacement in the slot
inherits no split recorded against the disk it replaced, and a manual link
that leaves the generation holding both observations as one disk is the
operator's later decision, so the poller pairs them again while it stands. The
decision reads the exclusions and that mapping, never what the pairing would
fill into the Proxmox row, whose serial it re-keys. A split Proxmox row takes
nothing from the agent's row: not its SMART attributes or I/O, health, serial,
WWN, type, controller, pool or temperature, whether from the row or from the
node's sensor lists. The node poll takes the SMART list from the linked agent
while the agent is reporting and its report carries a usable temperature (the
SSH collector's otherwise), and `mergeNVMeTempsIntoDisks` matches a list's rows
to the disk by the same identity, so `dropNodeSensorTemperature` drops a
reading taken from the agent's list, and the legacy NVMe list's guess by order,
which names no disk, while a reading from the SSH collector's list stays. The
rule is read from the agent's report as the disk poll sees it, so a report that
changed since the node poll can misjudge one poll. Nor does the disk
keep the agent's claim on its temperature alert (`AgentSMARTReported`): the
agent's `CheckHost` alerts on the agent's own disk and the PVE disk check has
no agent-supplied reading of the split disk to judge, so one reading never
alerts twice, and Proxmox's own health and wearout verdicts of the disk are
evaluated as for any disk. A refused row never makes another row match. The
record is marked `AgentSMARTSplit`, so the poll's evidence retention
(`preserveUnavailablePhysicalDiskEvidence`) restores nothing into it: nothing
records which retained value Proxmox collected and which the agent filled, so
its serial or ZFS pool is absent for a poll that cannot collect it and returns
with the next that can, and a Proxmox serial that goes unreported for a poll
can move the disk to a WWN- or slot-keyed ID, and its history to that key,
until it returns. The decider is asked once per matched row, about the
observation under its own ID, the key the registry holds it by, so the poller
follows the registry exactly: a split recorded under another key of the same
disk (a controller member's stand-in is keyed by its target, a Proxmox poll's
record by device alone) names an observation the other key's rows do not
carry, as for the registry. When the Proxmox disk query fails,
`physicalDisksFromHostAgentSMART` stands in for the node's disks except for
the rows the operator split from the Proxmox disk in their slot, judged under
the stand-in's own ID. Such a disk gets no stand-in, which would be the
agent's disk again, so while the query fails its Proxmox record is not listed
unless every row of the node is split, when the node keeps its last records as
any node Proxmox cannot be queried for does.
What the split does not reach. The poller applies a split on the first full
disk poll that collects the node from Proxmox and matches the agent's row
after the registry generation holds it. Until then a Proxmox record already
merged keeps the agent's data (the next full poll, five minutes by default,
`PhysicalDiskPollingMinutes`): the skipped polls between mark the record split
and drop its node-sensor temperature but leave the rest as it is, a node
Proxmox cannot be queried for keeps its last records, and a poll in which
the agent does not list the disk marks nothing, so retention restores the
earlier record's temperature, I/O, controller, target, serial and pool as
last-known evidence. The query-failure fallback judges its stand-in row by the
row's own identity, so it does not apply a split that names identity only the
Proxmox disk carries (an agent disk in standby reports no serial). Two split
rows that share a serial read one serial-keyed history
(`PhysicalDiskMetaMetricID`), and a PVE disk alert lifecycle row whose
recorded identity matches both, with no WWN only one of them shares, stays
@@ -7287,7 +7342,13 @@ the rebuilt listing or built from the snapshot, plain and for presentation;
then a relink of the split rows, the exclusions report-merge records on the
relinked disk (the link's own pair and both candidates), the agent's disk in
standby, a same-serial disk appearing on another machine, and a split
recorded while such a disk was present that holds once it leaves.
recorded while such a disk was present that holds once it leaves. At each of
those steps the test also asks `ProxmoxDiskAgentSMARTSplit` what the poller
would, and `TestOperatorSplitKeepsAgentSMARTOffTheProxmoxDisk` in
`internal/monitoring/physical_disk_roundtrip_test.go` drives the poller itself
through the same four requests for a SAS disk paired by path and an NVMe disk
paired by serial, with an open PVE temperature alert, the health alert the
agent's failure raised, and a failing Proxmox disk query.
### Seeded slot mapping refuses a replaced disk
+12 -9
View File
@@ -515,21 +515,24 @@ func TestCloneHostXCPNGInventoryIsolation(t *testing.T) {
}
}
// AgentSMARTReported is internal poll evidence: a state snapshot copy keeps it
// for the alert check, and it never reaches an API or websocket payload.
// AgentSMARTReported and AgentSMARTSplit are internal poll evidence: a state
// snapshot copy keeps them for the alert check and the poll's evidence
// retention, and neither reaches an API or websocket payload.
func TestPhysicalDiskAgentSMARTReportedStaysInternal(t *testing.T) {
disk := PhysicalDisk{ID: "lab-pve1-sda", DevPath: "/dev/sda", AgentSMARTReported: true}
if clone := clonePhysicalDisk(disk); !clone.AgentSMARTReported {
t.Fatalf("clone dropped AgentSMARTReported")
disk := PhysicalDisk{ID: "lab-pve1-sda", DevPath: "/dev/sda", AgentSMARTReported: true, AgentSMARTSplit: true}
if clone := clonePhysicalDisk(disk); !clone.AgentSMARTReported || !clone.AgentSMARTSplit {
t.Fatalf("clone dropped AgentSMARTReported/AgentSMARTSplit: %+v", clone)
}
if clones := clonePhysicalDisks([]PhysicalDisk{disk}); !clones[0].AgentSMARTReported {
t.Fatalf("slice clone dropped AgentSMARTReported")
if clones := clonePhysicalDisks([]PhysicalDisk{disk}); !clones[0].AgentSMARTReported || !clones[0].AgentSMARTSplit {
t.Fatalf("slice clone dropped AgentSMARTReported/AgentSMARTSplit: %+v", clones[0])
}
payload, err := json.Marshal(disk)
if err != nil {
t.Fatal(err)
}
if strings.Contains(strings.ToLower(string(payload)), "agentsmartreported") {
t.Fatalf("AgentSMARTReported leaked into JSON: %s", payload)
for _, field := range []string{"agentsmartreported", "agentsmartsplit"} {
if strings.Contains(strings.ToLower(string(payload)), field) {
t.Fatalf("%s leaked into JSON: %s", field, payload)
}
}
}
+7 -2
View File
@@ -2859,8 +2859,13 @@ type PhysicalDisk struct {
// AgentSMARTReported marks a disk a linked host agent lists in its SMART
// report, so the agent's CheckHost owns its temperature alert. Internal
// poll evidence, set by the PVE disk poller's agent SMART merge.
AgentSMARTReported bool `json:"-"`
LastChecked time.Time `json:"lastChecked"`
AgentSMARTReported bool `json:"-"`
// AgentSMARTSplit marks a disk the operator split from the disk a linked
// host agent's SMART row describes, so the PVE disk poller pairs neither
// that row nor an earlier poll's record, which held its readings, with
// it. Internal poll evidence, set by the same merge.
AgentSMARTSplit bool `json:"-"`
LastChecked time.Time `json:"lastChecked"`
}
// PBSInstance represents a Proxmox Backup Server instance
@@ -2268,7 +2268,7 @@ func TestProxmoxDiskAlertsRunOnMergedDiskState(t *testing.T) {
{
file: "monitor.go",
snippets: []string{
"func mergeHostAgentSMARTIntoDisks(disks []models.PhysicalDisk, nodes []models.Node, hosts []models.Host) []models.PhysicalDisk {",
"func mergeHostAgentSMARTIntoDisks(disks []models.PhysicalDisk, nodes []models.Node, hosts []models.Host, splits diskAgentSplitStore) []models.PhysicalDisk {",
"deriveWearoutFromSMARTAttributes(matched.Attributes)",
"storagehealth.RemainingLifeFromPercentageUsed(*attrs.PercentageUsed)",
"shouldUseHostAgentPhysicalDiskHealth(updated[i].Health, matched.Health)",
@@ -2278,7 +2278,7 @@ func TestProxmoxDiskAlertsRunOnMergedDiskState(t *testing.T) {
{
file: "monitor_pve.go",
snippets: []string{
"allDisks = mergeHostAgentSMARTIntoDisks(allDisks, nodesFromState, hosts)",
"allDisks = mergeHostAgentSMARTIntoDisks(allDisks, nodesFromState, hosts, splits)",
"m.checkPhysicalDiskAlerts(inst, disk, diskExcludeByNode[disk.Node])",
"m.alertManager.CheckDiskHealth(instance, disk.Node, proxmoxDiskFromPhysicalDisk(disk))",
"func proxmoxDiskFromPhysicalDisk(disk models.PhysicalDisk) proxmox.Disk {",
@@ -148,7 +148,7 @@ func TestIssue1595SASTopologySurvivesMergeRegistryAndReadState(t *testing.T) {
}
}
merged := mergeHostAgentSMARTIntoDisks(providerDisks, []models.Node{node}, []models.Host{host})
merged := mergeHostAgentSMARTIntoDisks(providerDisks, []models.Node{node}, []models.Host{host}, nil)
if len(merged) != 24 {
t.Fatalf("merged disk count = %d, want 24", len(merged))
}
@@ -286,6 +286,7 @@ func TestTrustedSMARTSerialPromotionDoesNotRewriteSATAOrNVMeIdentity(t *testing.
disks,
[]models.Node{{Name: "node", LinkedAgentID: "agent"}},
hosts,
nil,
)[0]
if got.Serial != "PROVIDER-SERIAL" || got.Type != diskType {
t.Fatalf("%s identity changed during SAS remediation: %+v", diskType, got)
@@ -948,6 +949,7 @@ func TestHostAgentSMARTMergeRefusesPathMatchAcrossContradictingIdentity(t *testi
[]models.Host{{ID: "agent-1", Sensors: models.HostSensorSummary{
SMART: append([]models.HostDiskSMART{tc.smart}, tc.others...),
}}},
nil,
)[0]
if !tc.merged {
if got.Serial != tc.disk.Serial || got.WWN != tc.disk.WWN || got.Type != tc.disk.Type ||
+2 -2
View File
@@ -394,7 +394,7 @@ func TestMergeHostAgentSMARTIntoDisks_EnrichesMissingIdentityAndZFSPool(t *testi
},
}
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts)
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts, nil)
if len(result) != 1 {
t.Fatalf("got %d disks, want 1", len(result))
}
@@ -465,7 +465,7 @@ func TestMergeHostAgentSMARTIntoDisks_PreservesExistingProxmoxIdentity(t *testin
},
}
got := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts)[0]
got := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts, nil)[0]
if got.Model != "Proxmox Model" || got.Serial != "PVE-SERIAL" || got.WWN != "0x5000c500a1b2c3d4" || got.Type != "sas" {
t.Fatalf("existing Proxmox identity was overwritten: %+v", got)
}
+91 -6
View File
@@ -390,7 +390,7 @@ func mergeNVMeTempsIntoDisks(disks []models.PhysicalDisk, nodes []models.Node) [
return diskinventory.IsUsableHardwareID(candidate.WWN) &&
strings.EqualFold(candidate.WWN, updated[i].WWN)
}); temp != nil && temp.Temperature > 0 && !temp.StandbySkipped {
setPhysicalDiskTemperature(&updated[i], temp.Temperature, "proxmox_node_smart")
setPhysicalDiskTemperature(&updated[i], temp.Temperature, proxmoxNodeSMARTTemperatureSource)
log.Debug().
Str("disk", updated[i].DevPath).
Str("wwn", updated[i].WWN).
@@ -405,7 +405,7 @@ func mergeNVMeTempsIntoDisks(disks []models.PhysicalDisk, nodes []models.Node) [
return diskinventory.IsUsableHardwareID(candidate.Serial) &&
strings.EqualFold(candidate.Serial, updated[i].Serial)
}); temp != nil && temp.Temperature > 0 && !temp.StandbySkipped {
setPhysicalDiskTemperature(&updated[i], temp.Temperature, "proxmox_node_smart")
setPhysicalDiskTemperature(&updated[i], temp.Temperature, proxmoxNodeSMARTTemperatureSource)
log.Debug().
Str("disk", updated[i].DevPath).
Str("serial", updated[i].Serial).
@@ -421,7 +421,7 @@ func mergeNVMeTempsIntoDisks(disks []models.PhysicalDisk, nodes []models.Node) [
normalizedTempDev := normalizeSMARTDeviceIdentifier(candidate.Device)
return normalizedTempDev != "" && normalizedTempDev == normalizedDevPath
}); temp != nil && temp.Temperature > 0 && !temp.StandbySkipped {
setPhysicalDiskTemperature(&updated[i], temp.Temperature, "proxmox_node_smart")
setPhysicalDiskTemperature(&updated[i], temp.Temperature, proxmoxNodeSMARTTemperatureSource)
log.Debug().
Str("disk", updated[i].DevPath).
Int("temp", temp.Temperature).
@@ -458,7 +458,7 @@ func mergeNVMeTempsIntoDisks(disks []models.PhysicalDisk, nodes []models.Node) [
continue
}
setPhysicalDiskTemperature(&updated[diskIdx], int(math.Round(tempVal)), "proxmox_node_nvme")
setPhysicalDiskTemperature(&updated[diskIdx], int(math.Round(tempVal)), proxmoxNodeNVMeTemperatureSource)
log.Debug().
Str("disk", updated[diskIdx].DevPath).
Int("temp", updated[diskIdx].Temperature).
@@ -486,6 +486,55 @@ func uniqueDiskTemperatureMatch(temperatures []models.DiskTemp, predicate func(m
return &temperatures[matchIndex]
}
// Collection sources of a disk temperature mergeNVMeTempsIntoDisks took from
// the node's sensor lists.
const (
proxmoxNodeSMARTTemperatureSource = "proxmox_node_smart"
proxmoxNodeNVMeTemperatureSource = "proxmox_node_nvme"
)
// hostSuppliesNodeSMARTTemperatures reports whether the node's SMART sensor
// list is this host's report: the node poll takes the linked agent's list when
// it carries a usable temperature and the SSH collector's otherwise
// (mergeTemperatureData in host_agent_temps.go). An agent whose lease lapsed
// supplies nothing, so callers also require it to be reporting.
func hostSuppliesNodeSMARTTemperatures(host models.Host) bool {
for _, row := range host.Sensors.SMART {
if row.Temperature > 0 && !row.Standby {
return true
}
}
return false
}
// dropNodeSensorTemperature removes a reading mergeNVMeTempsIntoDisks took
// from the node's sensor lists, for a disk the operator split from the
// linked agent's. The SMART list carries rows of the agent's disks when the
// agent supplies it (smartFromAgent), so a reading from it is the agent's row
// of the disk this one was split from; an SSH-supplied list is the node's own
// and stays. The legacy NVMe list names no disk, only an order, so its guess
// is dropped either way. A reading from another source, which Proxmox itself
// never supplies, is not touched.
func dropNodeSensorTemperature(disk *models.PhysicalDisk, smartFromAgent bool) {
if disk.Collection == nil {
return
}
switch disk.Collection.Temperature.Source {
case proxmoxNodeSMARTTemperatureSource:
if !smartFromAgent {
return
}
case proxmoxNodeNVMeTemperatureSource:
default:
return
}
disk.Temperature = 0
// The copy shares its collection with the caller's slice.
collection := diskinventory.CloneStatus(disk.Collection)
collection.Temperature = diskinventory.Unsupported("proxmox_disks", "Proxmox disk inventory does not expose temperature")
disk.Collection = collection
}
func setPhysicalDiskTemperature(disk *models.PhysicalDisk, temperature int, source string) {
if disk == nil || temperature <= 0 {
return
@@ -497,10 +546,34 @@ func setPhysicalDiskTemperature(disk *models.PhysicalDisk, temperature int, sour
disk.Collection.Temperature = diskinventory.Available(source)
}
// diskAgentSplitStore is implemented by resource stores that hold the
// operator's merge exclusions (unifiedresources.MonitorAdapter). It reports
// whether the operator split a Proxmox disk from the disk a linked host
// agent reports in one SMART row.
type diskAgentSplitStore interface {
ProxmoxDiskAgentSMARTSplit(disk models.PhysicalDisk, host models.Host, smart models.HostDiskSMART) bool
}
// proxmoxDiskAgentSplits returns the resource store's split decisions, or nil
// when the store holds none.
func (m *Monitor) proxmoxDiskAgentSplits() diskAgentSplitStore {
if m == nil {
return nil
}
m.mu.RLock()
store := m.resourceStore
m.mu.RUnlock()
splits, _ := store.(diskAgentSplitStore)
return splits
}
// mergeHostAgentSMARTIntoDisks merges SMART temperature data from linked host agents
// into physical disks for Proxmox nodes. This allows disk temps collected by the
// pulse-agent running on a PVE node to populate the Physical Disks view.
func mergeHostAgentSMARTIntoDisks(disks []models.PhysicalDisk, nodes []models.Node, hosts []models.Host) []models.PhysicalDisk {
// pulse-agent running on a PVE node to populate the Physical Disks view. A disk
// the operator split from the agent's disk its matching row describes (splits)
// takes nothing from that row, nor the node-sensor reading that stands in for
// it, and is marked AgentSMARTSplit instead.
func mergeHostAgentSMARTIntoDisks(disks []models.PhysicalDisk, nodes []models.Node, hosts []models.Host, splits diskAgentSplitStore) []models.PhysicalDisk {
if len(disks) == 0 || len(nodes) == 0 || len(hosts) == 0 {
return disks
}
@@ -513,6 +586,7 @@ func mergeHostAgentSMARTIntoDisks(disks []models.PhysicalDisk, nodes []models.No
// Build a map of node name to linked host's SMART data
smartByNodeName := make(map[string][]models.HostDiskSMART)
hostByNodeName := make(map[string]*models.Host)
reportingNodeNames := make(map[string]bool)
for _, node := range nodes {
if node.LinkedAgentID == "" {
@@ -523,6 +597,7 @@ func mergeHostAgentSMARTIntoDisks(disks []models.PhysicalDisk, nodes []models.No
continue
}
smartByNodeName[node.Name] = host.Sensors.SMART
hostByNodeName[node.Name] = host
// Only an agent still reporting owns its disks' temperature alerts;
// a silent one's retained rows still enrich the disk.
if !strings.EqualFold(strings.TrimSpace(host.Status), "offline") {
@@ -614,6 +689,16 @@ func mergeHostAgentSMARTIntoDisks(disks []models.PhysicalDisk, nodes []models.No
if matched == nil {
continue
}
// The operator split this disk from the agent's (report-merge on the
// merged disk), so the row describes another disk: its readings,
// identity and temperature alert stay the agent's, and the PVE disk
// check judges what Proxmox reported. The refusal never pairs another
// row, as in the registry's linked disk join.
if host := hostByNodeName[updated[i].Node]; splits != nil && splits.ProxmoxDiskAgentSMARTSplit(updated[i], *host, *matched) {
updated[i].AgentSMARTSplit = true
dropNodeSensorTemperature(&updated[i], reportingNodeNames[updated[i].Node] && hostSuppliesNodeSMARTTemperatures(*host))
continue
}
// The linked agent reports this disk, so while it keeps reporting its
// CheckHost owns the disk's temperature alert and the PVE disk check
// stands aside. Once its lease lapses, CheckHost no longer alerts on
+18 -18
View File
@@ -13,16 +13,16 @@ import (
func TestMergeHostAgentSMARTIntoDisks_EmptyInputs(t *testing.T) {
// All combinations of empty inputs should return disks unchanged
disks := []models.PhysicalDisk{{ID: "d1"}}
if got := mergeHostAgentSMARTIntoDisks(nil, nil, nil); got != nil {
if got := mergeHostAgentSMARTIntoDisks(nil, nil, nil, nil); got != nil {
t.Error("nil disks should return nil")
}
if got := mergeHostAgentSMARTIntoDisks(disks, nil, nil); len(got) != 1 {
if got := mergeHostAgentSMARTIntoDisks(disks, nil, nil, nil); len(got) != 1 {
t.Error("nil nodes/hosts should return disks unchanged")
}
if got := mergeHostAgentSMARTIntoDisks(disks, []models.Node{}, nil); len(got) != 1 {
if got := mergeHostAgentSMARTIntoDisks(disks, []models.Node{}, nil, nil); len(got) != 1 {
t.Error("empty nodes should return disks unchanged")
}
if got := mergeHostAgentSMARTIntoDisks(disks, []models.Node{{Name: "n1"}}, nil); len(got) != 1 {
if got := mergeHostAgentSMARTIntoDisks(disks, []models.Node{{Name: "n1"}}, nil, nil); len(got) != 1 {
t.Error("nil hosts should return disks unchanged")
}
}
@@ -44,7 +44,7 @@ func TestMergeHostAgentSMARTIntoDisks_MatchByWWN(t *testing.T) {
},
},
}
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts)
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts, nil)
if result[0].Temperature != 42 {
t.Errorf("expected temp 42 via WWN match, got %d", result[0].Temperature)
}
@@ -67,7 +67,7 @@ func TestMergeHostAgentSMARTIntoDisks_MatchBySerial(t *testing.T) {
},
},
}
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts)
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts, nil)
if result[0].Temperature != 38 {
t.Errorf("expected temp 38 via serial match (case-insensitive), got %d", result[0].Temperature)
}
@@ -90,7 +90,7 @@ func TestMergeHostAgentSMARTIntoDisks_MatchByDevPath(t *testing.T) {
},
},
}
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts)
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts, nil)
if result[0].Temperature != 35 {
t.Errorf("expected temp 35 via device path match (strip /dev/), got %d", result[0].Temperature)
}
@@ -113,7 +113,7 @@ func TestMergeHostAgentSMARTIntoDisks_SkipsStandby(t *testing.T) {
},
},
}
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts)
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts, nil)
if result[0].Temperature != 0 {
t.Error("standby disk should not have its temperature merged")
}
@@ -139,7 +139,7 @@ func TestMergeHostAgentSMARTIntoDisks_MergesSMARTAttributes(t *testing.T) {
},
},
}
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts)
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts, nil)
if result[0].SmartAttributes == nil {
t.Fatal("SMART attributes should be merged")
}
@@ -174,7 +174,7 @@ func TestMergeHostAgentSMARTIntoDisks_DerivesWearoutFromSMARTAttributes(t *testi
},
}
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts)
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts, nil)
if result[0].Wearout != 94 {
t.Fatalf("expected wearout derived from PercentageUsed, got %d", result[0].Wearout)
}
@@ -208,7 +208,7 @@ func TestMergeHostAgentSMARTIntoDisks_SMARTFailureAndWearoutOverrideCoarsePVEVal
},
}}
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts)
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts, nil)
if result[0].Health != "FAILED" {
t.Fatalf("SMART failure was suppressed by PVE health: %+v", result[0])
}
@@ -227,7 +227,7 @@ func TestMergeHostAgentSMARTIntoDisks_DoesNotReplaceFailureWithPassed(t *testing
},
}}
if got := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts)[0].Health; got != "FAILED" {
if got := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts, nil)[0].Health; got != "FAILED" {
t.Fatalf("PASSED replaced an existing failure: %q", got)
}
}
@@ -266,7 +266,7 @@ func TestMergeHostAgentSMARTIntoDisks_DuplicateIdentityRequiresUniqueTopology(t
},
}}
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts)
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts, nil)
if result[0].Temperature != 47 || result[0].Health != "PASSED" {
t.Fatalf("topology-specific SMART evidence was not selected: %+v", result[0])
}
@@ -274,7 +274,7 @@ func TestMergeHostAgentSMARTIntoDisks_DuplicateIdentityRequiresUniqueTopology(t
disks[0].Controller = ""
disks[0].Target = ""
disks[0].DevPath = "/dev/unknown"
result = mergeHostAgentSMARTIntoDisks(disks, nodes, hosts)
result = mergeHostAgentSMARTIntoDisks(disks, nodes, hosts, nil)
if result[0].Temperature != 0 || result[0].Health != "UNKNOWN" {
t.Fatalf("ambiguous SMART identity was guessed: %+v", result[0])
}
@@ -298,7 +298,7 @@ func TestMergeHostAgentSMARTIntoDisks_FillsMissingHealthFromHostSMART(t *testing
},
}
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts)
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts, nil)
if result[0].Health != "FAILED" {
t.Fatalf("expected host SMART health to fill unknown disk health, got %q", result[0].Health)
}
@@ -321,7 +321,7 @@ func TestMergeHostAgentSMARTIntoDisks_NoAgentLink(t *testing.T) {
},
},
}
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts)
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts, nil)
if result[0].Temperature != 0 {
t.Error("unlinked nodes should not get host agent temperatures")
}
@@ -344,7 +344,7 @@ func TestMergeHostAgentSMARTIntoDisks_PreservesExistingTemp(t *testing.T) {
},
},
}
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts)
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts, nil)
if result[0].Temperature != 50 {
t.Errorf("existing temperature should be preserved, got %d", result[0].Temperature)
}
@@ -360,7 +360,7 @@ func TestMergeHostAgentSMARTIntoDisks_OriginalUnchanged(t *testing.T) {
SMART: []models.HostDiskSMART{{Serial: "SER1", Temperature: 42}},
}},
}
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts)
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts, nil)
if disks[0].Temperature != 0 {
t.Error("original disk slice should not be mutated")
}
+34 -3
View File
@@ -911,7 +911,7 @@ func (m *Monitor) maybePollPhysicalDisksAsync(
nodes := nodesForInstanceFromReadState(readState, instanceName)
hosts := hostsFromReadState(readState)
updated := mergeNVMeTempsIntoDisks(existing, nodes)
updated = mergeHostAgentSMARTIntoDisks(updated, nodes, hosts)
updated = mergeHostAgentSMARTIntoDisks(updated, nodes, hosts, m.proxmoxDiskAgentSplits())
m.state.UpdatePhysicalDisks(instanceName, updated)
}
return
@@ -950,6 +950,7 @@ func (m *Monitor) maybePollPhysicalDisksAsync(
nodesFromState := nodesForInstanceFromReadState(readState, inst)
hosts := hostsFromReadState(readState)
existingDisks := physicalDisksForInstanceFromReadState(readState, inst)
splits := m.proxmoxDiskAgentSplits()
existingDisksMap := make(map[string]models.PhysicalDisk, len(existingDisks))
for _, disk := range existingDisks {
if disk.Instance == inst {
@@ -976,12 +977,14 @@ func (m *Monitor) maybePollPhysicalDisksAsync(
// Proxmox disk query fails can still populate its physical disks from
// the agent's smartctl view (#1516).
smartByNode := make(map[string][]models.HostDiskSMART)
linkedHostByNode := make(map[string]models.Host)
for _, n := range nodesFromState {
if n.LinkedAgentID == "" || n.Instance != inst {
continue
}
linkedNodes[n.Name] = true
if linkedHost, ok := hostByID[n.LinkedAgentID]; ok {
linkedHostByNode[n.Name] = linkedHost
if len(linkedHost.DiskExclude) > 0 {
diskExcludeByNode[n.Name] = linkedHost.DiskExclude
}
@@ -1044,7 +1047,7 @@ func (m *Monitor) maybePollPhysicalDisksAsync(
// exceed the API window PVE itself allows for disks/list, so
// without this fallback their Physical Disks view empties
// permanently (#1516).
if fallback := physicalDisksFromHostAgentSMART(inst, node.Node, smartByNode[node.Node]); len(fallback) > 0 {
if fallback := unsplitPhysicalDisksFromHostAgentSMART(inst, node.Node, linkedHostByNode[node.Node], smartByNode[node.Node], splits); len(fallback) > 0 {
polledNodes[node.Node] = true
allDisks = append(allDisks, fallback...)
log.Info().
@@ -1145,8 +1148,14 @@ func (m *Monitor) maybePollPhysicalDisksAsync(
}
allDisks = mergeNVMeTempsIntoDisks(allDisks, nodesFromState)
allDisks = mergeHostAgentSMARTIntoDisks(allDisks, nodesFromState, hosts)
allDisks = mergeHostAgentSMARTIntoDisks(allDisks, nodesFromState, hosts, splits)
for index := range allDisks {
// A split disk's earlier record held the agent's readings and
// identity until the split; retained, they would never leave it,
// and nothing records which retained value Proxmox collected.
if allDisks[index].AgentSMARTSplit {
continue
}
if previous, ok := previousPhysicalDiskEvidence(allDisks[index], existingDisks); ok {
allDisks[index] = preserveUnavailablePhysicalDiskEvidence(allDisks[index], previous)
}
@@ -1289,6 +1298,28 @@ func physicalDisksFromHostAgentSMART(inst, nodeName string, smartEntries []model
return disks
}
// unsplitPhysicalDisksFromHostAgentSMART is physicalDisksFromHostAgentSMART
// without the rows the operator split from the Proxmox disk in their slot.
// Such a row describes the agent's disk, not the slot's, and standing in for
// it would copy the agent's disk, readings and temperature alert included,
// into a second row that the PVE disk check alerts on as well. The stand-in
// is judged under its own ID, the key the registry holds it by once it is
// recorded.
func unsplitPhysicalDisksFromHostAgentSMART(inst, nodeName string, host models.Host, smartEntries []models.HostDiskSMART, splits diskAgentSplitStore) []models.PhysicalDisk {
if splits == nil {
return physicalDisksFromHostAgentSMART(inst, nodeName, smartEntries)
}
var disks []models.PhysicalDisk
for _, smart := range smartEntries {
for _, disk := range physicalDisksFromHostAgentSMART(inst, nodeName, []models.HostDiskSMART{smart}) {
if !splits.ProxmoxDiskAgentSMARTSplit(disk, host, smart) {
disks = append(disks, disk)
}
}
}
return disks
}
func cloneDiskIO(in *models.DiskIO) *models.DiskIO {
if in == nil {
return nil
@@ -3,6 +3,8 @@ package monitoring
import (
"context"
"encoding/json"
"errors"
"slices"
"strconv"
"sync"
"testing"
@@ -293,7 +295,7 @@ func TestMergeHostAgentSMARTIntoDisks_AgentWearoutDoesNotHideLowPVELife(t *testi
},
}}
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts)
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts, nil)
if result[0].Wearout != 0 {
t.Fatalf("agent endurance reading raised the remaining life of a worn disk: wearout=%d", result[0].Wearout)
}
@@ -327,7 +329,7 @@ func TestMergeHostAgentSMARTIntoDisks_AgentWearoutFillsUnreportedPVELife(t *test
},
}}
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts)
result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts, nil)
if result[0].Wearout != 60 {
t.Fatalf("agent endurance did not fill unreported PVE life: wearout=%d", result[0].Wearout)
}
@@ -535,7 +537,7 @@ func TestMergeHostAgentSMARTIntoDisks_LegacyAgentTemperatureFollowsLease(t *test
Device: "/dev/sdb", Serial: "LEGACY1", Temperature: 38,
}}}})
reporting := mergeHostAgentSMARTIntoDisks([]models.PhysicalDisk{proxmoxDisk}, nodes, state.GetHosts())[0]
reporting := mergeHostAgentSMARTIntoDisks([]models.PhysicalDisk{proxmoxDisk}, nodes, state.GetHosts(), nil)[0]
if reporting.Temperature != 38 || reporting.Collection.Temperature != diskinventory.Available(diskinventory.LegacyHostAgentSource) {
t.Fatalf("legacy agent temperature not recorded as collected: temp=%d collection=%+v", reporting.Temperature, reporting.Collection)
}
@@ -548,7 +550,7 @@ func TestMergeHostAgentSMARTIntoDisks_LegacyAgentTemperatureFollowsLease(t *test
"full poll": proxmoxDisk,
"skipped poll": reporting,
} {
got := mergeHostAgentSMARTIntoDisks([]models.PhysicalDisk{disk}, nodes, state.GetHosts())[0]
got := mergeHostAgentSMARTIntoDisks([]models.PhysicalDisk{disk}, nodes, state.GetHosts(), nil)[0]
if got.Temperature != 38 || got.Collection.Temperature.State != diskinventory.FieldUnavailable {
t.Fatalf("%s: silent legacy agent temperature presented as collected: temp=%d collection=%+v", name, got.Temperature, got.Collection)
}
@@ -591,7 +593,7 @@ func TestSilentLegacyAgentCanonicalDiskFollowsItsWithdrawal(t *testing.T) {
// The disk poll while the agent reported is the copy the Proxmox row
// keeps once its host stops being polled.
state.UpdatePhysicalDisks("pve1", mergeHostAgentSMARTIntoDisks(
[]models.PhysicalDisk{inventory}, state.GetSnapshot().Nodes, state.GetHosts()))
[]models.PhysicalDisk{inventory}, state.GetSnapshot().Nodes, state.GetHosts(), nil))
canonical := func() (int, *diskinventory.CollectionStatus) {
t.Helper()
adapter := unifiedresources.NewMonitorAdapter(unifiedresources.NewRegistry(nil))
@@ -791,6 +793,7 @@ type slotDiskPVEClient struct {
fakeStorageClient
mu sync.Mutex
disk proxmox.Disk
err error
}
func (client *slotDiskPVEClient) setDisk(disk proxmox.Disk) {
@@ -799,9 +802,20 @@ func (client *slotDiskPVEClient) setDisk(disk proxmox.Disk) {
client.disk = disk
}
// setError makes the disk query fail, as it does on a wide node that exceeds
// the API window (#1516), until it is cleared with nil.
func (client *slotDiskPVEClient) setError(err error) {
client.mu.Lock()
defer client.mu.Unlock()
client.err = err
}
func (client *slotDiskPVEClient) GetDisks(context.Context, string) ([]proxmox.Disk, error) {
client.mu.Lock()
defer client.mu.Unlock()
if client.err != nil {
return nil, client.err
}
return []proxmox.Disk{client.disk}, nil
}
@@ -1146,7 +1160,7 @@ func TestMergeHostAgentSMARTIntoDisksMarksDisksTheAgentReports(t *testing.T) {
{ID: "host-node3", Status: "offline", Sensors: models.HostSensorSummary{SMART: smart}},
}
merged := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts)
merged := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts, nil)
for i, want := range []bool{true, false, false, false} {
if merged[i].AgentSMARTReported != want {
t.Fatalf("%s/%s AgentSMARTReported = %v, want %v", merged[i].Node, merged[i].DevPath, merged[i].AgentSMARTReported, want)
@@ -1159,7 +1173,7 @@ func TestMergeHostAgentSMARTIntoDisksMarksDisksTheAgentReports(t *testing.T) {
t.Fatalf("a silent agent's retained row no longer enriches the disk: %+v", merged[3])
}
fallback := mergeHostAgentSMARTIntoDisks(physicalDisksFromHostAgentSMART("pve1", "node1", smart), nodes, hosts)
fallback := mergeHostAgentSMARTIntoDisks(physicalDisksFromHostAgentSMART("pve1", "node1", smart), nodes, hosts, nil)
if len(fallback) != 1 || !fallback[0].AgentSMARTReported {
t.Fatalf("a disk built from a reporting agent's SMART report is not the agent's: %+v", fallback)
}
@@ -1215,3 +1229,740 @@ func TestCheckPhysicalDiskAlertsRaisesProxmoxDiskTemperatureAlerts(t *testing.T)
t.Fatalf("an excluded device raised a temperature alert")
}
}
// An operator split of a Proxmox disk from the disk its linked agent reports
// (report-merge on the merged disk, the drawer's Split merged resource) holds
// for the PVE disk poller too. The poller paired the agent's SMART row with the
// Proxmox disk by WWN, serial or path before the registry saw either, and read
// no exclusions, so the registry kept two rows while the Proxmox record still
// carried the agent's attributes, I/O, health, serial, type and temperature,
// and the agent kept the disk's temperature alert (AgentSMARTReported). A split
// row carries what Proxmox reported, whatever an earlier poll merged into it,
// on a full poll and on the skipped polls between. That includes the
// temperature: a node with a linked agent takes its sensor lists from the
// agent, so the reading the node poll matches to the disk is the agent's row
// of the disk it was split from, and the disk neither shows it nor alerts on it
// a second time beside the agent's own alert.
func TestOperatorSplitKeepsAgentSMARTOffTheProxmoxDisk(t *testing.T) {
percentUsed, hours := 40, int64(1200)
agentSMART := func(device, model, serial, diskType string, temperature int) models.HostDiskSMART {
return models.HostDiskSMART{
Device: device, Model: model, Serial: serial, Type: diskType, Health: "FAILED", Temperature: temperature,
IO: &models.DiskIO{Device: device, ReadBytes: 4096, WriteBytes: 8192},
Attributes: &models.SMARTAttributes{PowerOnHours: &hours, PercentageUsed: &percentUsed},
Collection: &diskinventory.CollectionStatus{
Serial: diskinventory.Available("smartctl"),
Temperature: diskinventory.Available("smartctl"),
IO: diskinventory.Available("kernel_diskstats"),
},
}
}
for _, shape := range []struct {
name string
agent models.HostDiskSMART
pve proxmox.Disk
// nodeTemp is the node sensor list's row for the disk. The node poll
// takes that list from the linked agent, so it is the agent's reading.
nodeTemp *models.DiskTemp
// What a paired record carries: the agent's serial and type, and the
// temperature it reports unless the node's sensors read the disk.
pairedSerial, pairedType string
pairedTemperature int
// The temperature a skipped poll reads back from the joined canonical
// disk, and what the split disk shows once the agent's reading is gone.
skippedTemperature, splitTemperature int
// Whether the PVE disk check alerts on the split disk's reading.
splitAlert bool
}{
// Proxmox reports the SAS transport address as the serial (#1595), so
// only the device path pairs the rows, and the pairing promotes the
// agent's serial and transport over Proxmox's.
{
name: "sas-path",
agent: agentSMART("/dev/sda", "ST4000NM0023", "Z1Z0ABCD", "sas", 70),
pve: proxmox.Disk{DevPath: "/dev/sda", Model: "ST4000NM0023", Serial: "5000c500a1b2c3d4", Type: "hdd",
Health: "PASSED", Wearout: 100, Size: 4000787030016},
pairedSerial: "Z1Z0ABCD", pairedType: "sas", pairedTemperature: 70, skippedTemperature: 70,
},
// One serial on both sides, and the node's sensor list reads the disk,
// so it carries a reading and can raise a temperature alert.
{
name: "matching-serial",
agent: agentSMART("/dev/nvme0n1", "WD Black SN850X", "S6B0NL0W123456", "nvme", 40),
pve: proxmox.Disk{DevPath: "/dev/nvme0n1", Model: "WD Black SN850X", Serial: "S6B0NL0W123456", Type: "nvme",
Health: "PASSED", Wearout: 100, Size: 1000204886016},
nodeTemp: &models.DiskTemp{Device: "/dev/nvme0n1", Serial: "S6B0NL0W123456", Type: "nvme", Temperature: 85},
pairedSerial: "S6B0NL0W123456", pairedType: "nvme", pairedTemperature: 85, skippedTemperature: 40,
},
// The agent's report carries no usable temperature, so the node's SMART
// list is the SSH collector's: the reading is the node's own, survives the
// split, and the PVE disk check, no longer deferring to the agent, alerts.
{
name: "ssh-sensors",
agent: agentSMART("/dev/nvme1n1", "WD Black SN770", "S7SSH0000001", "nvme", 0),
pve: proxmox.Disk{DevPath: "/dev/nvme1n1", Model: "WD Black SN770", Serial: "S7SSH0000001", Type: "nvme",
Health: "PASSED", Wearout: 100, Size: 1000204886016},
nodeTemp: &models.DiskTemp{Device: "/dev/nvme1n1", Serial: "S7SSH0000001", Type: "nvme", Temperature: 85},
pairedSerial: "S7SSH0000001", pairedType: "nvme", pairedTemperature: 85, skippedTemperature: 85,
splitTemperature: 85, splitAlert: true,
},
} {
for _, request := range []struct {
name string
// exclusions are the pairs report-merge records for the merged disk.
exclusions func(merged, agentCandidate, proxmoxCandidate string) [][2]string
split bool
}{
{"report-merge", func(merged, agent, proxmox string) [][2]string {
return [][2]string{{merged, agent}, {merged, proxmox}}
}, true},
{"proxmox-candidate-only", func(merged, _, proxmox string) [][2]string {
return [][2]string{{merged, proxmox}}
}, true},
// The agent's row holds the merged ID, so a pair naming only its
// candidate separates nothing, here as in the registry.
{"agent-candidate-only", func(merged, agent, _ string) [][2]string {
return [][2]string{{merged, agent}}
}, false},
{"unrelated-exclusion", func(merged, _, _ string) [][2]string {
return [][2]string{{merged, "physical_disk-00000000deadbeef"}}
}, false},
} {
t.Run(shape.name+"/"+request.name, func(t *testing.T) {
t.Setenv("PULSE_DATA_DIR", t.TempDir())
now := time.Now()
const instance, nodeName = "pve1", "node1"
node := models.Node{
ID: "pve1-node1", Name: nodeName, Instance: instance, Host: "https://10.0.0.5:8006",
Status: "online", LastSeen: now, LinkedAgentID: "agent-1",
}
if shape.nodeTemp != nil {
reading := *shape.nodeTemp
reading.LastUpdated = now
node.Temperature = &models.Temperature{Available: true, HasSMART: true, SMART: []models.DiskTemp{reading}, LastUpdate: now}
}
state := models.NewState()
state.UpdateNodesForInstance(instance, []models.Node{node})
state.UpsertHost(models.Host{
ID: "agent-1", Hostname: nodeName, MachineID: "0123456789abcdef", LinkedNodeID: node.ID,
Status: "online", IntervalSeconds: 30, LastSeen: now,
Sensors: models.HostSensorSummary{SMART: []models.HostDiskSMART{shape.agent}},
})
store, err := unifiedresources.NewSQLiteResourceStore(t.TempDir(), "default")
if err != nil {
t.Fatal(err)
}
t.Cleanup(func() { _ = store.Close() })
alertManager := alerts.NewManager()
t.Cleanup(alertManager.Stop)
adapter := unifiedresources.NewMonitorAdapter(unifiedresources.NewRegistry(store))
m := &Monitor{
state: state, resourceStore: adapter, alertManager: alertManager,
metricsHistory: NewMetricsHistory(100, time.Hour),
startTime: now.Add(-time.Hour), lastPhysicalDiskPoll: make(map[string]time.Time),
}
client := &slotDiskPVEClient{}
client.setDisk(shape.pve)
disk := func() models.PhysicalDisk {
t.Helper()
disks := state.GetSnapshot().PhysicalDisks
if len(disks) != 1 {
t.Fatalf("physical disks = %+v, want one", disks)
}
return disks[0]
}
fullPoll := func() models.PhysicalDisk {
t.Helper()
adapter.PopulateFromSnapshot(state.GetSnapshot())
started := time.Now()
delete(m.lastPhysicalDiskPoll, instance)
m.maybePollPhysicalDisksAsync(context.Background(), instance, &config.PVEInstance{}, client,
[]proxmox.Node{{Node: nodeName, Status: "online"}}, map[string]string{nodeName: "online"}, nil)
deadline := time.Now().Add(3 * time.Second)
for {
if disks := state.GetSnapshot().PhysicalDisks; len(disks) == 1 && !disks[0].LastChecked.Before(started) {
return disks[0]
}
if time.Now().After(deadline) {
t.Fatal("full physical disk poll did not land in state")
}
time.Sleep(10 * time.Millisecond)
}
}
skippedPoll := func() models.PhysicalDisk {
t.Helper()
adapter.PopulateFromSnapshot(state.GetSnapshot())
m.lastPhysicalDiskPoll[instance] = time.Now()
m.maybePollPhysicalDisksAsync(context.Background(), instance, &config.PVEInstance{}, nil, nil, nil, nil)
return disk()
}
temperatureAlert := func() bool {
id := unifiedresources.ProxmoxPhysicalDiskAlertResourceID(instance, nodeName, shape.pve.DevPath) + "::metric-threshold:diskTemperature"
for _, alert := range alertManager.GetActiveAlerts() {
if alert.ID == id {
return true
}
}
return false
}
healthAlert := func() bool {
for _, alert := range alertManager.GetActiveAlerts() {
if alert.Type == "disk-health" {
return true
}
}
return false
}
// A poll whose Proxmox disk query fails carries the stored record
// (or the agent's rows) instead of a fresh one, so it is done when
// the state is written rather than when a disk is newly checked.
failedQueryPoll := func() models.PhysicalDisk {
t.Helper()
adapter.PopulateFromSnapshot(state.GetSnapshot())
client.setError(errors.New("disks/list timed out"))
defer client.setError(nil)
started := time.Now()
delete(m.lastPhysicalDiskPoll, instance)
m.maybePollPhysicalDisksAsync(context.Background(), instance, &config.PVEInstance{}, client,
[]proxmox.Node{{Node: nodeName, Status: "online"}}, map[string]string{nodeName: "online"}, nil)
deadline := time.Now().Add(3 * time.Second)
for {
if snapshot := state.GetSnapshot(); snapshot.LastUpdate.After(started) && len(snapshot.PhysicalDisks) == 1 {
return snapshot.PhysicalDisks[0]
}
if time.Now().After(deadline) {
t.Fatal("physical disk poll with a failing disk query did not land in state")
}
time.Sleep(10 * time.Millisecond)
}
}
// Whether the canonical disks are one joined row or two.
canonicalRows := func() (joined, agentOnly, proxmoxOnly []*unifiedresources.PhysicalDiskView) {
t.Helper()
adapter.PopulateFromSnapshot(state.GetSnapshot())
for _, view := range adapter.PhysicalDisks() {
_, fromAgent := view.SourceStatus(unifiedresources.SourceAgent)
_, fromProxmox := view.SourceStatus(unifiedresources.SourceProxmox)
switch {
case fromAgent && fromProxmox:
joined = append(joined, view)
case fromAgent:
agentOnly = append(agentOnly, view)
case fromProxmox:
proxmoxOnly = append(proxmoxOnly, view)
}
}
return joined, agentOnly, proxmoxOnly
}
assertPaired := func(step string, got models.PhysicalDisk, wantTemperature int) {
t.Helper()
if got.Serial != shape.pairedSerial || got.Type != shape.pairedType || got.Health != "FAILED" ||
got.Temperature != wantTemperature || got.Wearout != 60 ||
got.SmartAttributes == nil || got.IO == nil || !got.AgentSMARTReported || got.AgentSMARTSplit {
t.Fatalf("%s: the agent's row was not paired with the Proxmox disk: %+v", step, got)
}
}
assertProxmoxOnly := func(step string, got models.PhysicalDisk) {
t.Helper()
if got.Serial != shape.pve.Serial || got.Type != shape.pve.Type || got.Health != "PASSED" ||
got.Temperature != shape.splitTemperature || got.Wearout != shape.pve.Wearout ||
got.SmartAttributes != nil || got.IO != nil || got.AgentSMARTReported || !got.AgentSMARTSplit {
t.Fatalf("%s: the split disk carries the agent's data: %+v", step, got)
}
}
// The disk's temperature alert is open before the agent's row
// is paired with it, as when the node's sensors read it alone.
// A shape whose alert should open after the split starts without
// one: the alert manager suppresses an alert it just resolved.
if shape.nodeTemp != nil && !shape.splitAlert {
m.checkPhysicalDiskAlerts(instance, models.PhysicalDisk{
Instance: instance, Node: nodeName, DevPath: shape.pve.DevPath, Model: shape.pve.Model,
Type: shape.pve.Type, Health: "PASSED", Wearout: -1, Temperature: shape.nodeTemp.Temperature,
}, nil)
if !temperatureAlert() {
t.Fatal("fixture did not open the Proxmox disk's temperature alert")
}
}
assertPaired("before the request", fullPoll(), shape.pairedTemperature)
if !healthAlert() {
t.Fatal("the agent's failed health did not raise the disk health alert")
}
if joined, agentOnly, proxmoxOnly := canonicalRows(); len(joined) != 1 || len(agentOnly) != 0 || len(proxmoxOnly) != 0 {
t.Fatalf("fixture did not join the disks: joined %d, agent only %d, proxmox only %d", len(joined), len(agentOnly), len(proxmoxOnly))
}
if temperatureAlert() {
t.Fatal("the PVE disk's temperature alert stayed open for a disk the agent reports")
}
// Report-merge names the merged disk and each source's candidate.
ids := unifiedresources.NewRegistry(nil)
ids.IngestSnapshot(state.GetSnapshot())
var merged, agentCandidate, proxmoxCandidate string
for _, resource := range ids.List() {
if resource.Type == unifiedresources.ResourceTypePhysicalDisk {
merged = resource.ID
}
}
for _, target := range ids.SourceTargets(merged) {
switch target.Source {
case unifiedresources.SourceAgent:
agentCandidate = target.CandidateID
case unifiedresources.SourceProxmox:
proxmoxCandidate = target.CandidateID
}
}
if merged == "" || agentCandidate == "" || proxmoxCandidate == "" || proxmoxCandidate == merged {
t.Fatalf("merged disk %q lists candidates agent=%q proxmox=%q", merged, agentCandidate, proxmoxCandidate)
}
for _, pair := range request.exclusions(merged, agentCandidate, proxmoxCandidate) {
if err := store.AddExclusion(unifiedresources.ResourceExclusion{ResourceA: pair[0], ResourceB: pair[1], CreatedAt: time.Now().UTC()}); err != nil {
t.Fatal(err)
}
}
if !request.split {
assertPaired("after the request", fullPoll(), shape.pairedTemperature)
// A skipped poll merges the stored record, which the joined
// canonical disk reports at the agent's reading.
assertPaired("on a skipped poll", skippedPoll(), shape.skippedTemperature)
return
}
// A full poll builds the record from what Proxmox reported,
// and the poll after it must not retain what the one before
// held from the agent. The skipped polls between, which merge
// the stored record again, keep the agent's data off too.
assertProxmoxOnly("after the request", fullPoll())
// The split disk is still evaluated, and Proxmox's own verdict
// of it closes the health alert the agent's failure raised.
if healthAlert() {
t.Fatal("the disk health alert raised by the agent's row stayed open on the split disk")
}
assertProxmoxOnly("on a skipped poll", skippedPoll())
assertProxmoxOnly("on the next full poll", fullPoll())
assertProxmoxOnly("on the next skipped poll", skippedPoll())
// When Proxmox cannot list the node's disks, the agent's rows
// stand in for them (#1516), except the row split from the
// disk in its slot.
assertProxmoxOnly("when the disk query fails", failedQueryPoll())
joined, agentOnly, proxmoxOnly := canonicalRows()
if len(joined) != 0 || len(agentOnly) != 1 || len(proxmoxOnly) != 1 {
t.Fatalf("canonical disks: joined %d, agent only %d, proxmox only %d, want the two apart", len(joined), len(agentOnly), len(proxmoxOnly))
}
if proxmoxOnly[0].Health() != "PASSED" || proxmoxOnly[0].Serial() != shape.pve.Serial {
t.Fatalf("canonical Proxmox disk took the agent's data: health %q, serial %q", proxmoxOnly[0].Health(), proxmoxOnly[0].Serial())
}
if agentOnly[0].Health() != "FAILED" || agentOnly[0].Serial() != shape.agent.Serial {
t.Fatalf("canonical agent disk lost its own data: health %q, serial %q", agentOnly[0].Health(), agentOnly[0].Serial())
}
// A reading the agent supplied is gone, so the PVE disk check has
// none to judge and the alert the agent took over does not come
// back; one the SSH collector supplied is judged by the PVE check.
if temperatureAlert() != shape.splitAlert {
t.Fatalf("PVE temperature alert active = %v, want %v once the disks were split", temperatureAlert(), shape.splitAlert)
}
})
}
}
}
// fakeDiskAgentSplits answers the PVE disk poller's split question from a
// set of Proxmox device paths and records what it was asked.
type fakeDiskAgentSplits struct {
splitDevPaths map[string]bool
splitIDs map[string]bool
asked []string
askedIDs []string
}
func (f *fakeDiskAgentSplits) ProxmoxDiskAgentSMARTSplit(disk models.PhysicalDisk, host models.Host, smart models.HostDiskSMART) bool {
f.asked = append(f.asked, host.ID+":"+disk.DevPath+":"+smart.Serial)
f.askedIDs = append(f.askedIDs, disk.ID)
return f.splitDevPaths[disk.DevPath] || f.splitIDs[disk.ID]
}
// The agent's SMART merge asks the split decider once per disk it matches a
// row to. A split disk takes nothing from the row, not even the alert
// ownership, and is marked so the poll does not retain the agent's data for it
// either; a disk no row matches is not asked about; the rows of the disks that
// are not split still merge, as they do without a decider.
func TestMergeHostAgentSMARTIntoDisksLeavesDisksTheOperatorSplitAlone(t *testing.T) {
percentUsed := 40
smart := func(device, serial string) models.HostDiskSMART {
return models.HostDiskSMART{
Device: device, Serial: serial, Type: "sata", Health: "FAILED", Temperature: 61,
WWN: "5-c50-" + serial, Controller: "ctrl", Target: "0:1", Pool: "tank", Model: "AgentModel", SizeBytes: 1000,
IO: &models.DiskIO{Device: device, ReadBytes: 1},
Attributes: &models.SMARTAttributes{PercentageUsed: &percentUsed},
}
}
disk := func(devPath, serial string) models.PhysicalDisk {
return models.PhysicalDisk{ID: unifiedresources.ProxmoxPhysicalDiskSourceID("pve1", "node1", devPath, "", ""), Node: "node1", Instance: "pve1", DevPath: devPath, Serial: serial, Type: "sata", Health: "PASSED", Wearout: 100}
}
nodes := []models.Node{{Name: "node1", LinkedAgentID: "agent-1"}}
hosts := []models.Host{{ID: "agent-1", Status: "online", Sensors: models.HostSensorSummary{
SMART: []models.HostDiskSMART{smart("sda", "SERIAL-A"), smart("sdb", "SERIAL-B")},
}}}
disks := []models.PhysicalDisk{disk("/dev/sda", "SERIAL-A"), disk("/dev/sdb", "SERIAL-B"), disk("/dev/sdc", "SERIAL-C")}
// The node's sensor list, which a linked agent supplies, was matched to the
// split disk before the agent merge ran.
disks[0].Temperature = 58
disks[0].Collection = &diskinventory.CollectionStatus{Temperature: diskinventory.Available(proxmoxNodeSMARTTemperatureSource)}
// Proxmox's own verdict of the split disk, which the agent's PASSED/FAILED
// would otherwise replace or correct.
disks[0].Health, disks[0].Wearout = "FAILED", 30
splits := &fakeDiskAgentSplits{splitDevPaths: map[string]bool{"/dev/sda": true, "/dev/sdc": true}}
merged := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts, splits)
wantSplit := disk("/dev/sda", "SERIAL-A")
wantSplit.AgentSMARTSplit = true
if got := merged[0]; got.Temperature != 0 || got.Health != "FAILED" || got.Wearout != 30 || got.SmartAttributes != nil || got.IO != nil ||
got.AgentSMARTReported || !got.AgentSMARTSplit || got.ID != wantSplit.ID || got.Serial != wantSplit.Serial ||
got.WWN != "" || got.Controller != "" || got.Target != "" || got.StorageGroup != "" || got.Model != "" || got.Size != 0 {
t.Fatalf("a split disk took the agent's row: %+v", got)
}
if got := merged[1]; got.Temperature != 61 || got.Health != "FAILED" || got.Wearout != 60 || got.SmartAttributes == nil || got.IO == nil ||
!got.AgentSMARTReported || got.AgentSMARTSplit {
t.Fatalf("a disk that is not split lost the agent's row: %+v", got)
}
if got := merged[2]; got.AgentSMARTSplit || got.AgentSMARTReported {
t.Fatalf("a disk no row matches was marked: %+v", got)
}
if want := []string{"agent-1:/dev/sda:SERIAL-A", "agent-1:/dev/sdb:SERIAL-B"}; !slices.Equal(splits.asked, want) {
t.Fatalf("decider asked %v, want one question per matched disk %v", splits.asked, want)
}
if disks[0].AgentSMARTSplit || disks[0].Temperature != 58 || disks[0].Collection.Temperature.State != diskinventory.FieldAvailable {
t.Fatalf("the merge modified the caller's slice: %+v / %+v", disks[0], disks[0].Collection)
}
if merged[0].Collection == nil || merged[0].Collection.Temperature.State == diskinventory.FieldAvailable {
t.Fatalf("the split disk still counts the node sensors' reading as collected: %+v", merged[0].Collection)
}
// The decision is made under the observation's own ID, the key the registry
// holds it by: a controller member recorded by device alone is a different
// observation from the stand-in keyed by its target, so a split recorded
// under the other key is ignored, as it is for the registry.
memberRow := smart("sdd", "SERIAL-D")
memberRow.Controller, memberRow.Target = "megaraid", "megaraid,7"
memberHosts := []models.Host{{ID: "agent-1", Status: "online", Sensors: models.HostSensorSummary{SMART: []models.HostDiskSMART{memberRow}}}}
slotID := unifiedresources.ProxmoxPhysicalDiskSourceID("pve1", "node1", "/dev/sdd", "", "")
memberID := unifiedresources.ProxmoxPhysicalDiskSourceID("pve1", "node1", "/dev/sdd", "megaraid", "megaraid,7")
if slotID == memberID {
t.Fatal("fixture: a controller member's key should differ from the slot's")
}
bySlot := disk("/dev/sdd", "SERIAL-D")
byMember := bySlot
byMember.ID = memberID
for name, tc := range map[string]struct {
disk models.PhysicalDisk
splitID string
wantSplit bool
wantAsked string
}{
"slot-keyed record, split under its key": {bySlot, slotID, true, slotID},
"slot-keyed record, split under member key": {bySlot, memberID, false, slotID},
"member-keyed record, split under its key": {byMember, memberID, true, memberID},
"member-keyed record, split under slot key": {byMember, slotID, false, memberID},
} {
decider := &fakeDiskAgentSplits{splitIDs: map[string]bool{tc.splitID: true}}
got := mergeHostAgentSMARTIntoDisks([]models.PhysicalDisk{tc.disk}, nodes, memberHosts, decider)[0]
if got.AgentSMARTSplit != tc.wantSplit || got.AgentSMARTReported == tc.wantSplit {
t.Fatalf("%s: split = %v, reported = %v, want split %v", name, got.AgentSMARTSplit, got.AgentSMARTReported, tc.wantSplit)
}
if !slices.Equal(decider.askedIDs, []string{tc.wantAsked}) {
t.Fatalf("%s: decider asked about %v, want exactly the record's own ID %q", name, decider.askedIDs, tc.wantAsked)
}
}
// The node-sensor reading already on the disk stays; the agent's fills only an empty one.
if unchanged := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts, &fakeDiskAgentSplits{}); !unchanged[0].AgentSMARTReported || unchanged[0].Temperature != 58 || unchanged[0].Health != "FAILED" {
t.Fatalf("a decider that splits nothing changed the merge: %+v", unchanged[0])
}
}
// When the Proxmox disk query fails, the linked agent's SMART rows stand in
// for the node's disks (#1516). A row the operator split from the Proxmox disk
// in its slot is not that disk's stand-in: it would put the agent's disk, with
// its readings and temperature alert, in a second row beside the agent's own.
// The stand-in is judged under its own ID, the key the registry holds it by
// once recorded: a controller member's is qualified by its target.
func TestHostAgentSMARTFallbackSkipsRowsTheOperatorSplit(t *testing.T) {
rows := []models.HostDiskSMART{
{Device: "sda", Serial: "SERIAL-A", Type: "sata", Temperature: 61},
{Device: "sdb", Serial: "SERIAL-B", Type: "sata", Temperature: 62},
}
host := models.Host{ID: "agent-1"}
if got := unsplitPhysicalDisksFromHostAgentSMART("pve1", "node1", host, rows, nil); len(got) != 2 {
t.Fatalf("without split decisions the fallback listed %d disks, want 2", len(got))
}
if got := unsplitPhysicalDisksFromHostAgentSMART("pve1", "node1", host, rows, &fakeDiskAgentSplits{}); len(got) != 2 {
t.Fatalf("a decider that splits nothing left %d stand-ins, want 2", len(got))
}
splits := &fakeDiskAgentSplits{splitDevPaths: map[string]bool{"/dev/sda": true}}
got := unsplitPhysicalDisksFromHostAgentSMART("pve1", "node1", host, rows, splits)
if len(got) != 1 || got[0].DevPath != "/dev/sdb" || got[0].Temperature != 62 {
t.Fatalf("fallback disks = %+v, want only /dev/sdb", got)
}
if want := []string{"agent-1:/dev/sda:SERIAL-A", "agent-1:/dev/sdb:SERIAL-B"}; !slices.Equal(splits.asked, want) {
t.Fatalf("decider asked %v, want one question per row, with the row %v", splits.asked, want)
}
member := []models.HostDiskSMART{{Device: "sdc", Serial: "SERIAL-C", Type: "sat", Controller: "megaraid", Target: "megaraid,7"}}
stand := physicalDisksFromHostAgentSMART("pve1", "node1", member)
slot := unifiedresources.ProxmoxPhysicalDiskSourceID("pve1", "node1", "/dev/sdc", "", "")
if len(stand) != 1 || stand[0].ID == slot {
t.Fatalf("a controller member's stand-in should be keyed by its target: %+v", stand)
}
byOwnID := &fakeDiskAgentSplits{splitIDs: map[string]bool{stand[0].ID: true}}
if got := unsplitPhysicalDisksFromHostAgentSMART("pve1", "node1", host, member, byOwnID); len(got) != 0 {
t.Fatalf("a split recorded under the stand-in's key did not hold: %+v", got)
}
if want := []string{stand[0].ID}; !slices.Equal(byOwnID.askedIDs, want) {
t.Fatalf("decider asked about %v, want the stand-in's own ID %v", byOwnID.askedIDs, want)
}
// A split recorded under the slot's device-only key names a different
// observation, as for the registry: the stand-in is not that disk.
bySlot := &fakeDiskAgentSplits{splitIDs: map[string]bool{slot: true}}
if got := unsplitPhysicalDisksFromHostAgentSMART("pve1", "node1", host, member, bySlot); len(got) != 1 {
t.Fatalf("a split recorded under the slot's key applied to the member's stand-in: %+v", got)
}
if want := []string{stand[0].ID}; !slices.Equal(bySlot.askedIDs, want) {
t.Fatalf("decider asked about %v, want only the stand-in's own ID %v", bySlot.askedIDs, want)
}
}
// When the Proxmox disk query fails and the operator split one of the node's
// disks from its agent row, only the rows that were not split stand in for the
// node's disks, as before; the split disk gets no stand-in, which would be the
// agent's disk again, so its Proxmox record is not listed while the query fails.
func TestFailedDiskQueryListsOnlyTheRowsTheOperatorDidNotSplit(t *testing.T) {
t.Setenv("PULSE_DATA_DIR", t.TempDir())
now := time.Now()
state := models.NewState()
state.UpdateNodesForInstance("pve1", []models.Node{{
ID: "pve1-node1", Name: "node1", Instance: "pve1", Status: "online", LastSeen: now, LinkedAgentID: "agent-1",
}})
state.UpsertHost(models.Host{
ID: "agent-1", Hostname: "node1", LinkedNodeID: "pve1-node1", Status: "online", IntervalSeconds: 30, LastSeen: now,
Sensors: models.HostSensorSummary{SMART: []models.HostDiskSMART{
{Device: "/dev/sda", Serial: "AGENT-A", Type: "sata", Health: "FAILED", Temperature: 61},
{Device: "/dev/sdb", Serial: "SERIAL-B", Type: "sata", Health: "PASSED", Temperature: 62},
}},
})
record := func(device, serial string) models.PhysicalDisk {
return models.PhysicalDisk{
ID: unifiedresources.ProxmoxPhysicalDiskSourceID("pve1", "node1", device, "", ""), Node: "node1", Instance: "pve1",
DevPath: device, Serial: serial, Type: "sata", Health: "PASSED", Wearout: 100, LastChecked: now,
Collection: &diskinventory.CollectionStatus{Temperature: diskinventory.Unsupported("proxmox_disks", "no temperature")},
}
}
state.UpdatePhysicalDisks("pve1", []models.PhysicalDisk{record("/dev/sda", "PVE-A"), record("/dev/sdb", "SERIAL-B")})
alertManager := alerts.NewManager()
t.Cleanup(alertManager.Stop)
adapter := unifiedresources.NewMonitorAdapter(unifiedresources.NewRegistry(nil))
m := &Monitor{
state: state,
resourceStore: splitDecidingStore{adapter, &fakeDiskAgentSplits{splitDevPaths: map[string]bool{"/dev/sda": true}}},
alertManager: alertManager, metricsHistory: NewMetricsHistory(100, time.Hour),
startTime: now.Add(-time.Hour), lastPhysicalDiskPoll: make(map[string]time.Time),
}
adapter.PopulateFromSnapshot(state.GetSnapshot())
client := &slotDiskPVEClient{}
client.setError(errors.New("disks/list timed out"))
started := time.Now()
m.maybePollPhysicalDisksAsync(context.Background(), "pve1", &config.PVEInstance{}, client,
[]proxmox.Node{{Node: "node1", Status: "online"}}, map[string]string{"node1": "online"}, nil)
var disks []models.PhysicalDisk
for deadline := time.Now().Add(3 * time.Second); ; time.Sleep(10 * time.Millisecond) {
if snapshot := state.GetSnapshot(); snapshot.LastUpdate.After(started) {
disks = snapshot.PhysicalDisks
break
}
if time.Now().After(deadline) {
t.Fatal("physical disk poll with a failing disk query did not land in state")
}
}
byDevice := map[string]models.PhysicalDisk{}
for _, disk := range disks {
byDevice[disk.DevPath] = disk
}
if len(disks) != 1 {
t.Fatalf("disks = %+v, want only the row that was not split", disks)
}
if split, listed := byDevice["/dev/sda"]; listed {
t.Fatalf("the agent's row stood in for the disk the operator split from it: %+v", split)
}
if other := byDevice["/dev/sdb"]; other.Serial != "SERIAL-B" || other.Temperature != 62 || !other.AgentSMARTReported ||
other.ID != unifiedresources.ProxmoxPhysicalDiskSourceID("pve1", "node1", "/dev/sdb", "", "") {
t.Fatalf("the other row did not stand in for its disk: %+v", other)
}
}
// splitDecidingStore is the resource store a monitor test hands out when it
// wants to decide the operator's splits itself.
type splitDecidingStore struct {
*unifiedresources.MonitorAdapter
splits *fakeDiskAgentSplits
}
func (s splitDecidingStore) ProxmoxDiskAgentSMARTSplit(disk models.PhysicalDisk, host models.Host, smart models.HostDiskSMART) bool {
return s.splits.ProxmoxDiskAgentSMARTSplit(disk, host, smart)
}
// The node poll takes the node's SMART sensor list from a linked agent when
// the agent's report carries a usable temperature, and from the SSH collector
// otherwise, so only the first kind of reading is the agent's row of the disk
// a split Proxmox disk was split from. The legacy NVMe list names no disk, so
// a split disk drops its guess either way. A disk whose temperature has
// another source keeps it, and the caller's collection is never modified.
func TestDropNodeSensorTemperatureRemovesOnlyAgentSuppliedReadings(t *testing.T) {
for _, tc := range []struct {
source string
smartFromAgent bool
dropped bool
}{
{proxmoxNodeSMARTTemperatureSource, true, true},
{proxmoxNodeSMARTTemperatureSource, false, false},
{proxmoxNodeNVMeTemperatureSource, true, true},
{proxmoxNodeNVMeTemperatureSource, false, true},
} {
shared := &diskinventory.CollectionStatus{
Temperature: diskinventory.Available(tc.source), Serial: diskinventory.Available("proxmox_disks"),
IO: diskinventory.Unsupported("proxmox_disks", "no counters"), Controller: diskinventory.Missing("proxmox_disks", "none"),
Pool: diskinventory.Available("proxmox_zfs"),
}
disk := models.PhysicalDisk{Temperature: 60, Collection: shared}
dropNodeSensorTemperature(&disk, tc.smartFromAgent)
if tc.dropped {
other := *disk.Collection
other.Temperature = shared.Temperature
if disk.Temperature != 0 || disk.Collection.Temperature.State != diskinventory.FieldUnsupported ||
disk.Collection.Temperature.Source != "proxmox_disks" || other != *shared {
t.Fatalf("%+v: reading not dropped cleanly: %+v / %+v", tc, disk, disk.Collection)
}
} else if disk.Temperature != 60 || disk.Collection != shared {
t.Fatalf("%+v: an SSH-supplied reading was dropped: %+v", tc, disk)
}
if shared.Temperature.State != diskinventory.FieldAvailable || shared.Temperature.Source != tc.source {
t.Fatalf("%+v: the caller's collection was modified: %+v", tc, shared)
}
}
for _, collection := range []*diskinventory.CollectionStatus{
nil,
{Temperature: diskinventory.Available("smartctl")},
{Temperature: diskinventory.Unsupported("proxmox_disks", "no temperature")},
} {
disk := models.PhysicalDisk{Temperature: 60, Collection: collection}
dropNodeSensorTemperature(&disk, true)
if disk.Temperature != 60 || disk.Collection != collection {
t.Fatalf("a reading from another source was dropped: %+v", disk)
}
}
}
// mergeNVMeTempsIntoDisks labels what it takes from the node's sensor lists
// with the sources dropNodeSensorTemperature recognises: the SMART list
// matched by identity or path, and the legacy NVMe list matched by order.
func TestMergeNVMeTempsLabelsReadingsWithTheNodeSensorSources(t *testing.T) {
node := func(temperature *models.Temperature) []models.Node {
return []models.Node{{Name: "node1", Temperature: temperature}}
}
disk := models.PhysicalDisk{Node: "node1", DevPath: "/dev/nvme0n1", Serial: "S6B0NL0W123456", Type: "nvme"}
bySMART := mergeNVMeTempsIntoDisks([]models.PhysicalDisk{disk}, node(&models.Temperature{
Available: true, HasSMART: true,
SMART: []models.DiskTemp{{Device: "/dev/nvme0n1", Serial: "S6B0NL0W123456", Temperature: 55}},
}))[0]
if bySMART.Temperature != 55 || bySMART.Collection == nil || bySMART.Collection.Temperature.Source != proxmoxNodeSMARTTemperatureSource {
t.Fatalf("SMART list reading not labelled %q: %+v / %+v", proxmoxNodeSMARTTemperatureSource, bySMART, bySMART.Collection)
}
for name, row := range map[string]models.DiskTemp{
"WWN": {Device: "/dev/other", WWN: "0x5000c500a1b2c3d4", Temperature: 52},
"device path": {Device: "/dev/nvme0n1", Temperature: 53},
} {
wwnDisk := models.PhysicalDisk{Node: "node1", DevPath: "/dev/nvme0n1", WWN: "0x5000c500a1b2c3d4", Type: "nvme"}
if name == "device path" {
wwnDisk = models.PhysicalDisk{Node: "node1", DevPath: "/dev/nvme0n1", Type: "nvme"}
}
got := mergeNVMeTempsIntoDisks([]models.PhysicalDisk{wwnDisk}, node(&models.Temperature{
Available: true, HasSMART: true, SMART: []models.DiskTemp{row},
}))[0]
if got.Temperature != row.Temperature || got.Collection == nil || got.Collection.Temperature.Source != proxmoxNodeSMARTTemperatureSource {
t.Fatalf("%s match not labelled %q: %+v / %+v", name, proxmoxNodeSMARTTemperatureSource, got, got.Collection)
}
}
byOrder := mergeNVMeTempsIntoDisks([]models.PhysicalDisk{disk}, node(&models.Temperature{
Available: true, HasNVMe: true, NVMe: []models.NVMeTemp{{Device: "nvme0", Temp: 47}},
}))[0]
if byOrder.Temperature != 47 || byOrder.Collection == nil || byOrder.Collection.Temperature.Source != proxmoxNodeNVMeTemperatureSource {
t.Fatalf("legacy NVMe reading not labelled %q: %+v / %+v", proxmoxNodeNVMeTemperatureSource, byOrder, byOrder.Collection)
}
}
// Only a SMART report that carries a usable temperature makes the node's SMART
// list the agent's (mergeTemperatureData).
func TestHostSuppliesNodeSMARTTemperaturesFollowsUsableReadings(t *testing.T) {
host := func(rows ...models.HostDiskSMART) models.Host {
return models.Host{Sensors: models.HostSensorSummary{SMART: rows}}
}
for name, tc := range map[string]struct {
host models.Host
want bool
}{
"a reading": {host(models.HostDiskSMART{Device: "sda", Temperature: 40}), true},
"one reading of several": {host(models.HostDiskSMART{Device: "sda"}, models.HostDiskSMART{Device: "sdb", Temperature: 40}), true},
"no temperature": {host(models.HostDiskSMART{Device: "sda"}), false},
"standby row": {host(models.HostDiskSMART{Device: "sda", Temperature: 40, Standby: true}), false},
"negative reading": {host(models.HostDiskSMART{Device: "sda", Temperature: -1}), false},
"no rows": {host(), false},
} {
if got := hostSuppliesNodeSMARTTemperatures(tc.host); got != tc.want {
t.Errorf("%s: got %v, want %v", name, got, tc.want)
}
}
}
// When the linked agent's SMART report carries no usable temperature, the
// node's SMART sensor list comes from the SSH collector, so a reading of a
// split Proxmox disk taken from it is the node's own and stays, while the
// agent's row still takes nothing else from the disk.
func TestMergeHostAgentSMARTIntoDisksKeepsSSHNodeSensorReadingOfSplitDisk(t *testing.T) {
nodes := []models.Node{{Name: "node1", LinkedAgentID: "agent-1"}}
hosts := []models.Host{{ID: "agent-1", Status: "online", Sensors: models.HostSensorSummary{
SMART: []models.HostDiskSMART{{Device: "sda", Serial: "SERIAL-A", Type: "sata", Health: "FAILED"}},
}}}
disk := models.PhysicalDisk{
ID: "pve1-node1-/dev/sda", Node: "node1", Instance: "pve1", DevPath: "/dev/sda", Serial: "SERIAL-A", Type: "sata",
Health: "PASSED", Wearout: 100, Temperature: 52,
Collection: &diskinventory.CollectionStatus{Temperature: diskinventory.Available(proxmoxNodeSMARTTemperatureSource)},
}
splits := &fakeDiskAgentSplits{splitDevPaths: map[string]bool{"/dev/sda": true}}
got := mergeHostAgentSMARTIntoDisks([]models.PhysicalDisk{disk}, nodes, hosts, splits)[0]
if got.Temperature != 52 || got.Collection.Temperature.Source != proxmoxNodeSMARTTemperatureSource ||
got.Health != "PASSED" || got.AgentSMARTReported || !got.AgentSMARTSplit {
t.Fatalf("split disk with an SSH-supplied reading: %+v / %+v", got, got.Collection)
}
// An agent that stopped reporting keeps its rows, temperatures included,
// but the node poll ignores them once its lease lapsed, so the reading is
// the SSH collector's again.
hosts[0].Status = "offline"
hosts[0].Sensors.SMART[0].Temperature = 61
lapsed := mergeHostAgentSMARTIntoDisks([]models.PhysicalDisk{disk}, nodes, hosts, splits)[0]
if lapsed.Temperature != 52 || lapsed.Collection.Temperature.Source != proxmoxNodeSMARTTemperatureSource || !lapsed.AgentSMARTSplit {
t.Fatalf("split disk with a lapsed agent dropped the node's own reading: %+v / %+v", lapsed, lapsed.Collection)
}
hosts[0].Status = "online"
live := mergeHostAgentSMARTIntoDisks([]models.PhysicalDisk{disk}, nodes, hosts, splits)[0]
if live.Temperature != 0 || live.Collection.Temperature.State != diskinventory.FieldUnsupported {
t.Fatalf("split disk with a reporting agent kept the agent-supplied reading: %+v / %+v", live, live.Collection)
}
}
@@ -0,0 +1,67 @@
package unifiedresources
import "github.com/rcourtman/pulse-go-rewrite/internal/models"
// ProxmoxDiskAgentSMARTSplit reports whether the operator split a Proxmox
// disk observation from the disk a host agent reports in one SMART row:
// report-merge on the merged disk (the drawer's Split merged resource)
// recorded an exclusion that separates them by the rule the linked disk join
// applies (physicalDiskSplitLocked). The PVE disk poller asks before it pairs
// the agent's row with the Proxmox disk on the agent's node, so a split
// Proxmox row carries what Proxmox reported and the PVE disk check judges it.
//
// It reads the exclusions this generation loaded, and whether it holds the two
// observations as one disk, never what the poller's pairing would fill into
// the Proxmox row: the pairing re-keys a Proxmox disk whose serial it fills or
// promotes, so a decision read from those fields would flip with it. The
// generation holds them as one disk only through a manual link, which is the
// operator's later decision, so pairing resumes while it stands, and the link's
// removal or a new report-merge splits them again. A registry without a store
// carries no operator decisions. It takes no lock monitoring holds.
func (a *MonitorAdapter) ProxmoxDiskAgentSMARTSplit(disk models.PhysicalDisk, host models.Host, smart models.HostDiskSMART) bool {
registry := a.currentRegistry()
if registry == nil {
return false
}
return registry.proxmoxDiskAgentSMARTSplit(disk, host, smart)
}
// proxmoxDiskAgentSMARTSplit judges the pair as the linked disk join would:
// the Proxmox observation by its source-specific candidate ID (its slot) and
// the identity Proxmox reported, the agent's disk by the ID this generation
// gave it, or, absent from it, by the identity its row reports.
func (rr *ResourceRegistry) proxmoxDiskAgentSMARTSplit(disk models.PhysicalDisk, host models.Host, smart models.HostDiskSMART) bool {
proxmoxSourceID := normalizeSourceID(disk.ID)
agentSourceID := normalizeSourceID(HostSMARTDiskSourceID(host, smart))
if proxmoxSourceID == "" || agentSourceID == "" {
return false
}
rr.mu.RLock()
defer rr.mu.RUnlock()
if len(rr.exclusions) == 0 {
return false
}
agentDiskID := rr.bySource[SourceAgent][agentSourceID]
if agentDiskID != "" && agentDiskID == rr.bySource[SourceProxmox][proxmoxSourceID] {
return false
}
agentDisk := rr.resources[agentDiskID]
if agentDisk == nil {
resource, identity := resourceFromHostSMARTDisk(host, smart)
resource.Identity = identity
// A row without hardware identity takes its candidate ID, which
// report-merge records for it, as the merged disk's ID.
resource.ID = rr.sourceSpecificID(ResourceTypePhysicalDisk, SourceAgent, agentSourceID)
if hostID := rr.bySource[SourceAgent][normalizeSourceID(host.ID)]; hostID != "" {
resource.ParentID = &hostID
}
agentDisk = &resource
}
observation, identity := resourceFromPhysicalDisk(disk)
observation.Identity = identity
if nodeID := rr.bySource[SourceProxmox][proxmoxNodeSourceID(disk.Instance, disk.Node)]; nodeID != "" {
observation.ParentID = &nodeID
}
return rr.physicalDiskSplitLocked(agentDisk, &observation, rr.sourceSpecificID(ResourceTypePhysicalDisk, SourceProxmox, proxmoxSourceID))
}
@@ -4243,7 +4243,9 @@ func TestPhysicalDiskExclusionFallbackStaysPerMachine(t *testing.T) {
// on every rebuild. The registry ingests agent disks first, so the agent's
// row holds the merged ID and keeps it; the Proxmox row takes the ID it holds
// alone, or its source-specific ID where both share a hardware identity (the
// agent holds that ID), as findMatch's excluded branch does.
// agent holds that ID), as findMatch's excluded branch does. The PVE disk
// poller asks ProxmoxDiskAgentSMARTSplit before it pairs the agent's SMART
// row with the Proxmox disk, and it answers as the exclusions split the rows.
func TestOperatorSplitSeparatesDisksJoinedInsideALinkedHost(t *testing.T) {
now := time.Now().UTC()
for _, shape := range []struct {
@@ -4331,6 +4333,15 @@ func TestOperatorSplitSeparatesDisksJoinedInsideALinkedHost(t *testing.T) {
t.Fatalf("fixture did not join the disks under the agent's ID: merged %q, alone: agent %q, proxmox %q", merged, agentAlone, pveAlone)
}
ids := diskSplitCandidates(t, adapter.currentRegistry(), merged)
// The PVE disk poller asks the adapter before it pairs the
// agent's SMART row with the Proxmox disk on the agent's node.
pollerSplit := func(step string, host models.Host, smart models.HostDiskSMART, want bool) {
t.Helper()
if got := adapter.ProxmoxDiskAgentSMARTSplit(shape.pve, host, smart); got != want {
t.Fatalf("%s: poller split = %v, want %v", step, got, want)
}
}
pollerSplit("before the request", host, shape.agent, false)
addExclusions(t, store, request.exclusions(ids))
want := linkedDiskRows{joined: merged}
@@ -4371,6 +4382,13 @@ func TestOperatorSplitSeparatesDisksJoinedInsideALinkedHost(t *testing.T) {
check, describe := exactly(want)
assertViews("after the request", snapshot, check, describe)
pollerSplit("after the request", host, shape.agent, request.split)
// A generation that has not ingested the agent's disk yet judges
// its row by the ID and identity the row would be keyed under.
unseen := NewMonitorAdapter(NewRegistry(store))
if got := unseen.ProxmoxDiskAgentSMARTSplit(shape.pve, host, shape.agent); got != request.split {
t.Fatalf("a generation without the agent's disk judged the split = %v, want %v", got, request.split)
}
if !request.split {
return
}
@@ -4383,6 +4401,11 @@ func TestOperatorSplitSeparatesDisksJoinedInsideALinkedHost(t *testing.T) {
}
check, describe = exactly(linkedDiskRows{joined: merged})
assertViews("after relink", snapshot, check, describe)
// The link is the operator's later decision, whether it
// replaced an exclusion (the Proxmox row held its candidate
// ID) or left it standing: the generation holds the
// observations as one disk, so the poller pairs them again.
pollerSplit("after relink", host, shape.agent, false)
relinked := adapter.currentRegistry()
var again [][2]string
for _, fold := range relinked.ManualLinkFolds(merged) {
@@ -4393,6 +4416,22 @@ func TestOperatorSplitSeparatesDisksJoinedInsideALinkedHost(t *testing.T) {
addExclusions(t, store, again)
check, describe = exactly(want)
assertViews("after splitting again", snapshot, check, describe)
pollerSplit("after splitting again", host, shape.agent, true)
// The split names the IDs of the disks it separated, not the
// Proxmox slot, so a replacement in the slot (different
// hardware on both sides) is a different disk the poller pairs.
replacedPVE, replacedAgent := shape.pve, shape.agent
replacedPVE.Serial = "REPLACEMENT-0009"
replacedAgent.Serial = "Z9Z9REPLACED"
if shape.pve.Serial == shape.agent.Serial {
replacedAgent.Serial = replacedPVE.Serial
}
replacedHost := host
replacedHost.Sensors.SMART = []models.HostDiskSMART{replacedAgent}
if adapter.ProxmoxDiskAgentSMARTSplit(replacedPVE, replacedHost, replacedAgent) {
t.Fatalf("a replacement in the slot inherited the split recorded against the disk it replaced")
}
// smartctl reads no serial from a disk in standby, so the
// agent's row takes its device-keyed ID. A Proxmox
@@ -4405,8 +4444,10 @@ func TestOperatorSplitSeparatesDisksJoinedInsideALinkedHost(t *testing.T) {
assertViews("while the agent's disk is in standby", asleep, func(got linkedDiskRows) bool {
return apart(got) && got.proxmox == merged
}, "the disks apart, the Proxmox row on "+merged)
pollerSplit("while the agent's disk is in standby", asleep.Hosts[0], asleep.Hosts[0].Sensors.SMART[0], true)
check, describe = exactly(want)
assertViews("once the agent's disk wakes", snapshot, check, describe)
pollerSplit("once the agent's disk wakes", host, shape.agent, true)
}
// A same-serial disk on another machine re-keys the agent's
@@ -4414,6 +4455,7 @@ func TestOperatorSplitSeparatesDisksJoinedInsideALinkedHost(t *testing.T) {
// the unscoped ID, still holds.
if shape.agent.Serial != "" {
assertViews("after a same-serial disk appears elsewhere", withClone, apart, "the disks apart")
pollerSplit("after a same-serial disk appears elsewhere", host, shape.agent, true)
}
})
}
@@ -4451,6 +4493,9 @@ func TestOperatorSplitSeparatesDisksJoinedInsideALinkedHost(t *testing.T) {
t.Fatalf("%s rebuild %d %s: got %+v, want the disks apart", step.name, rebuild, view, got)
}
}
if !adapter.ProxmoxDiskAgentSMARTSplit(shape.pve, host, shape.agent) {
t.Fatalf("%s rebuild %d: the poller would pair the disks the rows split", step.name, rebuild)
}
}
}
})