diff --git a/server/collectors/SystemCollector.js b/server/collectors/SystemCollector.js index ecba788d..8acdc2c7 100644 --- a/server/collectors/SystemCollector.js +++ b/server/collectors/SystemCollector.js @@ -9,6 +9,38 @@ import { sshExec } from "./ssh.js"; * In Phase 2, this is the LOCAL path only (no SSH). * Remote path added in Phase 3. */ +/** + * Remote CPU probe command. Exported for testing. + * + * The sensor read is exit-tolerant on purpose. A host with no hwmon match and + * no readable thermal_zone temp file leaves the glob unexpanded, so the final + * `cat` exits 1 — and it is the last command in the ";"-joined chain, so the + * whole remote command exits non-zero. `sshExec` uses `execFile`, which rejects + * on a non-zero exit, so `_getRemoteCpu` would throw and fall back to + * `_defaultCpu()`: usage, draw and tdp lost as well as temperature. + * `_parseSensorTemp` already maps "nothing readable" to 0, so degrading to + * "no temperature" is the intended behaviour. + * + * @param {"spark"|"host"|undefined} kind + * @returns {string} + */ +export function buildRemoteCpuCommand(kind) { + return [ + "cat /proc/stat | head -1", + "echo '---'", + "cat /proc/cpuinfo | grep -E 'CPU architecture|aarch64' | head -1", + ...(kind === "host" + ? [ + "echo '---'", + // Same hwmon-then-thermal priority as local `_getCPUTemperature()`. + // GB10 also exposes nvme/mlx5 sensors; the name allowlist keeps those out. + 'for h in /sys/class/hwmon/*; do n=$(cat "$h/name" 2>/dev/null); case "$n" in coretemp|k10temp|zenpower|acpitz) for t in "$h"/temp*_input; do cat "$t" 2>/dev/null; break; done;; esac; done', + "cat /sys/class/thermal/thermal_zone*/temp 2>/dev/null || true", + ] + : []), + ].join("; "); +} + export class SystemCollector { constructor(spark) { this.spark = spark; @@ -1003,20 +1035,7 @@ export class SystemCollector { async _getRemoteCpu() { try { - const cmd = [ - "cat /proc/stat | head -1", - "echo '---'", - "cat /proc/cpuinfo | grep -E 'CPU architecture|aarch64' | head -1", - ...(this.spark.kind === "host" - ? [ - "echo '---'", - // Same hwmon-then-thermal priority as local `_getCPUTemperature()`. - // GB10 also exposes nvme/mlx5 sensors; the name allowlist keeps those out. - 'for h in /sys/class/hwmon/*; do n=$(cat "$h/name" 2>/dev/null); case "$n" in coretemp|k10temp|zenpower|acpitz) for t in "$h"/temp*_input; do cat "$t" 2>/dev/null; break; done;; esac; done', - "cat /sys/class/thermal/thermal_zone*/temp 2>/dev/null", - ] - : []), - ].join("; "); + const cmd = buildRemoteCpuCommand(this.spark.kind); const output = await sshExec(this.spark, cmd); const sections = output.split("---"); diff --git a/server/collectors/__tests__/SystemCollector.remoteCpuCommand.test.js b/server/collectors/__tests__/SystemCollector.remoteCpuCommand.test.js new file mode 100644 index 00000000..7e5942f9 --- /dev/null +++ b/server/collectors/__tests__/SystemCollector.remoteCpuCommand.test.js @@ -0,0 +1,42 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { execFile } from "node:child_process"; +import { promisify } from "node:util"; +import { buildRemoteCpuCommand } from "../SystemCollector.js"; + +const exec = promisify(execFile); + +// A kind "host" unit with no hwmon match and no readable thermal_zone temp file +// (e.g. a VM with a passed-through GPU) leaves the thermal glob unexpanded, so +// the final `cat` exits 1. It is the last command in the ";"-joined chain, so +// the whole remote command exits non-zero, and sshExec (execFile) rejects on a +// non-zero exit — _getRemoteCpu then falls back to _defaultCpu(), losing usage, +// draw and tdp as well as temperature. _parseSensorTemp already maps "nothing +// readable" to 0, so degrading to "no temperature" is the intended behaviour. + +test("host CPU probe exits 0 on a machine with no thermal sensors", async () => { + const cmd = buildRemoteCpuCommand("host"); + // execFile rejects on a non-zero exit, exactly how sshExec fails. + const { stdout } = await exec("/bin/sh", ["-c", cmd]); + assert.equal( + stdout.split("---").length, + 3, + "stat / cpuinfo / sensor sections must all be present even with no sensors" + ); +}); + +test("host CPU probe cannot fail on the sensor read", () => { + // On a box that does have thermal zones the test above passes trivially, so + // pin the exit-tolerance in the command itself too. + const cmd = buildRemoteCpuCommand("host"); + assert.match(cmd, /thermal_zone\*\/temp 2>\/dev\/null \|\| true/); +}); + +test("spark CPU probe omits the sensor probe entirely", () => { + // DGX Sparks deliberately skip the extra sensor read, so the GB10 path stays + // a two-section command. + const cmd = buildRemoteCpuCommand("spark"); + assert.doesNotMatch(cmd, /hwmon/); + assert.doesNotMatch(cmd, /thermal_zone/); + assert.equal(cmd.split("---").length, 2); +});