Adding a helm chart #81
Conversation
create priority class
|
@almahmoud That looks great :) The priorityClass should be The reason for that is how kubelet and the operating system treat pods with this priority class. If you for example run out of memory, you want to be certain that the csi pods are not terminated. This would lead to broken mounts in your application pods. |
|
Changed! Let me know if there's anything else! |
| # incremented each time you make changes to the application. Versions are not expected to | ||
| # follow Semantic Versioning. They should reflect the version the application is using. | ||
| # It is recommended to use it with quotes. | ||
| appVersion: "0.8.0" |
There was a problem hiding this comment.
why are we starting with 0.8.0?
There was a problem hiding this comment.
Tbh I wasn't sure how you're doing versioning, so happy to let you make the decision there!
There was a problem hiding this comment.
ah wait, is this the helm chart version or the csi gcs version?
There was a problem hiding this comment.
appVersion is the CSI GCS version, version is the chart version, which some people choose to version together, others choose to do separately, depending on how much out-of-band development there might be on the chart independent of features in the app I guess?
There was a problem hiding this comment.
Ok, I think versioning that together makes sense. @ofek We need to make sure to change this on releases.
| apiVersion: apps/v1 | ||
| kind: DaemonSet | ||
| metadata: | ||
| name: {{ include "csi-gcs.fullname" . }} |
There was a problem hiding this comment.
I think we should add the following labels (to most resources):
app.kubernetes.io/name: {{ template "csi-gcs.name" $ }}
app.kubernetes.io/managed-by: {{ $.Release.Service }}
app.kubernetes.io/instance: {{ $.Release.Name }}
app.kubernetes.io/version: {{ template "csi-gcs.version" $ }}
helm.sh/chart: {{ template "csi-gcs.chart" $ }}There was a problem hiding this comment.
| preemptionPolicy: Never | ||
| value: 1000000 |
There was a problem hiding this comment.
That should probably be configurable if the priority class is editable.
There was a problem hiding this comment.
I can make these and a few more things that are usually configurable (like the image pull policies, extra labels/annotations, etc) later today
|
Would very much like to see this merged :) |
|
Any update on this? |
|
Any reason why not merged yet? |
|
It is not merged yet because there’s outstanding tasks that have not been finished. If anyone feels like finishing the PR, please consider opening your own PR which incorporates this one plus the content of the discussions. |
Having a helm chart makes it easier to add the driver as a dependency to other charts in a k8s deployment stack.
I can also help add testing, auto-packaging, and auto-version bumping as github actions if you wish, as we have for our charts.
Other than codifying the helm chart, one noticeable change is also the customizability of the priority class (and optional creation) as opposed to hardcoding
SystemCriticalso that the chart can be used in a user namespace. This is needed for our project where we isolate users by nodepool and namespace.Many customizations can still be added but I only added the minimum that we needed to start.