diff --git a/.github/scripts/ci-outage.sh b/.github/scripts/ci-outage.sh new file mode 100755 index 0000000000..bb13509e36 --- /dev/null +++ b/.github/scripts/ci-outage.sh @@ -0,0 +1,118 @@ +#!/bin/bash +# Per-cluster circuit breaker for environment-wide outages. +# +# Some failures are not the node's fault and not the code's fault: pypi.org +# unreachable from a login node, a module tree mid-upgrade, a full project +# filesystem. Requeuing elsewhere cannot help, and every job that starts pays +# the same discovery cost -- on 2026-08-28, 17 Frontier jobs each spent ~33 +# minutes learning that PyPI was down. +# +# The first job to notice records a marker on the shared filesystem (every +# self-hosted runner for a cluster shares $HOME); later jobs check it and exit +# immediately instead of submitting a SLURM job that is going to fail. +# +# The breaker is deliberately self-healing. A marker expires after +# MFC_CI_OUTAGE_TTL_SECONDS, and a marker that cannot be parsed is ignored, so +# neither a stale file nor a truncated write can wedge CI. Only jobs that +# actually observe the outage re-mark it, so once the outage clears the breaker +# closes on its own. +# +# Usage: +# ci-outage.sh mark record an outage +# ci-outage.sh check exit 0 = clear, 1 = outage active +# ci-outage.sh clear reset the breaker +# +# Env: +# MFC_CI_STATE_DIR where markers live (default ~/.mfc-ci-state) +# MFC_CI_OUTAGE_TTL_SECONDS marker lifetime in seconds (default 1200) + +set -uo pipefail + +STATE_DIR="${MFC_CI_STATE_DIR:-$HOME/.mfc-ci-state}" +TTL="${MFC_CI_OUTAGE_TTL_SECONDS:-1200}" + +# A non-numeric TTL would make the age comparison below emit "integer expression +# expected" and exit with a code the caller reads as neither clear nor tripped. +# Fall back to the default rather than letting a typo gate CI. +case "$TTL" in + ''|*[!0-9]*) + echo "Ignoring non-numeric MFC_CI_OUTAGE_TTL_SECONDS='$TTL'; using 1200." >&2 + TTL=1200 + ;; +esac + +EXIT_CLEAR=0 +EXIT_TRIPPED=1 +EXIT_USAGE=2 + +usage() { + echo "Usage: $0 {mark |check |clear }" >&2 +} + +# Keep the marker name filesystem-safe regardless of what the caller passes. +marker_for() { + local cluster + cluster=$(printf '%s' "$1" | tr -c 'A-Za-z0-9_.-' '_') + printf '%s/outage-%s' "$STATE_DIR" "$cluster" +} + +cmd="${1:-}" +cluster="${2:-}" + +if [ -z "$cmd" ] || [ -z "$cluster" ]; then + usage + exit $EXIT_USAGE +fi + +marker=$(marker_for "$cluster") + +case "$cmd" in + mark) + reason="${3:-unspecified}" + mkdir -p "$STATE_DIR" || exit $EXIT_USAGE + # Write to a temporary file and rename so a concurrent `check` never + # observes a half-written marker. + tmp="${marker}.$$.tmp" + { + date +%s + printf '%s\n' "$reason" + } > "$tmp" && mv -f "$tmp" "$marker" + echo "Recorded $cluster outage: $reason" + echo " marker: $marker (expires after ${TTL}s)" + ;; + + check) + [ -f "$marker" ] || exit $EXIT_CLEAR + + stamp=$(head -n1 "$marker" 2>/dev/null) + reason=$(tail -n +2 "$marker" 2>/dev/null) + + # A marker we cannot parse is treated as absent: an unreadable breaker + # must never be an un-clearable one. + case "$stamp" in + ''|*[!0-9]*) + echo "Ignoring unparseable outage marker $marker" + exit $EXIT_CLEAR + ;; + esac + + age=$(( $(date +%s) - stamp )) + if [ "$age" -ge "$TTL" ] || [ "$age" -lt 0 ]; then + exit $EXIT_CLEAR + fi + + echo "::warning::Skipping: known $cluster outage recorded ${age}s ago: ${reason:-unspecified}" + echo "Clear it early by deleting $marker" + exit $EXIT_TRIPPED + ;; + + clear) + rm -f "$marker" + echo "Cleared any $cluster outage marker ($marker)" + ;; + + *) + usage + exit $EXIT_USAGE + ;; +esac diff --git a/.github/scripts/classify-build-failure.sh b/.github/scripts/classify-build-failure.sh new file mode 100755 index 0000000000..6f2e56e00b --- /dev/null +++ b/.github/scripts/classify-build-failure.sh @@ -0,0 +1,41 @@ +#!/bin/bash +# Decide whether a failed build was a cluster-wide dependency outage. +# +# MFC bootstraps its Python toolchain into build/venv on the first ./mfc.sh call +# of a job, pulling from pypi.org. On Phoenix clean_build has just moved build/ +# aside, so that happens every time; on Frontier it happens in the login-node +# "Fetch Dependencies" step. When the index is unreachable the build fails for a +# reason no other node improves on, so it is worth recording once and skipping +# the rest of the matrix rather than having each job spend ~33 minutes +# rediscovering it (17 Frontier jobs did exactly that on 2026-08-28). +# +# Usage: classify-build-failure.sh +# +# Exit codes: +# 78 cluster-wide dependency outage; it has been recorded +# 0 ordinary build failure, caller should keep its own exit code + +set -uo pipefail + +log="${1:-}" +cluster="${2:-}" + +if [ -z "$log" ] || [ -z "$cluster" ]; then + echo "Usage: $0 " >&2 + exit 0 +fi + +# No log means nothing to classify. Never claim an outage on absent evidence. +[ -f "$log" ] || exit 0 + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +# The URL may or may not be wrapped (uv quotes it in backticks, plain pip does +# not), so do not require a character between the colon and the scheme. +if grep -qE "Failed to fetch:[^h]*https?://pypi|uv install failed|\(venv\) Installation failed" "$log"; then + bash "$SCRIPT_DIR/ci-outage.sh" mark "$cluster" \ + "PyPI/uv dependency install failed during build" + exit 78 +fi + +exit 0 diff --git a/.github/scripts/monitor_slurm_job.sh b/.github/scripts/monitor_slurm_job.sh index be53337d56..7d42e119a4 100755 --- a/.github/scripts/monitor_slurm_job.sh +++ b/.github/scripts/monitor_slurm_job.sh @@ -99,6 +99,11 @@ if ! [[ "$SLURM_MAX_QUEUE_SECONDS" =~ ^[0-9]+$ ]]; then echo "ERROR: SLURM_MAX_QUEUE_SECONDS must be a non-negative integer (seconds), got '$SLURM_MAX_QUEUE_SECONDS'" >&2 exit 1 fi +# How long to wait between status polls and between output-stabilization +# checks. Overridable so tests can exercise this script without sleeping +# through it; CI leaves it at the default. +: "${MFC_MONITOR_POLL_SECONDS:=5}" + queue_start=$(date +%s) abort_queue_starvation() { @@ -142,7 +147,7 @@ while [ ! -f "$output_file" ]; do ;; PENDING|CONFIGURING) unknown_count=0 - sleep 5 + sleep "$MFC_MONITOR_POLL_SECONDS" ;; RUNNING|COMPLETING) unknown_count=0 @@ -155,7 +160,7 @@ while [ ! -f "$output_file" ]; do if [ $((unknown_count % 12)) -eq 1 ]; then echo "Warning: Could not query job $job_id state (SLURM may be temporarily unavailable)..." fi - sleep 5 + sleep "$MFC_MONITOR_POLL_SECONDS" ;; *) # Terminal state — job finished without creating output @@ -164,7 +169,7 @@ while [ ! -f "$output_file" ]; do exit 1 fi # Unrecognized state, keep waiting - sleep 5 + sleep "$MFC_MONITOR_POLL_SECONDS" ;; esac done @@ -205,7 +210,7 @@ while true; do last_heartbeat=$current_time fi - sleep 5 + sleep "$MFC_MONITOR_POLL_SECONDS" done # Give tail a moment to flush the final lines, then stop streaming. @@ -229,7 +234,7 @@ if [ -f "$output_file" ]; then if [ $same_count -ge 2 ]; then break fi - sleep 5 + sleep "$MFC_MONITOR_POLL_SECONDS" done fi @@ -262,6 +267,23 @@ if [ -z "$exit_code" ]; then exit 1 fi +# Infrastructure verdicts from the in-allocation preflight come back as the +# job's own exit code. Relay them verbatim: flattening them to 1 would leave the +# submit wrapper unable to tell "this node is unusable" (exclude it and try +# again) from "the tests failed" (report it). +case "$exit_code" in + 77:*) + echo "Job $job_id failed preflight: the node is unusable — signaling caller to exclude it and resubmit." + monitor_success=1 + exit 77 + ;; + 78:*) + echo "Job $job_id skipped: a cluster-wide outage is already recorded." + monitor_success=1 + exit 78 + ;; +esac + # Check if job succeeded if [ "$exit_code" != "0:0" ]; then echo "ERROR: Job $job_id failed with exit code $exit_code" diff --git a/.github/scripts/node-exclude.sh b/.github/scripts/node-exclude.sh new file mode 100755 index 0000000000..cc3e139470 --- /dev/null +++ b/.github/scripts/node-exclude.sh @@ -0,0 +1,55 @@ +#!/bin/bash +# Bookkeeping for the sbatch --exclude list used when a node fails preflight. +# +# The in-allocation preflight prints "MFC_FAULT_NODE=" into the job's +# output file when it finds the node unusable (dead GPU, SIGILL from a binary +# built for another microarchitecture). The submit wrapper reads that back, adds +# the node to --exclude and resubmits, so the retry lands elsewhere. +# +# Bad nodes are concentrated rather than scattered -- over 2026-08-18..31 one +# Phoenix node accounted for 25 of 29 ECC failures -- which is why excluding the +# offender is worth doing and why it was previously a hand-edited constant. +# +# Usage: +# node-exclude.sh node-from print the faulted node, if any +# node-exclude.sh merge print with added once + +set -uo pipefail + +usage() { + echo "Usage: $0 {node-from |merge }" >&2 +} + +case "${1:-}" in + node-from) + file="${2:-}" + [ -f "$file" ] || exit 0 + # Last marker wins: one output path is reused across resubmits, so an + # earlier attempt's marker can still be sitting above the current one. + sed -n 's/.*MFC_FAULT_NODE=\([A-Za-z0-9._-]\{1,\}\).*/\1/p' "$file" | tail -n1 + ;; + + merge) + csv="${2-}" + node="${3-}" + if [ -z "$node" ]; then + printf '%s\n' "$csv" + exit 0 + fi + if [ -z "$csv" ]; then + printf '%s\n' "$node" + exit 0 + fi + # Wrapping both sides in commas compares whole fields, so a shorter name + # that happens to be a prefix of the new one is not mistaken for a match. + case ",$csv," in + *",$node,"*) printf '%s\n' "$csv" ;; + *) printf '%s,%s\n' "$csv" "$node" ;; + esac + ;; + + *) + usage + exit 2 + ;; +esac diff --git a/.github/scripts/prebuild-case-optimization.sh b/.github/scripts/prebuild-case-optimization.sh index c31058a08a..bcd117f232 100755 --- a/.github/scripts/prebuild-case-optimization.sh +++ b/.github/scripts/prebuild-case-optimization.sh @@ -90,6 +90,15 @@ if [ -n "$shard" ] && [ "$shard_count" -gt 1 ]; then fi fi +# Deliberately no node probe here. This pre-build is submitted as a *cpu* +# allocation (see test.yml: it is --dry-run, so it only builds), while the +# binaries it produces are GPU builds. syscheck built with --gpu therefore +# asserts omp_get_num_devices() > 0 and exits non-zero on a node that has no +# GPU by design -- which a probe would report as a bad node. It did: three +# healthy Phoenix nodes were condemned and two excluded before the wrapper gave +# up. The GPU allocation that actually runs these cases is probed instead, in +# run_case_optimization.sh. + idx=0 for case in "${benchmarks[@]}"; do idx=$((idx + 1)) diff --git a/.github/scripts/preflight.sh b/.github/scripts/preflight.sh new file mode 100755 index 0000000000..e5e9f4a7b0 --- /dev/null +++ b/.github/scripts/preflight.sh @@ -0,0 +1,149 @@ +#!/bin/bash +# Prove this node can run MFC before spending an allocation on it. +# +# Runs syscheck -- the same binary the suite already builds -- as the first +# thing inside the allocation that will execute the tests. syscheck initialises +# MPI, creates a device context, launches a kernel and reads the result back, so +# a node with a dead GPU, a broken MPI layer, or a binary built for a different +# microarchitecture fails here in seconds instead of after the build. +# +# Why at the top of the *test* allocation: over 2026-08-18..31, 48 of 58 +# measurable jobs built on one node and tested on another, and in every ECC +# failure the build node was healthy while the tests landed on a bad one. A +# probe placed after the build checks the wrong machine. (Phoenix's combined +# build-and-test allocation avoids the split; Frontier still submits the two +# separately and lands elsewhere ~90% of the time.) +# +# Usage: preflight.sh +# +# Exit codes: +# 0 node looks healthy, carry on +# 77 node-local fault -- caller should exclude this node and resubmit +# 78 cluster-wide outage already recorded -- caller should skip, not requeue + +set -uo pipefail + +cluster="${1:-}" +device="${2:-}" + +if [ -z "$cluster" ] || [ -z "$device" ]; then + echo "Usage: $0 " >&2 + exit 2 +fi + +EXIT_HEALTHY=0 +EXIT_NODE_FAULT=77 +EXIT_OUTAGE=78 + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +node="${SLURMD_NODENAME:-$(hostname -s 2>/dev/null || hostname)}" + +# Only judge a node from inside its own allocation. `mfc.sh load` is also used +# for building on login nodes -- bench.yml and frontier/build.sh both load the +# GPU module set there -- and a login node has no GPU to probe. Reporting a node +# fault in that context would have the wrapper exclude a login node and requeue, +# which is both wrong and hard to diagnose. Nothing calls the probe from a login +# node today; this makes that a property of the probe rather than a convention +# every future caller has to remember. +if [ -z "${SLURM_JOB_ID:-}" ]; then + echo "Preflight: not inside a SLURM allocation; skipping the node probe." + exit $EXIT_HEALTHY +fi + +# --- Cluster-wide outage: requeuing cannot help, so skip rather than retry --- +outage_rc=0 +bash "$SCRIPT_DIR/ci-outage.sh" check "$cluster" || outage_rc=$? +if [ "$outage_rc" -eq 1 ]; then + echo "Preflight: skipping on $node because $cluster is known to be down." + exit $EXIT_OUTAGE +elif [ "$outage_rc" -ne 0 ]; then + # Only exit 1 means "tripped". Anything else means the breaker could not be + # read at all (missing script, unreadable state dir), which says nothing + # about the cluster -- treating it as an outage would halt CI on a bug here. + echo "Preflight: could not read the outage breaker (exit $outage_rc); continuing." +fi + +# --- Node health --- +# Pick the *newest* install matching this job's device (build/install is named +# e.g. gpu-acc-, gpu-mp-). Both halves matter: the device filter +# avoids probing another variant's binary, and newest-wins avoids probing a +# leftover from an earlier job. Not every caller nukes build/ first -- bench.sh +# only does so on Phoenix -- and a stale binary compiled for a different +# microarchitecture dies with SIGILL, which would be reported as a bad node and +# get a perfectly healthy one excluded. +newest_syscheck() { + find "$@" -name syscheck -type f -printf '%T@ %p\n' 2>/dev/null \ + | sort -rn | head -1 | cut -d' ' -f2- +} + +syscheck_bin=$(newest_syscheck build/install -path "*${device}*") +if [ -z "$syscheck_bin" ]; then + syscheck_bin=$(newest_syscheck build/install) +fi + +if [ -z "$syscheck_bin" ]; then + # Nothing to probe with. A missing binary is a build problem, not a bad + # node: requeuing would land somewhere healthy and fail the same way, so + # let the build or test step report it instead. + echo "Preflight: no syscheck binary under build/install; skipping node probe on $node." + exit $EXIT_HEALTHY +fi + +# A binary built for a device this allocation did not ask for tells us nothing +# about the node. The case-optimization pre-build is submitted as cpu (it is a +# --dry-run that only builds) while producing GPU binaries, so its syscheck +# asserts a device exists and fails on a GPU-less node by design. Read as a node +# fault, that condemned three healthy Phoenix nodes and excluded two of them. +# The mismatch is a property of the job, never of the machine. +case "$syscheck_bin" in + *gpu-*) binary_device="gpu" ;; + *) binary_device="cpu" ;; +esac +if [ "$device" = "cpu" ] && [ "$binary_device" = "gpu" ]; then + echo "Preflight: $device allocation but the available syscheck is a GPU build" + echo " ($syscheck_bin); it cannot pass here, so skipping rather than judging $node." + exit $EXIT_HEALTHY +fi + +echo "Preflight: probing $node with $syscheck_bin" + +# Launch the probe the way this cluster launches everything else. Phoenix uses +# mpirun -- its openmpi predates the PMIx that shipped with its Slurm upgrade, +# so a bare MPI binary misreads the environment and aborts in MPI_Init. Frontier +# and frontier_amd use srun and Cray MPICH ships no mpirun at all, so running +# one there fails 127 no matter how healthy the node is. See +# toolchain/templates/{phoenix,frontier,frontier_amd}.mako. +case "$cluster" in + phoenix) launcher=(mpirun -np 1) ;; + frontier|frontier_amd) launcher=(srun -n1) ;; + *) launcher=() ;; +esac + +# A launcher missing from PATH says nothing about the node. Probing bare is a +# weaker test, but calling a healthy node bad is far worse: it costs three +# allocations and blacklists three good nodes before giving up. +if [ "${#launcher[@]}" -gt 0 ] && ! command -v "${launcher[0]}" >/dev/null 2>&1; then + echo "Preflight: ${launcher[0]} is not on PATH; probing without a launcher." + launcher=() +fi + +# Output goes to the log verbatim. Only the exit status decides the verdict: +# PMIX_ERR_NO_PERMISSIONS and friends from dstore_base.c are benign and appear +# in more passing jobs than failing ones, so matching on log text would fail +# healthy nodes. +probe_rc=0 +if [ "${#launcher[@]}" -eq 0 ]; then + "$syscheck_bin" 2>&1 || probe_rc=$? +else + "${launcher[@]}" "$syscheck_bin" 2>&1 || probe_rc=$? +fi + +if [ "$probe_rc" -eq 0 ]; then + echo "Preflight: $node passed." + exit $EXIT_HEALTHY +fi + +echo "::error::Preflight failed on $node: syscheck could not run MFC here." +echo "This is an INFRASTRUCTURE fault, not a code or test failure." +echo "MFC_FAULT_NODE=$node" +exit $EXIT_NODE_FAULT diff --git a/.github/scripts/retry-build.sh b/.github/scripts/retry-build.sh index a0b6ce8cfe..60fc2559a3 100755 --- a/.github/scripts/retry-build.sh +++ b/.github/scripts/retry-build.sh @@ -8,6 +8,10 @@ # retry_build ./mfc.sh build -j 8 --gpu acc # RETRY_VALIDATE_CMD='./syscheck' retry_build ./mfc.sh build -j 8 +# Delay between build attempts. Overridable so tests can exercise the retry +# path without waiting on it; CI leaves it at the default. +: "${MFC_BUILD_RETRY_DELAY:=30}" + retry_build() { local max_attempts=2 local validate_cmd="${RETRY_VALIDATE_CMD:-}" @@ -36,7 +40,7 @@ retry_build() { if [ $attempt -lt $max_attempts ]; then echo " Build failed — nuking build directory before retry..." rm -rf build 2>/dev/null || true - sleep 30 + sleep "$MFC_BUILD_RETRY_DELAY" else echo "Build failed after $max_attempts attempts." return 1 diff --git a/.github/scripts/run_case_optimization.sh b/.github/scripts/run_case_optimization.sh index 26899da211..75ab3a44a4 100755 --- a/.github/scripts/run_case_optimization.sh +++ b/.github/scripts/run_case_optimization.sh @@ -60,6 +60,18 @@ else build_opts="--no-build" fi +# Probe this node before spending the allocation on it. Placed after the build, +# not in the sbatch template: these scripts nuke and rebuild build/ themselves +# (Phoenix does so precisely because its compute nodes are heterogeneous), so a +# probe running earlier would test a stale binary from a previous job -- likely +# built for another microarchitecture -- and a SIGILL there would be reported as +# a bad node, excluding a healthy one. +preflight_rc=0 +bash .github/scripts/preflight.sh "$job_cluster" "$job_device" || preflight_rc=$? +if [ "$preflight_rc" -ne 0 ]; then + exit "$preflight_rc" +fi + passed=0 failed=0 failed_cases="" diff --git a/.github/scripts/run_monitored_slurm_job.sh b/.github/scripts/run_monitored_slurm_job.sh index d414d8dbf1..f18af2b556 100644 --- a/.github/scripts/run_monitored_slurm_job.sh +++ b/.github/scripts/run_monitored_slurm_job.sh @@ -29,10 +29,23 @@ if [ "$monitor_exit" -eq 76 ]; then exit 76 fi +# 77 (node fault) and 78 (recorded outage) are verdicts the preflight reached +# inside the allocation, not monitor failures — there is nothing to re-check +# with sacct, so relay them straight through rather than falling into the +# recovery path below. +if [ "$monitor_exit" -eq 77 ]; then + echo "Monitor reports SLURM job $job_id failed preflight — signaling caller to exclude the node and resubmit." + exit 77 +fi +if [ "$monitor_exit" -eq 78 ]; then + echo "Monitor reports SLURM job $job_id was skipped due to a recorded outage." + exit 78 +fi + if [ "$monitor_exit" -ne 0 ]; then echo "Monitor exited with code $monitor_exit; re-checking SLURM job $job_id final state..." # Give the SLURM epilog time to finalize if the job just finished - sleep 30 + sleep "${MFC_MONITOR_RECHECK_SECONDS:-30}" final_state=$(sacct -j "$job_id" -n -X -P -o State 2>/dev/null | head -n1 | cut -d'|' -f1 | tr -d ' ' || true) final_state="${final_state:-UNKNOWN}" final_exit=$(sacct -j "$job_id" -X --format=ExitCode --noheader --parsable2 2>/dev/null | head -n1 | tr -d ' ' || true) @@ -44,6 +57,18 @@ if [ "$monitor_exit" -ne 0 ]; then echo "SLURM job $job_id final state PREEMPTED — signaling caller to resubmit." exit 76 fi + # The monitor may have been killed before it could classify the job; the + # preflight's verdict is still recorded in the job's exit code. + case "$final_exit" in + 77:*) + echo "SLURM job $job_id failed preflight — signaling caller to exclude the node and resubmit." + exit 77 + ;; + 78:*) + echo "SLURM job $job_id was skipped due to a recorded outage." + exit 78 + ;; + esac if [ "$final_state" = "COMPLETED" ] && [ "$final_exit" = "0:0" ]; then echo "SLURM job $job_id completed successfully despite monitor failure — continuing." else diff --git a/.github/scripts/submit-slurm-job.sh b/.github/scripts/submit-slurm-job.sh index cf9d924834..f5480d176f 100755 --- a/.github/scripts/submit-slurm-job.sh +++ b/.github/scripts/submit-slurm-job.sh @@ -29,6 +29,13 @@ if [ -z "$script_path" ] || [ -z "$device" ] || [ -z "$interface" ] || [ -z "$cl fi sbatch_script_contents=$(cat "$script_path") + +# Nodes this job must not be scheduled onto. Seeded below per cluster with hosts +# already known to be bad, and grown at runtime when the in-allocation preflight +# reports a node fault (exit 77). Growing it automatically is the point: the two +# Phoenix nodes in the seed list were each found by hand, diagnosed, and +# committed, after one of them alone had eaten 25 jobs. +node_exclude="" SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" # Detect job type from submitted script basename @@ -131,11 +138,13 @@ elif [ "$device" = "gpu" ]; then case "$cluster" in phoenix) + # --exclude is rendered separately (see $node_exclude) so the + # preflight can add a node to it and resubmit. sbatch_device_opts="\ #SBATCH -p $gpu_partition #SBATCH --ntasks-per-node=4 -#SBATCH -G2 -#SBATCH --exclude=atl1-1-03-007-29-0,atl1-1-03-007-31-0" +#SBATCH -G2" + node_exclude="atl1-1-03-007-29-0,atl1-1-03-007-31-0" ;; frontier|frontier_amd) sbatch_device_opts="\ @@ -186,14 +195,38 @@ rm -f "$output_file" # --- Module load mode (short form) --- module_mode=$([ "$device" = "gpu" ] && echo "g" || echo "c") +# --- Skip entirely if this cluster is already known to be down --- +# Checking only inside the allocation would mean every matrix job still pays the +# full queue wait -- hours on Phoenix 'embers' -- before finding the marker. Only +# exit 1 means "tripped"; any other failure means the breaker itself could not be +# read, which says nothing about the cluster. +outage_rc=0 +bash "${SCRIPT_DIR}/ci-outage.sh" check "$cluster" || outage_rc=$? +if [ "$outage_rc" -eq 1 ]; then + echo "::warning::Not submitting: $cluster is under a recorded outage." + exit 78 +elif [ "$outage_rc" -ne 0 ]; then + echo "Could not read the outage breaker (exit $outage_rc); submitting anyway." +fi + # --- Submit (with retries for transient SLURM errors) --- source "${SCRIPT_DIR}/retry-sbatch.sh" -_sbatch_script=$(cat < "$id_file" @@ -257,7 +302,28 @@ while :; do echo "::error::SLURM job preempted ${MAX_PREEMPT_RESUBMITS} times without completing; giving up." exit 1 fi - # Genuine failure (not preemption). + if [ "$monitor_rc" -eq 77 ]; then + # The in-allocation preflight found this node unusable before any real + # work started. Exclude it and draw another node. + faulted_node=$(bash "$SCRIPT_DIR/node-exclude.sh" node-from "$output_file") + if [ "$node_attempt" -lt "$MFC_MAX_NODE_RESUBMITS" ]; then + node_attempt=$((node_attempt + 1)) + node_exclude=$(bash "$SCRIPT_DIR/node-exclude.sh" merge "$node_exclude" "$faulted_node") + echo "::warning::SLURM job $job_id failed preflight on ${faulted_node:-an unidentified node}. Excluding it and resubmitting (attempt ${node_attempt}/${MFC_MAX_NODE_RESUBMITS}). Excluding: ${node_exclude}" + rm -f "$output_file" + continue + fi + echo "::error::Preflight failed on $((MFC_MAX_NODE_RESUBMITS + 1)) nodes in a row (excluded: ${node_exclude})." + echo "That is a cluster-wide problem rather than a bad draw; not resubmitting." + exit 1 + fi + if [ "$monitor_rc" -eq 78 ]; then + # A recorded cluster-wide outage. Another node cannot help, so stop + # rather than spend more allocations proving the same point. + echo "::warning::Not resubmitting: $cluster is under a recorded outage." + exit "$monitor_rc" + fi + # Genuine failure (not preemption or infrastructure). exit "$monitor_rc" done unset _sbatch_script diff --git a/.github/workflows/common/bench.sh b/.github/workflows/common/bench.sh index be83e57b87..5d108a2a66 100644 --- a/.github/workflows/common/bench.sh +++ b/.github/workflows/common/bench.sh @@ -34,6 +34,18 @@ fi source .github/scripts/retry-build.sh retry_build ./mfc.sh build -j $n_jobs $build_opts || exit 1 +# Probe this node before spending the allocation on it. Placed after the build, +# not in the sbatch template: these scripts nuke and rebuild build/ themselves +# (Phoenix does so precisely because its compute nodes are heterogeneous), so a +# probe running earlier would test a stale binary from a previous job -- likely +# built for another microarchitecture -- and a SIGILL there would be reported as +# a bad node, excluding a healthy one. +preflight_rc=0 +bash .github/scripts/preflight.sh "$job_cluster" "$job_device" || preflight_rc=$? +if [ "$preflight_rc" -ne 0 ]; then + exit "$preflight_rc" +fi + # --- Bench cluster flag --- if [ "$job_cluster" = "phoenix" ]; then bench_cluster="phoenix-bench" diff --git a/.github/workflows/common/build.sh b/.github/workflows/common/build.sh index 45c58ae0e7..b6cf2e14be 100755 --- a/.github/workflows/common/build.sh +++ b/.github/workflows/common/build.sh @@ -61,5 +61,51 @@ case "${job_variant:-}" in *) echo "ERROR: unknown job_variant '$job_variant'"; exit 1 ;; esac +# Run a build step with its output teed, and classify a failure before giving up. +# Every ./mfc.sh call needs this, not just the solver build: the *first* one in +# the job is what bootstraps build/venv from PyPI, so it is the one that sees a +# package-index outage. +log_base="build-${job_slug:-${job_device}-${job_interface}}" + +run_build_step() { + local log="$1" + shift + set +e + "$@" 2>&1 | tee "$log" + local rc=${PIPESTATUS[0]} + set -e + if [ "$rc" -ne 0 ]; then + local cls=0 + bash .github/scripts/classify-build-failure.sh "$log" "$job_cluster" || cls=$? + if [ "$cls" -ne 0 ]; then + exit "$cls" + fi + exit "$rc" + fi +} + +# --- Probe this node before committing the solver build to it --- +# syscheck is a standalone target that links in 5-19 seconds, and it is already +# built second in the ordinary build order. Building it on its own first and +# running it here rejects an unusable node in about a minute, rather than after +# the ~40 minute solver build that used to precede the first GPU touch. In the +# Aug 2026 ECC failures that gap was a median of 38 minutes per job. +# +# Through retry_build so a transient blip still gets its nuke-and-retry, which a +# bare invocation here would have quietly dropped. +run_build_step "${log_base}-syscheck.log" retry_build ./mfc.sh build -t syscheck -j 8 $build_opts + +preflight_rc=0 +bash .github/scripts/preflight.sh "$job_cluster" "$job_device" || preflight_rc=$? +if [ "$preflight_rc" -ne 0 ]; then + # 77 (bad node) and 78 (recorded outage) travel back to submit-slurm-job.sh + # as this job's exit code, which decides whether to requeue elsewhere. + exit "$preflight_rc" +fi + +# --- Solver build --- +# Output is teed so a failure can be classified afterwards. Some Frontier CCE +# and amdflang failures emit no compiler diagnostic at all, so whatever the +# build did print is the only evidence there is. RETRY_VALIDATE_CMD="$validate_cmd" \ - retry_build "${build_cmd[@]}" || exit 1 + run_build_step "${log_base}.log" retry_build "${build_cmd[@]}" diff --git a/.github/workflows/common/test.sh b/.github/workflows/common/test.sh index 14927ee314..04d5d0373b 100644 --- a/.github/workflows/common/test.sh +++ b/.github/workflows/common/test.sh @@ -19,6 +19,18 @@ if [ "$job_cluster" = "phoenix" ]; then trap 'rm -rf "$currentdir" || true' EXIT fi +# --- Probe this node before running the suite --- +# The build already probed a node, but not necessarily this one: outside +# Phoenix's combined allocation the Build and Test steps are separate SLURM +# submissions with no node affinity, and they landed on different nodes in 26 of +# 29 measurable Frontier jobs. In the Aug 2026 ECC failures the build node was +# healthy every time and the tests were what landed on the bad one. +preflight_rc=0 +bash .github/scripts/preflight.sh "$job_cluster" "$job_device" || preflight_rc=$? +if [ "$preflight_rc" -ne 0 ]; then + exit "$preflight_rc" +fi + # --- GPU detection and thread count --- device_opts="" rdma_opts="" diff --git a/.github/workflows/frontier/build.sh b/.github/workflows/frontier/build.sh index cd289ef074..4ad359a4b4 100644 --- a/.github/workflows/frontier/build.sh +++ b/.github/workflows/frontier/build.sh @@ -23,4 +23,21 @@ source .github/scripts/clean-build.sh clean_build source .github/scripts/retry-build.sh -retry_build ./mfc.sh build --deps-only -j 8 $build_opts || exit 1 + +# This login-node step is where Frontier's dependency install actually happens, +# and so where a PyPI outage actually lands -- 17 jobs spent ~33 minutes each +# rediscovering one on 2026-08-28. Tee the output and classify a failure so the +# first job to hit it records it and the rest of the matrix can skip. +# No set -e in this script, so capture the status rather than toggling it. +deps_log="deps-${cluster_name}-${job_device}-${job_interface}.log" +retry_build ./mfc.sh build --deps-only -j 8 $build_opts 2>&1 | tee "$deps_log" +deps_rc=${PIPESTATUS[0]} + +if [ "$deps_rc" -ne 0 ]; then + cls=0 + bash .github/scripts/classify-build-failure.sh "$deps_log" "$cluster_name" || cls=$? + if [ "$cls" -ne 0 ]; then + exit "$cls" + fi + exit 1 +fi diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index eba8b1d179..20ef3348b6 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -522,9 +522,13 @@ jobs: if: matrix.cluster != 'phoenix' with: name: logs-${{ strategy.job-index }}-${{ steps.log.outputs.test_slug }} + # build-*.log is the teed compiler output. Some CCE and amdflang + # failures emit no diagnostic into the step log at all, so this file is + # the only evidence left of why the build died. path: | ${{ steps.log.outputs.build_slug }}*.out ${{ steps.log.outputs.test_slug }}.out + build-*.log case-optimization: name: "Case Opt | ${{ matrix.cluster_name }} (${{ matrix.device }}-${{ matrix.interface }})" diff --git a/src/syscheck/syscheck.fpp b/src/syscheck/syscheck.fpp index c3ecad2e02..6a9f40cee2 100644 --- a/src/syscheck/syscheck.fpp +++ b/src/syscheck/syscheck.fpp @@ -71,7 +71,11 @@ program syscheck @:ACC(integer :: i, num_devices) @:ACC(real(8), allocatable, dimension(:) :: arr) @:ACC(integer, parameter :: N = 100) - @:OMP(integer :: num_devices_omp) + ! MFC_OpenACC and MFC_OpenMP are mutually exclusive (see build.py), so the + ! OpenMP path reuses these names rather than carrying a parallel set. + @:OMP(integer :: i, num_devices) + @:OMP(real(8), allocatable, dimension(:) :: arr) + @:OMP(integer, parameter :: N = 100) @:MPIC(call mpi_init(ierr)) @:MPIC(call mpi_comm_rank(MPI_COMM_WORLD, rank, ierr)) @@ -83,7 +87,7 @@ program syscheck @:ACCC('devtype = acc_get_device_type()') @:ACCC('num_devices = acc_get_num_devices(devtype)') @:ACCC(call assert(num_devices > 0)) - @:ACCC(call acc_set_device_num(mod(rank, nRanks), devtype)) + @:ACCC(call acc_set_device_num(mod(rank, num_devices), devtype)) @:ACCC(allocate(arr(1:N))) @:ACCC('!$acc enter data create(arr(1:N))') @:ACCC('!$acc parallel loop') @@ -91,11 +95,25 @@ program syscheck @:ACC(arr(i) = i) @:ACC(end do) @:ACCC('!$acc update host(arr(1:N))') + @:ACCC(call assert(nint(sum(arr)) == N*(N + 1)/2)) @:ACCC('!$acc exit data delete(arr)') - @:OMPC('num_devices_omp = omp_get_num_devices()') - @:OMPC(call assert(num_devices_omp > 0)) - @:OMPC(call omp_set_default_device(mod(rank, nRanks))) + ! Mirror of the OpenACC block above. Querying omp_get_num_devices() alone is + ! not enough: it answers from the host and so reports a healthy node even + ! when the GPU is unusable. Only a target region creates a context, and only + ! reading the result back proves the device computed anything. + @:OMPC('num_devices = omp_get_num_devices()') + @:OMPC(call assert(num_devices > 0)) + @:OMPC(call omp_set_default_device(mod(rank, num_devices))) + @:OMPC(allocate(arr(1:N))) + @:OMPC('!$omp target enter data map(alloc: arr(1:N))') + @:OMPC('!$omp target teams distribute parallel do') + @:OMP(do i = 1, N) + @:OMP(arr(i) = i) + @:OMP(end do) + @:OMPC('!$omp target update from(arr(1:N))') + @:OMPC(call assert(nint(sum(arr)) == N*(N + 1)/2)) + @:OMPC('!$omp target exit data map(delete: arr(1:N))') @:MPIC(call mpi_barrier(MPI_COMM_WORLD, ierr)) @:MPIC(call mpi_finalize(ierr)) diff --git a/toolchain/mfc/bench.py b/toolchain/mfc/bench.py index f697e3d6da..68146d5aa6 100644 --- a/toolchain/mfc/bench.py +++ b/toolchain/mfc/bench.py @@ -11,7 +11,7 @@ import rich.table from .build import DEFAULT_TARGETS, SIMULATION, get_targets -from .common import MFC_BENCH_FILEPATH, MFC_BUILD_DIR, MFCException, create_directory, file_dump_yaml, file_load_yaml, format_list_to_string, system +from .common import MFC_BENCH_FILEPATH, MFC_BUILD_DIR, MFCException, console_safe, create_directory, file_dump_yaml, file_load_yaml, format_list_to_string, log_tail, system from .printer import cons from .state import ARG, CFG @@ -87,7 +87,9 @@ def bench(targets=None): time.sleep(5) continue cons.print(f"[bold red]ERROR[/bold red]: Case {case.slug} failed with exit code {rc}") - cons.print(f"[bold red] Check log at: {log_filepath}[/bold red]") + # Print the log, not just its path: this file lives + # on the cluster and no artifact upload collects it. + cons.print(console_safe(log_tail(log_filepath))) failed_cases.append(case.slug) break @@ -99,6 +101,7 @@ def bench(targets=None): time.sleep(5) continue cons.print(f"[bold red]ERROR[/bold red]: Summary file not created for {case.slug}") + cons.print(console_safe(log_tail(log_filepath))) cons.print(f"[bold red] Expected: {summary_filepath}[/bold red]") failed_cases.append(case.slug) break diff --git a/toolchain/mfc/common.py b/toolchain/mfc/common.py index c3985dc966..c59b5d0867 100644 --- a/toolchain/mfc/common.py +++ b/toolchain/mfc/common.py @@ -1,3 +1,4 @@ +import collections import logging import os import shutil @@ -6,6 +7,7 @@ import typing from os.path import abspath, dirname, join, normpath, realpath +import rich.markup import yaml from .printer import cons @@ -91,6 +93,47 @@ def file_read(filepath: str): raise MFCException(f'Failed to read from "{filepath}": {exc}') from exc +def console_safe(text: str) -> str: + """Escape captured text so Rich renders it verbatim. + + The console prints with markup enabled, and compiler/MPI output is full of + square brackets: absolute paths inside diagnostics, and "[host:pid]" rank + prefixes. Rich reads those as markup tags -- it raises MarkupError on an + unmatched closing tag like "[/lustre/...]" and silently swallows "[node1:1]". + Either way the diagnostic is destroyed at the moment it matters most. + """ + return rich.markup.escape(text) + + +def log_tail(filepath: str, max_lines: int = 60) -> str: + """Return the end of a log file, ready to print into CI output. + + A failure that only prints the *path* to its log is undebuggable in CI: the + file sits on a cluster or inside a container that no artifact upload + collects. Benchmark cases dying with "exit code 143" and post_process + failures pointing at out_post.txt were both diagnosable only by someone with + a shell on the machine, minutes before the workspace was cleaned. + + Never raises: this runs on a path that is already failing, and the absence + of the log is itself worth reporting. + """ + header = f"--- last {max_lines} lines of {filepath} ---" + + try: + # A bounded deque, not f.read(): solver and benchmark logs reach tens of + # MB, and this runs on a path that is already failing. Memory stays + # proportional to max_lines rather than to the file. + with open(filepath, "r", encoding="utf-8", errors="replace") as f: + lines = [line.rstrip("\n") for line in collections.deque(f, maxlen=max_lines)] + except OSError as exc: + return f"{header}\n(could not be read: {exc})" + + if not lines: + return f"{header}\n(the log is empty -- the process likely died before writing anything)" + + return "\n".join([header, *lines]) + + def file_load_yaml(filepath: str): try: with open(filepath, "r") as f: @@ -130,8 +173,16 @@ def delete_directory(dirpath: str) -> None: shutil.rmtree(dirpath) -def get_program_output(arguments: typing.List[str] = None, cwd=None): - with subprocess.Popen([str(_) for _ in arguments] or [], cwd=cwd, stdout=subprocess.PIPE) as proc: +def get_program_output(arguments: typing.List[str] = None, cwd=None, merge_stderr: bool = False): + """Run a command and return (stdout, returncode). + + merge_stderr folds stderr into the captured output. Off by default because + callers that parse stdout must not start seeing stderr mixed in; on for + diagnostics, where tools like h5dump report the actual reason on stderr and + capturing only stdout leaves nothing to show. + """ + stderr = subprocess.STDOUT if merge_stderr else None + with subprocess.Popen([str(_) for _ in arguments] or [], cwd=cwd, stdout=subprocess.PIPE, stderr=stderr) as proc: return (proc.communicate()[0].decode(), proc.returncode) diff --git a/toolchain/mfc/test/test.py b/toolchain/mfc/test/test.py index 15b4fe6125..05720522d3 100644 --- a/toolchain/mfc/test/test.py +++ b/toolchain/mfc/test/test.py @@ -15,7 +15,7 @@ from .. import common, sched from ..build import HDF5, POST_PROCESS, PRE_PROCESS, SIMULATION, build -from ..common import MFCException, does_command_exist, format_list_to_string, get_program_output +from ..common import MFCException, console_safe, does_command_exist, format_list_to_string, get_program_output, log_tail from ..packer import packer from ..packer import tol as packtol from ..printer import cons @@ -28,6 +28,7 @@ nSKIP = 0 current_test_number = 0 total_test_count = 0 +nRESCUED = 0 # cases that failed and were recovered by a retry (#1798) errors = [] failed_tests = [] # Track failed test details for summary test_start_time = None # Track overall test duration @@ -434,7 +435,7 @@ def test(): seconds = total_duration % 60 # Build the summary report - _print_test_summary(nPASS, nFAIL, nSKIP, minutes, seconds, failed_tests, skipped_cases) + _print_test_summary(nPASS, nFAIL, nSKIP, minutes, seconds, failed_tests, skipped_cases, nRESCUED) # Write failed UUIDs to file for CI retry logic if failed_tests: @@ -447,7 +448,7 @@ def test(): sys.exit(nFAIL) -def _print_test_summary(passed: int, failed: int, skipped: int, minutes: int, seconds: float, failed_test_list: list, _skipped_cases: list): +def _print_test_summary(passed: int, failed: int, skipped: int, minutes: int, seconds: float, failed_test_list: list, _skipped_cases: list, rescued: int = 0): """Print a comprehensive test summary report.""" total = passed + failed + skipped @@ -474,6 +475,12 @@ def _print_test_summary(passed: int, failed: int, skipped: int, minutes: int, se f" [bold green]{passed:4d}[/bold green] passed", f" [bold red]{failed:4d}[/bold red] failed", f" [bold yellow]{skipped:4d}[/bold yellow] skipped", + ] + if rescued: + # How often a retry actually earned its cost. See #1798: without this the + # only measurable retry outcomes were the ones that failed anyway. + summary_lines.append(f" [yellow]{rescued:4d}[/yellow] recovered by a retry") + summary_lines += [ f" [dim]{'─' * 12}[/dim]", f" [bold]{total:4d}[/bold] total", "", @@ -519,10 +526,35 @@ def _process_silo_file(silo_filepath: str, case: TestCase, out_filepath: str): raise MFCException("h5dump couldn't be found.") h5dump = shutil.which("h5dump") - output, err = get_program_output([h5dump, silo_filepath]) + # merge_stderr: h5dump reports the actual reason on stderr, so capturing + # only stdout would leave the failure path with nothing to show. + output, err = get_program_output([h5dump, silo_filepath], merge_stderr=True) if err != 0: - raise MFCException(f"Test {case}: Failed to run h5dump. You can find the run's output in {out_filepath}, and the case dictionary in {case.get_filepath()}.") + # h5dump's own message and the post_process log are the only evidence of + # why the file could not be read, and both were being discarded: the + # failure reached CI as a bare path to a file on a machine nobody can + # reach. Whether the silo file is absent or merely unreadable is the + # first thing worth knowing. + # Never let describing the file replace the failure being reported: a + # broken symlink or an unreadable mount would raise OSError here and + # swallow the h5dump diagnostic entirely. + try: + exists = f"{os.path.getsize(silo_filepath)} bytes" if os.path.exists(silo_filepath) else "missing" + except OSError as size_exc: + exists = f"size unknown: {size_exc}" + # console_safe over the whole message: main.py renders this with Rich + # markup enabled, and the h5dump output and post_process log are full of + # bracketed paths. Unescaped, a MarkupError would be raised from inside + # the very handler meant to report this failure. + raise MFCException( + console_safe( + f"Test {case}: Failed to run h5dump on {silo_filepath} ({exists}).\n" + f"h5dump said: {output.strip() or '(no output)'}\n" + f"{log_tail(out_filepath)}\n" + f"Case dictionary: {case.get_filepath()}." + ) + ) if "nan," in output: raise MFCException(f"Test {case}: Post Process has detected a NaN. You can find the run's output in {out_filepath}, and the case dictionary in {case.get_filepath()}.") @@ -704,8 +736,49 @@ def _handle_case(case: TestCase, devices: typing.Set[int]): timeout_timer.cancel() # Cancel timeout timer +def classify_error(exc: Exception) -> str: + """Bucket a test failure into the categories the retry policy turns on. + + Whether a retry is worth its cost depends on the class: a tolerance mismatch + re-runs the same binary over the same input and can only reach the same + comparison, whereas an execution failure may be a transient launcher or node + problem. Counting rescues without the class cannot tell those apart, which + is what #1798 needs to distinguish. + """ + text = str(exc).lower() + + if "tolerance" in text or "golden" in text or "mismatch" in text: + return "tolerance mismatch" + if "timeout" in text: + return "timeout" + if "nan" in text: + return "NaN detected" + if "failed to execute" in text: + return "execution failed" + + return "" + + +def should_retry(attempt: int, max_attempts: int, aborting: bool) -> bool: + """Whether a failed case gets another attempt. + + Retries here are expensive and, as far as can be measured, rarely help: + bench.py's equivalent rescued 0 of 235 retried cases, and every recorded + failed test in a two-week sample shows the full attempt count. See #1798. + + `aborting` is the part that was missing. The suite-wide abort fires when the + failure rate says the environment itself is broken -- a dead GPU, a bad node + -- and in that state every remaining attempt is guaranteed to fail. Retrying + through an abort turns the fail-fast into a slow one. + """ + if aborting: + return False + + return attempt < max_attempts + + def handle_case(case: TestCase, devices: typing.Set[int]): - global nFAIL, nPASS, nSKIP # noqa: PLW0603 + global nFAIL, nPASS, nSKIP, nRESCUED # noqa: PLW0603 global errors, failed_tests # noqa: PLW0603 # Check if we should abort before processing this case @@ -713,6 +786,7 @@ def handle_case(case: TestCase, devices: typing.Set[int]): return # Exit gracefully if abort was requested nAttempts = 0 + last_error = None if ARG("single"): max_attempts = max(ARG("max_attempts"), 3) else: @@ -727,8 +801,19 @@ def handle_case(case: TestCase, devices: typing.Set[int]): nSKIP += 1 else: nPASS += 1 + if nAttempts > 1: + # A rescue: the case failed and a retry recovered it. Nothing + # recorded this before, so a pass on attempt 3 was + # indistinguishable from a pass on attempt 1 -- which is why + # the value of retrying could never be measured. See #1798. + # The class matters as much as the count: retrying is worth + # very different things for a tolerance mismatch than for an + # execution failure. + nRESCUED += 1 + cons.print(f" [yellow]recovered on attempt {nAttempts}[/yellow] ({classify_error(last_error) or 'unclassified'}): {case.trace}") except Exception as exc: - if nAttempts < max_attempts: + last_error = exc + if should_retry(nAttempts, max_attempts, abort_tests.is_set()): continue nFAIL += 1 @@ -754,16 +839,7 @@ def handle_case(case: TestCase, devices: typing.Set[int]): cons.print() # Track failed test details for summary - error_type = "" - exc_lower = str(exc).lower() - if "tolerance" in exc_lower or "golden" in exc_lower or "mismatch" in exc_lower: - error_type = "tolerance mismatch" - elif "timeout" in exc_lower: - error_type = "timeout" - elif "nan" in exc_lower: - error_type = "NaN detected" - elif "failed to execute" in exc_lower: - error_type = "execution failed" + error_type = classify_error(exc) failed_tests.append({"trace": case.trace, "uuid": case.get_uuid(), "error_type": error_type, "attempts": nAttempts}) diff --git a/toolchain/mfc/test/test_retry_policy.py b/toolchain/mfc/test/test_retry_policy.py new file mode 100644 index 0000000000..ce3d1218b7 --- /dev/null +++ b/toolchain/mfc/test/test_retry_policy.py @@ -0,0 +1,62 @@ +"""Tests for the test-suite retry decision (issue #1798). + +Retries in this harness cost 3x wall clock and, as far as anyone can measure, +rescue almost nothing: bench.py's equivalent rescued 0 of 235 retried cases, and +all 2,795 recorded failed-test entries show `Attempts: 3`. + +That second figure is one-sided by construction -- only failures record an +attempt count, so a case that passes on attempt 2 is indistinguishable from one +that passed first try. Measuring rescues is therefore the prerequisite for any +policy change, and is fixed here. + +Also fixed: the retry loop never consulted the suite-wide abort flag. On a bad +node, where the 30% failure-rate abort is exactly what should stop the run, +every in-flight case still burned its remaining attempts first. +""" + +from mfc.test.test import should_retry + + +def test_a_failure_is_retried_until_the_attempt_budget_is_spent(): + assert should_retry(attempt=1, max_attempts=3, aborting=False) + assert should_retry(attempt=2, max_attempts=3, aborting=False) + + +def test_the_last_attempt_is_not_retried(): + assert not should_retry(attempt=3, max_attempts=3, aborting=False) + + +def test_a_single_attempt_budget_never_retries(): + assert not should_retry(attempt=1, max_attempts=1, aborting=False) + + +def test_no_retry_once_the_suite_is_aborting(): + # The abort fires when the failure rate says the environment is broken -- + # a bad node, say. Spending two more attempts per in-flight case is the + # opposite of the fail-fast the abort exists to provide. + assert not should_retry(attempt=1, max_attempts=3, aborting=True) + assert not should_retry(attempt=2, max_attempts=3, aborting=True) + + +def test_the_summary_reports_how_often_a_retry_actually_helped(capsys): + from mfc.test.test import _print_test_summary + + _print_test_summary(10, 2, 0, 0, 1.0, [], [], rescued=3) + assert "recovered by a retry" in capsys.readouterr().out + + +def test_the_summary_stays_quiet_when_no_retry_helped(capsys): + from mfc.test.test import _print_test_summary + + _print_test_summary(10, 2, 0, 0, 1.0, [], [], rescued=0) + assert "recovered by a retry" not in capsys.readouterr().out + + +def test_failures_are_classified_into_the_categories_the_policy_cares_about(): + from mfc.test.test import classify_error + + assert classify_error(Exception("Variable n5282 is not within tolerance")) == "tolerance mismatch" + assert classify_error(Exception("Test X: Failed to execute MFC.")) == "execution failed" + assert classify_error(Exception("NaN or Inf detected in the case.")) == "NaN detected" + assert classify_error(Exception("Test case exceeded 1 hour timeout")) == "timeout" + assert classify_error(Exception("something else entirely")) == "" diff --git a/toolchain/mfc/test_bench_log_tail.py b/toolchain/mfc/test_bench_log_tail.py new file mode 100644 index 0000000000..2e3229851b --- /dev/null +++ b/toolchain/mfc/test_bench_log_tail.py @@ -0,0 +1,63 @@ +"""A failing benchmark case must say why in the CI log. + +bench.py writes each case's output to build/benchmarks//.out and, on +failure, prints only that path -- a path on the cluster that no artifact upload +ever collects. The result is a CI failure whose entire diagnosis is +"failed with exit code 143". Thirteen jobs failed that way over 2026-08-18..31, +eleven of them on Phoenix, and none can be debugged after the fact. +""" + +from mfc.common import log_tail + + +def test_shows_the_end_of_the_log_where_the_error_is(tmp_path): + log = tmp_path / "case.out" + log.write_text("start\nmiddle\nNaN(s) in timestep output\n") + assert "NaN(s) in timestep output" in log_tail(str(log)) + + +def test_keeps_only_the_last_lines_so_a_huge_log_cannot_flood_ci(tmp_path): + log = tmp_path / "case.out" + log.write_text("\n".join(f"line {i}" for i in range(5000))) + body = log_tail(str(log), max_lines=50) + assert "line 4999" in body + assert "line 4000" not in body + + +def test_says_so_when_the_log_was_never_written(tmp_path): + # A case killed before it opened its log is itself a useful signal, and must + # not turn into a traceback inside the bench harness. + body = log_tail(str(tmp_path / "absent.out")) + assert "absent.out" in body + assert body.strip() != "" + + +def test_says_so_when_the_log_is_empty(tmp_path): + log = tmp_path / "case.out" + log.write_text("") + assert log_tail(str(log)).strip() != "" + + +def test_names_the_log_so_the_full_file_can_still_be_found(tmp_path): + log = tmp_path / "case.out" + log.write_text("boom\n") + assert "case.out" in log_tail(str(log)) + + +def test_reads_only_the_tail_of_a_large_log(tmp_path): + # Bounded memory matters: this runs on an already-failing path, and solver + # and benchmark logs reach tens of MB. + import tracemalloc + + log = tmp_path / "big.out" + log.write_text("".join(f"line {i} {'x' * 200}\n" for i in range(200_000))) + assert log.stat().st_size > 40_000_000 + + tracemalloc.start() + body = log_tail(str(log), max_lines=20) + peak = tracemalloc.get_traced_memory()[1] + tracemalloc.stop() + + assert "line 199999" in body + assert "line 100000" not in body + assert peak < 5_000_000, f"peak {peak} suggests the whole file was read" diff --git a/toolchain/mfc/test_bench_preflight.py b/toolchain/mfc/test_bench_preflight.py new file mode 100644 index 0000000000..1ab38877f3 --- /dev/null +++ b/toolchain/mfc/test_bench_preflight.py @@ -0,0 +1,122 @@ +"""The benchmark and case-optimization paths must probe their node too. + +The probe was originally wired only into common/build.sh and common/test.sh, +which the test.yml `self` job uses. But three of the five entry points that +submit SLURM jobs -- the benchmark job and both case-optimization jobs -- go +straight to submit-slurm-job.sh and never touch those scripts. That left 134 of +540 first-attempt failures on unprobed paths, including every ECC failure +outside the `self` job. + +Ordering is load-bearing here, hence the second test. These scripts nuke and +rebuild build/ themselves -- bench.sh does so on Phoenix precisely because "compute +nodes are heterogeneous -> ISA mismatch risk" -- so a probe placed before that +would test a leftover binary from a previous job, quite possibly built for +another microarchitecture. The SIGILL would be reported as a bad node, and the +wrapper would exclude a perfectly healthy one. +""" + +import os +import shutil +import stat +import subprocess +from pathlib import Path + +import pytest + +REPO = Path(__file__).resolve().parents[2] + + +def _exe(path: Path, text: str): + path.write_text(text) + path.chmod(path.stat().st_mode | stat.S_IEXEC) + + +@pytest.fixture +def workspace(tmp_path): + shutil.copytree(REPO / ".github", tmp_path / ".github") + binz = tmp_path / "bin" + binz.mkdir() + trace = tmp_path / "trace.log" + + passthrough = '#!/bin/bash\nwhile [ "${1:0:1}" = "-" ]; do shift; case "$1" in [0-9]*) shift;; esac; done\nexec "$@"\n' + for name in ("mpirun", "srun"): + _exe(binz / name, passthrough) + _exe(binz / "nvidia-smi", "#!/bin/bash\necho 'GPU 0: fake'\n") + + def install_mfc(probe_exit=0): + _exe( + tmp_path / "mfc.sh", + f"""#!/bin/bash +echo "mfc.sh $*" >> {trace} +for a in "$@"; do + if [ "$a" = "build" ]; then + mkdir -p build/install/gpu/bin + printf '#!/bin/bash\\nexit {probe_exit}\\n' > build/install/gpu/bin/syscheck + chmod +x build/install/gpu/bin/syscheck + fi +done +exit 0 +""", + ) + # A stale binary from a "previous job" that a too-early probe would find. + stale = tmp_path / "build" / "install" / "stale-gpu" / "bin" + stale.mkdir(parents=True, exist_ok=True) + _exe(stale / "syscheck", "#!/bin/bash\necho 'stale SIGILL'\nexit 132\n") + + def run(): + env = { + **os.environ, + "PATH": f"{binz}:{os.environ['PATH']}", + "MFC_CI_STATE_DIR": str(tmp_path / "state"), + "MFC_BUILD_RETRY_DELAY": "0", + "SLURM_JOB_ID": "123456", + "SLURMD_NODENAME": "frontier4242", + "job_device": "gpu", + "job_interface": "acc", + "job_slug": "bench-gpu-acc", + "job_cluster": "frontier", + } + return subprocess.run( + ["bash", ".github/workflows/common/bench.sh"], + capture_output=True, + text=True, + cwd=tmp_path, + env=env, + check=False, + timeout=120, + ) + + return tmp_path, install_mfc, run, trace + + +def test_the_benchmark_job_probes_its_node(workspace): + _, install_mfc, run, _ = workspace + install_mfc(probe_exit=0) + result = run() + assert result.returncode == 0, result.stdout + result.stderr + assert "Preflight:" in result.stdout + + +def test_a_bad_node_stops_the_benchmark_before_it_runs(workspace): + _, install_mfc, run, trace = workspace + install_mfc(probe_exit=1) + result = run() + assert result.returncode == 77 + assert "mfc.sh bench" not in trace.read_text() + + +def test_the_probe_runs_after_the_build_not_before_it(workspace): + # If the probe ran first it would find the stale binary planted above, which + # exits 132 like a SIGILL, and condemn a healthy node. + _, install_mfc, run, trace = workspace + install_mfc(probe_exit=0) + result = run() + assert result.returncode == 0, result.stdout + result.stderr + calls = trace.read_text().splitlines() + assert any("build" in c for c in calls), calls + build_at = next(i for i, c in enumerate(calls) if "build" in c) + probe_at = result.stdout.index("Preflight:") + build_marker = result.stdout.find("mfc.sh build") + assert build_at == 0 + if build_marker != -1: + assert probe_at > build_marker diff --git a/toolchain/mfc/test_build_preflight.py b/toolchain/mfc/test_build_preflight.py new file mode 100644 index 0000000000..ab6d16e03c --- /dev/null +++ b/toolchain/mfc/test_build_preflight.py @@ -0,0 +1,184 @@ +"""Tests for .github/workflows/common/build.sh. + +Two behaviours are pinned here. + +Order: syscheck is a standalone target that links in 5-19 seconds and is already +built second, right after hipfort. Building it first and running it before the +solver build means a node with a dead GPU is rejected in about a minute instead +of after ~40 minutes of compilation. Across the Aug 2026 ECC failures the gap +between syscheck being available and the fault being noticed was a median of 38 +minutes, 28.5 hours in total. + +Outage: an unreachable pypi.org is not the node's fault and not fixable by +requeuing. The first job to hit it records it so the rest of the matrix skips +rather than each spending ~33 minutes rediscovering it. +""" + +import os +import shutil +import stat +import subprocess +from pathlib import Path + +import pytest + +REPO = Path(__file__).resolve().parents[2] + +PYPI_FAILURE = " x Failed to build `mfc @ file:///work/toolchain`\n" " |-> Failed to fetch: `https://pypi.org/simple/hatch-vcs/`\n" "mfc: ERROR > (venv) Installation failed.\n" + + +def _exe(path: Path, text: str): + path.write_text(text) + path.chmod(path.stat().st_mode | stat.S_IEXEC) + + +@pytest.fixture +def workspace(tmp_path): + shutil.copytree(REPO / ".github", tmp_path / ".github") + binz = tmp_path / "bin" + binz.mkdir() + passthrough = '#!/bin/bash\nwhile [ "${1:0:1}" = "-" ]; do shift; case "$1" in [0-9]*) shift;; esac; done\nexec "$@"\n' + _exe(binz / "mpirun", passthrough) + # frontier launches with srun; stub it so the real /usr/bin/srun on this box + # cannot stand in and try to submit an actual job. + _exe(binz / "srun", passthrough) + trace = tmp_path / "trace.log" + + def install_mfc(full_build_stdout="", full_build_rc=0, syscheck_rc=0, probe_build_stdout="", probe_build_rc=0): + _exe( + tmp_path / "mfc.sh", + f"""#!/bin/bash +echo "mfc.sh $*" >> {trace} +for a in "$@"; do + if [ "$a" = "syscheck" ]; then + printf '%s' {shlex_quote(probe_build_stdout)} + if [ {probe_build_rc} -ne 0 ]; then exit {probe_build_rc}; fi + mkdir -p build/install/gpu/bin + printf '#!/bin/bash\\necho probe ran\\nexit {syscheck_rc}\\n' > build/install/gpu/bin/syscheck + chmod +x build/install/gpu/bin/syscheck + exit 0 + fi +done +printf '%s' {shlex_quote(full_build_stdout)} +exit {full_build_rc} +""", + ) + + def run(): + env = { + **os.environ, + "PATH": f"{binz}:{os.environ['PATH']}", + "MFC_CI_STATE_DIR": str(tmp_path / "state"), + "MFC_BUILD_RETRY_DELAY": "0", + "job_device": "gpu", + "job_interface": "acc", + "job_shard": "", + "job_cluster": "frontier", + "job_variant": "", + "SLURMD_NODENAME": "frontier1234", + # These scripts only ever run inside a SLURM allocation + # (submit-slurm-job.sh is their sole caller), and the probe + # refuses to judge a node outside one. + "SLURM_JOB_ID": "123456", + } + return subprocess.run( + ["bash", ".github/workflows/common/build.sh"], + capture_output=True, + text=True, + cwd=tmp_path, + env=env, + check=False, + timeout=120, + ) + + return tmp_path, install_mfc, run, trace + + +def shlex_quote(s): + import shlex + + return shlex.quote(s) + + +def outage_recorded(tmp_path): + state = tmp_path / "state" + return state.exists() and any(state.glob("outage-*")) + + +def test_the_probe_is_built_before_the_solver(workspace): + tmp_path, install_mfc, run, trace = workspace + install_mfc() + assert run().returncode == 0 + calls = trace.read_text().splitlines() + assert any("syscheck" in c for c in calls), calls + assert "syscheck" in calls[0], f"probe must be built first, got {calls}" + + +def test_a_failing_probe_stops_before_the_solver_build(workspace): + tmp_path, install_mfc, run, trace = workspace + install_mfc(syscheck_rc=1) + result = run() + assert result.returncode == 77 + assert len(trace.read_text().splitlines()) == 1, "solver build must not be attempted" + + +def test_a_pypi_failure_records_a_cluster_outage(workspace): + tmp_path, install_mfc, run, _ = workspace + install_mfc(full_build_stdout=PYPI_FAILURE, full_build_rc=1) + run() + assert outage_recorded(tmp_path) + + +def test_a_pypi_failure_reports_the_outage_exit_code(workspace): + tmp_path, install_mfc, run, _ = workspace + install_mfc(full_build_stdout=PYPI_FAILURE, full_build_rc=1) + assert run().returncode == 78 + + +def test_an_ordinary_compile_error_is_not_treated_as_an_outage(workspace): + tmp_path, install_mfc, run, _ = workspace + install_mfc(full_build_stdout="NVFORTRAN-S-0034-Syntax error\n", full_build_rc=1) + result = run() + assert not outage_recorded(tmp_path) + assert result.returncode not in (77, 78) + + +def test_the_build_output_is_still_shown_when_it_fails(workspace): + # The CCE/flang failures that report no compiler diagnostic are only + # debuggable if whatever the build did print survives into the CI log. + tmp_path, install_mfc, run, _ = workspace + install_mfc(full_build_stdout="ftn-2116 ftn: INTERNAL\n", full_build_rc=1) + result = run() + assert "ftn-2116" in result.stdout + result.stderr + + +def test_a_pypi_failure_during_the_probe_build_records_an_outage(workspace): + # The probe build is now the first mfc.sh call in the job, so it is what + # bootstraps build/venv from PyPI -- and on Phoenix clean_build has just + # deleted that venv, so it is rebuilt every time. Classifying only the solver + # build leaves the breaker blind to the outage it exists for. + tmp_path, install_mfc, run, _ = workspace + install_mfc(probe_build_stdout=PYPI_FAILURE, probe_build_rc=1) + run() + assert outage_recorded(tmp_path) + + +def test_a_pypi_failure_during_the_probe_build_reports_the_outage_exit_code(workspace): + tmp_path, install_mfc, run, _ = workspace + install_mfc(probe_build_stdout=PYPI_FAILURE, probe_build_rc=1) + assert run().returncode == 78 + + +def test_an_ordinary_probe_build_failure_is_not_an_outage(workspace): + tmp_path, install_mfc, run, _ = workspace + install_mfc(probe_build_stdout="NVFORTRAN-S-0034-Syntax error\n", probe_build_rc=1) + result = run() + assert not outage_recorded(tmp_path) + assert result.returncode not in (77, 78) + + +def test_the_probe_build_output_is_shown_when_it_fails(workspace): + tmp_path, install_mfc, run, _ = workspace + install_mfc(probe_build_stdout="ftn-2116 ftn: INTERNAL\n", probe_build_rc=1) + result = run() + assert "ftn-2116" in result.stdout + result.stderr diff --git a/toolchain/mfc/test_ci_outage.py b/toolchain/mfc/test_ci_outage.py new file mode 100644 index 0000000000..24ba338b14 --- /dev/null +++ b/toolchain/mfc/test_ci_outage.py @@ -0,0 +1,114 @@ +"""Unit tests for .github/scripts/ci-outage.sh. + +When an environment-wide outage hits a cluster -- pypi.org unreachable from a +Frontier login node, say -- every queued CI job rediscovers it independently. +On 2026-08-28 that cost 17 Frontier jobs about 33 minutes each to learn the same +fact. The circuit breaker lets the first job that notices record it on the shared +filesystem so later jobs skip immediately instead of queueing SLURM to fail. + +A breaker that cannot reset is worse than none, so the time-to-live behaviour is +pinned here as tightly as the tripping behaviour. +""" + +import os +import subprocess +import time +from pathlib import Path + +import pytest + +SCRIPT = Path(__file__).resolve().parents[2] / ".github" / "scripts" / "ci-outage.sh" + +CLEAR = 0 +TRIPPED = 1 + + +@pytest.fixture +def state_dir(tmp_path): + return tmp_path / "state" + + +def run(state_dir, *args, ttl=None): + env = {**os.environ, "MFC_CI_STATE_DIR": str(state_dir)} + if ttl is not None: + env["MFC_CI_OUTAGE_TTL_SECONDS"] = str(ttl) + return subprocess.run( + ["bash", str(SCRIPT), *args], + capture_output=True, + text=True, + env=env, + check=False, + ) + + +def test_check_reports_clear_when_nothing_has_been_recorded(state_dir): + assert run(state_dir, "check", "phoenix").returncode == CLEAR + + +def test_check_trips_after_an_outage_is_marked(state_dir): + run(state_dir, "mark", "phoenix", "pypi unreachable") + assert run(state_dir, "check", "phoenix").returncode == TRIPPED + + +def test_check_reports_the_recorded_reason_so_the_log_explains_the_skip(state_dir): + run(state_dir, "mark", "phoenix", "pypi unreachable") + result = run(state_dir, "check", "phoenix") + assert "pypi unreachable" in result.stdout + result.stderr + + +def test_an_outage_on_one_cluster_leaves_the_other_alone(state_dir): + run(state_dir, "mark", "frontier", "pypi unreachable") + assert run(state_dir, "check", "phoenix").returncode == CLEAR + + +def test_an_outage_older_than_the_ttl_stops_tripping(state_dir): + run(state_dir, "mark", "phoenix", "pypi unreachable", ttl=1) + time.sleep(2) + assert run(state_dir, "check", "phoenix", ttl=1).returncode == CLEAR + + +def test_clear_resets_the_breaker(state_dir): + run(state_dir, "mark", "phoenix", "pypi unreachable") + run(state_dir, "clear", "phoenix") + assert run(state_dir, "check", "phoenix").returncode == CLEAR + + +def test_marking_works_when_the_state_directory_does_not_exist_yet(state_dir): + # The first job on a fresh runner must not fail just because nothing has + # created the directory. + assert not state_dir.exists() + assert run(state_dir, "mark", "phoenix", "pypi unreachable").returncode == 0 + + +def test_a_corrupt_marker_is_treated_as_clear_rather_than_wedging_ci(state_dir): + # A truncated or garbled marker must never become an un-clearable breaker. + run(state_dir, "mark", "phoenix", "pypi unreachable") + marker = next(state_dir.glob("*phoenix*")) + marker.write_text("not-a-timestamp\n") + assert run(state_dir, "check", "phoenix").returncode == CLEAR + + +def test_check_names_the_marker_file_so_a_human_can_clear_it(state_dir): + run(state_dir, "mark", "phoenix", "pypi unreachable") + result = run(state_dir, "check", "phoenix") + assert str(state_dir) in result.stdout + result.stderr + + +def test_an_unknown_subcommand_fails_with_a_usage_error(state_dir): + # Distinct from both CLEAR and TRIPPED so a typo in a workflow can never be + # silently read as "no outage". + result = run(state_dir, "frobnicate", "phoenix") + assert result.returncode == 2 + assert "usage" in (result.stdout + result.stderr).lower() + + +def test_a_non_numeric_ttl_falls_back_to_the_default_instead_of_erroring(state_dir): + # A typo in MFC_CI_OUTAGE_TTL_SECONDS must not make `check` exit with a code + # that is neither clear nor tripped -- that would gate CI on a config slip. + run(state_dir, "mark", "phoenix", "pypi unreachable") + assert run(state_dir, "check", "phoenix", ttl="not-a-number").returncode == TRIPPED + + +def test_an_empty_ttl_falls_back_to_the_default(state_dir): + run(state_dir, "mark", "phoenix", "pypi unreachable") + assert run(state_dir, "check", "phoenix", ttl="").returncode == TRIPPED diff --git a/toolchain/mfc/test_classify_build_failure.py b/toolchain/mfc/test_classify_build_failure.py new file mode 100644 index 0000000000..19bca303e6 --- /dev/null +++ b/toolchain/mfc/test_classify_build_failure.py @@ -0,0 +1,73 @@ +"""Tests for .github/scripts/classify-build-failure.sh. + +The classifier decides whether a failed build was a cluster-wide dependency +outage. Getting it wrong in either direction is costly: a missed outage means +every matrix job rediscovers it, and a false positive halts CI on an ordinary +compile error. +""" + +import os +import subprocess +from pathlib import Path + +import pytest + +SCRIPT = Path(__file__).resolve().parents[2] / ".github" / "scripts" / "classify-build-failure.sh" + +OUTAGE = 78 +NOT_AN_OUTAGE = 0 + + +@pytest.fixture +def classify(tmp_path): + def _run(log_text): + log = tmp_path / "build.log" + log.write_text(log_text) + return subprocess.run( + ["bash", str(SCRIPT), str(log), "frontier"], + capture_output=True, + text=True, + check=False, + env={**os.environ, "MFC_CI_STATE_DIR": str(tmp_path / "state")}, + ).returncode + + return _run + + +@pytest.mark.parametrize( + "line", + [ + " |-> Failed to fetch: `https://pypi.org/simple/hatch-vcs/`", # uv, backticked + "Failed to fetch: https://pypi.org/simple/hatch-vcs/", # plain, no wrapper + " Failed to fetch: https://pypi.org/simple/build/", # padded + "mfc: ERROR > (venv) Installation failed.", + "mfc: WARNING > (venv) uv install failed; clearing the uv cache", + ], +) +def test_dependency_outages_are_recognised_in_every_formatting_variant(classify, line): + assert classify(line + "\n") == OUTAGE + + +@pytest.mark.parametrize( + "line", + [ + "NVFORTRAN-S-0034-Syntax error at or near end of line", + "ftn-2116 ftn: INTERNAL", + "clang: error: ld.lld command failed with exit code 1", + "CMake Error: could not find HDF5", + "Failed to fetch: https://example.com/not-pypi", + ], +) +def test_ordinary_build_failures_are_not_outages(classify, line): + assert classify(line + "\n") == NOT_AN_OUTAGE + + +def test_an_absent_log_is_never_called_an_outage(tmp_path): + result = subprocess.run( + ["bash", str(SCRIPT), str(tmp_path / "nope.log"), "frontier"], + capture_output=True, + text=True, + check=False, + env={**os.environ, "MFC_CI_STATE_DIR": str(tmp_path / "state")}, + ) + assert result.returncode == NOT_AN_OUTAGE diff --git a/toolchain/mfc/test_diagnostic_rendering.py b/toolchain/mfc/test_diagnostic_rendering.py new file mode 100644 index 0000000000..c8007b0e93 --- /dev/null +++ b/toolchain/mfc/test_diagnostic_rendering.py @@ -0,0 +1,69 @@ +"""Captured diagnostics must survive being printed. + +The console prints through Rich with markup enabled (printer.py), so raw log +text is interpreted as markup. Compiler and MPI output is full of square +brackets -- absolute paths in diagnostics, `[host:pid]` rank prefixes -- and +Rich either raises MarkupError on them or silently eats them as tags. + +That turns diagnostic capture into the opposite of its purpose: a crash, or a +message with the identifying parts removed. It is worse inside an MFCException, +because main.py renders that message with markup from inside the handler, so the +failure surfaces as an unhandled traceback instead of the intended report. +""" + +from rich.console import Console + +from mfc.common import console_safe, get_program_output + +HOSTILE = [ + "ftn: error in [/lustre/orion/cfd154/scratch/x.f90] line 3", + "[node1:12345] MPI abort", + "nvlink error: undefined reference in [/gpfs/alpine/proj/m_riemann.o]", +] + + +def render(text): + """Render the way the CLI does: a Rich console with markup enabled.""" + console = Console(file=open("/dev/null", "w"), record=True, width=200) + console.print(text, soft_wrap=True) + return console.export_text() + + +def test_rich_really_does_mangle_raw_log_text(): + # Guards the premise: if Rich ever stops doing this, console_safe can go. + raised = False + try: + render(HOSTILE[0]) + except Exception: + raised = True + assert raised or "[/lustre/orion/cfd154/scratch/x.f90]" not in render(HOSTILE[0]) + + +def test_console_safe_text_renders_without_raising(): + for line in HOSTILE: + render(console_safe(line)) + + +def test_console_safe_preserves_bracketed_paths(): + out = render(console_safe(HOSTILE[0])) + assert "[/lustre/orion/cfd154/scratch/x.f90]" in out + + +def test_console_safe_preserves_mpi_rank_prefixes(): + # "[node1:12345]" is how you tell which rank died; Rich eats it as a tag. + assert "[node1:12345]" in render(console_safe(HOSTILE[1])) + + +def test_get_program_output_can_capture_stderr(): + # h5dump writes "unable to open file" to stderr, so without this the + # diagnostic added to the h5dump failure path is always "(no output)". + out, code = get_program_output(["bash", "-c", "echo to-stderr >&2; exit 3"], merge_stderr=True) + assert code == 3 + assert "to-stderr" in out + + +def test_get_program_output_still_ignores_stderr_by_default(): + # Other callers parse stdout and must not start seeing stderr mixed in. + out, _ = get_program_output(["bash", "-c", "echo out; echo err >&2"]) + assert "err" not in out + assert "out" in out diff --git a/toolchain/mfc/test_frontier_deps.py b/toolchain/mfc/test_frontier_deps.py new file mode 100644 index 0000000000..82695c7180 --- /dev/null +++ b/toolchain/mfc/test_frontier_deps.py @@ -0,0 +1,95 @@ +"""Tests for .github/workflows/frontier/build.sh. + +Frontier installs its Python dependencies on the login node, in the "Fetch +Dependencies" step, before any SLURM job exists. That is where the PyPI outage +of 2026-08-28 actually landed -- 17 jobs, ~33 minutes each, all learning the +same thing independently. Classifying only the in-allocation build would leave +the breaker blind to the case that motivated it. +""" + +import os +import shutil +import stat +import subprocess +from pathlib import Path + +import pytest + +REPO = Path(__file__).resolve().parents[2] + +PYPI_FAILURE = " |-> Failed to fetch: `https://pypi.org/simple/hatch-vcs/`\n" "mfc: ERROR > (venv) Installation failed.\n" + + +def _exe(path: Path, text: str): + path.write_text(text) + path.chmod(path.stat().st_mode | stat.S_IEXEC) + + +@pytest.fixture +def workspace(tmp_path): + shutil.copytree(REPO / ".github", tmp_path / ".github") + + def install_mfc(stdout="", rc=0): + # `. ./mfc.sh load` is sourced, so the stub must return rather than exit + # for that subcommand or it would terminate build.sh itself. + _exe( + tmp_path / "mfc.sh", + f'#!/bin/bash\nif [ "$1" = "load" ]; then return 0 2>/dev/null || exit 0; fi\n' f"printf '%s' {_q(stdout)}\nexit {rc}\n", + ) + + def run(): + env = { + **os.environ, + "MFC_CI_STATE_DIR": str(tmp_path / "state"), + "MFC_BUILD_RETRY_DELAY": "0", + } + return subprocess.run( + ["bash", ".github/workflows/frontier/build.sh", "gpu", "acc"], + capture_output=True, + text=True, + cwd=tmp_path, + env=env, + check=False, + timeout=120, + ) + + return tmp_path, install_mfc, run + + +def _q(s): + import shlex + + return shlex.quote(s) + + +def outage_recorded(tmp_path): + state = tmp_path / "state" + return state.exists() and any(state.glob("outage-*")) + + +def test_a_successful_dependency_fetch_records_nothing(workspace): + tmp_path, install_mfc, run = workspace + install_mfc() + assert run().returncode == 0 + assert not outage_recorded(tmp_path) + + +def test_a_pypi_outage_on_the_login_node_is_recorded(workspace): + tmp_path, install_mfc, run = workspace + install_mfc(stdout=PYPI_FAILURE, rc=1) + run() + assert outage_recorded(tmp_path) + + +def test_a_pypi_outage_on_the_login_node_reports_the_outage_exit_code(workspace): + tmp_path, install_mfc, run = workspace + install_mfc(stdout=PYPI_FAILURE, rc=1) + assert run().returncode == 78 + + +def test_an_ordinary_dependency_failure_is_not_an_outage(workspace): + tmp_path, install_mfc, run = workspace + install_mfc(stdout="CMake Error: could not find HDF5\n", rc=1) + result = run() + assert not outage_recorded(tmp_path) + assert result.returncode != 78 diff --git a/toolchain/mfc/test_monitor_exit_codes.py b/toolchain/mfc/test_monitor_exit_codes.py new file mode 100644 index 0000000000..2b87016e80 --- /dev/null +++ b/toolchain/mfc/test_monitor_exit_codes.py @@ -0,0 +1,93 @@ +"""The infrastructure exit codes must survive the trip back to the submit wrapper. + +preflight.sh exits 77 (node fault) or 78 (recorded outage) inside the SLURM job. +That becomes the job's ExitCode, which monitor_slurm_job.sh reads and +run_monitored_slurm_job.sh relays to the resubmit loop. If either layer flattens +them to 1 -- as both did for every non-zero code before -- the loop sees a +generic failure and the node is never excluded. +""" + +import os +import stat +import subprocess +from pathlib import Path + +import pytest + +SCRIPTS = Path(__file__).resolve().parents[2] / ".github" / "scripts" + + +def _exe(path: Path, text: str): + path.write_text(text) + path.chmod(path.stat().st_mode | stat.S_IEXEC) + + +@pytest.fixture +def slurm(tmp_path): + """Stub SLURM reporting a finished job whose ExitCode the test chooses.""" + binz = tmp_path / "bin" + binz.mkdir() + + def configure(exit_code, state="COMPLETED"): + _exe(binz / "squeue", "#!/bin/bash\nexit 0\n") + _exe(binz / "sacct", f'#!/bin/bash\nfor a in "$@"; do [ "$a" = "--format=ExitCode" ] && {{ echo "{exit_code}"; exit 0; }}; done\necho "{state}"\n') + _exe(binz / "scontrol", f'#!/bin/bash\necho "ExitCode={exit_code}"\n') + _exe(binz / "scancel", "#!/bin/bash\nexit 0\n") + out = tmp_path / "job.out" + out.write_text("job output\n") + return out + + return tmp_path, binz, configure + + +def run_script(tmp_path, binz, name, *args): + env = { + **os.environ, + "PATH": f"{binz}:{os.environ['PATH']}", + # This script really sleeps between polls; without these the file + # cost ~36s in every PR's Lint Gate. + "MFC_MONITOR_POLL_SECONDS": "0", + "MFC_MONITOR_RECHECK_SECONDS": "0", + } + return subprocess.run( + ["bash", str(SCRIPTS / name), *args], + capture_output=True, + text=True, + cwd=tmp_path, + env=env, + check=False, + timeout=180, + ) + + +@pytest.mark.parametrize("job_exit,expected", [("77:0", 77), ("78:0", 78)]) +def test_monitor_relays_the_infrastructure_exit_code(slurm, job_exit, expected): + tmp_path, binz, configure = slurm + out = configure(job_exit) + assert run_script(tmp_path, binz, "monitor_slurm_job.sh", "1234", str(out)).returncode == expected + + +def test_monitor_still_reports_an_ordinary_failure_as_one(slurm): + tmp_path, binz, configure = slurm + out = configure("2:0") + assert run_script(tmp_path, binz, "monitor_slurm_job.sh", "1234", str(out)).returncode == 1 + + +@pytest.mark.parametrize("monitor_exit", [77, 78]) +def test_the_runner_relays_the_infrastructure_exit_code(tmp_path, monitor_exit): + # Stub the inner monitor so this exercises only the relaying layer. + scripts = tmp_path / "scripts" + scripts.mkdir() + for name in ("run_monitored_slurm_job.sh", "monitor_slurm_job.sh"): + (scripts / name).write_text((SCRIPTS / name).read_text()) + _exe(scripts / "monitor_slurm_job.sh", f"#!/bin/bash\nexit {monitor_exit}\n") + + result = subprocess.run( + ["bash", str(scripts / "run_monitored_slurm_job.sh"), "1234", str(tmp_path / "job.out")], + capture_output=True, + text=True, + cwd=tmp_path, + check=False, + timeout=180, + ) + assert result.returncode == monitor_exit diff --git a/toolchain/mfc/test_node_exclude.py b/toolchain/mfc/test_node_exclude.py new file mode 100644 index 0000000000..fe5be6ce22 --- /dev/null +++ b/toolchain/mfc/test_node_exclude.py @@ -0,0 +1,93 @@ +"""Unit tests for .github/scripts/node-exclude.sh. + +When the in-allocation preflight finds the node's GPU unusable it records the +node name in the job's output file. The submit wrapper reads it back, adds it to +the sbatch --exclude list and resubmits, so the next attempt lands somewhere +else instead of failing the same way. + +This matters because bad nodes are concentrated, not spread: over 2026-08-18..31 +a single Phoenix node (atl1-1-03-007-29-0) accounted for 25 of 29 ECC failures +and two nodes for 32 of ~40. Both are currently in a hand-edited --exclude line +that a human had to notice, diagnose, and commit. +""" + +import os +import subprocess +from pathlib import Path + +import pytest + +SCRIPT = Path(__file__).resolve().parents[2] / ".github" / "scripts" / "node-exclude.sh" + + +def run(*args): + return subprocess.run( + ["bash", str(SCRIPT), *args], + capture_output=True, + text=True, + env={**os.environ}, + check=False, + ) + + +def out(*args): + result = run(*args) + assert result.returncode == 0, result.stderr + return result.stdout.strip() + + +@pytest.fixture +def job_output(tmp_path): + def _write(text): + path = tmp_path / "build-and-test-gpu-acc.out" + path.write_text(text) + return str(path) + + return _write + + +def test_node_from_reads_the_marker_the_preflight_wrote(job_output): + path = job_output("some build noise\nMFC_FAULT_NODE=atl1-1-03-007-29-0\nmore noise\n") + assert out("node-from", path) == "atl1-1-03-007-29-0" + + +def test_node_from_prints_nothing_when_the_job_recorded_no_fault(job_output): + assert out("node-from", job_output("ordinary output, no marker\n")) == "" + + +def test_node_from_prints_nothing_when_the_output_file_is_missing(tmp_path): + assert out("node-from", str(tmp_path / "absent.out")) == "" + + +def test_node_from_takes_the_last_marker_when_the_file_holds_several(job_output): + # submit-slurm-job.sh reuses one output path across resubmits, so a stale + # marker can precede the current one. + path = job_output("MFC_FAULT_NODE=oldnode\nMFC_FAULT_NODE=atl1-1-03-007-31-0\n") + assert out("node-from", path) == "atl1-1-03-007-31-0" + + +def test_merge_adds_the_first_node_to_an_empty_list(): + assert out("merge", "", "atl1-1-03-007-29-0") == "atl1-1-03-007-29-0" + + +def test_merge_appends_to_an_existing_list(): + assert out("merge", "nodeA", "nodeB") == "nodeA,nodeB" + + +def test_merge_does_not_repeat_a_node_already_excluded(): + assert out("merge", "nodeA,nodeB", "nodeA") == "nodeA,nodeB" + + +def test_merge_does_not_match_a_node_name_that_is_only_a_prefix(): + # "atl1-1-03-007-2" must not suppress excluding "atl1-1-03-007-29-0". + assert out("merge", "atl1-1-03-007-2", "atl1-1-03-007-29-0") == "atl1-1-03-007-2,atl1-1-03-007-29-0" + + +def test_merge_leaves_the_list_untouched_when_there_is_no_node_to_add(): + assert out("merge", "nodeA", "") == "nodeA" + + +def test_an_unknown_subcommand_fails_with_a_usage_error(): + result = run("frobnicate", "x") + assert result.returncode == 2 + assert "usage" in (result.stdout + result.stderr).lower() diff --git a/toolchain/mfc/test_prebuild_preflight.py b/toolchain/mfc/test_prebuild_preflight.py new file mode 100644 index 0000000000..4d1b7eb98c --- /dev/null +++ b/toolchain/mfc/test_prebuild_preflight.py @@ -0,0 +1,44 @@ +"""The case-optimization pre-build must NOT probe the node. + +Every other SLURM job MFC submits runs binaries built for the device it asked +for. This one does not: test.yml submits it as a *cpu* allocation, because it is +a --dry-run that only builds, while the binaries it produces are GPU builds. + +A syscheck built with --gpu asserts omp_get_num_devices() > 0 (or the OpenACC +equivalent) and so exits non-zero on a node that has no GPU by design. Probing +here reads that as a bad node. It did exactly that in CI: three healthy Phoenix +nodes were condemned and two added to --exclude before the wrapper gave up, +which is the precise false positive the whole guard exists to avoid. + +The GPU allocation that actually runs these cases is probed instead, in +run_case_optimization.sh, where the allocation and the binary agree. +""" + +from pathlib import Path + +import pytest + +SCRIPTS = Path(__file__).resolve().parents[2] / ".github" / "scripts" + +# Scripts that run binaries built for the device their allocation requested. +PROBES = ["run_case_optimization.sh"] +# Scripts whose allocation device deliberately differs from what they build. +DOES_NOT_PROBE = ["prebuild-case-optimization.sh"] + + +@pytest.mark.parametrize("script", DOES_NOT_PROBE) +def test_a_build_only_allocation_does_not_judge_its_node(script): + body = (SCRIPTS / script).read_text() + assert "preflight.sh" not in body, f"{script} runs on a cpu allocation but builds GPU binaries, so a probe " "there always fails and excludes a healthy node" + + +@pytest.mark.parametrize("script", PROBES) +def test_the_allocation_that_runs_the_cases_does_judge_its_node(script): + assert "preflight.sh" in (SCRIPTS / script).read_text() + + +def test_the_reason_is_recorded_where_someone_would_re_add_it(): + # The next person to notice case-opt's pre-build is unprobed should find the + # reason in the file rather than rediscovering it through a red CI run. + body = (SCRIPTS / "prebuild-case-optimization.sh").read_text() + assert "cpu" in body and "probe" in body.lower() diff --git a/toolchain/mfc/test_preflight.py b/toolchain/mfc/test_preflight.py new file mode 100644 index 0000000000..d376ca8380 --- /dev/null +++ b/toolchain/mfc/test_preflight.py @@ -0,0 +1,227 @@ +"""Unit tests for .github/scripts/preflight.sh. + +The preflight runs syscheck at the top of the allocation that will execute the +tests, before the expensive work starts. + +Placement is the whole point. Over 2026-08-18..31, 48 of 58 measurable jobs +built on one node and tested on another, and in the ECC failures the build node +was healthy every time while the tests landed on a bad one. A probe that runs +after the build validates the wrong machine. + +Cost of getting it wrong: the ECC jobs spent a median of 38 minutes between +syscheck being available and the GPU fault being noticed -- 28.5 hours in two +weeks. +""" + +import os +import stat +import subprocess +from pathlib import Path + +import pytest + +SCRIPTS = Path(__file__).resolve().parents[2] / ".github" / "scripts" +SCRIPT = SCRIPTS / "preflight.sh" + +HEALTHY = 0 +NODE_FAULT = 77 +OUTAGE = 78 + + +@pytest.fixture +def workspace(tmp_path): + """A bare workspace. No launcher is on PATH unless a test installs one. + + An earlier version of this fixture put a fake mpirun on PATH for every test, + which is exactly why it could not see that preflight ran mpirun on Frontier, + where the launcher is srun and Cray MPICH ships no mpirun at all. + """ + (tmp_path / "bin").mkdir() + (tmp_path / "state").mkdir() + + # A hermetic PATH holding only the utilities the scripts need. This box has + # a real mpirun *and* a real /usr/bin/srun, either of which would silently + # stand in for a launcher the test meant to be absent. + sysbin = tmp_path / "sysbin" + sysbin.mkdir() + for tool in ("bash", "find", "head", "tail", "cat", "sed", "grep", "tr", "cut", "date", "mkdir", "mv", "rm", "hostname", "env", "sort", "wc", "dirname", "basename"): + for root in ("/usr/bin", "/bin"): + src = Path(root) / tool + if src.exists(): + (sysbin / tool).symlink_to(src) + break + return tmp_path + + +def install_launcher(workspace, name): + """A passthrough launcher that records its argv, mirroring mpirun/srun.""" + launcher = workspace / "bin" / name + launcher.write_text("#!/bin/bash\n" f'echo "$@" >> {workspace}/launched.txt\n' 'while [ "${1:0:1}" = "-" ]; do shift; case "$1" in [0-9]*) shift;; esac; done\n' 'exec "$@"\n') + launcher.chmod(launcher.stat().st_mode | stat.S_IEXEC) + return launcher + + +def launched_with(workspace): + path = workspace / "launched.txt" + return path.read_text() if path.exists() else "" + + +def write_syscheck(workspace, exit_code, message="syscheck says hello"): + target = workspace / "build" / "install" / "gpu-acc-abc123" / "bin" + target.mkdir(parents=True, exist_ok=True) + binary = target / "syscheck" + binary.write_text(f'#!/bin/bash\necho "{message}"\nexit {exit_code}\n') + binary.chmod(binary.stat().st_mode | stat.S_IEXEC) + return binary + + +def run(workspace, *args, **overrides): + env = { + **os.environ, + # A controlled PATH, not the inherited one: this box has a real mpirun, + # and inheriting it makes "no launcher available" silently untestable. + "PATH": f"{workspace / 'bin'}:{workspace / 'sysbin'}", + "MFC_CI_STATE_DIR": str(workspace / "state"), + "SLURMD_NODENAME": "atl1-1-03-007-29-0", + # Present by default: these tests describe behaviour inside a job. + "SLURM_JOB_ID": "123456", + } + if overrides.pop("MFC_NO_SLURM", None): + env.pop("SLURM_JOB_ID", None) + env.update(overrides) + return subprocess.run( + ["bash", str(SCRIPT), *(args or ("phoenix", "gpu"))], + capture_output=True, + text=True, + cwd=workspace, + env=env, + check=False, + ) + + +def test_passes_when_syscheck_succeeds_on_this_node(workspace): + write_syscheck(workspace, 0) + assert run(workspace).returncode == HEALTHY + + +def test_reports_a_node_fault_when_syscheck_fails(workspace): + write_syscheck(workspace, 1) + assert run(workspace).returncode == NODE_FAULT + + +def test_names_the_faulted_node_so_the_wrapper_can_exclude_it(workspace): + write_syscheck(workspace, 1) + result = run(workspace) + assert "MFC_FAULT_NODE=atl1-1-03-007-29-0" in result.stdout + result.stderr + + +def test_shows_syscheck_output_when_it_fails(workspace): + # The previous post-build validation sent syscheck's output to /dev/null, + # which left the CI log with no evidence of why the node was rejected. + write_syscheck(workspace, 1, message="uncorrectable ECC error encountered") + result = run(workspace) + assert "uncorrectable ECC error encountered" in result.stdout + result.stderr + + +def test_passes_when_no_syscheck_binary_was_built(workspace): + # A missing binary is a build problem, not a bad node; failing here would + # requeue onto a healthy node and fail identically. + assert run(workspace).returncode == HEALTHY + + +def test_skips_when_the_cluster_is_already_known_to_be_down(workspace): + write_syscheck(workspace, 0) + subprocess.run( + ["bash", str(SCRIPTS / "ci-outage.sh"), "mark", "phoenix", "pypi unreachable"], + env={**os.environ, "MFC_CI_STATE_DIR": str(workspace / "state")}, + capture_output=True, + check=True, + ) + assert run(workspace).returncode == OUTAGE + + +def test_does_not_report_a_node_fault_merely_because_pmix_printed_a_warning(workspace): + # PMIX_ERR_NO_PERMISSIONS in dstore_base.c is benign noise: it appears in + # 16% of passing self-hosted jobs and only 9% of failing ones. Gating on it + # would fail roughly one healthy job in six. + write_syscheck(workspace, 0, message="PMIX ERROR: PMIX_ERR_NO_PERMISSIONS in file dstore_base.c at line 238") + assert run(workspace).returncode == HEALTHY + + +def test_uses_mpirun_on_phoenix(workspace): + install_launcher(workspace, "mpirun") + write_syscheck(workspace, 0) + assert run(workspace, "phoenix", "gpu").returncode == HEALTHY + assert "syscheck" in launched_with(workspace) + + +def test_uses_srun_on_frontier(workspace): + # Frontier and frontier_amd launch every binary with srun + # (toolchain/templates/frontier.mako, frontier_amd.mako); Cray MPICH ships no + # mpirun, so running one there is not merely wrong, it cannot work. + install_launcher(workspace, "srun") + write_syscheck(workspace, 0) + assert run(workspace, "frontier", "gpu").returncode == HEALTHY + assert "syscheck" in launched_with(workspace) + + +def test_a_missing_launcher_is_not_blamed_on_the_node(workspace): + # Without this, a launcher absent from PATH returns 127, preflight calls the + # node bad, and the requeue loop burns three allocations and blacklists three + # healthy nodes before declaring a cluster-wide problem. + write_syscheck(workspace, 0) + assert run(workspace, "frontier", "gpu").returncode == HEALTHY + + +def test_a_breaker_that_cannot_be_read_does_not_halt_the_job(workspace): + # Exit 1 from ci-outage.sh means "tripped"; any other failure means the check + # itself broke. Conflating them turns a bug in the breaker into a CI outage. + write_syscheck(workspace, 0) + (workspace / "state").chmod(0o000) + try: + assert run(workspace, "phoenix", "gpu").returncode == HEALTHY + finally: + (workspace / "state").chmod(0o755) + + +def test_it_refuses_to_judge_a_node_outside_a_slurm_allocation(workspace): + # mfc.sh load is used for building on login nodes too (bench.yml and + # frontier/build.sh both load the GPU module set there). A probe that ran in + # that context would find no usable GPU, call the login node bad, and have + # the wrapper exclude it and requeue. Only a real allocation can be judged. + write_syscheck(workspace, 1) + env_without_slurm = {"MFC_NO_SLURM": "1"} + assert run(workspace, "phoenix", "gpu", **env_without_slurm).returncode == HEALTHY + + +def test_a_gpu_binary_in_a_cpu_allocation_is_not_a_node_fault(workspace): + """The failure that condemned three healthy Phoenix nodes. + + The case-optimization pre-build is submitted as a cpu allocation but builds + GPU binaries, so the only syscheck available asserts a device exists and + exits non-zero on a node that has no GPU by design. That is a mismatch + between what the job asked for and what it built -- never evidence about the + node -- so the probe must decline to judge rather than blame it. + """ + install_launcher(workspace, "mpirun") + target = workspace / "build" / "install" / "gpu-mp-abc123" / "bin" + target.mkdir(parents=True) + probe = target / "syscheck" + probe.write_text("#!/bin/bash\necho 'num_devices == 0'\nexit 1\n") + probe.chmod(probe.stat().st_mode | stat.S_IEXEC) + + result = run(workspace, "phoenix", "cpu") + assert result.returncode == HEALTHY + assert "MFC_FAULT_NODE" not in result.stdout + result.stderr + + +def test_a_gpu_binary_in_a_gpu_allocation_is_still_judged(workspace): + # The guard above must not blunt the actual feature. + install_launcher(workspace, "mpirun") + target = workspace / "build" / "install" / "gpu-mp-abc123" / "bin" + target.mkdir(parents=True) + probe = target / "syscheck" + probe.write_text("#!/bin/bash\nexit 1\n") + probe.chmod(probe.stat().st_mode | stat.S_IEXEC) + + assert run(workspace, "phoenix", "gpu").returncode == NODE_FAULT diff --git a/toolchain/mfc/test_submit_requeue.py b/toolchain/mfc/test_submit_requeue.py new file mode 100644 index 0000000000..4fe0ceb633 --- /dev/null +++ b/toolchain/mfc/test_submit_requeue.py @@ -0,0 +1,169 @@ +"""Integration tests for the resubmit loop in .github/scripts/submit-slurm-job.sh. + +The loop already resubmits on preemption (exit 76). These tests cover the node +-fault path (exit 77): the wrapper must add the faulted node to sbatch's +--exclude and try again, so a persistently bad node stops eating jobs without +anyone hand-editing a constant and committing it. + +SLURM is stubbed. The scripts are copied into a temp tree so the monitor can be +replaced with one that returns a scripted sequence of exit codes; nothing here +waits on a scheduler. +""" + +import os +import shutil +import stat +import subprocess +from pathlib import Path + +import pytest + +SCRIPTS = Path(__file__).resolve().parents[2] / ".github" / "scripts" + + +def _exe(path: Path, text: str): + path.write_text(text) + path.chmod(path.stat().st_mode | stat.S_IEXEC) + + +@pytest.fixture +def rig(tmp_path): + """A workspace with stubbed SLURM commands and a scriptable monitor.""" + binz = tmp_path / "bin" + binz.mkdir() + scripts = tmp_path / "scripts" + shutil.copytree(SCRIPTS, scripts) + submissions = tmp_path / "submissions" + submissions.mkdir() + + # submit-slurm-job.sh inlines the payload script into the sbatch heredoc. + payload = tmp_path / ".github" / "workflows" / "common" + payload.mkdir(parents=True) + (payload / "build-and-test.sh").write_text("echo payload\n") + + # sbatch records each submitted script, then reports a fresh job id. + _exe( + binz / "sbatch", + f"""#!/bin/bash +n=$(ls {submissions} | wc -l) +cat > {submissions}/submission-$n.sh +echo "Submitted batch job 100$n" +""", + ) + for name in ("squeue", "scancel", "scontrol"): + _exe(binz / name, "#!/bin/bash\nexit 0\n") + _exe(binz / "sacct", "#!/bin/bash\necho COMPLETED\n") + _exe(binz / "sinfo", "#!/bin/bash\necho idle\n") + + # The monitor returns the next code from MONITOR_CODES on each call. + _exe( + scripts / "run_monitored_slurm_job.sh", + f"""#!/bin/bash +n=$(cat {tmp_path}/monitor_calls 2>/dev/null || echo 0) +echo $((n + 1)) > {tmp_path}/monitor_calls +code=$(echo "$MONITOR_CODES" | cut -d, -f$((n + 1))) +# Mimic the preflight's marker landing in the job output file. +if [ "$code" = "77" ]; then + echo "MFC_FAULT_NODE=badnode-$n" >> "$2" +fi +exit "${{code:-0}}" +""", + ) + + def run(monitor_codes, script="common/build-and-test.sh", **env_extra): + env = { + **os.environ, + "PATH": f"{binz}:{os.environ['PATH']}", + "MONITOR_CODES": monitor_codes, + "GITHUB_EVENT_NAME": "pull_request", + "MFC_CI_STATE_DIR": str(tmp_path / "state"), + **env_extra, + } + result = subprocess.run( + ["bash", str(scripts / "submit-slurm-job.sh"), f".github/workflows/{script}", "gpu", "acc", "phoenix"], + capture_output=True, + text=True, + cwd=tmp_path, + env=env, + check=False, + ) + result.submissions = sorted(submissions.glob("submission-*.sh")) + return result + + run.scripts = scripts + run.state_dir = tmp_path / "state" + return run + + +def excludes(submission: Path): + for line in submission.read_text().splitlines(): + if "--exclude=" in line: + return line.split("--exclude=", 1)[1].strip().strip('"') + return "" + + +def test_a_healthy_job_is_submitted_exactly_once(rig): + result = rig("0") + assert result.returncode == 0, result.stdout + result.stderr + assert len(result.submissions) == 1 + + +def test_a_node_fault_causes_a_resubmission(rig): + result = rig("77,0") + assert result.returncode == 0, result.stdout + result.stderr + assert len(result.submissions) == 2 + + +def test_the_resubmission_excludes_the_faulted_node(rig): + result = rig("77,0") + assert "badnode-0" in excludes(result.submissions[1]) + + +def test_the_resubmission_keeps_the_nodes_already_excluded(rig): + result = rig("77,0") + first = excludes(result.submissions[0]) + assert first, "expected the baseline --exclude list to be non-empty" + for node in first.split(","): + assert node in excludes(result.submissions[1]) + + +def test_node_faults_stop_after_the_bounded_number_of_resubmits(rig): + result = rig("77,77,77,77,77", MFC_MAX_NODE_RESUBMITS="2") + assert result.returncode != 0 + assert len(result.submissions) == 3 # original + 2 resubmits + + +def test_a_known_outage_is_not_resubmitted(rig): + # Requeuing cannot fix pypi.org being unreachable; trying again just burns + # another allocation. + result = rig("78") + assert len(result.submissions) == 1 + + +def test_no_job_is_submitted_while_the_cluster_is_under_a_recorded_outage(rig): + # ci-outage.sh's whole promise is that later jobs "exit immediately instead + # of submitting a SLURM job that is going to fail". Checking it only inside + # the allocation means every job still pays the queue wait first -- hours on + # Phoenix embers -- before discovering the marker. + subprocess.run( + ["bash", str(rig.scripts / "ci-outage.sh"), "mark", "phoenix", "pypi unreachable"], + env={**os.environ, "MFC_CI_STATE_DIR": str(rig.state_dir)}, + capture_output=True, + check=True, + ) + result = rig("0") + assert len(result.submissions) == 0 + assert result.returncode == 78 + + +def test_the_default_bound_is_one_requeue(rig): + """One requeue, not two. + + A wrong probe costs one node per attempt: the run that condemned three + healthy Phoenix nodes was a bounded loop doing exactly what it was told. + Bad nodes are concentrated -- one accounted for 25 of 29 ECC failures -- so a + single requeue captures nearly all the benefit at half the blast radius. + """ + result = rig("77,77,77,77") + assert result.returncode != 0 + assert len(result.submissions) == 2 # original + 1 requeue diff --git a/toolchain/mfc/test_syscheck_source.py b/toolchain/mfc/test_syscheck_source.py new file mode 100644 index 0000000000..7e8a0e9488 --- /dev/null +++ b/toolchain/mfc/test_syscheck_source.py @@ -0,0 +1,91 @@ +"""Unit tests for src/syscheck/syscheck.fpp. + +syscheck is CI's proof that a compute node can actually run MFC: it is the first +binary launched inside a SLURM allocation, and its exit code is what tells the +CI wrapper whether the node is healthy or should be excluded and requeued. + +That only works if syscheck genuinely exercises the device. A path that merely +queries the runtime ("how many GPUs are there?") passes on a node whose GPU is +dead, which is exactly what happened on Phoenix in Aug 2026: on the OpenACC +build syscheck died at cuCtxCreate with CUDA_ERROR_ECC_UNCORRECTABLE, while on +the OpenMP build it reported PASSED up to 111 times on the same broken nodes +before the solver fell over. + +These tests pin the offload down so the OpenMP path cannot silently regress to +a host-side query again. +""" + +import re +from pathlib import Path + +import fypp + +SYSCHECK_FPP = Path(__file__).resolve().parents[2] / "src" / "syscheck" / "syscheck.fpp" + + +def _expand_fypp() -> str: + return fypp.Fypp(fypp.FyppOptions()).process_text(SYSCHECK_FPP.read_text()) + + +def _preprocess(text: str, defines: set) -> str: + """Resolve the #ifdef/#else/#endif nesting the way a Fortran preprocessor would. + + syscheck.fpp only uses plain #ifdef, so a small evaluator is enough and keeps + the test free of a dependency on an external cpp. + """ + out, stack = [], [] + for line in text.splitlines(): + stripped = line.strip() + if stripped.startswith("#ifdef "): + stack.append(stripped.split(None, 1)[1].strip() in defines) + elif stripped == "#else": + stack[-1] = not stack[-1] + elif stripped == "#endif": + stack.pop() + elif all(stack): + out.append(line) + assert not stack, "unbalanced #ifdef in syscheck.fpp" + return "\n".join(out) + + +def _variant(*defines: str) -> str: + """The source as the compiler sees it for one GPU interface.""" + return _preprocess(_expand_fypp(), {"MFC_MPI", *defines}) + + +def _omp() -> str: + return _variant("MFC_OpenMP") + + +def _acc() -> str: + return _variant("MFC_OpenACC") + + +def test_openmp_path_launches_a_kernel_on_the_device(): + # A host-side omp_get_num_devices() query cannot fail on a node whose GPU is + # unusable; only an actual target region creates a context and runs code there. + assert re.search(r"!\$omp\s+target\s+teams\s+distribute\s+parallel\s+do", _omp()) + + +def test_openmp_path_copies_the_result_back_from_the_device(): + assert re.search(r"!\$omp\s+target\s+update\s+from\s*\(\s*arr", _omp()) + + +def test_openmp_path_checks_the_values_returned_from_the_device(): + # Copying back without inspecting the values would still pass on a device + # that returns garbage. + assert re.search(r"call\s+assert\s*\(.*\barr\b", _omp()) + + +def test_openacc_path_checks_the_values_returned_from_the_device(): + assert re.search(r"call\s+assert\s*\(.*\barr\b", _acc()) + + +def test_openmp_device_index_is_reduced_modulo_the_device_count(): + # mod(rank, nRanks) is always < nRanks and ignores how many GPUs exist, so a + # 4-rank job on a 2-GPU node selects devices 2 and 3, which are not there. + assert re.search(r"omp_set_default_device\s*\(\s*mod\s*\(\s*rank\s*,\s*num_devices", _omp()) + + +def test_openacc_device_index_is_reduced_modulo_the_device_count(): + assert re.search(r"acc_set_device_num\s*\(\s*mod\s*\(\s*rank\s*,\s*num_devices", _acc()) diff --git a/toolchain/mfc/test_test_sh_preflight.py b/toolchain/mfc/test_test_sh_preflight.py new file mode 100644 index 0000000000..e27d223bba --- /dev/null +++ b/toolchain/mfc/test_test_sh_preflight.py @@ -0,0 +1,87 @@ +"""The test allocation must probe its own node before running the suite. + +On Frontier the Build and Test steps are two separate SLURM submissions with no +node affinity, and they landed on different nodes in 26 of 29 measurable +post-#1763 jobs. In the Aug 2026 ECC failures the build node was healthy every +time while the tests landed on a bad one, so a probe that only runs during the +build proves nothing about the machine that runs the tests. +""" + +import os +import shutil +import stat +import subprocess +from pathlib import Path + +import pytest + +REPO = Path(__file__).resolve().parents[2] + + +def _exe(path: Path, text: str): + path.write_text(text) + path.chmod(path.stat().st_mode | stat.S_IEXEC) + + +@pytest.fixture +def workspace(tmp_path): + shutil.copytree(REPO / ".github", tmp_path / ".github") + binz = tmp_path / "bin" + binz.mkdir() + passthrough = '#!/bin/bash\nwhile [ "${1:0:1}" = "-" ]; do shift; case "$1" in [0-9]*) shift;; esac; done\nexec "$@"\n' + _exe(binz / "mpirun", passthrough) + # job_cluster here is frontier, whose launcher is srun. Without a stub the + # real /usr/bin/srun on this box would try to submit an actual job. + _exe(binz / "srun", passthrough) + _exe(binz / "nvidia-smi", "#!/bin/bash\necho 'GPU 0: fake'\n") + trace = tmp_path / "trace.log" + _exe(tmp_path / "mfc.sh", f'#!/bin/bash\necho "mfc.sh $*" >> {trace}\nexit 0\n') + + def install_probe(exit_code): + target = tmp_path / "build" / "install" / "gpu" / "bin" + target.mkdir(parents=True, exist_ok=True) + _exe(target / "syscheck", f'#!/bin/bash\necho "probe ran"\nexit {exit_code}\n') + + def run(): + env = { + **os.environ, + "PATH": f"{binz}:{os.environ['PATH']}", + "MFC_CI_STATE_DIR": str(tmp_path / "state"), + "job_device": "gpu", + "job_interface": "acc", + "job_shard": "", + "job_cluster": "frontier", + "job_variant": "", + "GITHUB_EVENT_NAME": "push", + "SLURMD_NODENAME": "frontier9999", + # These scripts only ever run inside a SLURM allocation + # (submit-slurm-job.sh is their sole caller), and the probe + # refuses to judge a node outside one. + "SLURM_JOB_ID": "123456", + } + return subprocess.run( + ["bash", ".github/workflows/common/test.sh"], + capture_output=True, + text=True, + cwd=tmp_path, + env=env, + check=False, + timeout=120, + ) + + return install_probe, run, trace + + +def test_a_healthy_node_runs_the_suite(workspace): + install_probe, run, trace = workspace + install_probe(0) + assert run().returncode == 0 + assert "test" in trace.read_text() + + +def test_a_bad_node_is_reported_before_the_suite_starts(workspace): + install_probe, run, trace = workspace + install_probe(1) + result = run() + assert result.returncode == 77 + assert not trace.exists() or "test" not in trace.read_text()