Puzletron v2 dockerfile - #2077
Conversation
Signed-off-by: mchochowski <mchochowski@nvidia.com>
Signed-off-by: mchochowski <mchochowski@nvidia.com>
📝 WalkthroughWalkthroughThe change adds a CUDA 12.9.2 Ubuntu 24.04 Docker image for Puzzletron. A shared setup script installs dependencies, external repositories, CUDA extensions, and Model Optimizer. The README documents the image build command. ChangesPuzzletron container setup
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Developer
participant Dockerfile
participant SetupEnv as setup_env.sh
participant VirtualEnvironment
participant ModelOptimizer
Developer->>Dockerfile: build puzzletron_v2:26.08
Dockerfile->>SetupEnv: run full setup
SetupEnv->>VirtualEnvironment: install dependencies and CUDA extensions
SetupEnv->>ModelOptimizer: install local source with hf extras
Dockerfile-->>Developer: produce configured image
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
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: 3
🤖 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 `@examples/puzzletron/README.md`:
- Around line 297-301: Add a blank line before the “Build docker image” heading
and another before its fenced bash block to satisfy Markdown spacing rules, and
correct “pelase” to “please” in the build instruction text.
In `@puzzletron_setup/docker/Dockerfile`:
- Around line 26-32: Add runtime-user setup to the Dockerfile after the
installation step: create a non-root user and group using the project’s
mount-policy UID/GID, grant ownership or access only to required runtime paths,
then set the final USER directive so the image runs as that user.
In `@puzzletron_setup/docker/setup_env.sh`:
- Around line 57-63: Update the repository setup around the VLLM_ROOT and
AUTOMODEL_ROOT clone commands to check out reviewed, immutable full commit
hashes instead of mutable branches before the existing installation steps at
lines 76 and 79. Preserve the idempotent directory checks and ensure both
external repositories are pinned before installation.
🪄 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: 405c070a-04c6-4c18-a465-e6b12e101220
📒 Files selected for processing (3)
examples/puzzletron/README.mdpuzzletron_setup/docker/Dockerfilepuzzletron_setup/docker/setup_env.sh
| ### Build docker image | ||
| Building a docker image encapsulates the steps above. To build the image pelase run | ||
| ```bash | ||
| docker build -f puzzletron_setup/docker/Dockerfile -t puzzletron_v2:26.08 . | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the Markdown spacing and build instruction text.
Add a blank line before the heading and before the fenced block to satisfy MD022 and MD031. Change pelase to please.
Proposed fix
+
### Build docker image
-Building a docker image encapsulates the steps above. To build the image pelase run
+Building a docker image encapsulates the steps above. To build the image please run.
+
```bash
docker build -f puzzletron_setup/docker/Dockerfile -t puzzletron_v2:26.08 .</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.23.2)</summary>
[warning] 297-297: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
---
[warning] 299-299: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @examples/puzzletron/README.md around lines 297 - 301, Add a blank line
before the “Build docker image” heading and another before its fenced bash block
to satisfy Markdown spacing rules, and correct “pelase” to “please” in the build
instruction text.
</details>
<!-- fingerprinting:phantom:poseidon:tapir -->
<!-- cr-indicator-types:potential_issue -->
<!-- cr-comment:v1:4c0f181bcb926b179a0be1a7 -->
_Source: Linters/SAST tools_
<!-- This is an auto-generated comment by CodeRabbit -->
| ENV VIRTUAL_ENV=/venv | ||
| ENV PATH=/venv/bin/:$PATH | ||
|
|
||
| # Copy the full repo after deps so source changes don't bust the cache above. | ||
| COPY . "${MODEL_OPT_ROOT}"/ | ||
|
|
||
| RUN MODEL_OPT_ROOT="${MODEL_OPT_ROOT}" bash /tmp/setup_env.sh --modelopt |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- Dockerfile ---'
cat -n puzzletron_setup/docker/Dockerfile
printf '%s\n' '--- Related container configuration ---'
rg -n --glob '!*node_modules*' --glob '!*.lock' \
'puzzletron_setup/docker|docker compose|docker run|--user|user:|USER |/workspace|MODEL_OPT_ROOT' \
README.md .github puzzletron_setup 2>/dev/null | head -250Repository: NVIDIA/Model-Optimizer
Length of output: 2875
🏁 Script executed:
#!/bin/bash
set -eu
cat -n puzzletron_setup/docker/Dockerfile
rg -n --glob '!*node_modules*' --glob '!*.lock' \
'puzzletron_setup/docker|docker compose|docker run|--user|user:|USER |/workspace|MODEL_OPT_ROOT' \
README.md .github puzzletron_setup 2>/dev/null | head -250Repository: NVIDIA/Model-Optimizer
Length of output: 2816
🌐 Web query:
nvidia/cuda:12.9.2-cudnn-devel-ubuntu24.04 Docker image default USER root metadata
💡 Result:
The nvidia/cuda:12.9.2-cudnn-devel-ubuntu24.04 Docker image, like the official NVIDIA CUDA container images generally, runs as the root user by default [1]. The Dockerfile for these images does not contain a USER instruction to switch away from root [2][3], meaning the container inherits the default behavior of Docker and most container runtimes, which start processes as root [1]. If you require the container to run as a non-root user, you must define the user and group within your own Dockerfile (using RUN useradd or similar) and explicitly set the USER directive, while also ensuring the user has appropriate permissions for NVIDIA device access if needed [4][5].
Citations:
- 1: https://run-ai-docs.nvidia.com/self-hosted/2.25/infrastructure-setup/advanced-setup/container-access/user-identity-in-containers
- 2: https://gitlab.com/nvidia/container-images/cuda/blob/master/dist/13.3.0/ubuntu2404/base/Dockerfile
- 3: https://gitlab.com/nvidia/container-images/cuda/blob/master/dist/13.3.0/ubuntu2204/runtime/Dockerfile
- 4: https://stackoverflow.com/questions/74554143/how-to-use-nvidia-docker-container-as-non-root-user
- 5: https://forums.developer.nvidia.com/t/using-nvidia-docker-containers-as-non-root-user/235252
Security Misconfiguration (CWE-250)
Exploitability: Moderate
Run the final image as a non-root user.
The NVIDIA CUDA base image runs as root by default, and this Dockerfile does not add a USER directive. Create a runtime user, grant access only to required paths, and set USER after installation. Align its UID and GID with the mount policy.
🤖 Prompt for 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.
In `@puzzletron_setup/docker/Dockerfile` around lines 26 - 32, Add runtime-user
setup to the Dockerfile after the installation step: create a non-root user and
group using the project’s mount-policy UID/GID, grant ownership or access only
to required runtime paths, then set the final USER directive so the image runs
as that user.
Source: Linters/SAST tools
| # Clone external repos (idempotent) | ||
| [[ -d "${VLLM_ROOT}" ]] || \ | ||
| git clone --branch feature/add_anymodel_to_vllm --single-branch \ | ||
| https://github.com/Separius/vllm.git "${VLLM_ROOT}" | ||
| [[ -d "${AUTOMODEL_ROOT}" ]] || \ | ||
| git clone --branch puzzletron --single-branch \ | ||
| https://github.com/Separius/Automodel.git "${AUTOMODEL_ROOT}" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- setup_env.sh relevant sections ---'
sed -n '45,105p' puzzletron_setup/docker/setup_env.sh
printf '%s\n' '--- repository references ---'
rg -n --glob '!node_modules' --glob '!dist' 'feature/add_anymodel_to_vllm|Separius/vllm|Separius/Automodel|pip install' puzzletron_setup examples README.md 2>/dev/null | head -120Repository: NVIDIA/Model-Optimizer
Length of output: 14343
Mutable Third-party Dependency Pin (CWE-494): Download of Code Without Integrity Check
Reachability: External · Exploitability: Difficult
Pin the external repository revisions.
Lines 57-63 clone mutable branches, and lines 76 and 79 install from those checkouts. Pin both repositories to reviewed full commit hashes before installation.
🤖 Prompt for 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.
In `@puzzletron_setup/docker/setup_env.sh` around lines 57 - 63, Update the
repository setup around the VLLM_ROOT and AUTOMODEL_ROOT clone commands to check
out reviewed, immutable full commit hashes instead of mutable branches before
the existing installation steps at lines 76 and 79. Preserve the idempotent
directory checks and ensure both external repositories are pinned before
installation.
Source: Coding guidelines
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feature/puzzletron_v2 #2077 +/- ##
==========================================================
+ Coverage 29.74% 60.70% +30.96%
==========================================================
Files 704 705 +1
Lines 90869 90970 +101
==========================================================
+ Hits 27028 55227 +28199
+ Misses 63841 35743 -28098
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:
|
What does this PR do?
Type of change: ?
Usage
# Add a code snippet demonstrating how to use thisTesting
Before your PR is "Ready for review"
Make sure you read and follow Contributor guidelines and your commits are signed (
git commit -s -S).Make sure you read and follow the Security Best Practices (e.g. avoiding hardcoded
trust_remote_code=True,torch.load(..., weights_only=False),pickle, etc.).CONTRIBUTING.md: ✅ / ❌ / N/AAdditional Information
Summary by CodeRabbit
New Features
Documentation