diff --git a/audit/internal/collector/cpu_telemetry.go b/audit/internal/collector/cpu_telemetry.go index 9f063b0..e35c770 100644 --- a/audit/internal/collector/cpu_telemetry.go +++ b/audit/internal/collector/cpu_telemetry.go @@ -37,12 +37,36 @@ func enrichCPUsWithTelemetry(cpus []schema.HardwareCPU, doc sensorsDoc) []schema } if value, ok := throttleBySocket[socket]; ok { cpus[i].Throttled = &value + if value { + escalateCPUThrottleStatus(&cpus[i]) + } } } return cpus } +// escalateCPUThrottleStatus raises a CPU's status to at least Warning when it +// has hit thermal throttling (cpuPackageThrottled: a cumulative since-boot +// sysfs counter, not "is throttling right now"). dmidecode-derived status +// (parseCPUStatus) only distinguishes populated/enabled/disabled and has no +// way to know about a live thermal event, so without this a CPU that +// throttled during e.g. a stress test still reports status "OK" — see +// enrichCPUsWithTelemetry. +func escalateCPUThrottleStatus(cpu *schema.HardwareCPU) { + current := "" + if cpu.Status != nil { + current = strings.TrimSpace(*cpu.Status) + } + if current != "" && current != statusUnknown && StatusSeverity(statusWarning) <= StatusSeverity(current) { + return + } + status := statusWarning + cpu.Status = &status + desc := "CPU hit thermal throttling (package/core throttle count > 0 since boot)" + cpu.ErrorDescription = &desc +} + func cpuTempsFromSensors(doc sensorsDoc, cpuCount int) map[int]float64 { out := map[int]float64{} if len(doc) == 0 { diff --git a/audit/internal/collector/cpu_telemetry_test.go b/audit/internal/collector/cpu_telemetry_test.go index ba89e19..572e8a3 100644 --- a/audit/internal/collector/cpu_telemetry_test.go +++ b/audit/internal/collector/cpu_telemetry_test.go @@ -49,6 +49,12 @@ func TestEnrichCPUsWithTelemetry(t *testing.T) { if got[0].Throttled == nil || !*got[0].Throttled { t.Fatalf("cpu0 throttled mismatch: %#v", got[0].Throttled) } + if got[0].Status == nil || *got[0].Status != statusWarning { + t.Fatalf("cpu0 status not escalated to Warning on throttle: %#v", got[0].Status) + } + if got[0].ErrorDescription == nil || *got[0].ErrorDescription == "" { + t.Fatalf("cpu0 error description not set on throttle") + } if got[1].TemperatureC == nil || *got[1].TemperatureC != 58.0 { t.Fatalf("cpu1 temperature mismatch: %#v", got[1].TemperatureC) } @@ -58,6 +64,26 @@ func TestEnrichCPUsWithTelemetry(t *testing.T) { if got[1].Throttled != nil && *got[1].Throttled { t.Fatalf("cpu1 throttled mismatch: %#v", got[1].Throttled) } + if got[1].Status == nil || *got[1].Status != statusOK { + t.Fatalf("cpu1 status should remain OK when not throttled: %#v", got[1].Status) + } +} + +func TestEscalateCPUThrottleStatusDoesNotDowngradeCritical(t *testing.T) { + status := statusCritical + desc := "pre-existing critical finding" + cpu := schema.HardwareCPU{ + HardwareComponentStatus: schema.HardwareComponentStatus{Status: &status, ErrorDescription: &desc}, + } + + escalateCPUThrottleStatus(&cpu) + + if *cpu.Status != statusCritical { + t.Fatalf("status downgraded from Critical: %#v", cpu.Status) + } + if *cpu.ErrorDescription != desc { + t.Fatalf("error description overwritten: %#v", cpu.ErrorDescription) + } } func mustWriteFile(t *testing.T, path, content string) { diff --git a/audit/internal/platform/platform_stress.go b/audit/internal/platform/platform_stress.go index 41b65d5..c56ab97 100644 --- a/audit/internal/platform/platform_stress.go +++ b/audit/internal/platform/platform_stress.go @@ -331,12 +331,24 @@ func writePlatformSummary(opts PlatformStressOptions, analyses []cycleAnalysis) } fmt.Fprintf(&b, "%s\n", strings.Repeat("=", 48)) + // overall_status is the machine-readable twin of the "Overall: ..." line + // below, in the key=value vocabulary ApplySATResultToDB/parseSATKV + // expects (OK/FAILED/PARTIAL/UNSUPPORTED). Without it, a throttle FAIL + // detected right here never reaches component-status.json: the DB writer + // only recognizes "overall_status=", and "Overall: FAIL — ..." has no + // "=" for parseSATKV to split on. FAILED is the only token that maps to + // DB status "Warning" (see satStatusToDBStatus), so both throttle and + // fan-spindown findings use it — there's no separate "WARN" token in the + // SAT summary vocabulary today. if totalThrottle > 0 { fmt.Fprintf(&b, "Overall: FAIL — throttle detected in %d/%d cycles\n", totalThrottle, len(analyses)) + fmt.Fprintf(&b, "overall_status=FAILED\n") } else if totalFanWarn > 0 { fmt.Fprintf(&b, "Overall: WARN — fast fan spindown in %d/%d cycles (cooling recovery risk)\n", totalFanWarn, len(analyses)) + fmt.Fprintf(&b, "overall_status=FAILED\n") } else { fmt.Fprintf(&b, "Overall: PASS\n") + fmt.Fprintf(&b, "overall_status=OK\n") } return b.String() } diff --git a/audit/internal/platform/platform_stress_test.go b/audit/internal/platform/platform_stress_test.go index cc25da9..f07990e 100644 --- a/audit/internal/platform/platform_stress_test.go +++ b/audit/internal/platform/platform_stress_test.go @@ -2,6 +2,7 @@ package platform import ( "runtime" + "strings" "testing" ) @@ -32,3 +33,52 @@ func TestPlatformStressMemoryMBOverride(t *testing.T) { t.Fatalf("platformStressMemoryMB=%d want 8192", got) } } + +// TestWritePlatformSummaryOverallStatusKV verifies writePlatformSummary emits +// a parseable "overall_status=" line matching its human-readable "Overall:" +// verdict. ApplySATResultToDB/parseSATKV (internal/app/component_status_db.go) +// only recognizes "key=value" lines — a summary with only "Overall: FAIL — +// ..." (no "=") silently never reaches component-status.json, so a real +// thermal-throttle FAIL detected by this test would never show up as cpu:all +// going Warning. +func TestWritePlatformSummaryOverallStatusKV(t *testing.T) { + opts := PlatformStressOptions{Cycles: []PlatformStressCycle{{LoadSec: 60, IdleSec: 30}}} + + tests := []struct { + name string + analyses []cycleAnalysis + want string + }{ + { + name: "clean run reports OK", + analyses: []cycleAnalysis{{maxCPUTemp: 70, maxGPUTemp: 60}}, + want: "overall_status=OK", + }, + { + name: "throttle detected reports FAILED", + analyses: []cycleAnalysis{{maxCPUTemp: 95, throttled: true}}, + want: "overall_status=FAILED", + }, + { + name: "fast fan spindown reports FAILED", + analyses: []cycleAnalysis{{fanAtCutAvg: 8000, fanMin15s: 2000, fanDropPct: 75}}, + want: "overall_status=FAILED", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + summary := writePlatformSummary(opts, tt.analyses) + found := false + for _, line := range strings.Split(summary, "\n") { + if line == tt.want { + found = true + break + } + } + if !found { + t.Fatalf("summary missing %q:\n%s", tt.want, summary) + } + }) + } +} diff --git a/audit/internal/platform/sat.go b/audit/internal/platform/sat.go index fe66d0c..6dfd0f0 100644 --- a/audit/internal/platform/sat.go +++ b/audit/internal/platform/sat.go @@ -793,16 +793,71 @@ func (s *System) RunSATStressPack(ctx context.Context, baseDir string, durationS }, logFunc) } +// cpuThermalThrottleSysDir is the sysfs root the throttle-check scripts glob +// under. Overridden in tests so they can point at a fake directory tree +// instead of the real /sys. +var cpuThermalThrottleSysDir = "/sys/devices/system/cpu" + +// cpuThrottleSumScript is the shell fragment both before/after scripts use to +// sum the kernel's cumulative-since-boot thermal throttle counters across +// every CPU. +func cpuThrottleSumScript() string { + return fmt.Sprintf(` +sum=0 +for f in %[1]s/cpu*/thermal_throttle/core_throttle_count %[1]s/cpu*/thermal_throttle/package_throttle_count; do + [ -f "$f" ] || continue + v=$(cat "$f" 2>/dev/null) + case "$v" in ''|*[!0-9]*) continue ;; esac + sum=$((sum + v)) +done +`, cpuThermalThrottleSysDir) +} + +// cpuThrottleBeforeScript snapshots the throttle counter sum into a file in +// {{run_dir}} so cpuThrottleCheckScript can later diff before/after despite +// each satJob running as an independent process. +func cpuThrottleBeforeScript() string { + return cpuThrottleSumScript() + `echo "$sum" | tee {{run_dir}}/.cpu-throttle-before` + "\n" +} + +// cpuThrottleCheckScript compares the after-run throttle counter sum against +// the snapshot cpuThrottleBeforeScript took, and fails (non-zero exit) if it +// increased — i.e. the CPU actually hit thermal throttling during this +// specific run, not just at some earlier point this boot. classifySATResult +// maps a failed job here to SAT status FAILED, which ApplySATResultToDB +// records as component status "Warning" for cpu:all — without this, the +// "cpu" SAT pack only checks stress-ng's exit code, which is 0 whether or +// not the CPU throttled while running it. +func cpuThrottleCheckScript() string { + return `before=$(cat {{run_dir}}/.cpu-throttle-before 2>/dev/null) +case "$before" in ''|*[!0-9]*) before=0 ;; esac +` + cpuThrottleSumScript() + `after=$sum +echo "throttle_count_before=$before" +echo "throttle_count_after=$after" +if [ "$after" -gt "$before" ]; then + echo "THROTTLE DETECTED: CPU package/core hit thermal throttling during this stress-ng run ($before -> $after)" + exit 1 +fi +echo "no new thermal throttling detected during this run" +` +} + +func cpuSATJobs(durationSec int) []satJob { + return []satJob{ + {name: "01-lscpu.log", cmd: []string{"lscpu"}}, + {name: "02-sensors-before.log", cmd: []string{"sensors"}}, + {name: "02-thermal-throttle-before.log", cmd: []string{"sh", "-c", cpuThrottleBeforeScript()}, informational: true}, + {name: "03-stress-ng.log", cmd: []string{"stress-ng", "--cpu", "0", "--cpu-method", "all", "--timeout", fmt.Sprintf("%d", durationSec)}, syncBracket: true}, + {name: "04-sensors-after.log", cmd: []string{"sensors"}}, + {name: "05-thermal-throttle-check.log", cmd: []string{"sh", "-c", cpuThrottleCheckScript()}}, + } +} + func (s *System) RunCPUAcceptancePack(ctx context.Context, baseDir string, durationSec int, logFunc func(string)) (string, error) { if durationSec <= 0 { durationSec = 60 } - return runAcceptancePackCtx(ctx, baseDir, "cpu", []satJob{ - {name: "01-lscpu.log", cmd: []string{"lscpu"}}, - {name: "02-sensors-before.log", cmd: []string{"sensors"}}, - {name: "03-stress-ng.log", cmd: []string{"stress-ng", "--cpu", "0", "--cpu-method", "all", "--timeout", fmt.Sprintf("%d", durationSec)}, syncBracket: true}, - {name: "04-sensors-after.log", cmd: []string{"sensors"}}, - }, logFunc) + return runAcceptancePackCtx(ctx, baseDir, "cpu", cpuSATJobs(durationSec), logFunc) } func (s *System) RunStorageAcceptancePack(ctx context.Context, baseDir string, extended bool, logFunc func(string)) (string, error) { diff --git a/audit/internal/platform/sat_test.go b/audit/internal/platform/sat_test.go index c6d18b0..58a112a 100644 --- a/audit/internal/platform/sat_test.go +++ b/audit/internal/platform/sat_test.go @@ -684,3 +684,94 @@ func TestRunAcceptancePackCtxRetriesJob(t *testing.T) { t.Fatalf("summary=%q want flaky_status=OK", got) } } + +func TestCPUSATJobsIncludeThrottleCheck(t *testing.T) { + t.Parallel() + + jobs := cpuSATJobs(60) + + if len(jobs) != 6 { + t.Fatalf("jobs=%d want 6", len(jobs)) + } + if jobs[2].name != "02-thermal-throttle-before.log" || !jobs[2].informational { + t.Fatalf("throttle-before job=%+v want informational before-snapshot", jobs[2]) + } + if jobs[5].name != "05-thermal-throttle-check.log" || jobs[5].informational { + t.Fatalf("throttle-check job=%+v want non-informational so it can fail overall_status", jobs[5]) + } +} + +// TestCPUThrottleCheckDetectsIncreaseDuringRun runs the real before/check +// scripts (the same ones RunCPUAcceptancePack shells out to) against a fake +// sysfs tree, guarding both that a genuine thermal-throttle event during the +// stress-ng run is caught, and that a host with no new throttling (or none +// at all — e.g. AMD/ARM, which lack this sysfs interface) stays OK. +func TestCPUThrottleCheckDetectsIncreaseDuringRun(t *testing.T) { + fakeSys := t.TempDir() + oldDir := cpuThermalThrottleSysDir + cpuThermalThrottleSysDir = fakeSys + t.Cleanup(func() { cpuThermalThrottleSysDir = oldDir }) + + writeCounter := func(cpu, file, value string) { + path := filepath.Join(fakeSys, cpu, "thermal_throttle", file) + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(path, []byte(value+"\n"), 0644); err != nil { + t.Fatalf("write: %v", err) + } + } + + run := func(t *testing.T, dir string) (string, error) { + t.Helper() + runDir, err := runAcceptancePackCtx(context.Background(), dir, "cpu-throttle-test", []satJob{ + {name: "02-thermal-throttle-before.log", cmd: []string{"sh", "-c", cpuThrottleBeforeScript()}, informational: true}, + {name: "05-thermal-throttle-check.log", cmd: []string{"sh", "-c", cpuThrottleCheckScript()}}, + }, nil) + if err != nil { + return "", err + } + summary, err := os.ReadFile(filepath.Join(runDir, "summary.txt")) + if err != nil { + t.Fatalf("reading summary.txt: %v", err) + } + return string(summary), nil + } + + t.Run("no throttling stays OK", func(t *testing.T) { + writeCounter("cpu0", "core_throttle_count", "0") + writeCounter("cpu1", "core_throttle_count", "0") + + summary, err := run(t, t.TempDir()) + if err != nil { + t.Fatalf("runAcceptancePackCtx error: %v", err) + } + if !strings.Contains(summary, "overall_status=OK") { + t.Fatalf("summary=%q want overall_status=OK", summary) + } + }) + + t.Run("throttle count increasing during run fails overall_status", func(t *testing.T) { + writeCounter("cpu0", "core_throttle_count", "3") + writeCounter("cpu1", "core_throttle_count", "0") + + dir := t.TempDir() + runDir, err := runAcceptancePackCtx(context.Background(), dir, "cpu-throttle-test", []satJob{ + {name: "02-thermal-throttle-before.log", cmd: []string{"sh", "-c", cpuThrottleBeforeScript()}, informational: true}, + }, nil) + if err != nil { + t.Fatalf("runAcceptancePackCtx (before) error: %v", err) + } + + // Simulate the stress-ng run itself tripping the throttle. + writeCounter("cpu0", "core_throttle_count", "5") + + out, err := exec.Command("sh", "-c", strings.ReplaceAll(cpuThrottleCheckScript(), "{{run_dir}}", runDir)).CombinedOutput() + if err == nil { + t.Fatalf("check script succeeded, want failure; output=%s", out) + } + if !strings.Contains(string(out), "THROTTLE DETECTED") { + t.Fatalf("output=%s want THROTTLE DETECTED", out) + } + }) +}