app/webui: replace generic "see summary.txt" SAT failure text with the real reason
Every place that surfaced a FAILED SAT result — task error messages, component-status.json detail, and the hardware snapshot's ErrorDescription/ StatusHistory — used to say only "SAT overall_status=FAILED (see summary.txt)" or "<label> failed", forcing an engineer to go dig through the run directory to find out what actually broke. nvidia-config's summary.txt now carries a "warnings" field with the specific GPU/NVLink finding. A new SATFailureDetail/satFailureDetailFromKV in component_status_db.go reads that field, or falls back to naming whichever generic SAT sub-job(s) reported non-OK/UNSUPPORTED status along with their exit code. This feeds both the task-runner error message and the component status DB. sat_overlay.go's satKeyStatus (which drives ErrorDescription on the exported hardware snapshot) now does the same, with storage kept per-device so one drive's rc doesn't get attributed to another's card. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
5aee146903
commit
b30d34199a
@@ -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 "<job>_status"/"<job>_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 {
|
||||
|
||||
@@ -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 "<job>_status"/"<job>_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
|
||||
|
||||
@@ -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, "<label> failed" alone
|
||||
// tells an engineer nothing they didn't already know from the Critical
|
||||
// badge — kv (the parsed summary.txt) is consulted via
|
||||
// satFailureDetailFromKV for the actual reason (which GPU/NVLink pair, or
|
||||
// which sub-job and its exit code) so ErrorDescription/StatusHistory carry
|
||||
// something actionable instead of a dead end.
|
||||
func satKeyStatus(rawStatus, label string, kv map[string]string) (string, string, bool) {
|
||||
switch strings.ToUpper(strings.TrimSpace(rawStatus)) {
|
||||
case "OK":
|
||||
// No error description on success — error_description is for problems only.
|
||||
@@ -246,7 +264,11 @@ func satKeyStatus(rawStatus, label string) (string, string, bool) {
|
||||
// Tool couldn't run or test was incomplete — we can't assert hardware health.
|
||||
return "Unknown", "", true
|
||||
case "FAILED":
|
||||
return "Critical", label + " failed", true
|
||||
desc := label + " failed"
|
||||
if reason := satFailureDetailFromKV(kv); reason != "" {
|
||||
desc = label + " failed: " + reason
|
||||
}
|
||||
return "Critical", desc, true
|
||||
default:
|
||||
return "", "", false
|
||||
}
|
||||
|
||||
@@ -3,6 +3,7 @@ package app
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"bee/audit/internal/collector"
|
||||
@@ -63,6 +64,66 @@ func TestApplyLatestSATStatusesMarksAMDGPUs(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestApplyLatestSATStatusesGPUFailureDescriptionNamesFailingSubJob guards
|
||||
// against ErrorDescription regressing to a bare "amd GPU SAT failed" — an
|
||||
// engineer reading the hardware snapshot (e.g. in a support bundle) needs to
|
||||
// know which sub-job failed and its exit code, not just that something did.
|
||||
func TestApplyLatestSATStatusesGPUFailureDescriptionNamesFailingSubJob(t *testing.T) {
|
||||
baseDir := t.TempDir()
|
||||
runDir := filepath.Join(baseDir, "gpu-amd-20260325-161436")
|
||||
if err := os.MkdirAll(runDir, 0755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
raw := "run_at_utc=2026-03-25T16:14:36Z\n" +
|
||||
"rocm-bandwidth-test_rc=1\n" +
|
||||
"rocm-bandwidth-test_status=FAILED\n" +
|
||||
"overall_status=FAILED\n"
|
||||
if err := os.WriteFile(filepath.Join(runDir, "summary.txt"), []byte(raw), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
class := "DisplayController"
|
||||
amdVendorID := collector.AMDVendorID
|
||||
snap := schema.HardwareSnapshot{
|
||||
PCIeDevices: []schema.HardwarePCIeDevice{{DeviceClass: &class, VendorID: &amdVendorID}},
|
||||
}
|
||||
|
||||
applyLatestSATStatuses(&snap, baseDir, nil)
|
||||
|
||||
desc := snap.PCIeDevices[0].ErrorDescription
|
||||
if desc == nil || !strings.Contains(*desc, "rocm-bandwidth-test=FAILED (rc=1)") {
|
||||
t.Fatalf("ErrorDescription=%v, want it to name the failing sub-job and rc", desc)
|
||||
}
|
||||
}
|
||||
|
||||
// TestApplyLatestSATStatusesStorageFailureDescriptionIncludesRC guards the
|
||||
// per-device storage path, which must attribute the rc of *that device's*
|
||||
// own failing job rather than any other device's.
|
||||
func TestApplyLatestSATStatusesStorageFailureDescriptionIncludesRC(t *testing.T) {
|
||||
baseDir := t.TempDir()
|
||||
runDir := filepath.Join(baseDir, "storage-20260325-161151")
|
||||
if err := os.MkdirAll(runDir, 0755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
raw := "run_at_utc=2026-03-25T16:11:51Z\n" +
|
||||
"sda_smartctl_health_rc=2\n" +
|
||||
"sda_smartctl_health_status=FAILED\n" +
|
||||
"overall_status=FAILED\n"
|
||||
if err := os.WriteFile(filepath.Join(runDir, "summary.txt"), []byte(raw), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
sda := schema.HardwareStorage{Telemetry: map[string]any{"linux_device": "/dev/sda"}}
|
||||
snap := schema.HardwareSnapshot{Storage: []schema.HardwareStorage{sda}}
|
||||
|
||||
applyLatestSATStatuses(&snap, baseDir, nil)
|
||||
|
||||
desc := snap.Storage[0].ErrorDescription
|
||||
if desc == nil || !strings.Contains(*desc, "rc=2") {
|
||||
t.Fatalf("ErrorDescription=%v, want it to include the failing job's rc", desc)
|
||||
}
|
||||
}
|
||||
|
||||
func TestApplyLatestSATStatusesMarksNvidiaGPUByPerGPUStatusFile(t *testing.T) {
|
||||
baseDir := t.TempDir()
|
||||
runDir := filepath.Join(baseDir, "gpu-nvidia-20260407-162123")
|
||||
|
||||
@@ -470,6 +470,11 @@ func renderNvidiaConfigCheckSummary(status NvidiaConfigCheckStatus) string {
|
||||
fmt.Fprintln(&b, "overall_status=OK")
|
||||
} else {
|
||||
fmt.Fprintln(&b, "overall_status=FAILED")
|
||||
// warnings carries the actual reason(s) in prose so a reader of
|
||||
// summary.txt (or anything parsing it, like sat.DescribeFailure)
|
||||
// never has to fall back to "see the report" — each entry already
|
||||
// names the specific GPU or NVLink pair and what's wrong with it.
|
||||
fmt.Fprintf(&b, "warnings=%s\n", strings.Join(status.Warnings, "; "))
|
||||
}
|
||||
return b.String()
|
||||
}
|
||||
|
||||
@@ -166,3 +166,25 @@ func TestRenderNvidiaConfigCheckSummaryOverallStatus(t *testing.T) {
|
||||
t.Fatalf("status with warnings missing overall_status=FAILED:\n%s", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRenderNvidiaConfigCheckSummaryIncludesWarningsField guards that a
|
||||
// failed run's summary.txt carries the actual reason in prose (a "warnings"
|
||||
// field), not just a bare overall_status=FAILED — app.SATFailureDetail reads
|
||||
// this field so a failed task's error message names the specific GPU/NVLink
|
||||
// problem instead of telling the engineer to go read summary.txt themselves.
|
||||
func TestRenderNvidiaConfigCheckSummaryIncludesWarningsField(t *testing.T) {
|
||||
status := NvidiaConfigCheckStatus{Warnings: []string{
|
||||
"GPU 0 (H100): ECC is disabled",
|
||||
"NVLink GPU0<->GPU1: 2/36 NVLinks inactive on a bonded pair",
|
||||
}}
|
||||
got := renderNvidiaConfigCheckSummary(status)
|
||||
want := "warnings=GPU 0 (H100): ECC is disabled; NVLink GPU0<->GPU1: 2/36 NVLinks inactive on a bonded pair\n"
|
||||
if !strings.Contains(got, want) {
|
||||
t.Fatalf("summary missing combined warnings field:\ngot:\n%s\nwant substring:\n%s", got, want)
|
||||
}
|
||||
|
||||
clean := NvidiaConfigCheckStatus{}
|
||||
if got := renderNvidiaConfigCheckSummary(clean); strings.Contains(got, "warnings=") {
|
||||
t.Fatalf("clean status summary should omit warnings field entirely:\n%s", got)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -437,7 +437,11 @@ func executeTaskWithOptions(opts *HandlerOptions, t *Task, j *jobState, ctx cont
|
||||
if archive != "" {
|
||||
archivePath := app.ExtractArchivePath(archive)
|
||||
if err == nil && app.ReadSATOverallStatus(archivePath) == "FAILED" {
|
||||
err = fmt.Errorf("SAT overall_status=FAILED (see summary.txt)")
|
||||
if reason := app.SATFailureDetail(archivePath); reason != "" {
|
||||
err = fmt.Errorf("SAT FAILED: %s", reason)
|
||||
} else {
|
||||
err = fmt.Errorf("SAT overall_status=FAILED (see summary.txt)")
|
||||
}
|
||||
}
|
||||
// See tasks.go's identical guard: a user-aborted run must not
|
||||
// overwrite the component status DB with a partial result.
|
||||
|
||||
@@ -1150,7 +1150,11 @@ func (q *taskQueue) runTask(t *Task, j *jobState, ctx context.Context) {
|
||||
archivePath := app.ExtractArchivePath(archive)
|
||||
if err == nil {
|
||||
if app.ReadSATOverallStatus(archivePath) == "FAILED" {
|
||||
err = fmt.Errorf("SAT overall_status=FAILED (see summary.txt)")
|
||||
if reason := app.SATFailureDetail(archivePath); reason != "" {
|
||||
err = fmt.Errorf("SAT FAILED: %s", reason)
|
||||
} else {
|
||||
err = fmt.Errorf("SAT overall_status=FAILED (see summary.txt)")
|
||||
}
|
||||
}
|
||||
}
|
||||
// A user-aborted run (ctx canceled) may still have produced a partial
|
||||
|
||||
Reference in New Issue
Block a user