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.

Sign up for GitHub

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

[chart] Allow resources override for node DaemonSet + priorityClassName #732

Merged
merged 2 commits into from
Feb 11, 2021
Merged

[chart] Allow resources override for node DaemonSet + priorityClassName #732

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f3a57d3
[chart] Allow resources override for node DaemonSet
dntosas Feb 9, 2021
5a03305
[chart] Ability to add priorityClassName
dntosas Feb 9, 2021
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
2 changes: 1 addition & 1 deletion charts/aws-ebs-csi-driver/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ apiVersion: v1
appVersion: "0.9.0"
name: aws-ebs-csi-driver
description: A Helm chart for AWS EBS CSI Driver
version: 0.9.4
version: 0.9.5
kubeVersion: ">=1.17.0-0"
home: https://github.com/kubernetes-sigs/aws-ebs-csi-driver
sources:
Expand Down
2 changes: 1 addition & 1 deletion charts/aws-ebs-csi-driver/templates/controller.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ spec:
{{ toYaml . | indent 8 }}
{{- end }}
serviceAccountName: {{ .Values.serviceAccount.controller.name }}
priorityClassName: system-cluster-critical
priorityClassName: {{ .Values.priorityClassName | default "system-cluster-critical" }}
{{- with .Values.affinity }}
affinity: {{ toYaml . | nindent 8 }}
{{- end }}
Expand Down
20 changes: 19 additions & 1 deletion charts/aws-ebs-csi-driver/templates/node.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ spec:
{{- end }}
hostNetwork: true
serviceAccountName: {{ .Values.serviceAccount.node.name }}
priorityClassName: system-node-critical
priorityClassName: {{ .Values.node.priorityClassName | default "system-cluster-critical" }}
tolerations:
{{- if .Values.node.tolerateAllTaints }}
- operator: Exists
Expand Down Expand Up @@ -80,9 +80,15 @@ spec:
timeoutSeconds: 3
periodSeconds: 10
failureThreshold: 5
{{- if .Values.node.resources }}
{{- with .Values.node.resources }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not a big fan of this change. This would be a breaking change and force the operators to update their setup. Can we somehow give this new value priority and if it doesn't exist fallback to .Values.resources? We can add a deprecation note for the old field on the values file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch ^^

Pushed some changes, adding a condition so the additions would be backwards compatible. Tell me if something like that would work for you :)

resources: {{ toYaml . | nindent 12 }}
{{- end }}
{{- else }}
{{- with .Values.resources }}
resources: {{ toYaml . | nindent 12 }}
{{- end }}
{{- end }}
- name: node-driver-registrar
image: {{ printf "%s:%s" .Values.sidecars.nodeDriverRegistrarImage.repository .Values.sidecars.nodeDriverRegistrarImage.tag }}
args:
Expand All @@ -103,19 +109,31 @@ spec:
mountPath: /csi
- name: registration-dir
mountPath: /registration
{{- if .Values.node.resources }}
{{- with .Values.node.resources }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly here. I understand this makes more sense, but we still have to make sure we don't break existing setups without any warning.

resources: {{ toYaml . | nindent 12 }}
{{- end }}
{{- else }}
{{- with .Values.resources }}
resources: {{ toYaml . | nindent 12 }}
{{- end }}
{{- end }}
- name: liveness-probe
image: {{ printf "%s:%s" .Values.sidecars.livenessProbeImage.repository .Values.sidecars.livenessProbeImage.tag }}
args:
- --csi-address=/csi/csi.sock
volumeMounts:
- name: plugin-dir
mountPath: /csi
{{- if .Values.node.resources }}
{{- with .Values.node.resources }}
resources: {{ toYaml . | nindent 12 }}
{{- end }}
{{- else }}
{{- with .Values.resources }}
resources: {{ toYaml . | nindent 12 }}
{{- end }}
{{- end }}
volumes:
- name: kubelet-dir
hostPath:
Expand Down
2 changes: 1 addition & 1 deletion charts/aws-ebs-csi-driver/templates/statefulset.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ spec:
{{- with .Values.nodeSelector }}
{{ toYaml . | indent 8 }}
{{- end }}
priorityClassName: system-cluster-critical
priorityClassName: {{ .Values.priorityClassName | default "system-cluster-critical" }}
{{- with .Values.affinity }}
affinity: {{ toYaml . | nindent 8 }}
{{- end }}
Expand Down
5 changes: 3 additions & 2 deletions charts/aws-ebs-csi-driver/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,10 @@ resources:
# cpu: 100m
# memory: 128Mi

priorityClassName: ""
nodeSelector: {}

tolerateAllTaints: true
tolerations: []

affinity: {}

# Extra volume tags to attach to each dynamically provisioned volume.
Expand All @@ -87,10 +86,12 @@ k8sTagClusterId: ""
region: ""

node:
priorityClassName: ""
nodeSelector: {}
podAnnotations: {}
tolerateAllTaints: true
tolerations: []
resources: {}

serviceAccount:
controller:
Expand Down