-
Notifications
You must be signed in to change notification settings - Fork 2
CSI compatibility mode #175
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
base: main
Are you sure you want to change the base?
Changes from all commits
2aa5c46
34b1e30
b26aa99
92697d7
05cd918
3703159
3c1fef4
2f32d47
8dc17ef
a03c8df
4c59bcd
f46ef40
5d15fee
f6b0a54
c923b5d
96a59fd
00827ca
5b71759
14e2fcc
6f82793
b353e21
2853620
ce762b9
a8ed75e
16445c6
0fe7653
f5bc131
cb98a89
2ca5373
802210a
2c7b98f
5c464af
e6f1909
d5cc0b4
bcf30d3
5d45900
2c876d0
1dc86cd
4051952
64ca6f6
d3e7c7a
cfb6787
e966458
08f1015
2787b75
913f3da
c047568
eef3bdd
5677f76
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| --- | ||
| {{- if not .Values.csi.enableCompatibilityMode }} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we dont deploy the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above. One snapshot-controller is suffice. |
||
| apiVersion: autoscaling.k8s.io/v1 | ||
| kind: VerticalPodAutoscaler | ||
| metadata: | ||
|
|
@@ -15,3 +15,4 @@ spec: | |
| name: {{.Values.prefix}}-csi-snapshot-controller | ||
| updatePolicy: | ||
| updateMode: Auto | ||
| {{- end }} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| {{- if not .Values.csi.enableCompatibilityMode }} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not needed as the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, correct. The leader of the controller is also hard-coded so it wouldn't even bring you anything if you have multiple controllers running. |
||
| apiVersion: apps/v1 | ||
| kind: Deployment | ||
| metadata: | ||
|
|
@@ -70,3 +71,4 @@ spec: | |
| path: token | ||
| name: shoot-access-csi-snapshot-controller | ||
| optional: false | ||
| {{- end }} | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -17,7 +17,11 @@ spec: | |||||
| metadata: | ||||||
| annotations: | ||||||
| checksum/configmap-cloud-provider-config: {{ include (print $.Template.BasePath "/configmap.yaml") . | sha256sum }} | ||||||
| node.gardener.cloud/wait-for-csi-node-stackit: block-storage.csi.stackit.cloud | ||||||
| {{- if eq .Values.driverName "cinder.csi.openstack.org" }} | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could use the
Suggested change
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is then also needed to switch the annotations in the if/else |
||||||
| node.gardener.cloud/wait-for-csi-node-openstack: {{ .Values.driverName }} | ||||||
| {{- else }} | ||||||
| node.gardener.cloud/wait-for-csi-node-stackit: {{ .Values.driverName }} | ||||||
| {{- end }} | ||||||
| labels: | ||||||
| node.gardener.cloud/critical-component: "true" | ||||||
| app: {{ .Values.prefix }}-csi | ||||||
|
|
@@ -48,6 +52,9 @@ spec: | |||||
| {{- end }} | ||||||
| - --v=2 | ||||||
| - --provide-controller-service=false | ||||||
| {{- if .Values.csi.enableCompatibilityMode }} | ||||||
| - --legacy-storage-mode=true | ||||||
| {{- end }} | ||||||
| env: | ||||||
| - name: CSI_ENDPOINT | ||||||
| value: unix://{{ .Values.socketPath }} | ||||||
|
|
@@ -62,7 +69,7 @@ spec: | |||||
| allowPrivilegeEscalation: true | ||||||
| ports: | ||||||
| - name: healthz | ||||||
| containerPort: 9908 | ||||||
| containerPort: {{ .Values.healthzPort }} | ||||||
| protocol: TCP | ||||||
| livenessProbe: | ||||||
| httpGet: | ||||||
|
|
@@ -94,7 +101,7 @@ spec: | |||||
| - name: ADDRESS | ||||||
| value: {{ .Values.socketPath }} | ||||||
| - name: DRIVER_REG_SOCK_PATH | ||||||
| value: /var/lib/kubelet/plugins/block-storage.csi.stackit.cloud/csi.sock | ||||||
| value: /var/lib/kubelet/plugins/{{ .Values.driverName }}/csi.sock | ||||||
| {{- if .Values.resources.nodeDriverRegistrar }} | ||||||
| resources: | ||||||
| {{ toYaml .Values.resources.nodeDriverRegistrar | indent 10 }} | ||||||
|
|
@@ -112,7 +119,7 @@ spec: | |||||
| args: | ||||||
| - --probe-timeout=3m | ||||||
| - --csi-address={{ .Values.socketPath }} | ||||||
| - --health-port=9908 | ||||||
| - --health-port={{ .Values.healthzPort }} | ||||||
| {{- if .Values.resources.livenessProbe }} | ||||||
| resources: | ||||||
| {{ toYaml .Values.resources.livenessProbe | indent 10 }} | ||||||
|
|
@@ -130,7 +137,7 @@ spec: | |||||
| type: Directory | ||||||
| - name: plugin-dir | ||||||
| hostPath: | ||||||
| path: /var/lib/kubelet/plugins/block-storage.csi.stackit.cloud/ | ||||||
| path: /var/lib/kubelet/plugins/{{ .Values.driverName }}/ | ||||||
| type: DirectoryOrCreate | ||||||
| - name: registration-dir | ||||||
| hostPath: | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we dont deploy the
PodDisruptionBudgetwhen enable the CompatibilityModeThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dergeberl We only need one snapshot controller per cluster. It would just be duplicated doing nothing.