diff --git a/audit/internal/collector/pcie_nvlink_bridge.go b/audit/internal/collector/pcie_nvlink_bridge.go index 4316bea..ef62e3d 100644 --- a/audit/internal/collector/pcie_nvlink_bridge.go +++ b/audit/internal/collector/pcie_nvlink_bridge.go @@ -12,6 +12,11 @@ import ( var nv5re = regexp.MustCompile(`(?i)^NV(\d+)$`) +// nvidia-smi underlines the topo -m header row with ANSI CSI sequences +// (ESC[4m...ESC[0m) even when stdout is not a TTY, so "GPU0" is not at the +// start of the trimmed header line unless they are stripped first. +var nvANSIRe = regexp.MustCompile("\x1b\\[[0-9;]*[A-Za-z]") + // isNVLinkBridgeCandidate returns true for Mellanox PCIe devices that look like // NVLink bridge mezzanine cards: narrow link (x2), no host net interfaces. // These are the CPU-side PCIe control plane of the NVSwitch fabric on HGX/DGX systems. @@ -163,7 +168,7 @@ func locateGPUTopologyColumns(lines []string) (headerIdx int, gpuColIndices []in // (NIC, CPU) which are ignored. Only GPU×GPU cells containing NV# values are // counted. X is self; non-NV tokens (NODE, SYS, PHB, PIX) are skipped. func parseNVIDIATopologyMatrix(raw string) nvlinkTopoResult { - lines := strings.Split(raw, "\n") + lines := strings.Split(nvANSIRe.ReplaceAllString(raw, ""), "\n") headerIdx, gpuColIndices, gpuCount := locateGPUTopologyColumns(lines) if headerIdx < 0 { return nvlinkTopoResult{} diff --git a/audit/internal/collector/pcie_nvlink_bridge_test.go b/audit/internal/collector/pcie_nvlink_bridge_test.go index 1d7ecba..28c1bb0 100644 --- a/audit/internal/collector/pcie_nvlink_bridge_test.go +++ b/audit/internal/collector/pcie_nvlink_bridge_test.go @@ -71,6 +71,25 @@ GPU1 NV0 X } } +func TestParseNVIDIATopologyMatrixANSIUnderlinedHeader(t *testing.T) { + t.Parallel() + + // nvidia-smi underlines the header row with ANSI escapes even when stdout + // is not a TTY: the header line starts with ESC[4m, not "GPU0". + input := "\x1b[4m\tGPU0\tGPU1\tNIC0\tCPU Affinity\x1b[0m\n" + + "GPU0\t X \tNV18\tNODE\t0-31,64-95\n" + + "GPU1\tNV18\t X \tNODE\t0-31,64-95\n" + + "NIC0\tNODE\tNODE\t X \n" + + got := parseNVIDIATopologyMatrix(input) + if got.GPUCount != 2 { + t.Fatalf("GPUCount=%d want 2", got.GPUCount) + } + if !got.AllActive || got.MinNVLinks != 18 { + t.Fatalf("AllActive=%v MinNVLinks=%d want true/18", got.AllActive, got.MinNVLinks) + } +} + func TestParseNVIDIATopologyMatrixEmpty(t *testing.T) { t.Parallel() diff --git a/audit/internal/webui/page_topo.go b/audit/internal/webui/page_topo.go index 097c5a2..8e87d83 100644 --- a/audit/internal/webui/page_topo.go +++ b/audit/internal/webui/page_topo.go @@ -178,13 +178,18 @@ type gpuPairLink struct { var topoNVRe = regexp.MustCompile(`(?i)^NV(\d+)$`) +// nvidia-smi underlines the topo -m header row with ANSI CSI sequences +// (ESC[4m...ESC[0m) even when stdout is not a TTY, so the captured techdump +// contains them and "GPU0" is not at the start of the trimmed header line. +var topoANSIRe = regexp.MustCompile("\x1b\\[[0-9;]*[A-Za-z]") + // parseGPUPairAdjacency returns every GPU pair with a nonzero NVLink bond // count from a "nvidia-smi topo -m" matrix. Unlike parseNVIDIATopologyMatrix // (collector package, aggregate-only: min/all-active/count), this returns // who is bonded to whom — required so GPU-GPU edges are drawn for actually // bonded pairs, not for adjacent boxes in the layout. func parseGPUPairAdjacency(raw string) []gpuPairLink { - lines := strings.Split(raw, "\n") + lines := strings.Split(topoANSIRe.ReplaceAllString(raw, ""), "\n") headerIdx := -1 var gpuColIndices []int for i, line := range lines { diff --git a/audit/internal/webui/page_topo_test.go b/audit/internal/webui/page_topo_test.go index 94df8e9..8b5abac 100644 --- a/audit/internal/webui/page_topo_test.go +++ b/audit/internal/webui/page_topo_test.go @@ -135,6 +135,30 @@ func TestParseGPUPairAdjacencyDoesNotChainUnrelatedGPUs(t *testing.T) { } } +func TestParseGPUPairAdjacencyANSIUnderlinedHeader(t *testing.T) { + // Real techdump capture: nvidia-smi underlines the header row with ANSI + // escapes even when writing to a file, so the header line starts with + // ESC[4m, not "GPU0". + input := "\x1b[4m\tGPU0\tGPU1\tGPU2\tGPU3\tNIC0\tNIC1\tCPU Affinity\x1b[0m\n" + + "GPU0\t X \tNV18\tPIX\tPIX\tNODE\tNODE\t0-31,64-95\t0\n" + + "GPU1\tNV18\t X \tPIX\tPIX\tNODE\tNODE\t0-31,64-95\t0\n" + + "GPU2\tPIX\tPIX\t X \tNV18\tNODE\tNODE\t0-31,64-95\t0\n" + + "GPU3\tPIX\tPIX\tNV18\t X \tNODE\tNODE\t0-31,64-95\t0\n" + + "NIC0\tNODE\tNODE\tNODE\tNODE\t X \tPIX\n" + + "NIC1\tNODE\tNODE\tNODE\tNODE\tPIX\t X \n" + + pairs := parseGPUPairAdjacency(input) + if len(pairs) != 2 { + t.Fatalf("pairs=%d want 2 (%#v)", len(pairs), pairs) + } + want := map[[2]int]bool{{0, 1}: true, {2, 3}: true} + for _, p := range pairs { + if !want[[2]int{p.GPUA, p.GPUB}] || p.NVLinks != 18 { + t.Fatalf("unexpected pair %#v", p) + } + } +} + func TestParseGPUPairAdjacencyEmptyOnNoMatrix(t *testing.T) { if pairs := parseGPUPairAdjacency("no gpus here"); pairs != nil { t.Fatalf("pairs=%v want nil", pairs) diff --git a/audit/internal/webui/raid_mgmt.go b/audit/internal/webui/raid_mgmt.go index a3b2124..125234d 100644 --- a/audit/internal/webui/raid_mgmt.go +++ b/audit/internal/webui/raid_mgmt.go @@ -576,8 +576,107 @@ func runRAIDForeignImportTask(ctx context.Context, j *jobState, ctrl int) error return streamCmdJob(j, cmd) } +// raidPrepareAction says what (if anything) must be done to a drive in the +// given storcli state before it can join a new VD. Derived from the Broadcom +// StorCLI drive-state matrix: only UGood drives are accepted by "add vd"; +// JBOD/UBad convert with "set good force"; hotspares must be released first; +// Frgn/Onln hold configuration data and must not be silently destroyed. +type raidPrepareAction int + +const ( + raidPrepNone raidPrepareAction = iota // UGood or unknown — try add vd as-is + raidPrepSetGood // JBOD, UBad — "set good force" + raidPrepHotspare // GHS, DHS — "delete hotsparedrive", then set good + raidPrepBlockedFrgn + raidPrepBlockedOnln +) + +func classifyRAIDPrepareAction(state string) raidPrepareAction { + switch strings.TrimSpace(state) { + case "JBOD", "UBad": + return raidPrepSetGood + case "GHS", "DHS": + return raidPrepHotspare + case "Frgn": + return raidPrepBlockedFrgn + case "Onln", "Offln": + return raidPrepBlockedOnln + default: // "UGood", "" (state unknown — let add vd decide) + return raidPrepNone + } +} + +// raidLSIDriveStates returns EID:Slt -> State for one controller, or nil if +// storcli/parsing fails (callers then fall back to unconditional prepare). +func raidLSIDriveStates(ctx context.Context, ctrl int) map[string]string { + out, err := exec.CommandContext(ctx, "storcli64", + fmt.Sprintf("/c%d/eall/sall", ctrl), "show", "all", "J").Output() + if err != nil { + return nil + } + var doc struct { + Controllers []struct { + ResponseData map[string]json.RawMessage `json:"Response Data"` + } `json:"Controllers"` + } + if err := json.Unmarshal(out, &doc); err != nil || len(doc.Controllers) == 0 { + return nil + } + states := map[string]string{} + for _, c := range doc.Controllers { + for _, d := range parseStorcliResponseDataDrives(c.ResponseData) { + states[strings.TrimSpace(d.EIDSlt)] = strings.TrimSpace(d.State) + } + } + return states +} + func runRAIDLSICreateMirrorTask(ctx context.Context, j *jobState, ctrl int, drives []string) error { driveList := strings.Join(drives, ",") + states := raidLSIDriveStates(ctx, ctrl) + + // Non-UGood drives cannot be added to a VD directly — storcli fails with + // "resources already in use" (exit 11) or similar. Fix what is safely + // fixable (JBOD/UBad/hotspare), refuse what holds data (Frgn/Onln). + for _, drive := range drives { + eid, slt, ok := parseRAIDSlot(drive) + if !ok { + return fmt.Errorf("invalid drive slot %q", drive) + } + state := states[drive] + slotPath := fmt.Sprintf("/c%d/e%d/s%d", ctrl, eid, slt) + + switch classifyRAIDPrepareAction(state) { + case raidPrepBlockedFrgn: + return fmt.Errorf("drive %s carries a foreign configuration; run the RAID Foreign Clear (or Import) task first, then retry", drive) + case raidPrepBlockedOnln: + return fmt.Errorf("drive %s is part of an existing virtual drive (state %s); delete that VD first", drive, state) + case raidPrepHotspare: + j.append(fmt.Sprintf("Drive %s is a hotspare (%s); releasing it...", drive, state)) + rel := exec.CommandContext(ctx, "storcli64", slotPath, "delete", "hotsparedrive") + if err := streamCmdJob(j, rel); err != nil { + return fmt.Errorf("release hotspare %s: %w", drive, err) + } + case raidPrepSetGood: + j.append(fmt.Sprintf("Drive %s is %s; converting to Unconfigured Good (set good force)...", drive, state)) + prep := exec.CommandContext(ctx, "storcli64", slotPath, "set", "good", "force") + if err := streamCmdJob(j, prep); err != nil { + return fmt.Errorf("set good on %s: %w", drive, err) + } + case raidPrepNone: + if state == "" { + // Drive state unknown (storcli query failed) — attempt the + // conversion anyway; harmless on an already-UGood drive with + // force, and add vd below is the real verdict. + j.append(fmt.Sprintf("Preparing drive %s (set good, force)...", drive)) + prep := exec.CommandContext(ctx, "storcli64", slotPath, "set", "good", "force") + if err := streamCmdJob(j, prep); err != nil { + j.append(fmt.Sprintf("note: set good on %s: %v (continuing)", drive, err)) + } + } + } + } + j.append(fmt.Sprintf("Creating RAID 1 on controller %d with drives: %s", ctrl, driveList)) cmd := exec.CommandContext(ctx, "storcli64", fmt.Sprintf("/c%d", ctrl), @@ -585,7 +684,17 @@ func runRAIDLSICreateMirrorTask(ctx context.Context, j *jobState, ctrl int, driv fmt.Sprintf("drives=%s", driveList), "pdperarray=2", ) - return streamCmdJob(j, cmd) + if err := streamCmdJob(j, cmd); err != nil { + // A blocked add vd is often preserved cache from a dead VD + // ("controller has data in cache for offline or missing virtual + // drives"). Surface it so the log is actionable. + j.append("add vd failed; checking for preserved cache...") + pc := exec.CommandContext(ctx, "storcli64", fmt.Sprintf("/c%d", ctrl), "show", "preservedcache") + _ = streamCmdJob(j, pc) + j.append(fmt.Sprintf("hint: if preserved cache is listed above, clear it with: storcli64 /c%d/vall delete preservedcache (invalidates cached data of dead VDs), then retry", ctrl)) + return err + } + return nil } // parseRAIDSlot splits a storcli "EID:Slt" identifier (e.g. "252:0") into diff --git a/audit/internal/webui/raid_mgmt_test.go b/audit/internal/webui/raid_mgmt_test.go index 32ae1b1..b6a5f32 100644 --- a/audit/internal/webui/raid_mgmt_test.go +++ b/audit/internal/webui/raid_mgmt_test.go @@ -82,3 +82,22 @@ func TestParseStorcliResponseDataDrivesPerSlotKeys(t *testing.T) { t.Fatalf("free=%d want 1 (JBOD is a free drive)", len(free)) } } + +func TestClassifyRAIDPrepareAction(t *testing.T) { + cases := map[string]raidPrepareAction{ + "UGood": raidPrepNone, + "": raidPrepNone, + "JBOD": raidPrepSetGood, + "UBad": raidPrepSetGood, + "GHS": raidPrepHotspare, + "DHS": raidPrepHotspare, + "Frgn": raidPrepBlockedFrgn, + "Onln": raidPrepBlockedOnln, + "Offln": raidPrepBlockedOnln, + } + for state, want := range cases { + if got := classifyRAIDPrepareAction(state); got != want { + t.Errorf("state %q: got %v want %v", state, got, want) + } + } +}