fix(collector): dedup GPUs and resolve model text on xFusion boards without PCI identity
A re-run against the G5500 V7 (previous fix commit a3567dd) surfaced two
follow-on bugs once the GPUs started being classified correctly:
- gpuDocDedupKey fell straight through to the full @odata.id path when a
GPU had neither SerialNumber nor BDF, so the same physical H100 exposed
under both Systems/1/PCIeDevices and Chassis/1/PCIeDevices (identical
resource Id, different collection root) was kept twice, doubling the
reported GPU count. Added an Id + resolved VendorID/DeviceID fallback
tier that collapses same-Id duplicates without collapsing genuinely
distinct GPUs that happen to share a vendor/device pair.
- isMissingOrRawPCIModel/isGenericRedfishInventoryName didn't recognize
xFusion's generic "PCIeCardN"/"OCPCardN" slot labels, so when the doc's
actual Model field was empty and GPU.Model fell back to the slot-like
Name field, the pci.ids VendorId/DeviceId resolution never fired and the
GPU surfaced with a meaningless model like "PCIeCard1" instead of the
real chip name.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
a3567dd5f6
commit
1e4ec513e1
@@ -4591,6 +4591,10 @@ func isGenericRedfishInventoryName(value string) bool {
|
|||||||
return true
|
return true
|
||||||
case value == "pciedevice", strings.HasPrefix(value, "pciedevice_"), strings.HasPrefix(value, "pciedevice "):
|
case value == "pciedevice", strings.HasPrefix(value, "pciedevice_"), strings.HasPrefix(value, "pciedevice "):
|
||||||
return true
|
return true
|
||||||
|
// xFusion (and similar OEM BMCs) label generic PCIe/OCP slots "PCIeCardN"/
|
||||||
|
// "OCPCardN" instead of a real model — these are slot labels, not model text.
|
||||||
|
case strings.HasPrefix(value, "pciecard"), strings.HasPrefix(value, "ocpcard"):
|
||||||
|
return true
|
||||||
case value == "pciefunction", strings.HasPrefix(value, "pciefunction_"), strings.HasPrefix(value, "pciefunction "):
|
case value == "pciefunction", strings.HasPrefix(value, "pciefunction_"), strings.HasPrefix(value, "pciefunction "):
|
||||||
return true
|
return true
|
||||||
case value == "ethernetinterface", strings.HasPrefix(value, "ethernetinterface_"), strings.HasPrefix(value, "ethernetinterface "):
|
case value == "ethernetinterface", strings.HasPrefix(value, "ethernetinterface_"), strings.HasPrefix(value, "ethernetinterface "):
|
||||||
@@ -4818,6 +4822,15 @@ func gpuDocDedupKey(doc map[string]interface{}, gpu models.GPU) string {
|
|||||||
if bdf := strings.TrimSpace(gpu.BDF); bdf != "" {
|
if bdf := strings.TrimSpace(gpu.BDF); bdf != "" {
|
||||||
return bdf
|
return bdf
|
||||||
}
|
}
|
||||||
|
// Some BMCs (e.g. xFusion) expose the same physical GPU under both
|
||||||
|
// Systems/{id}/PCIeDevices and Chassis/{id}/PCIeDevices with neither
|
||||||
|
// serial nor BDF populated in either copy. The resource Id is identical
|
||||||
|
// across both trees for the same device, so combine it with the resolved
|
||||||
|
// VendorID/DeviceID (stable, collection-independent) to still collapse
|
||||||
|
// them without risking collisions between distinct GPUs.
|
||||||
|
if id := strings.ToLower(strings.TrimSpace(asString(doc["Id"]))); id != "" && (gpu.VendorID != 0 || gpu.DeviceID != 0) {
|
||||||
|
return fmt.Sprintf("id:%s|vd:%04x:%04x", id, gpu.VendorID, gpu.DeviceID)
|
||||||
|
}
|
||||||
if path := normalizeRedfishPath(asString(doc["@odata.id"])); path != "" {
|
if path := normalizeRedfishPath(asString(doc["@odata.id"])); path != "" {
|
||||||
return "path:" + path
|
return "path:" + path
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -3908,6 +3908,76 @@ func TestLooksLikeGPU_ResolvesVendorDeviceIDWhenModelTextMissing(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestGpuDocDedupKey_CollapsesSameIdAcrossCollectionRootsWithoutIdentity
|
||||||
|
// covers xFusion BMCs (e.g. G5500 V7) where the same physical GPU is
|
||||||
|
// enumerated under both Systems/{id}/PCIeDevices and Chassis/{id}/PCIeDevices
|
||||||
|
// with neither SerialNumber nor BDF populated in either copy. Without a
|
||||||
|
// fallback keyed on the shared resource Id + resolved VendorID/DeviceID,
|
||||||
|
// collectGPUs would keep both copies and double the reported GPU count.
|
||||||
|
func TestGpuDocDedupKey_CollapsesSameIdAcrossCollectionRootsWithoutIdentity(t *testing.T) {
|
||||||
|
gpu := models.GPU{VendorID: 0x10de, DeviceID: 0x2330}
|
||||||
|
|
||||||
|
systemsDoc := map[string]interface{}{"@odata.id": "/redfish/v1/Systems/1/PCIeDevices/PCIeCard1", "Id": "PCIeCard1"}
|
||||||
|
chassisDoc := map[string]interface{}{"@odata.id": "/redfish/v1/Chassis/1/PCIeDevices/PCIeCard1", "Id": "PCIeCard1"}
|
||||||
|
|
||||||
|
key1 := gpuDocDedupKey(systemsDoc, gpu)
|
||||||
|
key2 := gpuDocDedupKey(chassisDoc, gpu)
|
||||||
|
if key1 == "" || key2 == "" {
|
||||||
|
t.Fatalf("expected non-empty dedup keys, got %q and %q", key1, key2)
|
||||||
|
}
|
||||||
|
if key1 != key2 {
|
||||||
|
t.Fatalf("expected same physical GPU under different collection roots to dedup to the same key, got %q vs %q", key1, key2)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestGpuDocDedupKey_DistinctIdsStayDistinct guards against the Id+VendorID/
|
||||||
|
// DeviceID fallback over-collapsing genuinely different GPUs that share a
|
||||||
|
// vendor/device pair but have distinct slot Ids.
|
||||||
|
func TestGpuDocDedupKey_DistinctIdsStayDistinct(t *testing.T) {
|
||||||
|
gpu := models.GPU{VendorID: 0x10de, DeviceID: 0x2330}
|
||||||
|
|
||||||
|
doc1 := map[string]interface{}{"@odata.id": "/redfish/v1/Chassis/1/PCIeDevices/PCIeCard1", "Id": "PCIeCard1"}
|
||||||
|
doc3 := map[string]interface{}{"@odata.id": "/redfish/v1/Chassis/1/PCIeDevices/PCIeCard3", "Id": "PCIeCard3"}
|
||||||
|
|
||||||
|
if gpuDocDedupKey(doc1, gpu) == gpuDocDedupKey(doc3, gpu) {
|
||||||
|
t.Fatal("expected distinct GPU slot Ids to produce distinct dedup keys")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestIsMissingOrRawPCIModel_TreatsGenericSlotLabelsAsMissing covers xFusion
|
||||||
|
// BMCs that label generic PCIe/OCP slots "PCIeCardN"/"OCPCardN" instead of a
|
||||||
|
// real model — these must be treated as missing so pci.ids VendorId/DeviceId
|
||||||
|
// resolution fills in the real GPU model instead of leaving the slot label.
|
||||||
|
func TestIsMissingOrRawPCIModel_TreatsGenericSlotLabelsAsMissing(t *testing.T) {
|
||||||
|
for _, model := range []string{"PCIeCard1", "PCIeCard3", "OCPCard1", "pciecard1"} {
|
||||||
|
if !isMissingOrRawPCIModel(model) {
|
||||||
|
t.Errorf("expected %q to be treated as a missing/raw model", model)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if isMissingOrRawPCIModel("H100 SXM5 80GB") {
|
||||||
|
t.Fatal("expected a real model name not to be treated as missing/raw")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestParseGPUWithSupplementalDocs_ResolvesModelWhenOnlySlotLabelPresent
|
||||||
|
// reproduces the G5500 V7 case end-to-end: the PCIeDevice doc has no Model,
|
||||||
|
// only Name="PCIeCard1" (a slot label), and identity comes solely from the
|
||||||
|
// linked PCIeFunction's raw VendorId/DeviceId. The parsed GPU's Model must
|
||||||
|
// resolve to the real chip name via pci.ids instead of surfacing "PCIeCard1".
|
||||||
|
func TestParseGPUWithSupplementalDocs_ResolvesModelWhenOnlySlotLabelPresent(t *testing.T) {
|
||||||
|
doc := map[string]interface{}{
|
||||||
|
"Id": "PCIeCard1",
|
||||||
|
"Name": "PCIeCard1",
|
||||||
|
}
|
||||||
|
functionDocs := []map[string]interface{}{
|
||||||
|
{"VendorId": "0x10de", "DeviceId": "0x2330"},
|
||||||
|
}
|
||||||
|
gpu := parseGPUWithSupplementalDocs(doc, functionDocs, nil, 1)
|
||||||
|
if gpu.Model == "PCIeCard1" || strings.TrimSpace(gpu.Model) == "" {
|
||||||
|
t.Fatalf("expected GPU model to be resolved via pci.ids, got %q", gpu.Model)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestFirmwareInventoryDeviceName_PrefersIDForGenericSoftwareInventory(t *testing.T) {
|
func TestFirmwareInventoryDeviceName_PrefersIDForGenericSoftwareInventory(t *testing.T) {
|
||||||
doc := map[string]interface{}{
|
doc := map[string]interface{}{
|
||||||
"Id": "HGX_FW_NVSwitch_0",
|
"Id": "HGX_FW_NVSwitch_0",
|
||||||
|
|||||||
Reference in New Issue
Block a user