fix(collector): stop NVIDIA enrichment from clobbering PCIe status
enrichPCIeWithNVIDIAData unconditionally overwrote dev.Status after collectPCIe() had already flagged a Warning/Critical (e.g. PCIe link speed degraded), so a clean ECC/remap/reset readout silently downgraded that finding back to OK while leaving the stale ErrorDescription behind. Add a severity-ordered merge (OK/Unknown < Warning < Critical) shared via mergeDeviceStatus in contract.go, and route both the NVIDIA status calculation and the driver-unavailable fallback through it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
6c8be629d2
commit
a34e823f82
@@ -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
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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 }
|
||||
|
||||
Reference in New Issue
Block a user