Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Helm compatibility for --reuse-values upgrades #1789

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions charts/aws-ebs-csi-driver/templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ Convert the `--extra-tags` command line arg from a map.
*/}}
{{- define "aws-ebs-csi-driver.extra-volume-tags" -}}
{{- $result := dict "pairs" (list) -}}
{{- range $key, $value := .Values.controller.extraVolumeTags -}}
{{- range $key, $value := (.Values.controller).extraVolumeTags -}}
{{- $noop := printf "%s=%v" $key $value | append $result.pairs | set $result "pairs" -}}
{{- end -}}
{{- if gt (len $result.pairs) 0 -}}
Expand All @@ -77,9 +77,9 @@ Handle http proxy env vars
*/}}
{{- define "aws-ebs-csi-driver.http-proxy" -}}
- name: HTTP_PROXY
value: {{ .Values.proxy.http_proxy | quote }}
value: {{ (.Values.proxy).http_proxy | quote }}
- name: HTTPS_PROXY
value: {{ .Values.proxy.http_proxy | quote }}
value: {{ (.Values.proxy).http_proxy | quote }}
- name: NO_PROXY
value: {{ .Values.proxy.no_proxy | quote }}
value: {{ (.Values.proxy).no_proxy | quote }}
{{- end -}}
126 changes: 91 additions & 35 deletions charts/aws-ebs-csi-driver/templates/_node-windows.tpl
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
{{- define "node-windows" }}
{{- if .Values.node.enableWindows }}
{{- if (.Values.node).enableWindows }}
---
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This --- is just for styling yes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it's for separating multiple YAML documents in the same file. It used to be inside node.yaml and node-windows.yaml but I moved it here so it's only rendered if the relevant DaemonSet is enabled, otherwise extra ---s will be rendered when the DaemonSet is disabled.

kind: DaemonSet
apiVersion: apps/v1
metadata:
Expand All @@ -12,54 +13,73 @@ spec:
matchLabels:
app: {{ .NodeName }}
{{- include "aws-ebs-csi-driver.selectorLabels" . | nindent 6 }}
{{- with (.Values.node).updateStrategy }}
updateStrategy:
{{ toYaml .Values.node.updateStrategy | nindent 4 }}
{{- toYaml . | nindent 4 }}
{{- else }}
updateStrategy:
type: RollingUpdate
rollingUpdate:
maxUnavailable: "10%"
{{- end }}
template:
metadata:
labels:
app: {{ .NodeName }}
{{- include "aws-ebs-csi-driver.labels" . | nindent 8 }}
{{- if .Values.node.podLabels }}
{{- toYaml .Values.node.podLabels | nindent 8 }}
{{- with (.Values.node).podLabels }}
{{- toYaml . | nindent 8 }}
{{- end }}
{{- with .Values.node.podAnnotations }}
{{- with (.Values.node).podAnnotations }}
annotations:
{{- toYaml . | nindent 8 }}
{{- end }}
spec:
{{- with .Values.node.affinity }}
affinity: {{- toYaml . | nindent 8 }}
{{- with (.Values.node).affinity }}
affinity:
{{- toYaml . | nindent 8 }}
{{- else }}
affinity:
nodeAffinity:
requiredDuringSchedulingIgnoredDuringExecution:
nodeSelectorTerms:
- matchExpressions:
- key: eks.amazonaws.com/compute-type
operator: NotIn
values:
- fargate
{{- end }}
nodeSelector:
kubernetes.io/os: windows
{{- with .Values.node.nodeSelector }}
{{- with (.Values.node).nodeSelector }}
{{- toYaml . | nindent 8 }}
{{- end }}
serviceAccountName: {{ .Values.node.serviceAccount.name }}
priorityClassName: {{ .Values.node.priorityClassName | default "system-node-critical" }}
serviceAccountName: {{ default "ebs-csi-node-sa" ((.Values.node).serviceAccount).name }}
priorityClassName: {{ default "system-node-critical" (.Values.node).priorityClassName }}
tolerations:
{{- if .Values.node.tolerateAllTaints }}
{{- if default true (.Values.node).tolerateAllTaints }}
- operator: Exists
{{- else }}
{{- with .Values.node.tolerations }}
{{- with (.Values.node).tolerations }}
{{- toYaml . | nindent 8 }}
{{- end }}
- key: "ebs.csi.aws.com/agent-not-ready"
operator: "Exists"
{{- end }}
hostNetwork: {{ default "false" (.Values.node).hostNetwork }}
containers:
- name: ebs-plugin
image: {{ printf "%s%s:%s" (default "" .Values.image.containerRegistry) .Values.image.repository (default (printf "v%s" .Chart.AppVersion) (toString .Values.image.tag)) }}
imagePullPolicy: {{ .Values.image.pullPolicy }}
image: {{ printf "%s%s:%s" (default "" (.Values.image).containerRegistry) (default "public.ecr.aws/ebs-csi-driver/aws-ebs-csi-driver" (.Values.image).repository) (toString (default (printf "v%s" .Chart.AppVersion) (.Values.image).tag)) }}
imagePullPolicy: {{ default "IfNotPresent" (.Values.image).pullPolicy }}
args:
- node
- --endpoint=$(CSI_ENDPOINT)
{{- with .Values.node.volumeAttachLimit }}
{{- with (.Values.node).volumeAttachLimit }}
- --volume-attach-limit={{ . }}
{{- end }}
{{- with .Values.node.loggingFormat }}
- --logging-format={{ . }}
{{- end }}
- --v={{ .Values.node.logLevel }}
{{- if .Values.node.otelTracing }}
- --logging-format={{ default "text" (.Values.node).loggingFormat }}
- --v={{ default "2" (.Values.node).logLevel }}
{{- if (.Values.node).otelTracing }}
- --enable-otel-tracing=true
{{- end}}
env:
Expand All @@ -69,18 +89,22 @@ spec:
valueFrom:
fieldRef:
fieldPath: spec.nodeName
{{- if .Values.proxy.http_proxy }}
{{- if (.Values.proxy).http_proxy }}
{{- include "aws-ebs-csi-driver.http-proxy" . | nindent 12 }}
{{- end }}
{{- with .Values.node.otelTracing }}
{{- with (.Values.node).otelTracing }}
- name: OTEL_SERVICE_NAME
value: {{ .otelServiceName }}
- name: OTEL_EXPORTER_OTLP_ENDPOINT
value: {{ .otelExporterEndpoint }}
{{- end }}
{{- with .Values.node.env }}
{{- with (.Values.node).env }}
{{- . | toYaml | nindent 12 }}
{{- end }}
{{- with (.Values.node).envFrom }}
envFrom:
{{- . | toYaml | nindent 12 }}
{{- end }}
volumeMounts:
- name: kubelet-dir
mountPath: C:\var\lib\kubelet
Expand All @@ -105,9 +129,16 @@ spec:
timeoutSeconds: 3
periodSeconds: 10
failureThreshold: 5
{{- with .Values.node.resources }}
{{- with (.Values.node).resources }}
resources:
{{- toYaml . | nindent 12 }}
{{- else }}
resources:
requests:
cpu: 10m
memory: 40Mi
limits:
memory: 256Mi
{{- end }}
securityContext:
windowsOptions:
Expand All @@ -117,23 +148,27 @@ spec:
exec:
command: ["/bin/aws-ebs-csi-driver", "pre-stop-hook"]
- name: node-driver-registrar
image: {{ printf "%s%s:%s" (default "" .Values.image.containerRegistry) .Values.sidecars.nodeDriverRegistrar.image.repository .Values.sidecars.nodeDriverRegistrar.image.tag }}
imagePullPolicy: {{ default .Values.image.pullPolicy .Values.sidecars.nodeDriverRegistrar.image.pullPolicy }}
image: {{ printf "%s%s:%s" (default "" (.Values.image).containerRegistry) (default "public.ecr.aws/eks-distro/kubernetes-csi/node-driver-registrar" (((.Values.sidecars).nodeDriverRegistrar).image).repository) (default "v2.9.0-eks-1-28-6" (((.Values.sidecars).nodeDriverRegistrar).image).tag) }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm against putting default sidecar tags in the chart that we would have to risk changing every release.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like this default value is not relevant to the chart tester CI job failing: Additional DaemonSets feature #1722

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dislike it, but all default values (including tags) must be duplicated into the chart for this support to work. Otherwise, users upgrading with --reuse-values will end up with an inconsistent (or in the case of tags, broken) experience.

imagePullPolicy: {{ default (default "IfNotPresent" (.Values.image).pullPolicy) (((.Values.sidecars).nodeDriverRegistrar).image).pullPolicy }}
args:
- --csi-address=$(ADDRESS)
- --kubelet-registration-path=$(DRIVER_REG_SOCK_PATH)
- --v={{ .Values.sidecars.nodeDriverRegistrar.logLevel }}
- --v={{ default "2" ((.Values.sidecars).nodeDriverRegistrar).logLevel }}
env:
- name: ADDRESS
value: unix:/csi/csi.sock
- name: DRIVER_REG_SOCK_PATH
value: C:\var\lib\kubelet\plugins\ebs.csi.aws.com\csi.sock
{{- if .Values.proxy.http_proxy }}
{{- if (.Values.proxy).http_proxy }}
{{- include "aws-ebs-csi-driver.http-proxy" . | nindent 12 }}
{{- end }}
{{- with .Values.sidecars.nodeDriverRegistrar.env }}
{{- with ((.Values.sidecars).nodeDriverRegistrar).env }}
{{- . | toYaml | nindent 12 }}
{{- end }}
{{- with (.Values.node).envFrom }}
envFrom:
{{- . | toYaml | nindent 12 }}
{{- end }}
livenessProbe:
exec:
command:
Expand All @@ -150,25 +185,46 @@ spec:
mountPath: C:\registration
- name: probe-dir
mountPath: C:\var\lib\kubelet\plugins\ebs.csi.aws.com
{{- with default .Values.node.resources .Values.sidecars.nodeDriverRegistrar.resources }}
{{- with default (.Values.node).resources ((.Values.sidecars).nodeDriverRegistrar).resources }}
resources:
{{- toYaml . | nindent 12 }}
{{- else }}
resources:
requests:
cpu: 10m
memory: 40Mi
limits:
memory: 256Mi
{{- end }}
- name: liveness-probe
image: {{ printf "%s%s:%s" (default "" .Values.image.containerRegistry) .Values.sidecars.livenessProbe.image.repository .Values.sidecars.livenessProbe.image.tag }}
imagePullPolicy: {{ default .Values.image.pullPolicy .Values.sidecars.livenessProbe.image.pullPolicy }}
image: {{ printf "%s%s:%s" (default "" (.Values.image).containerRegistry) (default "public.ecr.aws/eks-distro/kubernetes-csi/livenessprobe" (((.Values.sidecars).livenessProbe).image).repository) (default "v2.10.0-eks-1-28-6" (((.Values.sidecars).livenessProbe).image).tag) }}
imagePullPolicy: {{ default (default "IfNotPresent" (.Values.image).pullPolicy) (((.Values.sidecars).livenessProbe).image).pullPolicy }}
args:
- --csi-address=unix:/csi/csi.sock
{{- range ((.Values.sidecars).livenessProbe).additionalArgs }}
- {{ . }}
{{- end }}
{{- with (.Values.node).envFrom }}
envFrom:
{{- . | toYaml | nindent 12 }}
{{- end }}
volumeMounts:
- name: plugin-dir
mountPath: C:\csi
{{- with default .Values.node.resources .Values.sidecars.livenessProbe.resources }}
{{- with default (.Values.node).resources ((.Values.sidecars).livenessProbe).resources }}
resources:
{{- toYaml . | nindent 12 }}
{{- else }}
resources:
requests:
cpu: 10m
memory: 40Mi
limits:
memory: 256Mi
{{- end }}
{{- if .Values.imagePullSecrets }}
{{- with .Values.imagePullSecrets }}
imagePullSecrets:
{{- range .Values.imagePullSecrets }}
{{- range . }}
- name: {{ . }}
{{- end }}
{{- end }}
Expand Down
Loading