diff --git a/audit/internal/platform/benchmark_power_calibration.go b/audit/internal/platform/benchmark_power_calibration.go index 163d870..82331c5 100644 --- a/audit/internal/platform/benchmark_power_calibration.go +++ b/audit/internal/platform/benchmark_power_calibration.go @@ -197,11 +197,12 @@ func filterSensorGroup(sensors []sdrSensor, maxPerSensorW float64) (valid []sdrS // GPU slot sensors (GPU_POWER_SLOTx, GPU1 Power, …) are classified separately // since the audit collector does not track GPU PCIe slot power. func sampleIPMISDRPowerSensors() sdrPowerSnapshot { - raw, err := exec.Command("ipmitool", "sdr").Output() - if err != nil || len(raw) == 0 { + // Reads from the shared hardware telemetry cache (hwSnapshot) rather than + // shelling out to `ipmitool sdr` itself — see hw_telemetry.go for why. + sdrStr := hwSnapshot().Raw + if sdrStr == "" { return sdrPowerSnapshot{} } - sdrStr := string(raw) var snap sdrPowerSnapshot // ── PSU data via audit collector ───────────────────────────────────────── @@ -390,20 +391,16 @@ func summarizeSDRPowerSeries(samples []sdrPowerSnapshot) benchmarkSDRSeriesSumma return summary } -// queryIPMIServerPowerW reads the current server power draw via ipmitool dcmi. -// Returns 0 and an error if IPMI is unavailable or the output cannot be parsed. +// queryIPMIServerPowerW reads the current server power draw from the shared +// hardware telemetry cache (hwSnapshot), which runs `ipmitool dcmi power +// reading` itself on its own cadence — see hw_telemetry.go for why this +// doesn't shell out directly. Returns 0 and an error if IPMI dcmi power is +// unavailable on this BMC. func queryIPMIServerPowerW() (float64, error) { - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - cmd := exec.CommandContext(ctx, "ipmitool", "dcmi", "power", "reading") - out, err := cmd.Output() - if err != nil { - return 0, fmt.Errorf("ipmitool dcmi power reading: %w", err) - } - if w := parseDCMIPowerReading(string(out)); w > 0 { + if w := hwSnapshot().DCMIPowerW; w > 0 { return w, nil } - return 0, fmt.Errorf("could not parse ipmitool dcmi power reading output") + return 0, fmt.Errorf("ipmitool dcmi power reading unavailable") } // characterizeServerPower computes BenchmarkServerPower from idle and loaded diff --git a/audit/internal/platform/hw_telemetry.go b/audit/internal/platform/hw_telemetry.go new file mode 100644 index 0000000..7ad5229 --- /dev/null +++ b/audit/internal/platform/hw_telemetry.go @@ -0,0 +1,188 @@ +package platform + +import ( + "os/exec" + "sync" + "time" +) + +// hwSample is one full snapshot from the shared hardware telemetry pipeline. +type hwSample struct { + At time.Time + // Raw is the full `ipmitool sdr` text, for consumers that need to + // re-derive something the pre-parsed fields below don't cover. + Raw string + Fans []FanReading + Temps []TempReading + PSUs []PSUReading + FanStatus map[string]string // fan name -> ipmi status word (ok/cr/nr/...) + DCMIPowerW float64 + ReadDur time.Duration +} + +const ( + hwPollFloor = 1 * time.Second + hwPollCeil = 30 * time.Second + hwReadTMO = 8 * time.Second +) + +// hwTelemetry is the single owner of every recurring, hot-path IPMI poll for +// fan RPM, temperature and PSU power/status (plus DCMI system power). Before +// this existed, the webui metrics collector (every 5s), the PSU health +// poller and the fan-ceiling check each shelled out to ipmitool on their own +// schedule. The BMC's KCS interface serializes those calls anyway, so +// running several concurrently only produced queueing — that's what shows up +// as "ipmitool slow" backoff in blackbox dumps taken during a fan-ceiling +// run, even while the dashboard looked fine (it was quietly reading its own, +// separately-stale data from a different ipmitool invocation). +// +// Now exactly one `ipmitool sdr` (+ one `dcmi power reading`) round trip is +// ever in flight, on a single adaptively-throttled background goroutine; +// every consumer below reads the shared cache via hwSnapshot instead of +// shelling out itself. One-off, non-recurring ipmitool calls (audit bundle +// export, pre/post-test PSU fault diffing) are intentionally left alone — +// they don't run on a sustained cadence, so they aren't the source of +// contention this pipeline exists to remove. +type hwTelemetry struct { + mu sync.Mutex + sample hwSample + interval time.Duration + degraded bool + started bool +} + +var hw = &hwTelemetry{interval: hwPollFloor} + +// hwSnapshot returns the most recent shared telemetry sample, starting the +// background poller (and blocking for its first read) on first use. +func hwSnapshot() hwSample { + hw.ensureStarted() + hw.mu.Lock() + defer hw.mu.Unlock() + return hw.sample +} + +// hwCurrentInterval returns the poller's current adaptive polling interval — +// above hwPollFloor means IPMI has been observed slow recently. +func hwCurrentInterval() time.Duration { + hw.mu.Lock() + defer hw.mu.Unlock() + return hw.interval +} + +// HardwareSDRSnapshot exposes the shared `ipmitool sdr` cache to other +// packages (e.g. webui's PSU health poller) so they don't have to run their +// own independent ipmitool poll — see hw_telemetry.go for why that matters. +// Returns the raw SDR text and when it was captured; raw is empty if no +// successful read has happened yet. +func HardwareSDRSnapshot() (raw string, at time.Time) { + s := hwSnapshot() + return s.Raw, s.At +} + +func (p *hwTelemetry) ensureStarted() { + p.mu.Lock() + if p.started { + p.mu.Unlock() + return + } + p.started = true + p.mu.Unlock() + p.pollOnce() // synchronous warm-up: the first caller gets a real reading, not an empty cache + go p.loop() +} + +func (p *hwTelemetry) loop() { + for { + wait := p.pollOnce() + time.Sleep(wait) + } +} + +// pollOnce runs exactly one sequential BMC round trip (sdr, then dcmi), +// updates the cache and the observed-peak stores, adapts the polling +// interval to how slow the read was, and returns the interval to sleep +// before the next poll. +func (p *hwTelemetry) pollOnce() time.Duration { + start := time.Now() + raw, ok := runIPMIBounded(hwReadTMO, "sdr") + readDur := time.Since(start) + + var next time.Duration + p.mu.Lock() + next = p.interval + switch { + case !ok || readDur > hwReadTMO*3/4: + if next < hwPollCeil { + next = minDuration(next*2, hwPollCeil) + } + p.degraded = true + case next > hwPollFloor && readDur < hwPollFloor: + next = maxDuration(next*2/3, hwPollFloor) + } + p.interval = next + p.mu.Unlock() + + if !ok { + return next + } + + fans := parseFanSpeeds(raw) + temps := parseIPMITemps(raw) + psus := parsePSUReadings(raw) + fanStatus := parseFanStatuses(raw) + + var dcmiW float64 + if dcmiOut, dcmiOK := runIPMIBounded(hwReadTMO, "dcmi", "power", "reading"); dcmiOK { + dcmiW = parseDCMIPowerReading(dcmiOut) + } + + now := time.Now() + p.mu.Lock() + p.sample = hwSample{ + At: now, + Raw: raw, + Fans: fans, + Temps: temps, + PSUs: psus, + FanStatus: fanStatus, + DCMIPowerW: dcmiW, + ReadDur: readDur, + } + p.mu.Unlock() + + if len(fans) > 0 { + updateFanObservation(fans, now) + } + if len(psus) > 0 { + updatePSUObservation(psus, now) + } + return next +} + +// runIPMIBounded runs an ipmitool subcommand but never blocks the caller +// longer than timeout: the read happens in a goroutine that is abandoned if +// it does not return in time (a KCS read wedged in uninterruptible I/O +// cannot be killed, so we leave it and move on). ok=false means "no usable +// output this round" — the caller must treat that as missing data. +func runIPMIBounded(timeout time.Duration, args ...string) (string, bool) { + type result struct { + out string + ok bool + } + ch := make(chan result, 1) + go func() { + out, err := exec.Command("ipmitool", args...).Output() + if err != nil || len(out) == 0 { + ch <- result{} + return + } + ch <- result{string(out), true} + }() + select { + case r := <-ch: + return r.out, r.ok + case <-time.After(timeout): + return "", false + } +} diff --git a/audit/internal/platform/live_metrics.go b/audit/internal/platform/live_metrics.go index 53a7214..a8d6646 100644 --- a/audit/internal/platform/live_metrics.go +++ b/audit/internal/platform/live_metrics.go @@ -246,14 +246,23 @@ func sampleLiveTempsViaSensorsJSON() []TempReading { return temps } +// sampleLiveTempsViaIPMI reads temperatures from the shared hardware +// telemetry cache (hwSnapshot) instead of shelling out to ipmitool itself — +// see hw_telemetry.go for why. func sampleLiveTempsViaIPMI() []TempReading { - out, err := exec.Command("ipmitool", "sdr", "type", "Temperature").Output() - if err != nil || len(out) == 0 { + return hwSnapshot().Temps +} + +// parseIPMITemps parses temperature entries out of `ipmitool sdr` text +// (the full dump or a "type Temperature"-filtered one — both use the same +// per-line format). +func parseIPMITemps(raw string) []TempReading { + if raw == "" { return nil } var temps []TempReading seen := map[string]struct{}{} - for _, line := range strings.Split(strings.TrimSpace(string(out)), "\n") { + for _, line := range strings.Split(strings.TrimSpace(raw), "\n") { parts := strings.Split(line, "|") if len(parts) < 3 { continue @@ -353,16 +362,23 @@ func compactAmbientTempName(chip, name string) string { return chip + " / " + name } -// samplePSUPower reads per-PSU input power via IPMI SDR. +// samplePSUPower reads per-PSU power from the shared hardware telemetry +// cache (hwSnapshot) instead of shelling out to ipmitool itself — see +// hw_telemetry.go for why. The observed-capacity store is fed once, inside +// the shared poller, rather than by every caller. +func samplePSUPower() []PSUReading { + return hwSnapshot().PSUs +} + +// parsePSUReadings parses per-PSU power out of `ipmitool sdr` text. // Uses collector.PSUSlotsFromSDR (name-based matching) which works across // vendors where PSU sensors may not carry entity ID "10.N". -// Returns nil when IPMI is unavailable or no PSU Watt sensors exist. -func samplePSUPower() []PSUReading { - out, err := exec.Command("ipmitool", "sdr").Output() - if err != nil || len(out) == 0 { +// Returns nil when no PSU Watt sensors exist. +func parsePSUReadings(raw string) []PSUReading { + if raw == "" { return nil } - slots := collector.PSUSlotsFromSDR(string(out)) + slots := collector.PSUSlotsFromSDR(raw) if len(slots) == 0 { return nil } @@ -393,9 +409,5 @@ func samplePSUPower() []PSUReading { if len(psus) == 0 { return nil } - // Feed the observed-capacity store (the "autotune" for PSU load scaling on - // BMCs that report only instantaneous power) — every load run that samples - // PSU power, including the 5 s metrics collector, refines it. - updatePSUObservation(psus, time.Now()) return psus } diff --git a/audit/internal/platform/sat_fan_stress.go b/audit/internal/platform/sat_fan_stress.go index d13d173..cba1ac7 100644 --- a/audit/internal/platform/sat_fan_stress.go +++ b/audit/internal/platform/sat_fan_stress.go @@ -196,20 +196,19 @@ func (s *System) RunFanCheck(ctx context.Context, baseDir string, opts FanCheckO appendSATVerboseLog(verboseLog, fmt.Sprintf("%d load source(s) active; plateau clock effective from +%.0fs", activeLoads, readyAt.Sub(start).Seconds())) - // ── Sample loop with IPMI-hang protection. + // ── Sample loop, reading the shared hardware telemetry cache. // - // Under full load ipmitool over KCS can take tens of seconds per call or - // wedge outright. So: every fan read is time-boxed (readFansBounded runs - // it in a goroutine we abandon on timeout — a wedged KCS read can never - // block this loop), and the polling interval backs off geometrically when - // reads are slow and tightens again when they recover. A plateau is only - // declared while telemetry is healthy (interval near the floor); a - // degraded run just rides out to MaxLoadSec and records the peak it saw. - const ( - fanPollFloor = 1 * time.Second - fanPollCeil = 30 * time.Second - fanReadTMO = 8 * time.Second - ) + // Fan RPM, PSU power and IPMI-hang protection all live in the shared + // hwTelemetry poller (hw_telemetry.go) now: it is the only thing that + // ever shells out to ipmitool on a recurring cadence, backs its own + // polling interval off geometrically when reads are slow (or wedge + // outright) and tightens again when they recover. This loop just ticks + // at the cache's floor interval and picks up whatever the shared poller + // last saw — it never spawns an ipmitool call of its own, so it can't + // add to the exact contention that made ipmitool slow in the first + // place. A plateau is only declared while telemetry is healthy (shared + // poller interval near the floor); a degraded run just rides out to + // MaxLoadSec and records the peak it saw. type fanState struct { peak float64 lastRiseSec float64 @@ -220,7 +219,8 @@ func (s *System) RunFanCheck(ctx context.Context, baseDir string, opts FanCheckO aborted := false degraded := false goodSamples := 0 - poll := fanPollFloor + lastSampleAt := time.Time{} + lastLoggedInterval := hwPollFloor // PSU peak draw, sampled at a slow cadence off the same loop — the fan // check already drives the box to full power, so it is also the right run @@ -243,39 +243,34 @@ loop: break loop case <-loadCtx.Done(): break loop // MaxLoadSec reached - case <-time.After(poll): + case <-time.After(hwPollFloor): } elapsed := time.Since(start).Seconds() - readStart := time.Now() - fans, ok := readFansBounded(fanReadTMO) - readDur := time.Since(readStart) - - // Adapt the interval to how ipmitool is behaving. - switch { - case !ok || readDur > fanReadTMO*3/4: - if poll < fanPollCeil { - poll = minDuration(poll*2, fanPollCeil) + curInterval := hwCurrentInterval() + if curInterval != lastLoggedInterval { + if curInterval > lastLoggedInterval { degraded = true - appendSATVerboseLog(verboseLog, fmt.Sprintf("[%.0fs] ipmitool slow (%.1fs, ok=%v) — polling backed off to %s", - elapsed, readDur.Seconds(), ok, poll)) - logFunc(fmt.Sprintf("IPMI slow — fan polling backed off to %s", poll)) + appendSATVerboseLog(verboseLog, fmt.Sprintf("[%.0fs] ipmitool slow — shared polling backed off to %s", + elapsed, curInterval)) + logFunc(fmt.Sprintf("IPMI slow — fan polling backed off to %s", curInterval)) } - case poll > fanPollFloor && readDur < fanPollFloor: - poll = maxDuration(poll*2/3, fanPollFloor) + lastLoggedInterval = curInterval } - if !ok { - continue + + sample := hwSnapshot() + if sample.At.IsZero() || !sample.At.After(lastSampleAt) { + continue // no fresh reading from the shared poller yet } + lastSampleAt = sample.At goodSamples++ + fans := sample.Fans if elapsed-lastPSUSec >= 15 { lastPSUSec = elapsed - if ps, ok := readPSUPowerBounded(fanReadTMO); ok { - for i, p := range ps { - if p.PowerW > psuPeakW[i] { - psuPeakW[i] = p.PowerW - } + for i, p := range sample.PSUs { + if p.PowerW > psuPeakW[i] { + psuPeakW[i] = p.PowerW } } } @@ -302,7 +297,7 @@ loop: // Only trust a plateau while telemetry is healthy and we have enough // recent samples to have actually seen a flat window. - healthy := poll <= 2*fanPollFloor && goodSamples >= 5 + healthy := curInterval <= 2*hwPollFloor && goodSamples >= 5 if healthy && rampConfirmed && elapsed >= float64(opts.MinLoadSec) && time.Since(readyAt) >= time.Duration(opts.PlateauHoldSec)*time.Second && len(fanBy) > 0 { allFlat := true @@ -415,56 +410,6 @@ func maxDuration(a, b time.Duration) time.Duration { return b } -// readFansBounded runs "ipmitool sdr type Fan" but never blocks the caller -// longer than timeout: the read happens in a goroutine that is abandoned if it -// does not return in time (a KCS read wedged in uninterruptible I/O cannot be -// killed, so we leave it and move on). ok=false means "no usable sample this -// tick" — the caller must treat that as missing data, not as a flat fan. -func readFansBounded(timeout time.Duration) ([]FanReading, bool) { - type result struct { - fans []FanReading - ok bool - } - ch := make(chan result, 1) - go func() { - out, err := exec.Command("ipmitool", "sdr", "type", "Fan").Output() - if err != nil { - ch <- result{} - return - } - fans := parseFanSpeeds(string(out)) - if len(fans) == 0 { - ch <- result{} - return - } - ch <- result{fans, true} - }() - select { - case r := <-ch: - if r.ok { - updateFanObservation(r.fans, time.Now()) - } - return r.fans, r.ok - case <-time.After(timeout): - return nil, false - } -} - -// readPSUPowerBounded is readFansBounded's PSU sibling: a time-boxed, -// abandonable "ipmitool sdr" read of per-PSU power (samplePSUPower also feeds -// the observed-capacity store). Used at a slow cadence during the fan check so -// the same max-load run that finds fan ceilings also records peak PSU draw. -func readPSUPowerBounded(timeout time.Duration) ([]PSUReading, bool) { - ch := make(chan []PSUReading, 1) - go func() { ch <- samplePSUPower() }() - select { - case ps := <-ch: - return ps, len(ps) > 0 - case <-time.After(timeout): - return nil, false - } -} - // boundedGPUMaxTemp returns the hottest GPU temperature via a single // time-boxed nvidia-smi call, or 0 if unavailable. func boundedGPUMaxTemp(gpuIndices []int) float64 { @@ -570,14 +515,23 @@ func sanitizeSummaryKey(name string) string { } // readFanStatuses returns the per-fan IPMI status word ("ok", "cr", "nr", ...) -// from "ipmitool sdr type Fan". Empty map when ipmitool is unavailable. +// from the shared hardware telemetry cache (hwSnapshot) instead of shelling +// out to ipmitool itself — see hw_telemetry.go for why. func readFanStatuses() map[string]string { - out, err := exec.Command("ipmitool", "sdr", "type", "Fan").Output() - if err != nil { + return hwSnapshot().FanStatus +} + +// parseFanStatuses parses the per-fan IPMI status word out of `ipmitool sdr` +// text (the full dump or a "type Fan"-filtered one — both use the same +// per-line format). Non-fan lines end up in the map too (keyed by whatever +// sensor name they have) but that's harmless: callers only look up known fan +// names. +func parseFanStatuses(raw string) map[string]string { + if raw == "" { return nil } m := map[string]string{} - for _, line := range strings.Split(string(out), "\n") { + for _, line := range strings.Split(raw, "\n") { parts := strings.Split(line, "|") if len(parts) < 3 { continue @@ -625,23 +579,19 @@ func ResolveFanMaxRPM(current map[string]float64) map[string]float64 { return out } -// sampleFanSpeeds reads fan RPM values from ipmitool sdr. +// sampleFanSpeeds reads fan RPM values from the shared hardware telemetry +// cache (hwSnapshot) instead of shelling out to ipmitool itself — see +// hw_telemetry.go for why. Falls back to lm-sensors when IPMI has nothing +// (unavailable BMC, or a platform without IPMI fan sensors at all). func sampleFanSpeeds() ([]FanReading, error) { - out, err := exec.Command("ipmitool", "sdr", "type", "Fan").Output() - if err == nil { - if fans := parseFanSpeeds(string(out)); len(fans) > 0 { - updateFanObservation(fans, time.Now()) - return fans, nil - } + if fans := hwSnapshot().Fans; len(fans) > 0 { + return fans, nil } fans, sensorsErr := sampleFanSpeedsViaSensorsJSON() if len(fans) > 0 { updateFanObservation(fans, time.Now()) return fans, nil } - if err != nil { - return nil, err - } return nil, sensorsErr } @@ -900,13 +850,16 @@ func firstFanInputValue(feature map[string]any) (float64, bool) { return firstSensorInputValue(feature, "fan") } -// sampleCPUMaxTemp returns the highest CPU/inlet temperature from ipmitool or sensors. +// sampleCPUMaxTemp returns the highest CPU/inlet temperature, from the +// shared hardware telemetry cache (hwSnapshot) when IPMI has it, else +// lm-sensors directly. func sampleCPUMaxTemp() float64 { - out, err := exec.Command("ipmitool", "sdr", "type", "Temperature").Output() - if err != nil { - return sampleCPUTempViaSensors() + if raw := hwSnapshot().Raw; raw != "" { + if t := parseIPMIMaxTemp(raw); t > 0 { + return t + } } - return parseIPMIMaxTemp(string(out)) + return sampleCPUTempViaSensors() } // parseIPMIMaxTemp extracts the maximum temperature from "ipmitool sdr type Temperature". diff --git a/audit/internal/webui/health_poller.go b/audit/internal/webui/health_poller.go index ccaad5d..ace8b4e 100644 --- a/audit/internal/webui/health_poller.go +++ b/audit/internal/webui/health_poller.go @@ -1,14 +1,12 @@ package webui import ( - "bytes" - "context" "log/slog" - "os/exec" "time" "bee/audit/internal/app" "bee/audit/internal/collector" + "bee/audit/internal/platform" ) const ( @@ -18,7 +16,6 @@ const ( // is still caught within a bounded window even after a long quiet // stretch. healthPollIntervalMax = 30 * time.Minute - psuIPMITimeout = 15 * time.Second ) // healthPoller runs periodic health checks for hardware components that do not @@ -66,26 +63,26 @@ func nextHealthPollInterval(current time.Duration, changed bool) time.Duration { return next } -// pollPSU polls PSU status via ipmitool and records it to statusDB. Returns -// true if any PSU's status differs from what statusDB currently has for it, -// which run uses to reset the backoff. +// pollPSU reads PSU status from the shared hardware telemetry cache +// (platform.HardwareSDRSnapshot) and records it to statusDB, instead of +// shelling out to ipmitool itself — see platform/hw_telemetry.go for why: +// this poller used to run its own independent `ipmitool sdr` on a 60s+ +// cadence, competing with the webui metrics collector and any running SAT +// test for the same serialized BMC interface. Returns true if any PSU's +// status differs from what statusDB currently has for it, which run uses to +// reset the backoff. func (p *healthPoller) pollPSU() bool { if p.statusDB == nil { return false } - ctx, cancel := context.WithTimeout(context.Background(), psuIPMITimeout) - defer cancel() - - cmd := exec.CommandContext(ctx, "ipmitool", "sdr") - var out bytes.Buffer - cmd.Stdout = &out - if err := cmd.Run(); err != nil { - // IPMI not available or not a server — skip silently. - slog.Debug("health poller: ipmitool sdr unavailable", "err", err) + raw, at := platform.HardwareSDRSnapshot() + if raw == "" || at.IsZero() { + // IPMI not available, or the shared poller hasn't completed a read yet. + slog.Debug("health poller: no shared ipmitool sdr sample available") return false } - slots := collector.PSUSlotsFromSDR(out.String()) + slots := collector.PSUSlotsFromSDR(raw) if len(slots) == 0 { return false }