Automate Puzzletron GPU lifecycle tests - #2167
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (3)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughThe PR adds a pinned CUDA-based Puzzletron CI image, validates its image and environment contracts, updates Nox GPU verification, and adds a gated GitHub Actions workflow for GPU lifecycle tests. ChangesPuzzletron GPU CI
Estimated code review effort: 3 (Moderate) | ~25 minutes Mergeability Score: 🔵 Low · up to The new GPU test image currently runs its test process as root, leaving a bounded container-isolation risk that should remain explicitly owned and followed up; this is mergeable without blocking the PR. Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant PRGate
participant ImageResolver
participant GPUContainer
participant RequiredCheck
PullRequest->>PRGate: evaluate changed-file scope
PRGate->>ImageResolver: resolve pinned CI image
ImageResolver->>GPUContainer: provide image and digest cache key
GPUContainer->>RequiredCheck: report GPU test result
PRGate->>RequiredCheck: report gate result
ImageResolver->>RequiredCheck: report image result
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/puzzletron_gpu_tests.yml:
- Around line 19-23: Remove secrets: inherit from the pr-gate reusable workflow
invocation in puzzletron_gpu_tests.yml. Preserve the existing permissions and
uses configuration; if pr-gate requires a secret, pass only the specific named
secret through an explicit secrets mapping.
In `@examples/puzzletron/ci/Dockerfile`:
- Around line 20-31: Create a non-root runtime user in the Dockerfile, assign
ownership of /venv and /workspace/modelopt to that user, and add the
corresponding USER instruction before WORKDIR so GPU jobs execute checked-out
code without root privileges.
In `@examples/puzzletron/ci/resolve_ci_image.py`:
- Around line 18-22: Add the module-level __all__ declaration immediately after
the imports in resolve_ci_image.py, listing resolve_image_reference and
validate_repository_contract so both functions are explicitly exposed as the
resolver module’s public API.
In `@tests/unit/torch/puzzletron/test_ci_image_contract.py`:
- Line 87: Replace yaml.load with yaml.safe_load in the workflow parsing setup,
then update the related scalar assertions to expect PyYAML’s native values,
including True for “on” and integer 50 for “timeout-minutes.”
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ab0acb2b-83ac-4994-9a09-dc95f1ef40f2
📒 Files selected for processing (8)
.github/workflows/puzzletron_gpu_tests.ymlexamples/puzzletron/ci/Dockerfileexamples/puzzletron/ci/README.mdexamples/puzzletron/ci/resolve_ci_image.pyexamples/puzzletron/ci/setup_env.shexamples/puzzletron/ci_environment.jsonnoxfile.pytests/unit/torch/puzzletron/test_ci_image_contract.py
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## jrausch/puzzletron-gpu-quality-baseline-v4 #2167 +/- ##
==============================================================================
+ Coverage 62.09% 63.04% +0.94%
==============================================================================
Files 709 709
Lines 91929 91929
==============================================================================
+ Hits 57083 57954 +871
+ Misses 34846 33975 -871
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
c2b81f3 to
1b5940f
Compare
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/puzzletron_gpu_tests.yml:
- Around line 69-71: Remove the privileged NGC_API_KEY use from the
push-triggered GPU workflow around the credentials block. Move the GPU job and
secret access into a maintainer-controlled workflow revision, and require an
explicit trusted workflow_dispatch or maintainer approval before executing
copied PR content.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 402d3d89-0192-4214-a74a-3b6b28b3c9a9
📒 Files selected for processing (8)
.github/workflows/puzzletron_gpu_tests.ymlexamples/puzzletron/ci/Dockerfileexamples/puzzletron/ci/README.mdexamples/puzzletron/ci/resolve_ci_image.pyexamples/puzzletron/ci/setup_env.shexamples/puzzletron/ci_environment.jsonnoxfile.pytests/unit/torch/puzzletron/test_ci_image_contract.py
1b5940f to
3274cb6
Compare
3274cb6 to
278e060
Compare
|
@coderabbitai full review |
✅ Action performedFull review finished. |
278e060 to
20988e6
Compare
20988e6 to
5fdeeb2
Compare
Pin the execution image and dependency contract so the focused lifecycle workflow runs against a reproducible one-GPU environment. Signed-off-by: Johannes Rausch <jrausch@nvidia.com>
Signed-off-by: Johannes Rausch <jrausch@nvidia.com>
Require the immutable execution image to support anonymous pulls so copied pull-request branches never receive registry credentials. Signed-off-by: Johannes Rausch <jrausch@nvidia.com>
5fdeeb2 to
f022768
Compare
1 participant
What does this PR do?
Type of change: New tests.
The existing
gpu_puzzletronsession proves the current Puzzletron lifecycle but is only run manually. This change adds a digest-pinned repository-owned CUDA image recipe and a dedicated copied-PR, nightly, and manual workflow that runs that focused session against the reusable tiny-Qwen lifecycle test.The workflow accepts only immutable
nvcr.ioimage references, verifies the checked-out image contract before allocating a GPU, and installs the checked-out ModelOpt source without resolving dependencies at runtime. The image build preinstalls the declared ModelOpt runtime and test dependencies and verifies the pinned CUDA, Torch, Transformers, LMMS-Eval, AutoModel, AIPerf, and Nox environment.Testing
Adds CPU contract coverage for the recorded base-image digest, image-reference validation, Docker setup contract, workflow permissions and routing, and dependency-isolated runtime installation. The dedicated GPU workflow runs the existing one-GPU Puzzletron lifecycle test through
nox -s gpu_puzzletron.Additional Information
This PR is stacked on #2166. The registry image must be built and published separately, then
PUZZLETRON_GPU_CI_IMAGEmust be configured with its immutable digest before the workflow can run.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests