Skip to content

Conversation

@rgsl888prabhu
Copy link
Collaborator

@rgsl888prabhu rgsl888prabhu commented Jan 15, 2026

Description

Container had a failure with missing yaml dependency and also missing cuda_fp16.h header. It has been fixed in this PR.
This also fixes a CVE.

Checklist

  • I am familiar with the Contributing Guidelines.
  • Testing
    • New or existing tests cover these changes
    • Added tests
    • Created an issue to follow-up
    • NA
  • Documentation
    • The documentation is up to date with these changes
    • Added new documentation
    • NA

Summary by CodeRabbit

  • Chores

    • Added pyyaml>=6.0.0 as a runtime dependency across all environments.
    • Updated Docker image to include CUDA development headers at runtime.
    • Updated container build dependencies for enhanced environment consistency.
  • Tests

    • Enhanced test infrastructure with comprehensive CLI and server test coverage.

✏️ Tip: You can customize this high-level summary in your review settings.

@rgsl888prabhu rgsl888prabhu added this to the 26.02 milestone Jan 15, 2026
@rgsl888prabhu rgsl888prabhu self-assigned this Jan 15, 2026
@rgsl888prabhu rgsl888prabhu added bug Something isn't working non-breaking Introduces a non-breaking change labels Jan 15, 2026
@copy-pr-bot
Copy link

copy-pr-bot bot commented Jan 15, 2026

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.

@rgsl888prabhu rgsl888prabhu marked this pull request as ready for review January 15, 2026 19:42
@rgsl888prabhu rgsl888prabhu requested a review from a team as a code owner January 15, 2026 19:43
@coderabbitai
Copy link

coderabbitai bot commented Jan 15, 2026

📝 Walkthrough

Walkthrough

This pull request adds pyyaml>=6.0.0 as a runtime dependency across conda environments, dependencies configuration, and Python package metadata. The Docker image is augmented with a CUDA headers devel stage for runtime compilation support, gnupg2 is installed and upgraded for security updates, and a new test script is introduced in the Docker test image.

Changes

Cohort / File(s) Summary
Docker Build & Testing
ci/docker/Dockerfile, ci/docker/test_image.sh
Dockerfile: Introduces cuda-headers devel stage to provide CUDA headers (/usr/local/cuda/include/); installs and upgrades gnupg2 during base image setup; adds pyyaml to Python packages; updates cleanup to remove both gcc and gnupg2. test_image.sh: Updates copyright year to 2025-2026; adds test script with set -euo pipefail, pytest/pexpect installation, and execution of CLI, CUOPT, and CUOPT SERVER test suites.
Conda Environments
conda/environments/all_cuda-129_arch-aarch64.yaml, conda/environments/all_cuda-129_arch-x86_64.yaml, conda/environments/all_cuda-131_arch-aarch64.yaml, conda/environments/all_cuda-131_arch-x86_64.yaml
All four environment YAML files add pyyaml>=6.0.0 as a dependency entry.
Dependency Declarations
dependencies.yaml, python/cuopt/pyproject.toml
Both files add pyyaml>=6.0.0 as a runtime dependency (dependencies.yaml uses &pyyaml anchor under run_cuopt common packages; pyproject.toml adds entry to [project] dependencies list).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix container build and test' directly relates to the changeset, which fixes container build failures (missing yaml dependency and CUDA headers) and adds container testing.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

🧹 Recent nitpick comments
ci/docker/Dockerfile (1)

59-59: Consider adding version constraint for consistency.

The conda environment files specify pyyaml>=6.0.0, but here no version constraint is provided. For consistency across environments and to ensure the security improvements in PyYAML 6.0+ are explicitly required:

Suggested change
-      "pyyaml" \
+      "pyyaml>=6.0.0" \

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7bb76f7 and 5ef46d9.

📒 Files selected for processing (8)
  • ci/docker/Dockerfile
  • ci/docker/test_image.sh
  • conda/environments/all_cuda-129_arch-aarch64.yaml
  • conda/environments/all_cuda-129_arch-x86_64.yaml
  • conda/environments/all_cuda-131_arch-aarch64.yaml
  • conda/environments/all_cuda-131_arch-x86_64.yaml
  • dependencies.yaml
  • python/cuopt/pyproject.toml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Check that hard-coded GPU device IDs and resource limits are made configurable; abstract multi-backend support for different CUDA versions

Applied to files:

  • ci/docker/Dockerfile
🔇 Additional comments (12)
conda/environments/all_cuda-131_arch-aarch64.yaml (1)

58-58: LGTM!

The pyyaml>=6.0.0 dependency is correctly added in alphabetical order. The version constraint is appropriate and aligns with the other environment files and the source dependencies.yaml.

conda/environments/all_cuda-129_arch-x86_64.yaml (1)

58-58: LGTM!

Consistent addition of pyyaml>=6.0.0 matching the other conda environment files.

conda/environments/all_cuda-131_arch-x86_64.yaml (1)

58-58: LGTM!

Consistent addition of pyyaml>=6.0.0 matching the other conda environment files.

dependencies.yaml (1)

352-352: LGTM!

The pyyaml>=6.0.0 dependency is correctly added under run_cuopt with appropriate output types. The YAML anchor &pyyaml follows the existing pattern used for other dependencies (e.g., &pandas, &numpy) even though it's not currently referenced elsewhere—this is fine for consistency and potential future use.

python/cuopt/pyproject.toml (1)

32-32: LGTM!

The pyyaml>=6.0.0 runtime dependency is correctly added, maintaining alphabetical order and matching the version constraint specified in dependencies.yaml.

conda/environments/all_cuda-129_arch-aarch64.yaml (1)

58-58: LGTM!

The addition of pyyaml>=6.0.0 addresses the missing YAML dependency. The version constraint is appropriate and the placement maintains alphabetical ordering in the dependencies list.

Note: Since this file is auto-generated from dependencies.yaml, ensure the source file is also updated (per the AI summary, it appears to be).

ci/docker/test_image.sh (2)

27-28: LGTM!

Adding set -euo pipefail inside the generated test script ensures fail-fast behavior during test execution, matching the outer script's error handling. This is a good defensive practice.


26-42: Well-structured test script with clear markers.

The test execution flow is well-organized with clear log markers. The runtime pip install of pytest and pexpect works, though pre-installing them in the test image could marginally speed up repeated test runs.

ci/docker/Dockerfile (4)

12-15: Good use of multi-stage build for CUDA headers.

This cleanly extracts the necessary CUDA headers (like cuda_fp16.h) from the devel image without bloating the final image. The comment clearly explains the rationale for CuPy/NVRTC runtime compilation support.


33-37: CVE mitigation pattern looks correct.

Installing gnupg2 then immediately upgrading it ensures the container gets the latest security patches, addressing the CVE mentioned in the PR objectives. The --only-upgrade flag is slightly redundant here since gnupg2 is installed just above, but it doesn't cause issues and makes the intent explicit.


64-65: Good security hygiene by purging build-only dependencies.

Removing both gcc and gnupg2 after they've served their purpose (building psutils and applying security updates) reduces the CVE surface area and final image size.


112-113: LGTM!

This completes the fix for the missing cuda_fp16.h header issue by copying the CUDA headers from the devel stage. The comment clearly documents the purpose for CuPy NVRTC runtime compilation.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@rgsl888prabhu
Copy link
Collaborator Author

/merge

@rapids-bot rapids-bot bot merged commit dd3a8da into NVIDIA:main Jan 15, 2026
195 of 197 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants