Support the DevicePluginCDIDevices feature gate
#1007
Conversation
jfroy
requested review from
ArangoGutierrez,
cdesiniotis,
elezar,
shivamerla and
tariq1890
as code owners
jfroy
force-pushed
the
deviceplugincdidevices
branch
2 times, most recently
from
435a38f to
0b0151d
Compare
jfroy
changed the title
DevicePluginCDIDevices featureDevicePluginCDIDevices feature gate
| // +operator-sdk:gen-csv:customresourcedefinitions.specDescriptors.x-descriptors="urn:alm:descriptor:com.tectonic.ui:booleanSwitch" | ||
| UseOpenShiftDriverToolkit *bool `json:"use_ocp_driver_toolkit,omitempty"` | ||
|
|
||
| // UseDevicePluginCDIDevicesFeature indicates if the device plug-in should be configured to use the CDI devices feature |
There was a problem hiding this comment.
One question on the UX for this: Should this be under the cdi object in the cluster policy?
There was a problem hiding this comment.
It's an option that only affects the operator, but is related to CDI. I don't know to which it binds more strongly. So I picked one. It's easy enough to change that in the PR, but of course hard after. Maybe other folks can chime in. Ultimately I will defer to you and your team.
| } else { | ||
| setContainerEnv(&(obj.Spec.Template.Spec.Containers[0]), DeviceListStrategyEnvName, "envvar,cdi-annotations") | ||
| } | ||
| setContainerEnv(&(obj.Spec.Template.Spec.Containers[0]), CDIAnnotationPrefixEnvName, "nvidia.cdi.k8s.io/") |
There was a problem hiding this comment.
This block is also not relevant when using cdi-cri.
There was a problem hiding this comment.
Do you mean to say that useDevicePluginCDIDevicesFeature should be stronger than cdi.enabled? The current design and implementation is that useDevicePluginCDIDevicesFeature does nothing unless cdi.enabled is set.
This patch adds support for the `DevicePluginCDIDevices` feature gate by adding `spec.operator.useDevicePluginCDIDevicesFeature` to `ClusterPolicy`. When this field is set, the operator sets the `DEVICE_LIST_STRATEGY` device plug-in environment variable to `cdi-cri`. Signed-off-by: Jean-Francois Roy <[email protected]>
jfroy
force-pushed
the
deviceplugincdidevices
branch
from
0b0151d to
ef79ad3
Compare
|
@cdesiniotis I imagine #1285 obsoletes this PR? |
Ah wait, no, it doesn't use "native CDI" but instead relies on annotations. I'll comment more in the internal document about this. |
|
@jfroy I've updated #1285 to use the CRI instead of annotations, so yes, if we proceed with #1285 it will obsolete this PR. See the discussion here: #1285 (comment) |
|
This PR is stale because it has been open 90 days with no activity. This PR will be closed in 30 days unless new comments are made or the stale label is removed. To skip these checks, apply the "lifecycle/frozen" label. |
github-actions
bot
added
the
lifecycle/stale
|
This is no longer needed. 🚀 |
3 participants
This patch adds support for the
DevicePluginCDIDevicesfeature gate by addingspec.operator.useDevicePluginCDIDevicesFeaturetoClusterPolicy. When this field is set, the operator sets theDEVICE_LIST_STRATEGYdevice plug-in environment variable tocdi-cri.