diff --git a/audit/internal/app/component_status_db.go b/audit/internal/app/component_status_db.go index 689cc4f..638916f 100644 --- a/audit/internal/app/component_status_db.go +++ b/audit/internal/app/component_status_db.go @@ -177,12 +177,21 @@ func ApplySATResultToDB(db *ComponentStatusDB, target, archivePath string) { source := "sat:" + target dbStatus := satStatusToDBStatus(overall) - // Map SAT target to component keys. + // Map SAT target to component keys. GPU targets are keyed by vendor, not + // by the raw target string: "nvidia" (Check tier) and "nvidia-stress" / + // "nvidia-targeted-stress" (Load/Burn tier) all exercise the same + // physical GPUs, so they must share one severity-tracked record — a + // severity-1 Check run after a severity-3 Load failure must not lose + // that failure just because it ran more recently. Recording each target + // under its own key (the previous behavior) also silently broke + // applyComponentStatusDB below, which expects "pcie:gpu:" and + // otherwise fails to match any real BDF. switch target { case "nvidia", "nvidia-targeted-stress", "nvidia-compute", "nvidia-targeted-power", "nvidia-pulse", - "nvidia-interconnect", "nvidia-bandwidth", "amd", "nvidia-stress", - "amd-stress", "amd-mem", "amd-bandwidth": - db.Record("pcie:gpu:"+target, source, dbStatus, target+" SAT: "+overall) + "nvidia-interconnect", "nvidia-bandwidth", "nvidia-stress": + db.Record("pcie:gpu:nvidia", source, dbStatus, target+" SAT: "+overall) + case "amd", "amd-stress", "amd-mem", "amd-bandwidth": + db.Record("pcie:gpu:amd", source, dbStatus, target+" SAT: "+overall) case "memory", "memory-stress", "sat-stress": db.Record("memory:all", source, dbStatus, target+" SAT: "+overall) case "cpu", "platform-stress": diff --git a/audit/internal/app/component_status_db_test.go b/audit/internal/app/component_status_db_test.go index d818bcc..ab1f89f 100644 --- a/audit/internal/app/component_status_db_test.go +++ b/audit/internal/app/component_status_db_test.go @@ -4,6 +4,8 @@ import ( "os" "path/filepath" "testing" + + "bee/audit/internal/schema" ) func TestExtractArchivePath(t *testing.T) { @@ -40,3 +42,58 @@ func TestReadSATOverallStatus_HandlesActionResultPrefix(t *testing.T) { t.Errorf("ReadSATOverallStatus(bare) = %q, want FAILED", got) } } + +func writeSATSummary(t *testing.T, overall string) string { + t.Helper() + runDir := t.TempDir() + summary := "run_at_utc=2026-07-06T17:47:22Z\noverall_status=" + overall + "\n" + if err := os.WriteFile(filepath.Join(runDir, "summary.txt"), []byte(summary), 0644); err != nil { + t.Fatal(err) + } + return runDir +} + +func TestApplySATResultToDBNormalizesGPUKeyByVendor(t *testing.T) { + // "nvidia" (Check tier) and "nvidia-stress" (Load/Burn tier) exercise the + // same physical GPUs and must collapse onto one component key so a later + // clean Check run can't erase an earlier Load-tier failure. + db, err := OpenComponentStatusDB(filepath.Join(t.TempDir(), "component-status.json")) + if err != nil { + t.Fatal(err) + } + + ApplySATResultToDB(db, "nvidia-stress", writeSATSummary(t, "FAILED")) + ApplySATResultToDB(db, "nvidia", writeSATSummary(t, "OK")) + + rec, ok := db.Get("pcie:gpu:nvidia") + if !ok { + t.Fatalf("expected pcie:gpu:nvidia record to exist") + } + if rec.Status != "Warning" { + t.Fatalf("status=%q, want Warning (FAILED) to survive the later OK Check run", rec.Status) + } +} + +func TestApplyComponentStatusDBMatchesGPUByVendor(t *testing.T) { + db, err := OpenComponentStatusDB(filepath.Join(t.TempDir(), "component-status.json")) + if err != nil { + t.Fatal(err) + } + db.Record("pcie:gpu:nvidia", "sat:nvidia-stress", "Critical", "nvidia-stress SAT: FAILED") + + class := "VideoController" + vendor := 0x10de // collector.NvidiaVendorID + snap := &schema.HardwareSnapshot{ + PCIeDevices: []schema.HardwarePCIeDevice{ + {DeviceClass: &class, VendorID: &vendor, BDF: strPtr("0000:c8:00.0")}, + }, + } + + applyComponentStatusDB(snap, db) + + if snap.PCIeDevices[0].Status == nil || *snap.PCIeDevices[0].Status != "Critical" { + t.Fatalf("expected GPU device status Critical, got %v", snap.PCIeDevices[0].Status) + } +} + +func strPtr(s string) *string { return &s } diff --git a/audit/internal/app/sat_overlay.go b/audit/internal/app/sat_overlay.go index c4d8675..50f888d 100644 --- a/audit/internal/app/sat_overlay.go +++ b/audit/internal/app/sat_overlay.go @@ -347,14 +347,19 @@ func applyComponentStatusDB(snap *schema.HardwareSnapshot, db *ComponentStatusDB ts := rec.LastChangedAt.UTC().Format("2006-01-02T15:04:05Z") switch { + case key == "pcie:gpu:nvidia" || key == "pcie:gpu:amd": + vendor := strings.TrimPrefix(key, "pcie:gpu:") + for i := range snap.PCIeDevices { + if matchesGPUVendor(snap.PCIeDevices[i], vendor) { + mergeComponentStatus(&snap.PCIeDevices[i].HardwareComponentStatus, ts, status, detail) + } + } case strings.HasPrefix(key, "pcie:"): bdf := strings.TrimPrefix(key, "pcie:") - bdf = strings.TrimPrefix(bdf, "gpu:") // strip sub-type if present - // bdf may be empty (e.g. "pcie:gpu:nvidia") — skip BDF matching - if sanitizeBDFForLookup(bdf) == "" { + normalized := sanitizeBDFForLookup(bdf) + if normalized == "" { break } - normalized := sanitizeBDFForLookup(bdf) for i := range snap.PCIeDevices { if snap.PCIeDevices[i].BDF == nil { continue diff --git a/audit/internal/platform/techdump.go b/audit/internal/platform/techdump.go index c82e524..2b95080 100644 --- a/audit/internal/platform/techdump.go +++ b/audit/internal/platform/techdump.go @@ -47,6 +47,12 @@ var techDumpNvidiaCommands = []struct { {Name: "nvidia-smi", Args: []string{"-q"}, File: "nvidia-smi-q.txt"}, {Name: "nvidia-smi", Args: []string{"--query-gpu=index,pci.bus_id,serial,vbios_version,temperature.gpu,power.draw,ecc.errors.uncorrected.aggregate.total,ecc.errors.corrected.aggregate.total,clocks_throttle_reasons.hw_slowdown", "--format=csv,noheader,nounits"}, File: "nvidia-smi-query.csv"}, {Name: "nvidia-smi", Args: []string{"conf-compute", "-q"}, File: "nvidia-smi-conf-compute-q.txt"}, + // Consumed by webui's /topo page (GPU-GPU NVLink adjacency and per-link + // status/errors) so that page can render from this persisted dump instead + // of shelling out to nvidia-smi on every HTTP request. + {Name: "nvidia-smi", Args: []string{"topo", "-m"}, File: "nvidia-smi-topo.txt"}, + {Name: "nvidia-smi", Args: []string{"nvlink", "-s"}, File: "nvidia-smi-nvlink-status.txt"}, + {Name: "nvidia-smi", Args: []string{"nvlink", "-e"}, File: "nvidia-smi-nvlink-errors.txt"}, } type lsblkDumpRoot struct { diff --git a/audit/internal/webui/page_topo.go b/audit/internal/webui/page_topo.go index 3eca577..097c5a2 100644 --- a/audit/internal/webui/page_topo.go +++ b/audit/internal/webui/page_topo.go @@ -4,7 +4,8 @@ import ( "encoding/json" "fmt" "html" - "os/exec" + "os" + "path/filepath" "regexp" "sort" "strconv" @@ -16,9 +17,11 @@ import ( // renderTopo renders the /topo page: a read-only visualization of the server // topology (CPU sockets, NUMA-affine PCIe devices, PSU/BMC) plus a separate // NVLink topology card. It is pure visualization: everything it reads either -// already exists in the audit.json contract, or is a live (non-persisted) -// query — nothing here writes to schema.HardwarePCIeDevice or any other -// contract type, and nothing here talks to Reanimator Core. +// already exists in the audit.json contract, or comes from the persisted +// techdump captured once per audit cycle (platform.CaptureTechnicalDump) — +// nothing here shells out to nvidia-smi itself, writes to +// schema.HardwarePCIeDevice or any other contract type, or talks to +// Reanimator Core. func renderTopo(opts HandlerOptions) string { data, err := loadSnapshot(opts.AuditPath) if err != nil { @@ -31,8 +34,8 @@ func renderTopo(opts HandlerOptions) string { hw := ingest.Hardware var b strings.Builder - b.WriteString(renderTopoMainDiagram(hw)) - if nv := renderTopoNVLinkCard(hw); nv != "" { + b.WriteString(renderTopoMainDiagram(hw, opts.ExportDir)) + if nv := renderTopoNVLinkCard(hw, opts.ExportDir); nv != "" { b.WriteString(nv) } return b.String() @@ -130,18 +133,36 @@ func topoEdgeColorVar(dev schema.HardwarePCIeDevice) string { // NUMA node -> CPU socket join (heuristic, no guaranteed hardware mapping) // --------------------------------------------------------------------------- -// buildSocketIndex maps a NUMA node number to the index into cpus whose -// Socket field equals that NUMA node number. This is a best-effort heuristic -// (NUMANode == Socket) documented as such in the /topo design — Linux NUMA -// node numbering and dmidecode socket designation are different numbering -// domains with no guaranteed 1:1 mapping, but in practice agree for the -// common case of N-socket boards. +// buildSocketIndex maps a NUMA node number to the index into cpus for the +// socket occupying that position in ascending Socket-designation order. +// +// Linux NUMA node numbering is always 0-based (node0, node1, ...), but +// dmidecode's "Socket Designation" is board-defined and frequently 1-based +// ("CPU1", "CPU2", ...). Mapping NUMA node N to the CPU whose Socket field +// equals N (as an earlier version of this function did) silently fails on +// any 1-indexed board: node 0 has no match (dropped into the "unknown" +// column) and node 1 wrongly maps to the first CPU. Ranking by Socket value +// instead assumes only that node order follows socket order — true for the +// common case of N-socket boards — without depending on the numbering base. func buildSocketIndex(cpus []schema.HardwareCPU) map[int]int { - idx := map[int]int{} - for i, cpu := range cpus { - if cpu.Socket != nil { - idx[*cpu.Socket] = i + order := make([]int, len(cpus)) + for i := range cpus { + order[i] = i + } + sort.SliceStable(order, func(a, b int) bool { + ca, cb := cpus[order[a]], cpus[order[b]] + sa, sb := 0, 0 + if ca.Socket != nil { + sa = *ca.Socket } + if cb.Socket != nil { + sb = *cb.Socket + } + return sa < sb + }) + idx := map[int]int{} + for numaNode, cpuIdx := range order { + idx[numaNode] = cpuIdx } return idx } @@ -245,31 +266,41 @@ func parseGPUPairAdjacency(raw string) []gpuPairLink { return pairs } -func queryGPUTopologyMatrix() (string, error) { - out, err := exec.Command("nvidia-smi", "topo", "-m").Output() +// readTopoTechDump reads a file previously captured into the persistent +// techdump directory by platform.System.CaptureTechnicalDump (run once per +// audit cycle), rather than shelling out to nvidia-smi from the HTTP request +// handler — a live call here would block page rendering on a wedged driver, +// exactly the failure mode this tool exists to diagnose. +func readTopoTechDump(exportDir, name string) (string, error) { + out, err := os.ReadFile(filepath.Join(exportDir, "techdump", name)) if err != nil { return "", err } return string(out), nil } -// queryNVIDIAIndexByBDF runs a lightweight live nvidia-smi query mapping -// PCI bus address (matching HardwarePCIeDevice.Slot) to the GPU index -// nvidia-smi/dcgmi/topo -m report, so GPU-GPU edges (keyed by index) can be -// anchored to the correct box (keyed by BDF) in the diagram. -func queryNVIDIAIndexByBDF() (map[string]int, error) { - out, err := exec.Command("nvidia-smi", "--query-gpu=index,pci.bus_id", "--format=csv,noheader").Output() +func readGPUTopologyMatrix(exportDir string) (string, error) { + return readTopoTechDump(exportDir, "nvidia-smi-topo.txt") +} + +// readNVIDIAIndexByBDF parses the persisted nvidia-smi-query.csv techdump +// (index,pci.bus_id,...) to map PCI bus address (matching +// HardwarePCIeDevice.Slot) to the GPU index nvidia-smi/topo -m reports, so +// GPU-GPU edges (keyed by index) can be anchored to the correct box (keyed +// by BDF) in the diagram. +func readNVIDIAIndexByBDF(exportDir string) (map[string]int, error) { + raw, err := readTopoTechDump(exportDir, "nvidia-smi-query.csv") if err != nil { return nil, err } result := map[string]int{} - for _, line := range strings.Split(string(out), "\n") { + for _, line := range strings.Split(raw, "\n") { line = strings.TrimSpace(line) if line == "" { continue } - parts := strings.SplitN(line, ",", 2) - if len(parts) != 2 { + parts := strings.Split(line, ",") + if len(parts) < 2 { continue } idx, err := strconv.Atoi(strings.TrimSpace(parts[0])) @@ -328,7 +359,7 @@ type topoBox struct { detailType string // "" = not clickable } -func renderTopoMainDiagram(hw schema.HardwareSnapshot) string { +func renderTopoMainDiagram(hw schema.HardwareSnapshot, exportDir string) string { socketIdx := buildSocketIndex(hw.CPUs) numCols := len(hw.CPUs) if numCols == 0 { @@ -383,11 +414,13 @@ func renderTopoMainDiagram(hw schema.HardwareSnapshot) string { totalCols++ } - // Live GPU index<->BDF map + pairwise NVLink adjacency, best-effort: - // if nvidia-smi is unavailable, GPU-GPU edges are simply omitted. - bdfToIndex, _ := queryNVIDIAIndexByBDF() + // GPU index<->BDF map + pairwise NVLink adjacency, read from the + // persisted techdump captured during the last audit cycle, best-effort: + // if the dump is missing (older audit, no NVIDIA GPUs), GPU-GPU edges are + // simply omitted. + bdfToIndex, _ := readNVIDIAIndexByBDF(exportDir) var pairs []gpuPairLink - if topoMatrix, err := queryGPUTopologyMatrix(); err == nil { + if topoMatrix, err := readGPUTopologyMatrix(exportDir); err == nil { pairs = parseGPUPairAdjacency(topoMatrix) } @@ -601,7 +634,8 @@ func truncateTopoLabel(s string, max int) string { } // --------------------------------------------------------------------------- -// Separate NVLink topology card (live query, not persisted to any contract) +// Separate NVLink topology card (read from techdump, not written to any +// ingest contract) // --------------------------------------------------------------------------- type topoNVLinkPort struct { @@ -620,12 +654,12 @@ var ( topoNVLinkErrorLineRe = regexp.MustCompile(`^Link (\d+):\s*(Replay|Recovery|CRC) Errors:\s*(\d+)`) ) -func queryTopoNVLinkStatus() (map[int][]topoNVLinkPort, error) { - out, err := exec.Command("nvidia-smi", "nvlink", "-s").Output() +func readTopoNVLinkStatus(exportDir string) (map[int][]topoNVLinkPort, error) { + raw, err := readTopoTechDump(exportDir, "nvidia-smi-nvlink-status.txt") if err != nil { return nil, err } - return parseTopoNVLinkStatus(string(out)), nil + return parseTopoNVLinkStatus(raw), nil } func parseTopoNVLinkStatus(raw string) map[int][]topoNVLinkPort { @@ -657,12 +691,12 @@ func parseTopoNVLinkStatus(raw string) map[int][]topoNVLinkPort { return result } -func queryTopoNVLinkErrors() (map[int]map[int][3]int64, error) { - out, err := exec.Command("nvidia-smi", "nvlink", "-e").Output() +func readTopoNVLinkErrors(exportDir string) (map[int]map[int][3]int64, error) { + raw, err := readTopoTechDump(exportDir, "nvidia-smi-nvlink-errors.txt") if err != nil { return nil, err } - return parseTopoNVLinkErrors(string(out)), nil + return parseTopoNVLinkErrors(raw), nil } // parseTopoNVLinkErrors returns, per GPU then link index, [replay, recovery, crc]. @@ -702,8 +736,9 @@ func parseTopoNVLinkErrors(raw string) map[int]map[int][3]int64 { } // renderTopoNVLinkCard renders the separate NVLink topology card. Returns "" -// if there are fewer than 2 NVIDIA GPUs or nvidia-smi is unavailable. -func renderTopoNVLinkCard(hw schema.HardwareSnapshot) string { +// if there are fewer than 2 NVIDIA GPUs, or the nvidia-smi nvlink techdump +// wasn't captured (older audit, or nvidia-smi unavailable on that run). +func renderTopoNVLinkCard(hw schema.HardwareSnapshot, exportDir string) string { gpuCount := 0 for _, dev := range hw.PCIeDevices { if dev.DeviceClass != nil && isGPUDeviceClass(*dev.DeviceClass) { @@ -714,13 +749,13 @@ func renderTopoNVLinkCard(hw schema.HardwareSnapshot) string { return "" } - status, err := queryTopoNVLinkStatus() + status, err := readTopoNVLinkStatus(exportDir) if err != nil || len(status) == 0 { - return topoCard("NVLink Topology", `nvidia-smi nvlink unavailable`) + return topoCard("NVLink Topology", `nvidia-smi nvlink data unavailable`) } - errors, _ := queryTopoNVLinkErrors() + errors, _ := readTopoNVLinkErrors(exportDir) - topoMatrix, _ := queryGPUTopologyMatrix() + topoMatrix, _ := readGPUTopologyMatrix(exportDir) pairs := parseGPUPairAdjacency(topoMatrix) var bodyB strings.Builder diff --git a/audit/internal/webui/page_topo_test.go b/audit/internal/webui/page_topo_test.go index 74935c9..94df8e9 100644 --- a/audit/internal/webui/page_topo_test.go +++ b/audit/internal/webui/page_topo_test.go @@ -175,6 +175,19 @@ func TestBuildSocketIndex(t *testing.T) { } } +func TestBuildSocketIndexOneIndexedSocketDesignation(t *testing.T) { + // dmidecode "Socket Designation" is frequently 1-indexed ("CPU1", "CPU2") + // while Linux NUMA nodes are always 0-indexed. NUMA node 0 must still + // resolve to the first CPU in Socket order, not fall through to the + // "unknown" column (the bug reported against the /topo page). + s1, s2 := 1, 2 + cpus := []schema.HardwareCPU{{Socket: &s1}, {Socket: &s2}} + idx := buildSocketIndex(cpus) + if idx[0] != 0 || idx[1] != 1 { + t.Fatalf("idx=%#v want {0:0, 1:1}", idx) + } +} + func TestIsRAIDControllerClass(t *testing.T) { if !isRAIDControllerClass("StorageController") || !isRAIDControllerClass("MassStorageController") { t.Fatalf("expected known RAID/storage classes to match") diff --git a/audit/internal/webui/task_runner.go b/audit/internal/webui/task_runner.go index eab4e3b..8a907f7 100644 --- a/audit/internal/webui/task_runner.go +++ b/audit/internal/webui/task_runner.go @@ -439,7 +439,9 @@ func executeTaskWithOptions(opts *HandlerOptions, t *Task, j *jobState, ctx cont if err == nil && app.ReadSATOverallStatus(archivePath) == "FAILED" { err = fmt.Errorf("SAT overall_status=FAILED (see summary.txt)") } - if opts.App != nil && opts.App.StatusDB != nil { + // See tasks.go's identical guard: a user-aborted run must not + // overwrite the component status DB with a partial result. + if opts.App != nil && opts.App.StatusDB != nil && ctx.Err() == nil { app.ApplySATResultToDB(opts.App.StatusDB, t.Target, archivePath) } } diff --git a/audit/internal/webui/tasks.go b/audit/internal/webui/tasks.go index c99cd4f..7feca99 100644 --- a/audit/internal/webui/tasks.go +++ b/audit/internal/webui/tasks.go @@ -1153,7 +1153,11 @@ func (q *taskQueue) runTask(t *Task, j *jobState, ctx context.Context) { err = fmt.Errorf("SAT overall_status=FAILED (see summary.txt)") } } - if db := q.statusDB(); db != nil { + // A user-aborted run (ctx canceled) may still have produced a partial + // archive/summary.txt — that incomplete result must not overwrite the + // component status DB, which is why this is skipped here rather than + // relying on satKeyStatus's PARTIAL/UNSUPPORTED handling below. + if db := q.statusDB(); db != nil && ctx.Err() == nil { app.ApplySATResultToDB(db, t.Target, archivePath) } }