Remove the init container used for NRI enablement

[fmuyassarov]

NRI has been supported and enabled by default for quite some time (since CRI-O 1.30 and containerd 2.0). Given this, it should be safe to remove the initContainer currently used to enable NRI at runtime in all installations. At the moment, the initContainer only enables NRI for containerd. This creates a problem for CRI-O based setups, where the init container will fail and does not provide any benefit. Additionally, I tend to believe that enabling NRI is typically the job of admin, rather than something that should be handled within the app running on k8s. Based on these points, I would suggest that we drop the initContainer to simplify install.yaml.

[fmuyassarov]

@ffromani @pravk03 any objections?

[ffromani]

No objections, actually this is something I'm tinkering with myself. IMO we should move the configuration stage to a separate, explicit step, e.g. a job or something, which users will run once on their clusters.
Doing so - separating config from runtime - will allow us to drop privileges and mounts in our main pod.

The caveat is that is very likely true that production configuration usually enable all the bits (CDI, NRI...) but this is not necessarily the case for kind and minikube, which are important flows for the DEVEX.
In general, for the CPU feature, we likely don't need any setup, but for the memory feature (#36) we will.

Here is a repo on which I'm playing with test environments and integrating DRA drivers for the low-latency network workflow.
I'm splitting the setup helpers (and rewriting them in go) and running them in a setup job.
ffromani/dra-driver-integration-env@13c115d

I think this is a good model we can iterate on, but I'm open to hear suggestions.

[AutuSnow]

I feel that while implementing this idea, we can also remove the current configurations set in the pod spec, as these configurations come with a high cost in terms of security in production environments

privileged: true
hostPID: true
Mount the host machine's /etc directory

[ffromani]

I feel that while implementing this idea, we can also remove the current configurations set in the pod spec, as these configurations come with a high cost in terms of security in production environments

privileged: true
hostPID: true
Mount the host machine's /etc directory

Absolutely. These are privileges needed only at setup stage, not during normal running operations.

[fmuyassarov]

No objections, actually this is something I'm tinkering with myself. IMO we should move the configuration stage to a separate, explicit step, e.g. a job or something, which users will run once on their clusters.
Doing so - separating config from runtime - will allow us to drop privileges and mounts in our main pod.

Yes, I believe the Job approach will work similarly to the current setup. In this model, the containerd configuration would still be defined within the Pod spec. An alternative approach we’ve used with NRI plugins is to separate the runtime tuning logic into a dedicated binary. This keeps the implementation cleaner and makes it runtime agnostic. That binary can then be packaged as an image and executed via initContainer. In practice, this isn’t a significant diff from the current approach. The main advantage is separation of concerns, specifically, isolating container runtime configuration changes into a standalone binary.

The caveat is that is very likely true that production configuration usually enable all the bits (CDI, NRI...) but this is not necessarily the case for kind and minikube, which are important flows for the DEVEX. In general, for the CPU feature, we likely don't need any setup, but for the memory feature (#36) we will.

True. Minikube has already moved to containerd 2.0 and CRI-O 1.35 (see the linked references: containerd and crio).
It seems the only case that might still require manual enabling is a CRI-O-based Kind setup. Even the latest Kind configuration references containerd v2.

But I think, it might still be beneficial to leave an option to developers to turn on the NRI. This boils down to the way we install the driver. For example, it would be fairly simple with Helm to have a flag to do the job. This could be implemented as part of #83 in.

Here is a repo on which I'm playing with test environments and integrating DRA drivers for the low-latency network workflow. I'm splitting the setup helpers (and rewriting them in go) and running them in a setup job. ffromani/dra-driver-integration-env@13c115d

I think this is a good model we can iterate on, but I'm open to hear suggestions.

Definitely, thanks for sharing.

[fmuyassarov]

Since we are here, this might be somewhat related. But after the separation of manifests in 9eaefd8, I think it is fairly simple commit that we can introduce kustomize based installation + make the runtime modification as an optional layer. WDYT?

[ffromani]

@fmuyassarov note in my repo I'm not using helm for unrelated accidental reasons. I'm very open to use helm here if it makes sense (I just haven't check yet).

For the sake of clarity and posterity, the key points are:

• split one-time(/rarely-used) configuration from actual runtime
• pack the setup helpers in a container image, possibly the same runtime image (tools just sit here idle)
• use a job or any other kubernetes-native construct as much as possible to do the one-time configuration using the setup helpers image.

Basically, it's about remixing the components we already have, with minimal/limited additions. (Re)writing in go helps to smoothen the experience as it is more practical and with better tools than shell, but is not necessary.

[ffromani]

Since we are here, this might be somewhat related. But after the separation of manifests in 9eaefd8, I think it is fairly simple commit that we can introduce kustomize based installation + make the runtime modification as an optional layer. WDYT?

generally very open. My guiding principles are

1. reuse tools as much as possible (kustomize, helm)
2. don't bring complexity/configurations in unless there's a clear use case

post-release we can revisit the manifest situation from the ground up as needed.

[fmuyassarov]

Thanks @ffromani - that’s great to hear.
Based on the discussion above, I’d propose the following next steps. Please let me know if this diverges from your expectations:

1. Remove the init container currently used for configuring the containerd
2. Introduce a Helm chart option to enable NRI support in the runtime, basically on top of feat: add Helm chart for driver deployment #83.

[ffromani]

@fmuyassarov Thank you!
I'm generally favorable pending further review (and @pravk03 's review). For clarity, all of this needs to wait till 0.1.0 is out of the door. We can merge targeted fixes but such big overhauls need to wait.

[fmuyassarov]

@fmuyassarov Thank you!
I'm generally favorable pending further review (and @pravk03 's review). For clarity, all of this needs to wait till 0.1.0 is out of the door. We can merge targeted fixes but such big overhauls need to wait.

Totally understandable, this isn't urgent per say.