Add PodSecurityContext support for DaemonSets#2120
Conversation
faganihajizada
requested review from
ArangoGutierrez,
cdesiniotis,
karthikvetrivel,
shivamerla and
tariq1890
as code owners
faganihajizada
force-pushed
the
feat/custom-labels-and-security-context
branch
from
57fee73 to
cc94407
Compare
deployments/gpu-operator/charts/node-feature-discovery/values.yaml
Outdated
Show resolved
Hide resolved
faganihajizada
force-pushed
the
feat/custom-labels-and-security-context
branch
2 times, most recently
from
7b5211a to
0e21f29
Compare
|
We need to add this to nvidiadriver CR as well if we are adding it for clusterpolicy. Basically here: https://github.com/NVIDIA/gpu-operator/blob/main/api/nvidia/v1alpha1/nvidiadriver_types.go#L199 After that, we need to update the nvidia-driver specific daemonset manifest to also include securityContext if set here: https://github.com/NVIDIA/gpu-operator/blob/main/manifests/state-driver/0500_daemonset.yaml#L86 |
|
@faganihajizada Thanks again for the contribution! Can you amend the commit message and PR title so that it reflects the changes after review? Please commit with the |
faganihajizada
force-pushed
the
feat/custom-labels-and-security-context
branch
from
0e21f29 to
23df463
Compare
faganihajizada
changed the title
Thanks @tariq1890 I updated commit message and also addressed comment from @rahulait |
faganihajizada
force-pushed
the
feat/custom-labels-and-security-context
branch
from
23df463 to
5013497
Compare
|
/ok to test 5013497 |
|
@rahulait seems like there's no mechanism to regenerate the golden test files in I can add a small |
|
@faganihajizada unit-tests will need updates so that CI can pass. I think they are generated manually for now as I don't see any script doing that. You can just update the failing test for now to match the new fields added. |
Allow users to set pod-level security context for DaemonSet pods via spec.daemonsets.podSecurityContext (ClusterPolicy) and spec.podSecurityContext (NVIDIADriver CR). Kubernetes applies PodSecurityContext as defaults to all containers and init containers in the pod (e.g. runAsUser, runAsGroup, runAsNonRoot, fsGroup).
faganihajizada
force-pushed
the
feat/custom-labels-and-security-context
branch
from
5013497 to
e13fd80
Compare
|
/ok to test e13fd80 |
| fieldPath: status.hostIP | ||
| - name: DRIVER_CONFIG_DIGEST | ||
| value: "3041669332" | ||
| value: "4192128261" |
There was a problem hiding this comment.
I am a bit concerned that the DRIVER_CONFIG_DIGEST is changed here even though we don't set the .Driver.Spec.PodSecurityContext field. Is this because the struct schema has changed?
We need to keep in mind that this change will cancel fast path re-deployments where is was previously possible.
There was a problem hiding this comment.
can I help on this?
There was a problem hiding this comment.
Taking a look at this. No action required on your side right now @faganihajizada, thanks!
4 participants
Description
Partially fixes #1030
Adds
PodSecurityContextsupport to both ClusterPolicy and NVIDIADriver CRs:spec.daemonsets.podSecurityContextsets pod-level security context for all DaemonSet pods managed by ClusterPolicyspec.podSecurityContextsets pod-level security context for the nvidia-driver DaemonSet podDesign notes
applyCommonDaemonsetConfig. No embedded DaemonSet manifest currently sets a pod-level SecurityContext, so there is nothing to lose on replace.Checklist
make lint)make validate-generated-assets)make validate-modules)Testing
mergeSecurityContext(5 cases: nil target, nil defaults, empty target, merge with existing, no-overwrite)applyCommonDaemonsetConfig(4 new cases: podSecurityContext, nil container SecurityContext, merge into existing, initContainers)applyDaemonsetMetadataToPod(4 cases: no-op, label skip, annotations, nil labels)applyCommonDaemonsetMetadata(existing tests still pass)