Skip to content

Puzletron v2 dockerfile - #2077

Open
chochowski wants to merge 3 commits into
feature/puzzletron_v2from
puzletron_v2_dockerfile
Open

Puzletron v2 dockerfile#2077
chochowski wants to merge 3 commits into
feature/puzzletron_v2from
puzletron_v2_dockerfile

Conversation

@chochowski

@chochowski chochowski commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Type of change: ?

Usage

# Add a code snippet demonstrating how to use this

Testing

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.).

  • Is this change backward compatible?: ✅ / ❌ / N/A
  • If you copied code from any other sources or added a new PIP dependency, did you follow guidance in CONTRIBUTING.md: ✅ / ❌ / N/A
  • Did you write any new necessary tests?: ✅ / ❌ / N/A
  • Did you update Changelog?: ✅ / ❌ / N/A
  • Did you get Claude approval on this PR?: ✅ / ❌ / N/A

Additional Information

Summary by CodeRabbit

  • New Features

    • Added a Docker-based development environment with CUDA 12.9.2 and Ubuntu 24.04.
    • Added automated setup for Python dependencies, virtual environments, Model Optimizer, and required CUDA extensions.
    • Added support for dependency-only, Model Optimizer-only, and complete environment setup modes.
  • Documentation

    • Added instructions for building the Puzzletron Docker image.

Signed-off-by: mchochowski <mchochowski@nvidia.com>
Signed-off-by: mchochowski <mchochowski@nvidia.com>
Signed-off-by: mchochowski <mchochowski@nvidia.com>
@chochowski
chochowski requested a review from a team as a code owner August 5, 2026 10:45
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The 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.

Changes

Puzzletron container setup

Layer / File(s) Summary
Environment setup flow
puzzletron_setup/docker/setup_env.sh
The script supports dependency-only, Model Optimizer-only, and full setup modes. It configures CUDA settings, installs dependencies, manages repositories and the virtual environment, and installs Model Optimizer.
Docker image integration
puzzletron_setup/docker/Dockerfile, examples/puzzletron/README.md
The Dockerfile defines the CUDA development image and runs the setup flow. The README documents the puzzletron_v2:26.08 image build command.

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
Loading
🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the primary change, which is adding a Dockerfile for Puzzletron v2, despite a minor spelling error.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Security Anti-Patterns ✅ Passed The full PR adds only Markdown, Dockerfile, and shell files; it changes no Python source, pyproject.toml, or requirements.txt, and adds none of the listed security anti-patterns.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch puzletron_v2_dockerfile

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

👉 Steps to fix this

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

📥 Commits

Reviewing files that changed from the base of the PR and between 3485fd0 and 44989ad.

📒 Files selected for processing (3)
  • examples/puzzletron/README.md
  • puzzletron_setup/docker/Dockerfile
  • puzzletron_setup/docker/setup_env.sh

Comment on lines +297 to +301
### 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 .
```

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 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 -->

Comment on lines +26 to +32
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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔒 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 -250

Repository: 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 -250

Repository: 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:


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

Comment on lines +57 to +63
# 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}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔒 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 -120

Repository: 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

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.70%. Comparing base (eda8b3a) to head (44989ad).
⚠️ Report is 2 commits behind head on feature/puzzletron_v2.

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     
Flag Coverage Δ
examples 29.69% <ø> (?)
puzzletron 32.87% <ø> (?)
unit 29.75% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants