ci(docker): PR-side image build and run the GPU matrix inside the freshly built image - #1791
Merged
Conversation
TE 2.17 dlopens libcuda.so.1 at import time, which does not exist in the GPU-less build container; find_spec returns the package path without executing the module.
Zhichenzzz
force-pushed
the
zhichen/te-wheel-version-gate
branch
from
July 24, 2026 21:40
736ddb2 to
c44a3e9
Compare
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
Zhichenzzz
requested review from
guapisolo,
yueming-yuan and
yushengsu-thu
as code owners
July 24, 2026 21:48
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
The build job pushes a pr-<num> tag (same-repo PRs), and a chained _run-ci.yml call runs the stage-b 2-GPU suite in that image, so a Dockerfile change is validated by tests rather than only by the build finishing. Fork PRs still get the no-push build.
Zhichenzzz
changed the title
Collaborator
Collaborator
|
Looks like the docker re-built is no exec before gpu job run. |
Collaborator
|
Could we modify |
PRs touching the image inputs now build and push a pr-<num> tag first; resolve-ci-image redirects every suite into that image, so GPU results prove the new image rather than the stale dev one. The standalone docker-build-pr workflow is superseded; the tag is deleted when the PR closes so re-runs keep working while it is open.
cpu_runner stages run on bare ubuntu-latest and never use the container image; needing resolve-ci-image only delayed their fast feedback behind the image build on docker PRs.
TE 2.17 renamed custom_recipes.quantization_nvfp4 to quantization_ref_nvfp4; import the new path with a fallback for pre-2.17 environments.
Zhichenzzz
requested review from
Shi-Dong,
fzyzcjy and
maocheng23
as code owners
July 24, 2026 23:17
Zhichenzzz
changed the title
Zhichenzzz
changed the title
docker-paths matches changed files by prefix, so an edit to docker/Dockerfile.rocm also queued the PR-side cu13-x86 build — an image that never consumes that file. Filter it out of the changed-file list before the path regex so rocm-only PRs keep running on dev. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
resolve-ci-image assigns pr-<num> before it reads the PR body, so on a docker PR a successful build wins and the ci-image-tag: directive only applies when no PR image was built (non-docker or fork PRs). The doc claimed the opposite precedence. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Zhichenzzz
added a commit
that referenced
this pull request
Jul 26, 2026
#1791 made resolve-ci-image depend on docker-build so that a docker PR runs its suites inside the freshly built image. docker-build only runs when docker files change, so on every other PR it is skipped -- and GitHub propagates a skip down the entire needs closure, including through resolve-ci-image's always() guard. stage-a-cpu carries no if: at all, so it was skipped, and stage-b/stage-c gate on stage-a-cpu succeeding, so the whole matrix went with it. The run still reports success, because a skipped job is not a failed one. Since #1791 merged, PRs 1792/1793/1794 each completed "green" with zero test jobs expanded; the last run that actually executed anything was #1790 at 07-24T21:17Z, before the merge. #1791's own CI was green and did run, because it changed docker/Dockerfile -- the regression could not appear in the PR that introduced it. Fix keeps the new behavior and restores the old graph: docker-paths and docker-build now always reach a conclusion instead of skipping, so nothing downstream is poisoned. With nothing to build, docker-build no-ops on ubuntu-latest rather than occupying a GPU runner, and reports built=false; resolve-ci-image reads that output instead of the job result. All seven stage-* jobs are byte-identical to their pre-#1791 definitions. docker-paths also loses its `if: github.event_name == 'pull_request'`, which had the same effect on the nightly cron: no PR context meant a skipped job and a silently empty scheduled run. It now reports changed=false for non-PR events.
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
3 participants
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.
Problem
build-and-pushonly runs after merge, so Dockerfile breakage lands onmainfirst — and the tests that do run on a docker PR execute in the olddevimage, proving nothing about the image the PR produces.Change
PR-side docker CI, integrated into
pr-test.yml. When a PR touchesdocker/Dockerfile,docker/build.py,docker/patch/**, orrequirements.txt:docker-pathsdetects the change;docker-buildbuildscu13-x86and pushes a PR-scopedradixark/miles:pr-<num>tag (same-repo PRs; forks skip and stay ondev).resolve-ci-imagewaits for the build and redirects every GPU suite into the freshly built image; a failed build stops the matrix instead of green-lighting the stale image. An explicitci-image-tag:PR-body directive still wins.docker-pr-tag-cleanup.ymldeletes the tag when the PR closes; while the PR is open the tag stays available for re-runs.Carried fixes, all surfaced by this pipeline validating itself:
importlib.util.find_spec— TE 2.17 dlopenslibcuda.so.1at import, which doesn't exist in the GPU-less build container.quantization_ref_nvfp4(TE 2.17 module rename); caught by the GPU matrix running inside the new image, where the old path no longer exists.docs/ci/02-docker-build.mddocuments the new flow.Verification
Self-validating on this PR (head
6bc79c4):docker-build23:25→23:35,pr-1791tag pushed 23:35:06,resolve-ci-image23:35:34, GPU suites from 23:40 — strictly ordered after the fresh image. Full matrix (stage-b 2-GPU, stage-c 2/4/8-GPU on H100+H200) green insideradixark/miles:pr-1791.