Skip to content
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 plugin for CDI device injection #327

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

elezar
Copy link
Contributor

@elezar elezar commented Jun 7, 2024

This PR adds a plugin for CDI injection.

The values of defined pod annotations are interpreted as a comma-separated list of CDI devices. The local CDI cache is read and the corresponding container adjusments are generated.

Some items to do:

  • Make the CDI folders configurable
  • Consider which of this code should be upstreamed into the CDI packages

In the longer-term, it should also be possible to restrict the injection of devices to containers in a particular namespace / based on properties of a namespace.

@elezar
Copy link
Contributor Author

elezar commented Jun 7, 2024

/cc @klihub

@klihub klihub requested review from kad and askervin June 7, 2024 12:51
@klihub
Copy link
Collaborator

klihub commented Jun 7, 2024

/cc @klihub

I can take a look at adding native CDI injection support to NRI based on this/an earlier very similar CDI injection sample plugin we had in the first versions of v0.2.0 NRI PR...

Signed-off-by: Evan Lezar <elezar@nvidia.com>
Signed-off-by: Evan Lezar <elezar@nvidia.com>
This change adds an allowed-cdi-device-pattern command line option
to the CDI device injector to allow CDI device names to be filtered.

Signed-off-by: Evan Lezar <elezar@nvidia.com>
@klihub
Copy link
Collaborator

klihub commented Jul 3, 2024

@elezar @kad @askervin I rolled a proof-of-concept/initial implementation of CDI device injection in NRI, directly supported by the protocol and supporting libraries. I also updated containerd 1.7 and main/2.0 and cri-o main to allow CDI injection through. The NRI bits also update the test/sample device-injector plugin (in the NRI repo itself) to allow injecting CDI devices by annotation. This is what I used for testing against all of the updated containerd and cri-o trees.

Here are the related working trees/branches:

Based on this, it looks technically feasible, fairly straightforward, and not very intrusive to implement. Next we should decide if we want to push this or not. It has pros but maybe semantically it is a bit of an outlier compared to the rest of our features.


var allowedCDIDevices []string
for _, cdiDevice := range cdiDevices {
match, _ := filepath.Match(p.allowedCDIDevicePattern, cdiDevice)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it would be better to use path.Match() for globbing instead of filepath.Match().

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.

None yet

2 participants