Add nvidia-container-toolkit install and CDI generation#22
Open
bogdando wants to merge 4 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds optional NVIDIA Container Toolkit installation and CDI generation, separates toolkit setup from CUDA repository handling, adds reboot recovery, removes explicit ChangesNVIDIA Container Toolkit extraction and validation setup
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant SetupTasks
participant DNF
participant Reboot
participant NvidiaCTK
SetupTasks->>DNF: Install nvidia-container-toolkit
SetupTasks->>Reboot: Reboot VM
SetupTasks->>NvidiaCTK: Configure containerd runtime
NvidiaCTK->>NvidiaCTK: Generate /etc/cdi/nvidia.yaml
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Author
|
I am re-verifying this for L4 (non-MIG) on my downstream testproject again, after the recent MIG changes landed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@gpu-validation/tasks/nvidia-container-toolkit.yaml`:
- Around line 34-36: The task "Configure NVIDIA container runtime"
unconditionally runs the nvidia-ctk command for containerd, which will fail on
systems where containerd is not installed or in use, particularly in this
Podman-focused role. Add a conditional to the task using the when clause that
checks whether containerd is actually available or in use before executing the
ansible.builtin.command step. This ensures the configuration only runs on
systems where containerd is relevant, allowing the role to work correctly on
Podman-only systems.
In `@gpu-validation/tasks/nvidia-cuda-repos.yaml`:
- Around line 16-17: The unconditional import of nvidia-container-toolkit.yaml
at line 16-17 bypasses the nvidia_container_toolkit_install gate, causing the
toolkit installation to run even when the flag is false and potentially creating
duplicate executions. Remove the import statement for
nvidia-container-toolkit.yaml from this location entirely and rely on the single
gated import path in gpu-validation/tasks/main.yaml to control toolkit
installation based on the nvidia_container_toolkit_install flag.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: b211d255-db78-425c-bd39-7df856e8ea3e
📒 Files selected for processing (4)
gpu-validation/defaults/main.yamlgpu-validation/tasks/main.yamlgpu-validation/tasks/nvidia-container-toolkit.yamlgpu-validation/tasks/nvidia-cuda-repos.yaml
Contributor
Author
|
I tested it for L4 non-mig case. Would be nice to re-ferify this for a MIG setup as well to catch possible regressions |
Contributor
|
Hi @bogdando I need this PR for NVIDIA RTX PRO 6000 GPUs via PCI passthrough too |
MiguelCarpio
reviewed
Jul 9, 2026
There was a problem hiding this comment.
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 `@gpu-validation/defaults/main.yaml`:
- Around line 65-67: The setting comment for
gpu_validation_nvidia_container_toolkit_install describes podman GPU access, but
the related nvidia-container-toolkit task configures containerd and CDI
generation instead. Update the wording in the defaults entry to refer to
containerd or generic container GPU access so it matches the behavior
implemented in nvidia-container-toolkit.yaml.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 2ce7c7ac-ccba-4d76-b639-ecd7fc6251d7
📒 Files selected for processing (4)
gpu-validation/defaults/main.yamlgpu-validation/tasks/main.yamlgpu-validation/tasks/nvidia-container-toolkit.yamlgpu-validation/tasks/nvidia-cuda-repos.yaml
💤 Files with no reviewable changes (1)
- gpu-validation/tasks/nvidia-cuda-repos.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- gpu-validation/tasks/main.yaml
- gpu-validation/tasks/nvidia-container-toolkit.yaml
MiguelCarpio
requested changes
Jul 9, 2026
| - name: Reboot if system updates require it | ||
| ansible.builtin.import_tasks: reboot_if_needed.yaml | ||
|
|
||
| - name: Install NVIDIA CUDA repos |
Contributor
There was a problem hiding this comment.
This is needed for Install NVIDIA container toolkit and configure CDI the conditional should change
when: gpu_validation_nvidia_container_toolkit_install | bool
Contributor
Author
There was a problem hiding this comment.
I believe managing cuda repos and toolkit are independent things
Contributor
Author
There was a problem hiding this comment.
CUDA repos may be needed even if not installing toolkit
Contributor
There was a problem hiding this comment.
With the condition:
when:
- ansible_distribution == "RedHat"
- gpu_validation_mode == "mig"
The NVIDIA RTX PRO 6000 Blackwell in PCI passthrough (not MIG mode) won't install the CUDA repositories, so the gpu-validation tool won't be able to Install NVIDIA container toolkit and configure CDI. It will fail because no packages/repo were found.
Besides, by default, the gpu-validation machine is CentOS, so it will skipt the CUDA repo configuration. I can send another PR fixing this, but I believe we can take advantage of this change.
Contributor
Author
There was a problem hiding this comment.
@MiguelCarpio regarding Centos - we don't plan to use this for upstream CI, and downstream we only use RHEL VM guest, so I wouldn't bother of Centos support much
MiguelCarpio
requested changes
Jul 9, 2026
MiguelCarpio
reviewed
Jul 9, 2026
MiguelCarpio
previously requested changes
Jul 9, 2026
MiguelCarpio
reviewed
Jul 9, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
gpu-validation/defaults/main.yaml (1)
70-70: 🔒 Security & Privacy | 🔵 Trivial | ⚖️ Poor tradeoffHardcoded external repo URL as default with no integrity check.
Defaulting
gpu_validation_nvidia_container_toolkit_repo_urlto a third-party.repofile fetched viaget_urlwith no checksum/signature verification means every run trusts the remote file's current content. Consider pinning a checksum or at least documenting the trust assumption.🤖 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 `@gpu-validation/defaults/main.yaml` at line 70, The default for gpu_validation_nvidia_container_toolkit_repo_url is hardcoded to a remote third-party .repo file without any integrity verification. Update the defaults in main.yaml to avoid blindly trusting the fetched content, either by removing the remote URL as the default, pinning it to a verified checksum/signature-based source, or clearly documenting the trust assumption in the gpu_validation_nvidia_container_toolkit_repo_url setting so users understand how it is validated.
🤖 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 `@gpu-validation/defaults/main.yaml`:
- Line 67: The default for gpu_validation_nvidia_container_toolkit_install is
forcing toolkit installation on every host, including non-RedHat systems. Update
the defaults in main.yaml to keep this setting opt-in or guard the install path
in gpu-validation/tasks/main.yaml with the same ansible_distribution == "RedHat"
check used by the CUDA repo task. Also avoid hardcoding the nvidia.github.io
repo URL as a default value so the install only runs when explicitly intended.
---
Nitpick comments:
In `@gpu-validation/defaults/main.yaml`:
- Line 70: The default for gpu_validation_nvidia_container_toolkit_repo_url is
hardcoded to a remote third-party .repo file without any integrity verification.
Update the defaults in main.yaml to avoid blindly trusting the fetched content,
either by removing the remote URL as the default, pinning it to a verified
checksum/signature-based source, or clearly documenting the trust assumption in
the gpu_validation_nvidia_container_toolkit_repo_url setting so users understand
how it is validated.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: fe3592a5-bc0c-4471-94d5-f6a3d912732f
📒 Files selected for processing (4)
gpu-validation/defaults/main.yamlgpu-validation/tasks/main.yamlgpu-validation/tasks/nvidia-container-toolkit.yamlgpu-validation/tasks/nvidia-cuda-repos.yaml
💤 Files with no reviewable changes (1)
- gpu-validation/tasks/nvidia-cuda-repos.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- gpu-validation/tasks/main.yaml
- gpu-validation/tasks/nvidia-container-toolkit.yaml
bogdando
force-pushed
the
dev
branch
2 times, most recently
from
July 13, 2026 09:39
b38fb96 to
5016f69
Compare
There was a problem hiding this comment.
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 `@gpu-validation/tasks/model_download_and_serve.yaml`:
- Line 63: Update the retries expression in the model download and serve task to
derive the polling count by rounding the timeout division up and clamping the
result to at least one, so sub-10-second and partial-interval timeouts never
produce zero retries or lose the final interval.
In `@gpu-validation/tasks/nvidia-container-toolkit.yaml`:
- Around line 10-21: Register the result of the “Install
nvidia-container-toolkit” task and condition the “Reboot the VM to find the
installed drivers” block on that result’s changed state. Preserve the existing
reboot behavior when the package is newly installed, while skipping it on
unchanged runs.
- Around line 20-21: Update the ansible.builtin.reboot task in
nvidia-container-toolkit.yaml to use the shared gpu_validation_reboot_timeout
and gpu_validation_reboot_post_delay variables, matching the reboot settings in
reboot_if_needed.yaml instead of hard-coding the timeout.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 67290351-8ce1-496a-8302-be1d6d5a7175
📒 Files selected for processing (6)
gpu-validation/defaults/main.yamlgpu-validation/tasks/main.yamlgpu-validation/tasks/model_download_and_serve.yamlgpu-validation/tasks/nvidia-container-toolkit.yamlgpu-validation/tasks/nvidia-cuda-repos.yamlgpu-validation/tasks/setup.yaml
💤 Files with no reviewable changes (2)
- gpu-validation/tasks/nvidia-cuda-repos.yaml
- gpu-validation/tasks/setup.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- gpu-validation/tasks/main.yaml
- gpu-validation/defaults/main.yaml
There was a problem hiding this comment.
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 `@gpu-validation/tasks/nvidia-container-toolkit.yaml`:
- Around line 23-29: Update the rescue blocks in
gpu-validation/tasks/nvidia-container-toolkit.yaml lines 23-29 and
gpu-validation/tasks/reboot_if_needed.yaml lines 28-34 so reboot failures are
ignored only in local-runner mode via an explicit guard; remove unconditional
ignore_errors behavior, allowing non-local ansible.builtin.reboot failures to
propagate while preserving the local-runner guidance message.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 199dda2e-089d-470f-b670-6a6f5180f70a
📒 Files selected for processing (3)
gpu-validation/tasks/model_download_and_serve.yamlgpu-validation/tasks/nvidia-container-toolkit.yamlgpu-validation/tasks/reboot_if_needed.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- gpu-validation/tasks/model_download_and_serve.yaml
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@gpu-validation/tasks/reboot_if_needed.yaml`:
- Line 34: Update the ignore_errors setting in the reboot task to use the
lowercase YAML boolean true instead of True, preserving the setting's current
behavior.
- Around line 28-34: Update the rescue task in the reboot flow to stop masking
remote reboot failures: remove ignore_errors from the ansible.builtin.fail task,
or restrict this local-runner rescue message to the local-runner case while
allowing remote reboot and boot failures to propagate and halt the play.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 0c22e28f-1463-4d9b-9b6b-a3a1da059add
📒 Files selected for processing (3)
gpu-validation/tasks/model_download_and_serve.yamlgpu-validation/tasks/nvidia-container-toolkit.yamlgpu-validation/tasks/reboot_if_needed.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- gpu-validation/tasks/nvidia-container-toolkit.yaml
- gpu-validation/tasks/model_download_and_serve.yaml
There was a problem hiding this comment.
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 `@gpu-validation/tasks/nvidia-container-toolkit.yaml`:
- Around line 45-49: Move the “Generate NVIDIA CDI configuration” task to run
after the nvidia-driver-cuda-libs installation task, or add a second generation
task immediately afterward. Ensure the post-installation generation always runs
so /etc/cdi/nvidia.yaml includes libcuda.so.1.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: a7832c2e-c433-4f49-85b7-88839b92c77f
📒 Files selected for processing (3)
gpu-validation/tasks/model_download_and_serve.yamlgpu-validation/tasks/nvidia-container-toolkit.yamlgpu-validation/tasks/reboot_if_needed.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- gpu-validation/tasks/reboot_if_needed.yaml
- gpu-validation/tasks/model_download_and_serve.yaml
skovili
previously approved these changes
Jul 14, 2026
In a L4 GPU passtrhough setup (vllm-cuda-rhel9:3.3.3, vLLM v0.13.0, supports CUDA 12.8 / GPU driver 570+) gpu validation could not pass unless CDI was generated (even though there is no MIG mode to apply). The role vars accept nvidia_container_toolkit_install but never acts on it unless that is a MIG-enabled setup. Make CUDA toolkit installation, node rebooting, and config/generation a dedicated task gated on the new default variable. Also increase wait for reboot to a 30 min (multiple GPUs with large VRAM take lot of time to initialize for VMs). This fixes the "unresolvable CDI devices nvidia.com/gpu=all" error when running vLLM containers on CentOS Stream 9 VMs with passthrough GPUs. Signed-off-by: Bohdan Dobrelia <bdobreli@redhat.com>
Ansible core 2.14.18 which comes from RHEL 9.6 repos does not support this param. Signed-off-by: Bohdan Dobrelia <bdobreli@redhat.com>
skovili
previously approved these changes
Jul 14, 2026
Add rescue block for reboot calls to handle the error "Running ansible.builtin.reboot with local connection would reboot the control node". In local runner mode unconditional reboot action would always fail the playbook (VM as a control node cannot self-reboot). We want it to only print a RED error msg and continue. The user is expected to notice it in the log after the validation fails later because reboot wasn't done. Then user should reboot the node, re-run the suit, and ignore the red error msg after that. Signed-off-by: Bohdan Dobrelia <bdobreli@redhat.com>
Fix CDI generation was skipped if /etc/cdi/nvidia.yaml already exists — so a stale spec (generated before nvidia-driver-cuda-libs was installed) is never refreshed. Make CDI always be regenerated after a toolkit install/reboot — the command is fast and idempotent in output; the cost of skipping is a broken container runtime. Also make sure the delayed CDI generation is imported at the intended point and the earlier task file no longer generates the spec prematurely. Signed-off-by: Bohdan Dobrelia <bdobreli@redhat.com>
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.
The role vars accept nvidia_container_toolkit_install but never acts on it unless that is a MIG-enabled setup.
Make CUDA toolkit installation and config/generation a dedicated task gated on the new default variable.
This fixes the "unresolvable CDI devices nvidia.com/gpu=all" error when running vLLM containers on CentOS Stream 9 VMs with passthrough GPUs.
Fix CDI generation was skipped if /etc/cdi/nvidia.yaml already exists —
so a stale spec (generated before nvidia-driver-cuda-libs was installed)
is never refreshed.
Make CDI always be regenerated after a toolkit install/reboot — the
command is fast and idempotent in output; the cost of skipping is a
broken container runtime.
Also make sure the delayed CDI generation is imported at the intended
point and the earlier task file no longer generates the spec
prematurely.
Drop unsupported dnf use_backend.
Ansible core 2.14.18 which comes from RHEL 9.6 repos does not support
this param.
Allow running for pre-deployed VM with drivers
Add rescue block for reboot calls to handle the error
"Running ansible.builtin.reboot with local connection would reboot the
control node".
In local runner mode unconditional reboot action would always fail the
playbook (VM as a control node cannot self-reboot). We want it to only
print a RED error msg and continue. The user is expected to notice it
in the log after the validation fails later because reboot wasn't done.
Then user should reboot the node, re-run the suit, and ignore the red
error msg after that.
Related: RHOSRFE-311