diff --git a/.github/workflows/e2e-selfhosted.yml b/.github/workflows/e2e-selfhosted.yml index 0408cd21c..cb33e90b9 100644 --- a/.github/workflows/e2e-selfhosted.yml +++ b/.github/workflows/e2e-selfhosted.yml @@ -215,7 +215,13 @@ jobs: min_free=$(( MIN_FREE_GIB * 1024 * 1024 * 1024 )) deadline=$(( SECONDS + CEILING_SECS )) reason="rocm-smi never returned within its timeout (driver wedged or GPU absent)" - while [ "$SECONDS" -lt "$deadline" ]; do + # Check the deadline only after a poll, never before the first one. + # $SECONDS counts whole wall-clock seconds, so a short ceiling can + # lapse before the loop is entered, and the verdict below would then + # describe a reading that was never taken. + first_poll=1 + while [ "$first_poll" -eq 1 ] || [ "$SECONDS" -lt "$deadline" ]; do + first_poll=0 # rocm-smi itself can hang on a wedged driver — bound it with timeout. out=$(timeout 15 rocm-smi --showmeminfo vram 2>/dev/null) || { sleep 5; continue; } # Parse the byte value AFTER the colon; the line prefix "GPU[0]" would @@ -461,7 +467,13 @@ jobs: # Neither is ever set from a call that was merely cut short. combined=1 gtt_absent=0 - while [ "$SECONDS" -lt "$deadline" ]; do + # Check the deadline only after a poll, never before the first one. + # $SECONDS counts whole wall-clock seconds, so a short ceiling can + # lapse before the loop is entered, and the verdict below would then + # describe a reading that was never taken. + first_poll=1 + while [ "$first_poll" -eq 1 ] || [ "$SECONDS" -lt "$deadline" ]; do + first_poll=0 out="" rc=0 if [ "$combined" -eq 1 ]; then @@ -1265,7 +1277,13 @@ jobs: # See the native Ubuntu lane for both memos. combined=1 gtt_absent=0 - while [ "$SECONDS" -lt "$deadline" ]; do + # Check the deadline only after a poll, never before the first one. + # $SECONDS counts whole wall-clock seconds, so a short ceiling can + # lapse before the loop is entered, and the verdict below would then + # describe a reading that was never taken. + first_poll=1 + while [ "$first_poll" -eq 1 ] || [ "$SECONDS" -lt "$deadline" ]; do + first_poll=0 out="" rc=0 if [ "$combined" -eq 1 ]; then diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 71fb11192..8a80b6ad3 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -400,7 +400,13 @@ jobs: min_free=$(( MIN_FREE_GIB * 1024 * 1024 * 1024 )) deadline=$(( SECONDS + CEILING_SECS )) reason="rocm-smi never returned within its timeout (driver wedged or GPU absent)" - while [ "$SECONDS" -lt "$deadline" ]; do + # Check the deadline only after a poll, never before the first one. + # $SECONDS counts whole wall-clock seconds, so a short ceiling can + # lapse before the loop is entered, and the verdict below would then + # describe a reading that was never taken. + first_poll=1 + while [ "$first_poll" -eq 1 ] || [ "$SECONDS" -lt "$deadline" ]; do + first_poll=0 # rocm-smi itself can hang on a wedged driver — bound it with timeout. out=$(timeout 15 rocm-smi --showmeminfo vram 2>/dev/null) || { sleep 5; continue; } # Parse the byte value AFTER the colon; the line prefix "GPU[0]" would @@ -521,7 +527,13 @@ jobs: min_free=$(( MIN_FREE_GIB * 1024 * 1024 * 1024 )) deadline=$(( SECONDS + CEILING_SECS )) reason="rocm-smi never returned within its timeout (driver wedged or GPU absent)" - while [ "$SECONDS" -lt "$deadline" ]; do + # Check the deadline only after a poll, never before the first one. + # $SECONDS counts whole wall-clock seconds, so a short ceiling can + # lapse before the loop is entered, and the verdict below would then + # describe a reading that was never taken. + first_poll=1 + while [ "$first_poll" -eq 1 ] || [ "$SECONDS" -lt "$deadline" ]; do + first_poll=0 out=$(timeout 15 rocm-smi --showmeminfo vram 2>/dev/null) || { sleep 5; continue; } total=$(printf '%s\n' "$out" | grep -i 'VRAM Total Memory' | sed 's/.*: *//' | grep -oE '[0-9]+' | tail -1) used=$(printf '%s\n' "$out" | grep -i 'VRAM Total Used Memory' | sed 's/.*: *//' | grep -oE '[0-9]+' | tail -1) @@ -619,7 +631,13 @@ jobs: min_free=$(( MIN_FREE_GIB * 1024 * 1024 * 1024 )) deadline=$(( SECONDS + CEILING_SECS )) reason="rocm-smi never returned within its timeout (driver wedged or GPU absent)" - while [ "$SECONDS" -lt "$deadline" ]; do + # Check the deadline only after a poll, never before the first one. + # $SECONDS counts whole wall-clock seconds, so a short ceiling can + # lapse before the loop is entered, and the verdict below would then + # describe a reading that was never taken. + first_poll=1 + while [ "$first_poll" -eq 1 ] || [ "$SECONDS" -lt "$deadline" ]; do + first_poll=0 out=$(timeout 15 rocm-smi --showmeminfo vram 2>/dev/null) || { sleep 5; continue; } total=$(printf '%s\n' "$out" | grep -i 'VRAM Total Memory' | sed 's/.*: *//' | grep -oE '[0-9]+' | tail -1) used=$(printf '%s\n' "$out" | grep -i 'VRAM Total Used Memory' | sed 's/.*: *//' | grep -oE '[0-9]+' | tail -1) @@ -744,7 +762,13 @@ jobs: # Neither is ever set from a call that was merely cut short. combined=1 gtt_absent=0 - while [ "$SECONDS" -lt "$deadline" ]; do + # Check the deadline only after a poll, never before the first one. + # $SECONDS counts whole wall-clock seconds, so a short ceiling can + # lapse before the loop is entered, and the verdict below would then + # describe a reading that was never taken. + first_poll=1 + while [ "$first_poll" -eq 1 ] || [ "$SECONDS" -lt "$deadline" ]; do + first_poll=0 out="" rc=0 if [ "$combined" -eq 1 ]; then @@ -1378,7 +1402,13 @@ jobs: # See the native Ubuntu lane for both memos. combined=1 gtt_absent=0 - while [ "$SECONDS" -lt "$deadline" ]; do + # Check the deadline only after a poll, never before the first one. + # $SECONDS counts whole wall-clock seconds, so a short ceiling can + # lapse before the loop is entered, and the verdict below would then + # describe a reading that was never taken. + first_poll=1 + while [ "$first_poll" -eq 1 ] || [ "$SECONDS" -lt "$deadline" ]; do + first_poll=0 out="" rc=0 if [ "$combined" -eq 1 ]; then diff --git a/xtask/src/workflow_contract.rs b/xtask/src/workflow_contract.rs index 77463e4cf..cd68785b2 100644 --- a/xtask/src/workflow_contract.rs +++ b/xtask/src/workflow_contract.rs @@ -1356,6 +1356,31 @@ case "$*" in esac "#; + /// Stands in for `scripts/reclaim-gpu.sh`, which a native lane runs with + /// `--report-holders` when it gives up, and says so in the log. Without it + /// the call resolves against whatever directory the test runs from, fails, + /// and the holder-reporting path is never exercised at all. + #[cfg(unix)] + const RECLAIM_STUB: &str = "#!/bin/sh\necho \"reclaim-gpu.sh stub called: $*\"\n"; + + /// What a lane logs when it reached the step that names what holds the GPU. + #[cfg(unix)] + const REPORTED_HOLDERS: &str = "reclaim-gpu.sh stub called: --report-holders"; + + /// A preflight's exit code, or a panic naming the signal that killed it: + /// `code()` is `None` exactly when there is no code to report. + #[cfg(unix)] + fn exit_code(status: std::process::ExitStatus, what: &str) -> i32 { + use std::os::unix::process::ExitStatusExt; + + status.code().unwrap_or_else(|| { + panic!( + "{what} exited with no status code: killed by signal {:?}", + status.signal() + ) + }) + } + /// Run a preflight script against fixture `rocm-smi` output, returning its /// exit code and everything it logged. `rocm-smi` is stubbed on `PATH`; /// nothing touches a real GPU. @@ -1380,6 +1405,11 @@ esac std::fs::write(&stub, SMI_STUB).expect("write rocm-smi stub"); std::fs::set_permissions(&stub, std::fs::Permissions::from_mode(0o755)) .expect("make the stub executable"); + // The lanes name `scripts/reclaim-gpu.sh` relative to the checkout, so + // run from a directory that has one. + std::fs::create_dir(dir.path().join("scripts")).expect("scripts dir"); + std::fs::write(dir.path().join("scripts/reclaim-gpu.sh"), RECLAIM_STUB) + .expect("write reclaim-gpu.sh stub"); let path = format!( "{}:{}", @@ -1392,6 +1422,7 @@ esac } let out = cmd .arg(&script_path) + .current_dir(dir.path()) .env("PATH", path) .env("FIXTURE", &fixture_path) .env("COUNTER", dir.path().join("calls")) @@ -1403,12 +1434,7 @@ esac .expect("running the preflight script"); let mut log = String::from_utf8_lossy(&out.stdout).into_owned(); log.push_str(&String::from_utf8_lossy(&out.stderr)); - ( - out.status - .code() - .expect("preflight exited with a status code"), - log, - ) + (exit_code(out.status, "preflight"), log) } /// The two APU lanes of `e2e-selfhosted.yml`, with the shell each runs under. @@ -1493,12 +1519,7 @@ esac .expect("running the preflight script under pwsh"); let mut log = String::from_utf8_lossy(&out.stdout).into_owned(); log.push_str(&String::from_utf8_lossy(&out.stderr)); - ( - out.status - .code() - .expect("preflight exited with a status code"), - log, - ) + (exit_code(out.status, "pwsh preflight"), log) } /// One host reading, and what each lane must make of it. @@ -1681,6 +1702,16 @@ esac for handle in handles { let (case, native, advisory, windows) = handle.join().expect("preflight case thread"); + // A native lane that gives up must also say what holds the card. + if case.native.0 == 1 { + assert!( + native.1.contains(REPORTED_HOLDERS), + "native preflight, {}: failed without reporting the GPU's \ + holders, logged:\n{}", + case.label, + native.1 + ); + } assert_gate("native", case.label, native, case.native); assert_gate("advisory", case.label, advisory, case.advisory); // The Windows block guards the same hardware class with the @@ -1715,7 +1746,12 @@ esac // Two polls: one at t=0, one after the script's own 5s backoff. The // first spends two calls (combined, then GTT alone); everything after // that fails, so the second poll reads nothing. - let env = [("GOOD_CALLS", "2"), ("GPU_PREFLIGHT_CEILING_SECS", "6")]; + // + // The ceiling is counted in bash's whole-second `$SECONDS`, so a + // ceiling of N buys between N-1 and N real seconds. 7 keeps the second + // check, at just over 5s, inside the budget; 6 left it a coin toss, and + // a skipped second poll passes this test without exercising it. + let env = [("GOOD_CALLS", "2"), ("GPU_PREFLIGHT_CEILING_SECS", "7")]; let fixture = smi_fixture(512 * MIB, 200 * MIB, None); let (native, advisory) = apu_lanes(); @@ -1735,6 +1771,91 @@ esac ); } + /// A `BASH_ENV` file that stalls the script once, for just over a second, + /// as soon as its `deadline` is set — before anything else runs. + /// + /// That is the descheduling a loaded host can impose at the same spot. + /// `$SECONDS` ticks on whole wall-clock seconds, so with a 1s ceiling the + /// deadline is already past when the script resumes. + #[cfg(unix)] + const STALL_AFTER_DEADLINE: &str = "trap '[ -z \"${__stalled:-}\" ] && [ -n \"${deadline:-}\" ] \ + && { __stalled=1; sleep 1.05; } || true' DEBUG\n"; + + #[cfg(unix)] + #[test] + fn every_shell_preflight_polls_at_least_once_whatever_the_ceiling() { + const GIB: u64 = 1024 * 1024 * 1024; + + // A lane whose deadline expires before its first poll reports a verdict + // no reading produced: the native lanes "rocm-smi never returned within + // its timeout", the advisory ones "no VRAM figures under WSL". This + // flaked the gate test above whenever the second boundary passed + // between computing the deadline and the loop's first check. Every + // shell copy in both workflows is driven, not just the APU twins the + // drift test pins, because each was written with the same loop. + if !has_timeout() { + eprintln!("skipping: no `timeout` on PATH (GNU coreutils)"); + return; + } + let trap_dir = tempfile::tempdir().expect("temp dir"); + let bash_env = trap_dir.path().join("stall.sh"); + std::fs::write(&bash_env, STALL_AFTER_DEADLINE).expect("write BASH_ENV"); + let bash_env = bash_env.to_str().expect("utf-8 temp path"); + + let mut lanes = Vec::new(); + for (workflow, text) in self_hosted_workflows() { + for (i, step) in gpu_preflight_steps(&text).iter().enumerate() { + let Some(script) = preflight_shell_script(step) else { + continue; + }; + let shell = if step.contains("-Script @'") { + PreflightShell::WslBareBash + } else { + PreflightShell::DefaultRunStep + }; + lanes.push((format!("{workflow} shell lane #{i}"), script, shell)); + } + } + assert_eq!( + lanes.len(), + 8, + "expected 8 shell GPU preflight blocks across both workflows — if a \ + lane was added or removed, update this count" + ); + + // A card held by a leftover serve: every lane, native or advisory, + // discrete or APU, must take a reading and fail on it. + let fixture = smi_fixture(192 * GIB, 191 * GIB, None); + let env = [("BASH_ENV", bash_env), ("GPU_PREFLIGHT_CEILING_SECS", "1")]; + std::thread::scope(|scope| { + let handles: Vec<_> = lanes + .iter() + .map(|(label, script, shell)| { + let (fixture, env) = (&fixture, &env); + scope.spawn(move || (label, shell, run_preflight(script, *shell, fixture, env))) + }) + .collect(); + for handle in handles { + let (label, shell, got) = handle.join().expect("preflight lane thread"); + // The advisory lanes `cd` to the runner's absolute checkout + // path first, so only the native ones reach the stub here. + if matches!(shell, PreflightShell::DefaultRunStep) { + assert!( + got.1.contains(REPORTED_HOLDERS), + "{label}: failed without reporting the GPU's holders, logged:\n{}", + got.1 + ); + } + assert_gate( + label, + "deadline already past at the first check", + got, + (1, &["VRAM never dropped below the floor"]), + ); + } + }); + } + #[test] fn every_self_hosted_lane_pins_a_hardware_label() { // `e2e-gpu` and `e2e-gpu-nightly` shipped as `[self-hosted, linux,