Raise disk health alerts for mock physical disks (#2633)

`checkMockAlerts` evaluated guests, agents, nodes, storage, PBS, PMG and
Docker against the mock estate but never physical disks. The mock
generator deliberately keeps a FAILED health cohort and worn SSDs stable
across restarts, so mock and demo stacks showed failing and "Replace
Now" disks with no `disk-health` or `disk-wearout` alert, and the disk
alert lifecycle could not be exercised without real hardware.

## Change

- `checkMockAlerts` now evaluates fixture disks through
`checkMockPhysicalDiskAlerts`, applying the physical disk poller's
boundary: only disks on online nodes (matched by instance and node
name), with the linked agent's `--disk-exclude` patterns forcing a
matched device healthy.
- The poller's per-disk rule (exclusion forces healthy, else
`CheckDiskHealth`) moves into `checkPhysicalDiskAlerts`, shared by
`maybePollPhysicalDisksAsync` and the mock pass. The poller's behaviour
is unchanged; the source guardrail
`TestProxmoxDiskAlertsRunOnMergedDiskState` now pins the helper call.
- The poller's host-agent link settle deferral (#1674) is not applied in
mock mode: fixture links and exclusions are complete from the first
pass.

## Demo-facing effect

Measured on private stacks with the public demo's exact estate
(`/etc/pulse/.env`: 8 nodes, `DEMO_MODE`), baseline vs patched: the demo
estate has 12 Proxmox disks and none FAILED. The only new disk alert is
one warning-level `disk-wearout` ("SSD has less than 10% life remaining
(7% wearout)" on West Production E), whose resource ID is one of the
disk resource's canonical aliases. No `disk-health` alert opens on that
estate. The default test estate (50 nodes) raises five `disk-health` and
one `disk-wearout` alert.

## Known gaps, left out of scope

- Like the other loops in `checkMockAlerts`, the disk loop relies on the
pass's single up-front mode check, so a pass in flight when mock mode is
switched off can reopen alerts after `ClearActiveAlerts`. That race
predates this change and also involves the frontend refresh and backup
rollup paths, so it needs a monitor-wide fix rather than a lock around
this pass alone.
- `CheckDiskHealth` logs at error/warn on every evaluation of a FAILED
or worn disk, so in mock mode such a disk logs once per alert tick.
- The wearout message calls 7% life remaining "7% wearout".

## Verification

- `TestCheckMockAlertsEvaluatesPhysicalDisks` (pinned default estate)
and `TestCheckMockPhysicalDiskAlertsFollowsPollerBoundary` (offline
node, same node name on another instance, exclusion resolving existing
alerts, node departure cleanup). Mutations that drop the pass, ignore
offline nodes, ignore exclusions, or key by node name only each fail a
test.
- Full `go test ./internal/monitoring/` and the mock tests under `-race`
pass.
This commit is contained in:
rcourtman
2026-10-07 01:00:56 +01:00
committed by GitHub
5 changed files with 238 additions and 10 deletions
@@ -1679,6 +1679,62 @@ boundaries:
`TestMockKubernetesLoneHeavyPodStaysUnderNodeCeiling` in
`internal/mock/generator_test.go`.
Mock alert evaluation covers physical disks as well. Live Proxmox disks are
evaluated by the physical disk poller in `maybePollPhysicalDisksAsync`, which
only polls configured Proxmox instances, so fixture disks had no evaluation
path. The fixture deliberately keeps a FAILED health cohort and worn SSDs
stable across restarts, so the estate showed failing and "Replace Now" disks
with no `disk-health` or `disk-wearout` alert. `checkMockAlerts` now evaluates
fixture disks through `checkMockPhysicalDiskAlerts`. Both paths share the
per-disk rule in `checkPhysicalDiskAlerts` (a device matched by the linked
agent's `--disk-exclude` patterns is evaluated as healthy, otherwise
`CheckDiskHealth` judges the disk), and the mock pass applies the poller's
boundary around it:
- Only disks on online nodes are evaluated, as the poller only evaluates nodes
it reached. A disk on an offline node keeps whatever alert it had. The mock
pass treats every disk on an online fixture node as collected; the poller
also skips a node whose disk query and SMART fallback both failed. Nodes are
matched by instance and name, as the poller only pairs a disk with its own
instance's nodes.
- Excluding a disk resolves the alerts it already raised, because the excluded
device is evaluated as healthy rather than skipped.
- The poller's wait for host-agent links to settle after a restart (#1674) is
not applied: fixture links and exclusions are complete from the first pass.
- A disk whose node name leaves the estate loses its alerts through
`CleanupAlertsForNodes`, which the same pass already runs. That cleanup
matches node names only, so it relies on fixture node names being unique
across instances, which they are.
The disk loop relies on the pass's single up-front mode check, like every
loop in `checkMockAlerts` except the host-agent step, which re-checks under its
own lock. A pass already past that check when mock mode is switched off can
reopen its fixture alerts after `SetMockMode(false)` clears them, and
other paths that evaluate fixture data (frontend refresh into unified alert
evaluation, backup rollups) have the same window. That mode-exit race predates
the disk loop and is not closed here; a lock around `checkMockAlerts` alone
would leave the other paths open.
The disk evaluation runs on the mock alert tick, not the poller's disk
interval (five minutes by default, `PhysicalDiskPollingMinutes` per instance).
`CheckDiskHealth` logs at error level whenever it evaluates a FAILED disk
(models with a known firmware bug skip the health check) and at warn level for
a worn SSD, so an evaluated fixture disk in either state logs on every tick. On
2026-10-06, private stacks with the public demo's estate (8 nodes) carried 12
Proxmox disks, none FAILED. Baseline against patched, the only new disk alert
was one warning-level `disk-wearout` (a 7% SSD on `pve5`), whose resource ID is
one of the disk resource's canonical aliases, and neither stack had a
`disk-health` alert. `CheckDiskHealth` raises only those two alert types, so
the other differences between the two boots (ZFS and Docker service alerts) are
not from the disk pass; those fixtures draw random state each boot. The default
test estate raises five `disk-health` and one `disk-wearout` alert. The tests
pin the boundary: `TestCheckMockAlertsEvaluatesPhysicalDisks` and
`TestCheckMockPhysicalDiskAlertsFollowsPollerBoundary` in
`internal/monitoring/monitor_mock_alerts_test.go`, with
`TestProxmoxDiskAlertsRunOnMergedDiskState` in
`internal/monitoring/canonical_guardrails_test.go` keeping the poller on the
shared helper.
Host and container-runtime disk collection supports an explicit include list
for filesystems hidden by Pulse's automatic virtual/container filtering. The
include list is bounded to that automatic filter; explicit disk exclusions
@@ -2234,7 +2234,8 @@ func TestProxmoxDiskAlertsRunOnMergedDiskState(t *testing.T) {
file: "monitor_pve.go",
snippets: []string{
"allDisks = mergeHostAgentSMARTIntoDisks(allDisks, nodesFromState, hosts)",
"m.alertManager.CheckDiskHealth(inst, disk.Node, proxmoxDiskFromPhysicalDisk(disk))",
"m.checkPhysicalDiskAlerts(inst, disk, diskExcludeByNode[disk.Node])",
"m.alertManager.CheckDiskHealth(instance, disk.Node, proxmoxDiskFromPhysicalDisk(disk))",
"func proxmoxDiskFromPhysicalDisk(disk models.PhysicalDisk) proxmox.Disk {",
},
},
+43
View File
@@ -716,6 +716,14 @@ func (m *Monitor) checkMockAlerts() {
m.alertManager.CheckStorageWithCapacityTrend(storage, m.storageCapacityTrend(storage, time.Now()))
}
// Check alerts for physical disks. Live disks are evaluated by the
// physical disk poller, which only polls configured Proxmox instances, and
// the fixture deliberately keeps a FAILED cohort and worn SSDs, so without
// this pass the estate shows failing disks with no disk-health or
// disk-wearout alert.
log.Debug().Int("diskCount", len(state.PhysicalDisks)).Msg("checking physical disk alerts")
m.checkMockPhysicalDiskAlerts(state.PhysicalDisks, state.Nodes, state.Hosts)
// Check alerts for PBS instances
log.Debug().Int("pbsCount", len(state.PBSInstances)).Msg("checking PBS alerts")
for _, pbsInst := range state.PBSInstances {
@@ -815,3 +823,38 @@ func (m *Monitor) checkMockDockerHostAlerts(host models.DockerHost) {
}
m.alertManager.CheckDockerHost(host)
}
// checkMockPhysicalDiskAlerts applies the physical disk poller's alert
// boundary to fixture disks. The poller only evaluates disks on nodes it
// reached, so a disk on a node that is not online keeps whatever alert it
// had, and a device matched by the linked agent's --disk-exclude patterns is
// evaluated as healthy. The poller's wait for host-agent links to settle after
// a restart does not apply: fixture links and exclusions are complete from the
// first pass.
func (m *Monitor) checkMockPhysicalDiskAlerts(disks []models.PhysicalDisk, nodes []models.Node, hosts []models.Host) {
type nodeKey struct{ instance, name string }
excludeByHost := make(map[string][]string, len(hosts))
for _, host := range hosts {
if len(host.DiskExclude) > 0 {
excludeByHost[host.ID] = host.DiskExclude
}
}
onlineNodes := make(map[nodeKey]bool, len(nodes))
excludeByNode := make(map[nodeKey][]string)
for _, node := range nodes {
key := nodeKey{instance: node.Instance, name: node.Name}
onlineNodes[key] = node.Status == "online"
if patterns := excludeByHost[node.LinkedAgentID]; node.LinkedAgentID != "" && len(patterns) > 0 {
excludeByNode[key] = patterns
}
}
for _, disk := range disks {
key := nodeKey{instance: disk.Instance, name: disk.Node}
if !onlineNodes[key] {
continue
}
m.checkPhysicalDiskAlerts(disk.Instance, disk, excludeByNode[key])
}
}
@@ -239,3 +239,124 @@ func TestMockHostAgentPassAfterLeavingMockModeRegistersNothing(t *testing.T) {
}
t.Fatal("stale mock pass registered fixture hostname pve1 and suppressed the real node's memory alert")
}
// diskAlertKeys lists the active disk-health and disk-wearout alerts as
// "type instance/node device" so assertions name the disk, not the alert ID.
func diskAlertKeys(manager *alerts.Manager) map[string]bool {
keys := make(map[string]bool)
for _, alert := range manager.GetActiveAlerts() {
if alert.Type != "disk-health" && alert.Type != "disk-wearout" {
continue
}
devPath, _ := alert.Metadata["disk_path"].(string)
keys[alert.Type+" "+alert.Instance+"/"+alert.Node+" "+devPath] = true
}
return keys
}
func TestCheckMockAlertsEvaluatesPhysicalDisks(t *testing.T) {
// Pin the default estate: a PULSE_MOCK_* environment can select one with
// no FAILED disk (the public demo's eight nodes have none).
previousEnabled, previousConfig := mock.IsMockEnabled(), mock.GetConfig()
t.Cleanup(func() {
mustSetMockEnabled(t, false)
mock.SetMockConfig(previousConfig)
mustSetMockEnabled(t, previousEnabled)
})
mustSetMockEnabled(t, false)
cfg := mock.DefaultConfig
cfg.UpdateInterval = 5 * time.Minute
mock.SetMockConfig(cfg)
mustSetMockEnabled(t, true)
manager := newMockHostAlertTestManager(t)
monitor := &Monitor{alertManager: manager}
monitor.checkMockAlerts()
// The fixture keeps a FAILED cohort and worn SSDs stable across restarts
// so the disk alert lifecycle has evidence to act on. Each must raise the
// alert the physical disk poller raises for a live disk, and a disk on a
// node the poller would not reach raises nothing.
state := mock.CurrentFixtureGraph().State
onlineNodes := make(map[string]bool, len(state.Nodes))
for _, node := range state.Nodes {
onlineNodes[node.Instance+"/"+node.Name] = node.Status == "online"
}
want := make(map[string]bool)
failed, worn := 0, 0
for _, disk := range state.PhysicalDisks {
if !onlineNodes[disk.Instance+"/"+disk.Node] {
continue
}
if strings.EqualFold(disk.Health, "FAILED") {
want["disk-health "+disk.Instance+"/"+disk.Node+" "+disk.DevPath] = true
failed++
}
if disk.Wearout >= 0 && disk.Wearout < 10 {
want["disk-wearout "+disk.Instance+"/"+disk.Node+" "+disk.DevPath] = true
worn++
}
}
if failed == 0 || worn == 0 {
t.Fatalf("mock fixture has %d FAILED and %d worn disks on online nodes, want at least one of each", failed, worn)
}
got := diskAlertKeys(manager)
for key := range got {
if !want[key] {
t.Errorf("mock pass raised unexpected disk alert %q", key)
}
}
for key := range want {
if !got[key] {
t.Errorf("mock pass raised no alert %q", key)
}
}
}
func TestCheckMockPhysicalDiskAlertsFollowsPollerBoundary(t *testing.T) {
manager := newMockHostAlertTestManager(t)
monitor := &Monitor{alertManager: manager}
nodes := []models.Node{
{ID: "west-pve1", Name: "pve1", Instance: "west", Status: "online", LinkedAgentID: "host-west-pve1"},
{ID: "east-pve1", Name: "pve1", Instance: "east", Status: "offline"},
}
hosts := []models.Host{{ID: "host-west-pve1", Hostname: "pve1", Status: "online"}}
disks := []models.PhysicalDisk{
{Instance: "west", Node: "pve1", DevPath: "/dev/sda", Model: "Crucial MX500 2TB", Type: "sata", Health: "FAILED", Wearout: -1},
{Instance: "west", Node: "pve1", DevPath: "/dev/nvme0n1", Model: "Samsung 980 PRO 2TB", Type: "nvme", Health: "PASSED", Wearout: 6},
// Same node name on another instance: that node is offline, so the
// poller would not reach it and its disk must not alert.
{Instance: "east", Node: "pve1", DevPath: "/dev/sdb", Model: "WD Red Pro 8TB", Type: "sata", Health: "FAILED", Wearout: -1},
}
monitor.checkMockPhysicalDiskAlerts(disks, nodes, hosts)
got := diskAlertKeys(manager)
if len(got) != 2 || !got["disk-health west/pve1 /dev/sda"] || !got["disk-wearout west/pve1 /dev/nvme0n1"] {
t.Fatalf("disk alerts = %v, want only FAILED health on west/pve1 /dev/sda and wearout on west/pve1 /dev/nvme0n1", got)
}
// The linked agent's --disk-exclude forces matched devices healthy, so
// excluding a disk resolves the alerts it already raised. Endurance
// recovery needs three healthy-looking passes.
hosts[0].DiskExclude = []string{"/dev/sda", "nvme0n1"}
for i := 0; i < 3; i++ {
monitor.checkMockPhysicalDiskAlerts(disks, nodes, hosts)
}
if got := diskAlertKeys(manager); len(got) != 0 {
t.Fatalf("excluded devices kept disk alerts %v", got)
}
// A node that leaves the estate takes its disk alerts with it through the
// node cleanup the mock pass runs first.
hosts[0].DiskExclude = nil
monitor.checkMockPhysicalDiskAlerts(disks, nodes, hosts)
if got := diskAlertKeys(manager); len(got) != 2 {
t.Fatalf("disk alerts after removing the exclusion = %v, want both west/pve1 alerts back", got)
}
manager.CleanupAlertsForNodes(map[string]bool{"pve2": true})
if got := diskAlertKeys(manager); len(got) != 0 {
t.Fatalf("disk alerts outlived their departed node: %v", got)
}
}
+16 -9
View File
@@ -1175,15 +1175,7 @@ func (m *Monitor) maybePollPhysicalDisksAsync(
Int("wearout", disk.Wearout).
Msg("Checking disk health")
if excludePatterns, ok := diskExcludeByNode[disk.Node]; ok && fsfilters.MatchesDeviceExclude(disk.DevPath, excludePatterns) {
healthyDisk := proxmoxDiskFromPhysicalDisk(disk)
healthyDisk.Health = "PASSED"
healthyDisk.Wearout = 100
m.alertManager.CheckDiskHealth(inst, disk.Node, healthyDisk)
continue
}
m.alertManager.CheckDiskHealth(inst, disk.Node, proxmoxDiskFromPhysicalDisk(disk))
m.checkPhysicalDiskAlerts(inst, disk, diskExcludeByNode[disk.Node])
}
// Write SMART metrics to persistent store
@@ -1380,6 +1372,21 @@ func preserveUnavailablePhysicalDiskEvidence(current, previous models.PhysicalDi
return current
}
// checkPhysicalDiskAlerts evaluates one Proxmox physical disk's health and
// wearout alerts. A device matched by the linked host agent's --disk-exclude
// patterns is evaluated as healthy, so excluding a disk also resolves the
// alerts it already raised.
func (m *Monitor) checkPhysicalDiskAlerts(instance string, disk models.PhysicalDisk, excludePatterns []string) {
if len(excludePatterns) > 0 && fsfilters.MatchesDeviceExclude(disk.DevPath, excludePatterns) {
healthyDisk := proxmoxDiskFromPhysicalDisk(disk)
healthyDisk.Health = "PASSED"
healthyDisk.Wearout = 100
m.alertManager.CheckDiskHealth(instance, disk.Node, healthyDisk)
return
}
m.alertManager.CheckDiskHealth(instance, disk.Node, proxmoxDiskFromPhysicalDisk(disk))
}
func proxmoxDiskFromPhysicalDisk(disk models.PhysicalDisk) proxmox.Disk {
return proxmox.Disk{
DevPath: disk.DevPath,