Skip to content

Conversation

@a-mccarthy
Copy link
Collaborator

No description provided.

Signed-off-by: Abigail McCarthy <20771501+a-mccarthy@users.noreply.github.com>
Signed-off-by: Abigail McCarthy <20771501+a-mccarthy@users.noreply.github.com>
@github-actions
Copy link

Documentation preview

https://nvidia.github.io/cloud-native-docs/review/pr-165

@a-mccarthy a-mccarthy marked this pull request as draft March 14, 2025 03:19
Signed-off-by: Abigail McCarthy <20771501+a-mccarthy@users.noreply.github.com>
@a-mccarthy a-mccarthy marked this pull request as ready for review March 17, 2025 19:57
Signed-off-by: Abigail McCarthy <20771501+a-mccarthy@users.noreply.github.com>
Signed-off-by: Abigail McCarthy <20771501+a-mccarthy@users.noreply.github.com>
Signed-off-by: Abigail McCarthy <20771501+a-mccarthy@users.noreply.github.com>
nvidia/gpu-operator \
--version=${version} \
--set driver.useOpenKernelModules=true \
--set driver.kernelModuleType=open \
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's be consistent across the two install commands presented here in this section. Either both commands should specify --set driver.kernelModuleType=open or both commands should omit setting this field. I would be in favor of omitting this field as auto should take care of installing the open modules on supported systems. We can highlight that setting driver.kernelModuleType=open is only needed for older driver container versions where auto is not supported -- either in the text below or in a note.

I would be in favor of removing this field altogether since it is optional for users to configure it. The default setting, auto, should be sufficient as called out in the text.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see you have already implemented my suggestion in the Installing the GPU Operator and Enabling GPUDirect Storage section :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated the sample here to match the sample in the other section!

Signed-off-by: Abigail McCarthy <20771501+a-mccarthy@users.noreply.github.com>
Signed-off-by: Abigail McCarthy <20771501+a-mccarthy@users.noreply.github.com>
Signed-off-by: Abigail McCarthy <20771501+a-mccarthy@users.noreply.github.com>
Signed-off-by: Abigail McCarthy <20771501+a-mccarthy@users.noreply.github.com>
Copy link
Contributor

@cdesiniotis cdesiniotis left a comment

Choose a reason for hiding this comment

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

@a-mccarthy I think we are close to proceeding with this PR -- just a few small open items / suggestions to accommodate.

Signed-off-by: Abigail McCarthy <20771501+a-mccarthy@users.noreply.github.com>
Currently, ``auto`` is only supported with the 570.86.15 and 570.124.06 or later driver containers.
550 and 535 branch drivers do not yet support this mode.

In previous versions, the ``useOpenKernelModules`` field specified the driver containers to install the NVIDIA Open GPU Kernel module driver.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
In previous versions, the ``useOpenKernelModules`` field specified the driver containers to install the NVIDIA Open GPU Kernel module driver.
In previous versions, the ``useOpenKernelModules`` field specified the driver containers to install the NVIDIA Open GPU kernel module driver.

Signed-off-by: Abigail McCarthy <20771501+a-mccarthy@users.noreply.github.com>
Copy link
Contributor

@cdesiniotis cdesiniotis left a comment

Choose a reason for hiding this comment

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

@a-mccarthy thanks for the work and patience on this one!

@a-mccarthy a-mccarthy merged commit 7788e32 into NVIDIA:main Mar 26, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants