diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index a912bfce62..0c751ad9be 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -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 diff --git a/internal/monitoring/canonical_guardrails_test.go b/internal/monitoring/canonical_guardrails_test.go index 3c398a8cbf..6234aff22a 100644 --- a/internal/monitoring/canonical_guardrails_test.go +++ b/internal/monitoring/canonical_guardrails_test.go @@ -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 {", }, }, diff --git a/internal/monitoring/monitor_alerts.go b/internal/monitoring/monitor_alerts.go index 89e870836f..a75e7dd557 100644 --- a/internal/monitoring/monitor_alerts.go +++ b/internal/monitoring/monitor_alerts.go @@ -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]) + } +} diff --git a/internal/monitoring/monitor_mock_alerts_test.go b/internal/monitoring/monitor_mock_alerts_test.go index 66e69d616a..3c5c933339 100644 --- a/internal/monitoring/monitor_mock_alerts_test.go +++ b/internal/monitoring/monitor_mock_alerts_test.go @@ -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) + } +} diff --git a/internal/monitoring/monitor_pve.go b/internal/monitoring/monitor_pve.go index 8b03fac7e0..581307f263 100644 --- a/internal/monitoring/monitor_pve.go +++ b/internal/monitoring/monitor_pve.go @@ -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,