diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index fc280e4cc..01450aad8 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -282,6 +282,15 @@ Physical-disk refresh/merge logic now derives physical disks, nodes, and linked host-agent context from canonical `ReadState` before applying NVMe temperature and SMART merges, so skipped or background disk refresh no longer treats the snapshot as internal truth for that path. +That same monitoring-owned disk merge path must also treat host-agent SMART +attributes as canonical fill data for the Proxmox disk view. When a linked +host agent reports SMART health or NVMe `percentage_used` for a physical disk +that Proxmox itself exposes without trustworthy health or wearout, the merge +path in `internal/monitoring/monitor.go` must promote that data into the +canonical physical-disk model and the Proxmox polling runtime in +`internal/monitoring/monitor_pve.go` must evaluate disk alerts only after that +merged disk view exists, so controller-backed disks do not lose health and +endurance coverage between collection and alerting. Backup polling and recovery guest identity assembly now derive workload node, name, and type context from canonical `ReadState` instead of from diff --git a/internal/monitoring/canonical_guardrails_test.go b/internal/monitoring/canonical_guardrails_test.go index fc9be5302..fda09603b 100644 --- a/internal/monitoring/canonical_guardrails_test.go +++ b/internal/monitoring/canonical_guardrails_test.go @@ -440,6 +440,43 @@ func TestUnifiedPhysicalDiskMetricsUseCanonicalDiskHistoryPath(t *testing.T) { } } +func TestProxmoxDiskAlertsRunOnMergedDiskState(t *testing.T) { + cases := []struct { + file string + snippets []string + }{ + { + file: "monitor.go", + snippets: []string{ + "func mergeHostAgentSMARTIntoDisks(disks []models.PhysicalDisk, nodes []models.Node, hosts []models.Host) []models.PhysicalDisk {", + "deriveWearoutFromSMARTAttributes(matched.Attributes)", + `strings.EqualFold(updated[i].Health, "unknown")`, + }, + }, + { + file: "monitor_pve.go", + snippets: []string{ + "allDisks = mergeHostAgentSMARTIntoDisks(allDisks, nodesFromState, hosts)", + "m.alertManager.CheckDiskHealth(inst, disk.Node, proxmoxDiskFromPhysicalDisk(disk))", + "func proxmoxDiskFromPhysicalDisk(disk models.PhysicalDisk) proxmox.Disk {", + }, + }, + } + + for _, tc := range cases { + data, err := os.ReadFile(tc.file) + if err != nil { + t.Fatalf("failed to read %s: %v", tc.file, err) + } + source := string(data) + for _, snippet := range tc.snippets { + if !strings.Contains(source, snippet) { + t.Fatalf("%s must contain %q", tc.file, snippet) + } + } + } +} + func TestUnifiedPhysicalDiskMetricsAllowNativeHistoryProviders(t *testing.T) { monitorData, err := os.ReadFile("monitor.go") if err != nil { diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go index 4eb1323c7..af363dbfe 100644 --- a/internal/monitoring/monitor.go +++ b/internal/monitoring/monitor.go @@ -526,13 +526,37 @@ func mergeHostAgentSMARTIntoDisks(disks []models.PhysicalDisk, nodes []models.No // Always merge SMART attributes from host agent if matched.Attributes != nil { - updated[i].SmartAttributes = matched.Attributes + updated[i].SmartAttributes = smartAttributesCopy(matched.Attributes) + if updated[i].Wearout < 0 { + if derivedWearout := deriveWearoutFromSMARTAttributes(matched.Attributes); derivedWearout >= 0 { + updated[i].Wearout = derivedWearout + } + } + } + + if (strings.TrimSpace(updated[i].Health) == "" || strings.EqualFold(updated[i].Health, "unknown")) && strings.TrimSpace(matched.Health) != "" { + updated[i].Health = matched.Health } } return updated } +func deriveWearoutFromSMARTAttributes(attrs *models.SMARTAttributes) int { + if attrs == nil || attrs.PercentageUsed == nil { + return -1 + } + + used := *attrs.PercentageUsed + if used < 0 { + used = 0 + } + if used > 100 { + used = 100 + } + return 100 - used +} + func physicalDiskFromReadStateView(view *unifiedresources.PhysicalDiskView) models.PhysicalDisk { if view == nil { return models.PhysicalDisk{} diff --git a/internal/monitoring/monitor_integration_test.go b/internal/monitoring/monitor_integration_test.go index 6b60cea59..88446cb92 100644 --- a/internal/monitoring/monitor_integration_test.go +++ b/internal/monitoring/monitor_integration_test.go @@ -148,6 +148,65 @@ func TestMergeHostAgentSMARTIntoDisks_MergesSMARTAttributes(t *testing.T) { } } +func TestMergeHostAgentSMARTIntoDisks_DerivesWearoutFromSMARTAttributes(t *testing.T) { + disks := []models.PhysicalDisk{ + {ID: "d1", Node: "pve1", Serial: "SER1", Wearout: -1}, + } + nodes := []models.Node{ + {Name: "pve1", LinkedAgentID: "host-1"}, + } + used := 6 + hosts := []models.Host{ + { + ID: "host-1", + Sensors: models.HostSensorSummary{ + SMART: []models.HostDiskSMART{ + { + Device: "/dev/sda [megaraid,7]", + Serial: "SER1", + Health: "PASSED", + Attributes: &models.SMARTAttributes{ + PercentageUsed: &used, + }, + }, + }, + }, + }, + } + + result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts) + if result[0].Wearout != 94 { + t.Fatalf("expected wearout derived from PercentageUsed, got %d", result[0].Wearout) + } + if result[0].SmartAttributes == nil || result[0].SmartAttributes.PercentageUsed == nil || *result[0].SmartAttributes.PercentageUsed != 6 { + t.Fatalf("expected merged PercentageUsed, got %+v", result[0].SmartAttributes) + } +} + +func TestMergeHostAgentSMARTIntoDisks_FillsMissingHealthFromHostSMART(t *testing.T) { + disks := []models.PhysicalDisk{ + {ID: "d1", Node: "pve1", Serial: "SER1", Health: "UNKNOWN"}, + } + nodes := []models.Node{ + {Name: "pve1", LinkedAgentID: "host-1"}, + } + hosts := []models.Host{ + { + ID: "host-1", + Sensors: models.HostSensorSummary{ + SMART: []models.HostDiskSMART{ + {Device: "/dev/sda", Serial: "SER1", Health: "FAILED"}, + }, + }, + }, + } + + result := mergeHostAgentSMARTIntoDisks(disks, nodes, hosts) + if result[0].Health != "FAILED" { + t.Fatalf("expected host SMART health to fill unknown disk health, got %q", result[0].Health) + } +} + func TestMergeHostAgentSMARTIntoDisks_NoAgentLink(t *testing.T) { disks := []models.PhysicalDisk{ {ID: "d1", Node: "pve1", Serial: "SER1", Temperature: 0}, diff --git a/internal/monitoring/monitor_pve.go b/internal/monitoring/monitor_pve.go index 9dd33f12f..dc6061c6a 100644 --- a/internal/monitoring/monitor_pve.go +++ b/internal/monitoring/monitor_pve.go @@ -807,9 +807,9 @@ func (m *Monitor) maybePollPhysicalDisksAsync( // Mark this node as successfully polled polledNodes[node.Node] = true - // Check each disk for health issues and add to state + // Record each disk; alert evaluation happens after host-agent SMART merges + // so the canonical disk view includes post-merge health/wearout data. for _, disk := range disks { - // Create PhysicalDisk model diskID := fmt.Sprintf("%s-%s-%s", inst, node.Node, strings.ReplaceAll(disk.DevPath, "/", "-")) physicalDisk := models.PhysicalDisk{ ID: diskID, @@ -829,53 +829,6 @@ func (m *Monitor) maybePollPhysicalDisksAsync( } allDisks = append(allDisks, physicalDisk) - - log.Debug(). - Str("node", node.Node). - Str("disk", disk.DevPath). - Str("model", disk.Model). - Str("health", disk.Health). - Int("wearout", disk.Wearout). - Msg("Checking disk health") - - // If the linked host agent has --disk-exclude patterns that match - // this disk, send a synthetic healthy status to clear any existing - // alerts and skip normal health/wearout checks. - if excludePatterns, ok := diskExcludeByNode[node.Node]; ok { - if fsfilters.MatchesDeviceExclude(disk.DevPath, excludePatterns) { - healthyDisk := disk - healthyDisk.Health = "PASSED" - healthyDisk.Wearout = 100 - m.alertManager.CheckDiskHealth(inst, node.Node, healthyDisk) - continue - } - } - - normalizedHealth := strings.ToUpper(strings.TrimSpace(disk.Health)) - if normalizedHealth != "" && normalizedHealth != "UNKNOWN" && normalizedHealth != "PASSED" && normalizedHealth != "OK" { - // Disk has failed or is failing - alert manager will handle this - log.Warn(). - Str("node", node.Node). - Str("disk", disk.DevPath). - Str("model", disk.Model). - Str("health", disk.Health). - Int("wearout", disk.Wearout). - Msg("Disk health issue detected") - - // Pass disk info to alert manager - m.alertManager.CheckDiskHealth(inst, node.Node, disk) - } else if disk.Wearout > 0 && disk.Wearout < 10 { - // Low wearout warning (less than 10% life remaining) - log.Warn(). - Str("node", node.Node). - Str("disk", disk.DevPath). - Str("model", disk.Model). - Int("wearout", disk.Wearout). - Msg("SSD wearout critical - less than 10% life remaining") - - // Pass to alert manager for wearout alert - m.alertManager.CheckDiskHealth(inst, node.Node, disk) - } } } @@ -894,6 +847,29 @@ func (m *Monitor) maybePollPhysicalDisksAsync( allDisks = mergeNVMeTempsIntoDisks(allDisks, nodesFromState) allDisks = mergeHostAgentSMARTIntoDisks(allDisks, nodesFromState, hosts) + for _, disk := range allDisks { + if !polledNodes[disk.Node] { + continue + } + + log.Debug(). + Str("node", disk.Node). + Str("disk", disk.DevPath). + Str("model", disk.Model). + Str("health", disk.Health). + 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)) + } // Write SMART metrics to persistent store if m.metricsStore != nil { @@ -913,6 +889,21 @@ func (m *Monitor) maybePollPhysicalDisksAsync( }(instanceName, client, nodes, nodeEffectiveStatus, modelNodes) } +func proxmoxDiskFromPhysicalDisk(disk models.PhysicalDisk) proxmox.Disk { + return proxmox.Disk{ + DevPath: disk.DevPath, + Model: disk.Model, + Serial: disk.Serial, + Type: disk.Type, + Health: disk.Health, + Wearout: disk.Wearout, + Size: disk.Size, + RPM: disk.RPM, + Used: disk.Used, + WWN: disk.WWN, + } +} + // pollPVEInstance polls a single PVE instance func (m *Monitor) pollPVEInstance(ctx context.Context, instanceName string, client PVEClientInterface) { defer recoverFromPanic(fmt.Sprintf("pollPVEInstance-%s", instanceName))