fix: surface CPU thermal throttling in status and SAT results
A CPU that had thermally throttled (sysfs thermal_throttle counter > 0) still reported status "OK" everywhere: dmidecode-derived CPU status only distinguishes populated/enabled/disabled and never looked at the throttle flag the collector already recorded next to it, and neither SAT path meant to catch this actually could: - The routine "cpu" SAT pack (RunCPUAcceptancePack) only checked lscpu/sensors/stress-ng exit codes — stress-ng exits 0 whether or not the CPU throttled while running it, so an 89°C/throttled CPU right after a "successful" run still showed cpu:all as OK in component-status.json. - The more thorough platform-stress test already detected throttling and fan-spindown correctly, but wrote its verdict as "Overall: FAIL — ..." with no "=", which parseSATKV can't parse — so even a real detected throttle event never reached the component-status DB. Fixes: - cpu_telemetry.go: escalate a CPU's status to Warning (only-escalate, same severity ranking already used elsewhere) when Throttled is set. - sat.go: add a before/after thermal-throttle-counter check job around the "cpu" pack's stress-ng run, so a throttle event during the run fails that job and (via the existing FAILED->Warning DB mapping) flips cpu:all to Warning. - platform_stress.go: emit a machine-readable overall_status= line alongside the human-readable verdict so platform-stress results actually reach ApplySATResultToDB. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
56d12b1f3c
commit
198567dffe
@@ -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 {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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()
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user