Repository navigation
Conversation
The barrier helper was renamed to _run_startup_barrier_until_stop and now wraps both PD checks, and EngineDeath derives exit_status from observed/returncode. The test still imported the old name, so the whole file failed to collect and the engine tier went red on every run that reached a GPU node. Signed-off-by: LI MOU <lxglbk@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Runner listeners outlive scheduler config changes: the 005 runners still held the old SPUR_CONTROLLER_ADDR, so every srun/sbatch was refused with an auth error that the dispatcher retried as transient. Each GPU job now re-reads the host's SPUR_* settings (.github/scripts/refresh_spur_env.sh), and an auth refusal stops the tier at once with the cause named. The account/QoS rung is now shared by every submission in a run. Re-walking the always-full top rungs for each re-picked disagg pair spent two of the five submissions per pair, so one slow burst hold was enough to give up. run_tests.sh keeps its CLI and messages; the logic moves to tests/lib (env, slurm, disagg, tiers, and an in-container pytest helper), and a dispatched tier now carries the remote leg's failure lines into the local summary. Signed-off-by: LI MOU <lxglbk@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
limou102
requested review from
JohnQinAMD,
jiejingzhangamd and
xiaobochen-amd
as code owners
October 8, 2026 03:36
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The scheduler refresh script can expose host-provided credential values in GitHub Actions logs.
1 open finding
What changed in this PR
Improves GPU CI reliability and modularizes the test harness.
Changes:
- Refreshes scheduler configuration and improves SLURM retry handling.
- Splits
run_tests.shinto focused library scripts. - Repairs stale SGLang tests and adds scheduler regressions.
| File | Description |
|---|---|
.github/workflows/ci.yml |
Refreshes scheduler variables before GPU jobs. |
.github/scripts/refresh_spur_env.sh |
Loads current host scheduler configuration. |
tests/run_tests.sh |
Becomes the modular harness entry point. |
tests/lib/env.sh |
Provides environment and scratch setup. |
tests/lib/slurm.sh |
Implements scheduling and retry behavior. |
tests/lib/disagg.sh |
Implements disaggregated test orchestration. |
tests/lib/tiers.sh |
Defines unit, engine, and mixed tiers. |
tests/lib/container_pytest.sh |
Runs pytest inside containers. |
tests/engine/sglang/test_wait_for_decode_args.py |
Updates tests for renamed startup APIs. |
tests/unit/e2e_harness/test_slurm_retry_limit.py |
Adds scheduler retry regressions. |
tests/unit/e2e_harness/test_site_profiles.py |
Scans modular harness readers. |
tests/unit/e2e_harness/test_arch_overlay.py |
Validates image references across libraries. |
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
| for f in /etc/environment /etc/profile.d/spur.sh; do | ||
| [ -r "$f" ] || continue | ||
| sed -nE 's/^[[:space:]]*(export[[:space:]]+)?(SPUR_[A-Z_]+)=["'\'']?([^"'\'']*)["'\'']?[[:space:]]*$/\2=\3/p' "$f" | ||
| done | awk -F= '!seen[$1]++' | tee -a "${GITHUB_ENV:?not running under GitHub Actions}" |
Signed-off-by: LI MOU <lxglbk@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…-run-tests-refactor
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why the engine / e2e tiers kept failing
Runner.Listeners already sit near 290 MB, so a freshRunner.Workeris throttled past the listener's 30 s handshake, gets killed (exit 137), and GitHub reports "The self-hosted runner lost communication with the server" 10 minutes later. Every GPU job that landed there failed this way. Not fixable in this repo: those four runners stay in thecrusoepool, and jobs landing on them will keep failing this way until the cluster admins restore the hosts.SPUR_CONTROLLER_ADDR(three IPs). After the controller moved, everysrun/sbatchfailed withThe request does not have valid authentication credentials ... refusing to re-forward an already-forwarded request, which the dispatcher retried as a transient error until its five submissions were spent.amd-primus-cicd-qosandamd-primus-qosanswerQOSGrpNodeLimitin every recent run, but each re-picked pair started the ladder from rung 0 again, spending two of the five submissions per pair.tests/engine/sglang/test_wait_for_decode_args.pyimported_wait_for_decode_until_stop(renamed to_run_startup_barrier_until_stopin de69b2b) and builtEngineDeath(exit_status=...), so the file failed to collect.Changes
.github/scripts/refresh_spur_env.sh+ a step inengine,e2e-mixedande2e-disag: export the host's currentSPUR_*into$GITHUB_ENV, so both the tests and the reclaim step talk to the live controller.tests/run_tests.shis split intotests/lib/{env,slurm,disagg,tiers}.shpluscontainer_pytest.sh(the in-container pytest that used to be inlined as quoted strings). CLI and env knobs are unchanged, and log lines are unchanged except the shortened "submitted to SLURM" banner and the unwritable-TMPDIR hint; comments are trimmed to at most two lines each. A dispatched tier now copies the remote leg's failure lines into the local summary instead of printing "no per-test detail was captured".Testing
tests/unit/e2e_harnessmocked-scheduler tests (test_slurm_retry_limit,test_site_profiles,test_arch_overlay): all 69 pass on the refactored script.tests/run_tests.sh enginedispatched, built both images, and passed (vllm all files; sglang after the test fix). On this PR's CI, every GPU job that ran on a 005 runner passed: engine, e2e-mixed (sglang), e2e-disag (vllm). The ones on 004 died with "runner lost communication".e2e vllm mixedandenginewere both accepted by SLURM (worker logs in node-local /tmp, output streamed back) and then cancelled; the INT/TERM trap scancelled the job each time.