Question: Adding Kubernetes priority class to NRI plugin deployment manifests

[ffuerste]

Description

As described in the NRI (security) docs NRI plugins should be considered as part of the container runtime. Therefore, maybe the Kubernetes priority class system-node-critical could be added for the NRI plugin deployment manifests.

Or maybe adding the priority class not in the deployment manifest as a default, but discussing it as an option/recommendation as part of the README of the plugins?

Rationale

Adding the Kubernetes built-in priority class system-node-critical would ensure, that the particular NRI plugin is not evicted (easily) and available when pods are scheduled for which the respective plugin would be responsible for.

E.g. if a pod is created by a user for which a limit for EPC memory is defined, the sgx-epc NRI plugin must be up and running in order to ensure, that the limit is configured correctly as part of the misc cgroup of the container. Currently the sgx-epc NRI plugin supports only creating new containers. However, if containers are already existing when the plugin registers itself to the container runtime, it would need to request an update for existing containers as described here. This use case is currently not supported by the sgx-epc plugin. Therefore, adding the system-node-critical priority class to the deployment manifest for it, could mitigate potential risk in a running system under load.

However, I am not sure, if this makes sense for all currently available plugins or future ones. But it would be great, if we could define a common policy/best practice for it (as suggested by @mythi here).

[mythi]

Therefore, maybe the Kubernetes priority class system-node-critical could be added for the NRI plugin deployment manifests.

@fmuyassarov any thoughts on this? I think it makes sense but before I add it to my deployment, I thought we could align with the direction.

[fmuyassarov]

I don't see reason to say no and that can be made optional in our Helm charts so that whoever is interested can opt in for the feature by setting a flag during the Helm installation. Any objections @kad @klihub @marquiz ?

[kad]

yes, I think it makes sense to add priorityClass