Add support for featureGates#220
Conversation
Signed-off-by: svarnam <svarnam@nvidia.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Sreeja1725 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
k8s-ci-robot
added
cncf-cla: yes
| // VFIOGPUEnabled enables PCI passthrough device discovery and preparation via | ||
| // the vfio-gpu profile. When enabled, the driver selects that profile | ||
| // instead of the default gpu profile. | ||
| VFIOGPUEnabled featuregate.Feature = "VFIOGPUEnabled" |
There was a problem hiding this comment.
This is bleeding in from the other PR? Should this be left out?
alaypatel07
left a comment
alaypatel07
left a comment
There was a problem hiding this comment.
Adding feature gate is a good feature for the example driver, it will help everyone follow the good practices.
Please have a very tight scoped change of introducing the feature gate package. You can leave an example feature gate in comment for documentation and remove vfio traces from this PR
| Destination: &flags.healthcheckPort, | ||
| EnvVars: []string{"HEALTHCHECK_PORT"}, | ||
| }, | ||
| &cli.StringFlag{ |
There was a problem hiding this comment.
we can keep this flag right?
we should document with vfio that the profile will be changed to vfio automatically
|
I think we've mostly decided against the "feature gate" form factor in #91. Device profiles, command line flags, and Helm values should be enough to enable/disable certain features. |
|
@nojnhuh @pohly we have a community dra driver that uses feature gates: https://github.com/kubernetes-sigs/dra-driver-nvidia-gpu/tree/main/pkg/featuregates The example dra driver is supposed to be an example of how to implement the dra driver. For features like vfio driver and enabling discoverable metadata, KEP-5304, the dra-driver-nvidia-gpu uses feature gates. Is there any value in aligning both of them either to profile or feature-gate? My main reason for having feature gates was because inspired from the DRA Driver for NVIDIA GPUs. |
|
The mechanism to toggle features isn't an integral part of how DRA drivers should be implemented IMO. It's ok if different drivers do that in different ways. If anything, the example driver and the NVIDIA driver doing different things can help reinforce that and empower other drivers to do what works for them. |
|
I agree with @nojnhuh. The example driver has to support different "personas". I think profiles work well for that. A production driver may only have a single persona. Then boolean flags to toggle something can make sense. Those don't have to be feature gates, a plain boolean flag or config option can be better choice because those can be associated with a dedicated help text. |
k8s-ci-robot
added
the
needs-rebase
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
6 participants
Adds a shared feature gate support and uses VFIOGPUEnabled to select the active driver profile at runtime.