[FEATURE] [HELM] Add support to setup a custom serviceAccount #1179
Conversation
Signed-off-by: Marino Borges <[email protected]>
There was a problem hiding this comment.
Hey @marinoborges thanks for the PR! a nice addition for sure, happy to review
| @@ -52,9 +52,13 @@ spec: | |||
| {{- end }} | |||
| securityContext: | |||
| {{- toYaml .Values.podSecurityContext | nindent 8 }} | |||
| {{- if $useServiceAccount }} | |||
There was a problem hiding this comment.
Not sure about this, it could introduce a breaking change , we need to find a way that won't break existing users scripts that may rely on this existing HELM setup
There was a problem hiding this comment.
It's not a breaking change because the existing logic is respected. It replaces this if with another one but still considering $useServiceAccount.
There was a problem hiding this comment.
@ArangoGutierrez did you check what I mentioned?
There was a problem hiding this comment.
PR Overview
This PR adds support for setting up a custom serviceAccount for the NVIDIA device plugin, ensuring backward compatibility. Key changes include adding new conditional checks in multiple templates to use a custom service account if specified, introducing new configuration values in values.yaml, and updating the service account creation logic across all relevant manifests.
Reviewed Changes
| File | Description |
|---|---|
| deployments/helm/nvidia-device-plugin/templates/service-account.yml | Updated the service account creation logic to conditionally use a custom name and annotations if provided. |
| deployments/helm/nvidia-device-plugin/templates/daemonset-gfd.yml | Modified the condition for including serviceAccountName to support a custom service account. |
| deployments/helm/nvidia-device-plugin/templates/daemonset-mps-control-daemon.yml | Adjusted the service account condition and variable assignment to align with custom service account usage. |
| deployments/helm/nvidia-device-plugin/values.yaml | Added new configuration options to control service account creation and custom naming. |
| deployments/helm/nvidia-device-plugin/templates/daemonset-device-plugin.yml | Applied similar changes as in other daemonset manifests for custom service account support. |
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
| @@ -14,6 +14,7 @@ | |||
| {{- if .Values.devicePlugin.enabled }} | |||
| --- | |||
| {{- $options := (include "nvidia-device-plugin.options" . | fromJson) }} | |||
| {{- $useServiceAccount := $options.hasConfigMap }} | |||
There was a problem hiding this comment.
The variable $useServiceAccount is assigned solely from $options.hasConfigMap, which may lead to inconsistent service account creation behavior compared to other templates that consider .Values.serviceAccount.create. Consider updating this assignment to include .Values.serviceAccount.create for consistency.
| {{- $useServiceAccount := $options.hasConfigMap }} | |
| {{- $useServiceAccount := or $options.hasConfigMap .Values.serviceAccount.create }} |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
Fixes #1174 with backward compatibility so this should be a minor change.