Skip to content

fix(ci): reserve GPUs for SPUR hardware tests - #137

Merged
amd-ivaganev merged 1 commit into
mainfrom
fix/spur-tiny-test-gpu-reservation
Aug 14, 2026
Merged

amd-ivaganev merged 1 commit into
mainfrom
fix/spur-tiny-test-gpu-reservation

Conversation

@amd-ivaganev

@amd-ivaganev amd-ivaganev commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • reserve one Slurm GPU for smoke and tiny-test jobs on every cluster, including SPUR, through configurable AIC_TEST_GRES (default: gpu:1)
  • propagate the Slurm-selected visible device into Docker and Compose instead of hardcoding GPU 0
  • derive the vLLM container name from the selected device
  • fail immediately and print vLLM logs when the container exits during readiness polling

Root cause

The SPUR path omitted --gres=gpu:1, so Slurm could co-locate hardware CI with an existing GPU workload. In the failing run, vLLM found only 16.22 GiB free on a 287.98 GiB device, below the 86.4 GiB requested by --gpu-memory-utilization 0.30. Engine initialization failed, but tiny-test continued polling the unavailable endpoint for the full ten-minute readiness timeout.

Scope

This PR intentionally changes only .slurm/run-build-distribute.sh. The self-hosted runner script refactor, run-attempt worktree isolation, and orphan cancellation handling are split into #138.

Validation

  • bash -n .slurm/run-build-distribute.sh
  • git diff --check
  • no-submit harness generated and syntax-checked both smoke and tiny-test Slurm scripts
  • harness verified that both jobs submit with --gres=gpu:1
  • live SPUR probe verified that a one-GPU GRES allocation sets ROCR_VISIBLE_DEVICES=0 and exposes one gfx950 device

Request AIC_TEST_GRES (gpu:1 by default) for smoke and tiny-test Slurm jobs on every cluster, including SPUR, so hardware CI is scheduled with GPU ownership.

Propagate Slurm’s selected visible device into Docker and Compose instead of hardcoding GPU 0, and derive the vLLM container name from that device.

Detect a stopped vLLM container during readiness polling and print its logs immediately instead of waiting for the full endpoint timeout.
@amd-ivaganev

Copy link
Copy Markdown
Collaborator Author

/run-ci-fast

@github-actions

Copy link
Copy Markdown

❌ Tiny-test (fast) failed for 7691223. View logs.

@amd-ivaganev
amd-ivaganev force-pushed the fix/spur-tiny-test-gpu-reservation branch from b018ff0 to 2c72e08 Compare August 13, 2026 18:03
@amd-ivaganev

Copy link
Copy Markdown
Collaborator Author

/run-ci-fast

@github-actions

Copy link
Copy Markdown

✅ Fast hardware CI passed for ca2b75c (dist-build-fast + smoke-test-fast + tiny-test). This is a single-arch dev check — run /run-ci for the full gate.

@amd-ivaganev

Copy link
Copy Markdown
Collaborator Author

@sbates130272 please take a look -- this fixes a long-standing problem where tiny-test jobs were launched without gpu constraints leading to failures and possibly interfering with others' workloads that they collided with. Also fails faster and cleans up orphaned jobs.

I believe that once this merges the other PR #136 will merge cleanly, too.

@sbates130272

Copy link
Copy Markdown
Collaborator

@amd-ivaganev SPUR was recently updated and only just got suppport for --gres. This fixes the issue thanks!

@sbates130272 sbates130272 added the bug Something isn't working label Aug 13, 2026
@sbates130272 sbates130272 self-assigned this Aug 13, 2026

@sbates130272 sbates130272 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amd-ivaganev there are two differnt things here. One is the workflow fix for tiny-test but the rest is a refactor of the .github/scripts/* which are installed on the github runner and which are a seperate topic. Can you fix this PR to only address the tiny-test workflow and then create another PR for the refactor and optomization of the runner scripts?

@amd-ivaganev

Copy link
Copy Markdown
Collaborator Author

/run-ci-fast

@amd-ivaganev

Copy link
Copy Markdown
Collaborator Author

@amd-ivaganev there are two differnt things here. One is the workflow fix for tiny-test but the rest is a refactor of the .github/scripts/* which are installed on the github runner and which are a seperate topic. Can you fix this PR to only address the tiny-test workflow and then create another PR for the refactor and optomization of the runner scripts?

Ah, right... The reason for the refactor was this:

[tiny-test] OK: got a non-empty completion via LMCacheMPConnector
[build-distribute] exit code from file: 0 (/home/johtyler/Projects/rocm-aic.7691223/logs/9650/tiny-test.exit)
[build-distribute] tiny-test complete
make: Leaving directory '/home/johtyler/Projects/rocm-aic.7691223'
=== tiny-test-fast complete ===
=== Cleaning up ===
rm: cannot remove '/home/johtyler/Projects/rocm-aic.7691223/.slurm': Directory not empty
Error: Process completed with exit code 1.

Re-triggering keeps the same names and since it's done by me and not John the cleanup fails and propagates as tiny-test failure, even though the test itself succeeded...

@github-actions

Copy link
Copy Markdown

✅ Fast hardware CI passed for 7691223 (dist-build-fast + smoke-test-fast + tiny-test). This is a single-arch dev check — run /run-ci for the full gate.

@john00003

Copy link
Copy Markdown
Collaborator

@amd-ivaganev there are two differnt things here. One is the workflow fix for tiny-test but the rest is a refactor of the .github/scripts/* which are installed on the github runner and which are a seperate topic. Can you fix this PR to only address the tiny-test workflow and then create another PR for the refactor and optomization of the runner scripts?

Ah, right... The reason for the refactor was this:

[tiny-test] OK: got a non-empty completion via LMCacheMPConnector
[build-distribute] exit code from file: 0 (/home/johtyler/Projects/rocm-aic.7691223/logs/9650/tiny-test.exit)
[build-distribute] tiny-test complete
make: Leaving directory '/home/johtyler/Projects/rocm-aic.7691223'
=== tiny-test-fast complete ===
=== Cleaning up ===
rm: cannot remove '/home/johtyler/Projects/rocm-aic.7691223/.slurm': Directory not empty
Error: Process completed with exit code 1.

Re-triggering keeps the same names and since it's done by me and not John the cleanup fails and propagates as tiny-test failure, even though the test itself succeeded...

Do you need me to update the scripts on my runner? Does #138 cover any $USER changes that you need to be made?

@john00003

Copy link
Copy Markdown
Collaborator

@amd-ivaganev I am working on a follow-up PR that addresses similar issues to this PR. We need to pass the devices to Docker in a special way, and a couple of other items. If this PR is still open by the time I am done, I will make my PR against your branch.

@amd-ivaganev

Copy link
Copy Markdown
Collaborator Author

@amd-ivaganev I am working on a follow-up PR that addresses similar issues to this PR. We need to pass the devices to Docker in a special way, and a couple of other items. If this PR is still open by the time I am done, I will make my PR against your branch.

Feel free to drop this PR and replace with your larger changes if it makes it easier. It was a band-aid for the troubles with clearing PR #136 where the superficial errors were blocking the merge of clean code.

@john00003

john00003 commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

@amd-ivaganev sure, if it would be easiest, I can open a PR that includes your changes here as well as mine.

But could be good to get any band-aids in to unblock CI.

@amd-ivaganev

Copy link
Copy Markdown
Collaborator Author

@amd-ivaganev sure, if it would be easiest, I can open a PR that includes your changes here as well as mine.

But could be good to get any band-aids in to unblock CI.

Do I read that as "let's merge this now"?

@john00003

Copy link
Copy Markdown
Collaborator

@amd-ivaganev Yes, I think it would be good to merge this now.

@amd-ivaganev
amd-ivaganev merged commit 622b66d into main Aug 14, 2026
15 checks passed
@amd-ivaganev
amd-ivaganev deleted the fix/spur-tiny-test-gpu-reservation branch August 14, 2026 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants