api: add support for NRI-native CDI injection #98
Conversation
klihub
changed the title
|
/cc |
klihub
force-pushed
the
devel/native-cdi-injection
branch
2 times, most recently
from
e8cec5a
to
9aadde9
Compare
|
This change would make containers/nri-plugins#327 redundant. Is there a way to detect that we're working with a runtime / plugin that supports CDI devices natively? |
klihub
force-pushed
the
devel/native-cdi-injection
branch
2 times, most recently
from
cbfdea5
to
c304705
Compare
klihub
force-pushed
the
devel/native-cdi-injection
branch
from
c304705
to
460b41a
Compare
I rolled this PR actually with that CDI injecting NRI plugin in mind (I mentioned this in the initial review of that plugin PR). The idea was to test/see how we could do native CDI injection through NRI. And, if we're happy enough with the result to merge it, to make the implementation of actual CDI device injection in that plugin much easier. There are both pros and cons with this approach. The pros are
The cons are maybe that it is a bit superficial, as currently the container mutation capabilities offered by NRI are a superset of what is needed for CDI injection.
Currently no other means than checking |
| // apply additions to collected adjustments | ||
| for _, d := range devices { | ||
| if err := r.owners.claimCDIDevice(id, d.Name, plugin); err != nil { | ||
| return err |
There was a problem hiding this comment.
// TODO: consider support for a higher level management plugin having the cap to adjust, esp. outside k8s scenarios
There was a problem hiding this comment.
Do you mean to verbatim include that comment here... as a reminder to consider that in the future ?
There was a problem hiding this comment.
sort of.. suggesting the consideration of a TODO here, not that explicit text, or we can just open an issue though the way issues get removed when the bots are turned on :-)
plugins/device-injector/README.md
Outdated
| Devices are annotated using the `cdi-devices.nri.io` annotation key prefix. | ||
| The key `cdi-devices.nri.io/container.$CONTAINER_NAME` annotates CDI devices | ||
| to be injected into `$CONTAINER_NAME`. The keys `cdi-devices.nri.io` and | ||
| `cdi-devices.nri.io/pod` annotate CDI devices to be injected into all |
There was a problem hiding this comment.
reasoning for both with and without /pod?
There was a problem hiding this comment.
We've used in some of our existing components a similar annotation syntax/precedence. There the reasoning was to try provide both regularly symmetric annotation syntax ($KEY/pod vs. $KEY/container.$CONTAINER_NAME) and 'convenience' ($KEY interpreted as a shorthand notation/alias for $KEY/pod).
klihub
force-pushed
the
devel/native-cdi-injection
branch
from
4c99f6c
to
7d223a9
Compare
klihub
force-pushed
the
devel/native-cdi-injection
branch
4 times, most recently
from
4f12658
to
7859c0d
Compare
klihub
force-pushed
the
devel/native-cdi-injection
branch
2 times, most recently
from
466a173
to
257d1ee
Compare
Add support for native CDI injection. With this in place, a plugin can now directly request injection of CDI devices by device name. This is much simpler than first performing CDI device resolution and injection, followed by a set of low- level NRI adjustments for devices, mounts, env. vars. and OCI hooks to effectively do the same. Signed-off-by: Krisztian Litkey <[email protected]>
Signed-off-by: Krisztian Litkey <[email protected]>
Signed-off-by: Krisztian Litkey <[email protected]>
klihub
force-pushed
the
devel/native-cdi-injection
branch
from
257d1ee
to
7e6841a
Compare
Signed-off-by: Krisztian Litkey <[email protected]>
klihub
force-pushed
the
devel/native-cdi-injection
branch
from
7e6841a
to
6598645
Compare
Add support for injecting annotated CDI devices using the new native NRI CDI injection API. Signed-off-by: Krisztian Litkey <[email protected]>
Co-authored-by: Mike Brown <[email protected]> Signed-off-by: Krisztian Litkey <[email protected]>
klihub
force-pushed
the
devel/native-cdi-injection
branch
from
6598645
to
4eb7075
Compare
Clarify order of preference for the possible annotations. Co-authored-by: Mike Brown <[email protected]> Signed-off-by: Krisztian Litkey <[email protected]>
Add support for native CDI injection through NRI. With this patch set in place, a plugin can now directly request injection of CDI devices by fully qualified CDI device names. This is considerably simpler than what is currently required from an NRI plugin to perform CDI injection: perform CDI device resolution, then injection to an (almost) empty OCI Spec, inspect the resulting OCI Spec and convert the differences to a set of low-level NRI adjustments for devices, mounts, env. vars. and OCI hooks to effectively do the same through the NRI protocol.
/cc @elezar