-
Notifications
You must be signed in to change notification settings - Fork 152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add NRI device injector package #345
Conversation
4865808
to
e227caf
Compare
log.Infof("Container %s: annotated device %q...", ctrName, d.Path) | ||
deviceNRI, err := d.toNRIDevice() | ||
if err != nil { | ||
return nil, nil, err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there any difference on returning nil or the empty value here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so, as long as the plugin returns error, there won't be any adjustment made.
nri_device_injector/README.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samuelkarp @aojea added readme for the directory, PTAL, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks really good. A few suggestions, but otherwise looks good.
Can someone add some context on what this newly integrated NRI plugin is designed to accomplish? On the surface it seems like CDI is a better fit (and how we now do things in NVIDIA's GPU device plugin). If we have any hope of eventually merging the GKE GPU plugin with NVIDIA's we will need to understand what is being added here and why it is necessary / how it differs from just using CDI. |
Also worth noting -- CDI devices were added to the device plugin API in Kubernetes 1.28, graduated to beta in 1.29, and will be promoted to GA in 1.30. And both containerd 1.7+ and cri-o support injecting them natively (without the need for a custom NRI plugin). |
@klueska Hi Kevin, thanks for your comments! This NRI plugin is not related to GPU device plugin itself, but is related to the GPU feature TCPX, that's only supported on GKE 1.27 for now, and this plugin is used to provide unprivileged GPU access on a TCPX sidecar container. |
No description provided.