diff --git a/audit/internal/collector/contract.go b/audit/internal/collector/contract.go index 8556b1b..0a40ab9 100644 --- a/audit/internal/collector/contract.go +++ b/audit/internal/collector/contract.go @@ -1,6 +1,9 @@ package collector -import "strings" +import ( + "bee/audit/internal/schema" + "strings" +) const ( statusOK = "OK" @@ -62,3 +65,43 @@ func isRAIDClass(class string) bool { return false } } + +// statusSeverity ranks component statuses so merges can only escalate, never +// downgrade. Unknown ranks with OK: it means "couldn't tell", not "healthy", +// so it must not silently clear a Warning/Critical raised by an earlier stage. +func statusSeverity(status string) int { + switch strings.TrimSpace(status) { + case statusCritical: + return 3 + case statusWarning: + return 2 + case statusOK: + return 1 + case statusUnknown: + return 1 + default: + return 0 + } +} + +// mergeDeviceStatus applies status/description to dev only if it is at least +// as severe as whatever is already set. This lets later enrichment stages +// (e.g. NVIDIA telemetry) report their own findings without silently +// clobbering a Warning/Critical raised earlier in the collector pipeline +// (e.g. a PCIe link-speed degradation). +func mergeDeviceStatus(dev *schema.HardwarePCIeDevice, status, description string) { + if dev == nil || status == "" { + return + } + current := "" + if dev.Status != nil { + current = strings.TrimSpace(*dev.Status) + } + if current != "" && current != statusUnknown && statusSeverity(status) <= statusSeverity(current) { + return + } + dev.Status = &status + if strings.TrimSpace(description) != "" { + dev.ErrorDescription = &description + } +} diff --git a/audit/internal/collector/nvidia.go b/audit/internal/collector/nvidia.go index b6625c3..9a0beef 100644 --- a/audit/internal/collector/nvidia.go +++ b/audit/internal/collector/nvidia.go @@ -88,23 +88,24 @@ func enrichPCIeWithNVIDIAData(devs []schema.HardwarePCIeDevice, gpuByBDF map[str } status := statusOK + desc := "" if info.ECCUncorrected != nil && *info.ECCUncorrected > 0 { status = statusWarning - devs[i].ErrorDescription = stringPtr("GPU reports uncorrected ECC errors") + desc = "GPU reports uncorrected ECC errors" } if info.RemapUncorrectable != nil && *info.RemapUncorrectable > 0 { status = statusWarning - devs[i].ErrorDescription = stringPtr("GPU has uncorrectable row remap events (bad HBM cell repair scheduled)") + desc = "GPU has uncorrectable row remap events (bad HBM cell repair scheduled)" } if info.RemapFailure != nil && *info.RemapFailure { status = statusCritical - devs[i].ErrorDescription = stringPtr("GPU row remap failed to commit to InfoROM (XID 64)") + desc = "GPU row remap failed to commit to InfoROM (XID 64)" } if info.ResetRequired != nil && *info.ResetRequired { status = statusCritical - devs[i].ErrorDescription = stringPtr("GPU requires a reset") + desc = "GPU requires a reset" } - devs[i].Status = &status + mergeDeviceStatus(&devs[i], status, desc) injectNVIDIATelemetry(&devs[i], info) enriched++ } @@ -326,8 +327,7 @@ func isNVIDIADevice(dev schema.HardwarePCIeDevice) bool { } func setPCIeFallback(dev *schema.HardwarePCIeDevice) { - status := statusUnknown - dev.Status = &status + mergeDeviceStatus(dev, statusUnknown, "") } func injectNVIDIATelemetry(dev *schema.HardwarePCIeDevice, info nvidiaGPUInfo) { diff --git a/audit/internal/collector/nvidia_test.go b/audit/internal/collector/nvidia_test.go index 44bd0ea..cd076b7 100644 --- a/audit/internal/collector/nvidia_test.go +++ b/audit/internal/collector/nvidia_test.go @@ -192,6 +192,108 @@ func TestEnrichPCIeWithNVIDIAData_remapFailureCritical(t *testing.T) { } } +func TestEnrichPCIeWithNVIDIAData_preservesExistingPCIeLinkWarning(t *testing.T) { + vendorID := NvidiaVendorID + bdf := "0000:65:00.0" + status := statusWarning + desc := "PCIe link speed degraded: running at Gen1, capable of Gen4" + devices := []schema.HardwarePCIeDevice{ + { + HardwareComponentStatus: schema.HardwareComponentStatus{Status: &status, ErrorDescription: &desc}, + VendorID: &vendorID, + BDF: &bdf, + }, + } + + // Clean ECC/remap/reset: NVIDIA enrichment itself sees nothing wrong. + byBDF := map[string]nvidiaGPUInfo{ + "0000:65:00.0": {}, + } + + out := enrichPCIeWithNVIDIAData(devices, byBDF, true) + if out[0].Status == nil || *out[0].Status != statusWarning { + t.Fatalf("status: got %v, want %v (must not be overwritten by clean NVIDIA telemetry)", out[0].Status, statusWarning) + } + if out[0].ErrorDescription == nil || *out[0].ErrorDescription != desc { + t.Fatalf("error_description: got %v, want %q", out[0].ErrorDescription, desc) + } +} + +func TestEnrichPCIeWithNVIDIAData_eccWarningDoesNotDowngradeExistingWarning(t *testing.T) { + vendorID := NvidiaVendorID + bdf := "0000:65:00.0" + status := statusWarning + desc := "PCIe link speed degraded: running at Gen1, capable of Gen4" + devices := []schema.HardwarePCIeDevice{ + { + HardwareComponentStatus: schema.HardwareComponentStatus{Status: &status, ErrorDescription: &desc}, + VendorID: &vendorID, + BDF: &bdf, + }, + } + + byBDF := map[string]nvidiaGPUInfo{ + "0000:65:00.0": {ECCUncorrected: ptrInt64(1)}, + } + + out := enrichPCIeWithNVIDIAData(devices, byBDF, true) + // ECCUncorrected only raises Warning (same severity as the pre-existing + // PCIe-link Warning), so the merge must not lose the already-detected + // (and equally severe) status/description. + if out[0].Status == nil || *out[0].Status != statusWarning { + t.Fatalf("status: got %v, want %v", out[0].Status, statusWarning) + } + if out[0].ErrorDescription == nil || *out[0].ErrorDescription != desc { + t.Fatalf("error_description: got %v, want %q (equal-severity ECC finding must not clobber it)", out[0].ErrorDescription, desc) + } +} + +func TestEnrichPCIeWithNVIDIAData_resetRequiredEscalatesOverExistingWarning(t *testing.T) { + vendorID := NvidiaVendorID + bdf := "0000:65:00.0" + status := statusWarning + desc := "PCIe link speed degraded: running at Gen1, capable of Gen4" + devices := []schema.HardwarePCIeDevice{ + { + HardwareComponentStatus: schema.HardwareComponentStatus{Status: &status, ErrorDescription: &desc}, + VendorID: &vendorID, + BDF: &bdf, + }, + } + + byBDF := map[string]nvidiaGPUInfo{ + "0000:65:00.0": {ResetRequired: ptrBool(true)}, + } + + out := enrichPCIeWithNVIDIAData(devices, byBDF, true) + if out[0].Status == nil || *out[0].Status != statusCritical { + t.Fatalf("status: got %v, want %v (reset-required Critical must escalate over the PCIe-link Warning)", out[0].Status, statusCritical) + } + if out[0].ErrorDescription == nil || *out[0].ErrorDescription != "GPU requires a reset" { + t.Fatalf("error_description: got %v", out[0].ErrorDescription) + } +} + +func TestEnrichPCIeWithNVIDIAData_regressionNoOverride(t *testing.T) { + vendorID := NvidiaVendorID + bdf := "0000:65:00.0" + devices := []schema.HardwarePCIeDevice{ + {VendorID: &vendorID, BDF: &bdf}, + } + + byBDF := map[string]nvidiaGPUInfo{ + "0000:65:00.0": {}, + } + + out := enrichPCIeWithNVIDIAData(devices, byBDF, true) + if out[0].Status == nil || *out[0].Status != statusOK { + t.Fatalf("status: got %v, want %v", out[0].Status, statusOK) + } + if out[0].ErrorDescription != nil { + t.Fatalf("error_description: got %v, want nil", out[0].ErrorDescription) + } +} + func ptrInt64(v int64) *int64 { return &v } func ptrFloat(v float64) *float64 { return &v } func ptrBool(v bool) *bool { return &v }