diff --git a/audit/internal/app/component_status_db.go b/audit/internal/app/component_status_db.go index 5450aff..8eab2b1 100644 --- a/audit/internal/app/component_status_db.go +++ b/audit/internal/app/component_status_db.go @@ -2,8 +2,10 @@ package app import ( "encoding/json" + "fmt" "os" "path/filepath" + "sort" "strings" "sync" "time" @@ -231,6 +233,12 @@ func ApplySATResultToDB(db *ComponentStatusDB, target, archivePath string) { source := "sat:" + target dbStatus := satStatusToDBStatus(overall) + detail := target + " SAT: " + overall + if overall != "OK" { + if reason := satFailureDetailFromKV(kv); reason != "" { + detail += " — " + reason + } + } // Map SAT target to component keys. GPU targets are keyed by vendor, not // by the raw target string: "nvidia" (Check tier) and "nvidia-stress" / @@ -244,13 +252,13 @@ func ApplySATResultToDB(db *ComponentStatusDB, target, archivePath string) { switch target { case "nvidia", "nvidia-targeted-stress", "nvidia-compute", "nvidia-targeted-power", "nvidia-pulse", "nvidia-interconnect", "nvidia-bandwidth", "nvidia-stress", "nvidia-config": - db.Record("pcie:gpu:nvidia", source, dbStatus, target+" SAT: "+overall) + db.Record("pcie:gpu:nvidia", source, dbStatus, detail) case "amd", "amd-stress", "amd-mem", "amd-bandwidth": - db.Record("pcie:gpu:amd", source, dbStatus, target+" SAT: "+overall) + db.Record("pcie:gpu:amd", source, dbStatus, detail) case "memory", "memory-stress", "sat-stress": - db.Record("memory:all", source, dbStatus, target+" SAT: "+overall) + db.Record("memory:all", source, dbStatus, detail) case "cpu", "platform-stress": - db.Record("cpu:all", source, dbStatus, target+" SAT: "+overall) + db.Record("cpu:all", source, dbStatus, detail) case "storage": // Try to record per-device if available in summary. recordedAny := false @@ -265,11 +273,19 @@ func ApplySATResultToDB(db *ComponentStatusDB, target, archivePath string) { } devName := base[:idx] devStatus := satStatusToDBStatus(strings.ToUpper(strings.TrimSpace(val))) - db.Record("storage:"+devName, source, devStatus, "storage SAT: "+val) + devDetail := "storage SAT: " + val + if strings.ToUpper(strings.TrimSpace(val)) != "OK" { + if rc, ok := kv[base+"_rc"]; ok { + devDetail = fmt.Sprintf("storage SAT job %q: %s (rc=%s)", base, val, rc) + } else { + devDetail = fmt.Sprintf("storage SAT job %q: %s", base, val) + } + } + db.Record("storage:"+devName, source, devStatus, devDetail) recordedAny = true } if !recordedAny { - db.Record("storage:all", source, dbStatus, "storage SAT: "+overall) + db.Record("storage:all", source, dbStatus, detail) } } } @@ -309,6 +325,66 @@ func ReadSATOverallStatus(archivePath string) string { return strings.ToUpper(strings.TrimSpace(kv["overall_status"])) } +// SATFailureDetail explains *why* a SAT run's overall_status isn't OK, read +// from the summary.txt next to archivePath. "SAT overall_status=FAILED (see +// summary.txt)" tells an engineer nothing without opening the run directory +// themselves; this pulls the specific reason out so it can be surfaced +// directly in the task's error message and in the component status DB. +// Returns "" if summary.txt is unreadable or carries no identifiable reason. +func SATFailureDetail(archivePath string) string { + if strings.TrimSpace(archivePath) == "" { + return "" + } + runDir := strings.TrimSuffix(extractArchivePath(archivePath), ".tar.gz") + data, err := os.ReadFile(filepath.Join(runDir, "summary.txt")) + if err != nil { + return "" + } + return satFailureDetailFromKV(parseSATKV(string(data))) +} + +// satFailureDetailFromKV inspects an already-parsed summary.txt for the +// reason behind a non-OK overall_status. +// +// - Checks that write structured findings (nvidia-config's GPU config / +// NVLink topology check) put the human-readable reason straight into a +// "warnings" field — return that verbatim. +// - Generic SAT acceptance packs (nvidia, amd, memory, cpu, storage, ...) +// instead record one "_status"/"_rc" pair per sub-job; walk +// those and report whichever job(s) didn't come back OK/UNSUPPORTED. +func satFailureDetailFromKV(kv map[string]string) string { + if w := strings.TrimSpace(kv["warnings"]); w != "" { + return w + } + + keys := make([]string, 0, len(kv)) + for k := range kv { + keys = append(keys, k) + } + sort.Strings(keys) + + var failed []string + for _, k := range keys { + if k == "overall_status" || !strings.HasSuffix(k, "_status") { + continue + } + v := strings.ToUpper(strings.TrimSpace(kv[k])) + if v == "" || v == "OK" || v == "UNSUPPORTED" { + continue + } + job := strings.TrimSuffix(k, "_status") + if rc, ok := kv[job+"_rc"]; ok && strings.TrimSpace(rc) != "" { + failed = append(failed, fmt.Sprintf("%s=%s (rc=%s)", job, v, rc)) + } else { + failed = append(failed, fmt.Sprintf("%s=%s", job, v)) + } + } + if len(failed) == 0 { + return "" + } + return "failed sub-job(s): " + strings.Join(failed, ", ") +} + func extractArchivePath(s string) string { s = strings.TrimSpace(s) if rest, ok := strings.CutPrefix(s, "Archive written to "); ok { diff --git a/audit/internal/app/component_status_db_test.go b/audit/internal/app/component_status_db_test.go index 81bd9ff..c440d88 100644 --- a/audit/internal/app/component_status_db_test.go +++ b/audit/internal/app/component_status_db_test.go @@ -98,6 +98,64 @@ func TestApplyComponentStatusDBMatchesGPUByVendor(t *testing.T) { func strPtr(s string) *string { return &s } +// TestSATFailureDetailPrefersWarningsField guards the real bug this +// exercises: an engineer looking at a failed nvidia-config task used to see +// only "SAT overall_status=FAILED (see summary.txt)" — no indication of +// which GPU or NVLink pair was the problem. nvidia-config's summary.txt +// (nvidia_config_check.go's renderNvidiaConfigCheckSummary) writes the +// specific reason into a "warnings" field; SATFailureDetail must surface it +// verbatim instead of falling through to the generic sub-job scan. +func TestSATFailureDetailPrefersWarningsField(t *testing.T) { + runDir := t.TempDir() + summary := "run_at_utc=2026-07-09T18:40:57Z\n" + + "nvlink_pairs_checked=1\n" + + "nvlink_pairs_with_issues=1\n" + + "overall_status=FAILED\n" + + "warnings=NVLink GPU0<->GPU1: 2/36 NVLinks inactive on a bonded pair\n" + if err := os.WriteFile(filepath.Join(runDir, "summary.txt"), []byte(summary), 0644); err != nil { + t.Fatal(err) + } + want := "NVLink GPU0<->GPU1: 2/36 NVLinks inactive on a bonded pair" + if got := SATFailureDetail(runDir); got != want { + t.Fatalf("SATFailureDetail() = %q, want %q", got, want) + } +} + +// TestSATFailureDetailFallsBackToFailedSubJobs guards generic SAT acceptance +// packs (nvidia, memory, cpu, storage, ...), which have no "warnings" field +// — they record one "_status"/"_rc" pair per sub-job instead. +// SATFailureDetail must name the specific job(s) that failed rather than +// telling the reader nothing beyond "see summary.txt". +func TestSATFailureDetailFallsBackToFailedSubJobs(t *testing.T) { + runDir := t.TempDir() + summary := "run_at_utc=2026-07-09T18:40:57Z\n" + + "nvidia-smi-q_rc=0\n" + + "nvidia-smi-q_status=OK\n" + + "bee-gpu-burn_rc=1\n" + + "bee-gpu-burn_status=FAILED\n" + + "job_ok=1\n" + + "job_failed=1\n" + + "overall_status=FAILED\n" + if err := os.WriteFile(filepath.Join(runDir, "summary.txt"), []byte(summary), 0644); err != nil { + t.Fatal(err) + } + want := "failed sub-job(s): bee-gpu-burn=FAILED (rc=1)" + if got := SATFailureDetail(runDir); got != want { + t.Fatalf("SATFailureDetail() = %q, want %q", got, want) + } +} + +// TestSATFailureDetailEmptyWhenNoReasonFound guards the ultimate fallback: +// when summary.txt carries no identifiable per-job or warnings detail (e.g. +// unreadable or from an older binary version), callers must get "" so they +// know to fall back to their own generic message rather than silently +// printing an empty explanation. +func TestSATFailureDetailEmptyWhenNoReasonFound(t *testing.T) { + if got := SATFailureDetail(filepath.Join(t.TempDir(), "does-not-exist")); got != "" { + t.Fatalf("SATFailureDetail() = %q, want empty for missing summary.txt", got) + } +} + // TestApplySATResultToDBCoversAllHealthCheckTargets guards against a target // silently falling through ApplySATResultToDB's switch with no matching // case — exactly what happened to the old "confidential-computing" target diff --git a/audit/internal/app/sat_overlay.go b/audit/internal/app/sat_overlay.go index 50f888d..211de80 100644 --- a/audit/internal/app/sat_overlay.go +++ b/audit/internal/app/sat_overlay.go @@ -1,6 +1,7 @@ package app import ( + "fmt" "os" "path/filepath" "sort" @@ -61,7 +62,7 @@ func applyNvidiaPerGPUStatus(devs []schema.HardwarePCIeDevice, baseDir string) { if !ok { continue } - status, description, ok := satKeyStatus(st.runStatus, firstNonEmpty(strings.TrimSpace(st.reason), "nvidia GPU SAT")) + status, description, ok := satKeyStatus(st.runStatus, firstNonEmpty(strings.TrimSpace(st.reason), "nvidia GPU SAT"), nil) if !ok { continue } @@ -221,10 +222,20 @@ func parseStorageSATStatus(summary satSummary) map[string]satStatusResult { } devName := base[:idx] step := strings.ReplaceAll(base[idx+1:], "_", "-") - stepStatus, desc, ok := satKeyStatus(strings.ToUpper(strings.TrimSpace(value)), "storage "+step) + label := "storage " + step + // Storage steps are per-device, per-job — satFailureDetailFromKV(summary.kv) + // would combine every device's failures into one description here. + // Build the reason from this one job's own rc instead so a failing + // nvme1n1 self-test doesn't get another drive's error attributed to it. + stepStatus, desc, ok := satKeyStatus(strings.ToUpper(strings.TrimSpace(value)), label, nil) if !ok { continue } + if stepStatus == "Critical" { + if rc, hasRC := summary.kv[base+"_rc"]; hasRC && strings.TrimSpace(rc) != "" { + desc = fmt.Sprintf("%s failed (rc=%s)", label, rc) + } + } current := result[devName] if !current.ok || statusSeverity(stepStatus) > statusSeverity(current.status) { result[devName] = satStatusResult{status: stepStatus, description: desc, ok: true} @@ -234,10 +245,17 @@ func parseStorageSATStatus(summary satSummary) map[string]satStatusResult { } func satSummaryStatus(summary satSummary, label string) (string, string, bool) { - return satKeyStatus(summary.overall, label) + return satKeyStatus(summary.overall, label, summary.kv) } -func satKeyStatus(rawStatus, label string) (string, string, bool) { +// satKeyStatus maps a raw SAT overall_status to a hardware component status +// and a human-readable description. For FAILED, "