Helm Chart: statefulset template, broken logic

[loft-sh]

Helm Chart: statefulset template, broken logic

[vsantos]

@matskiv I can try to fix this minor issue if that's okay (especially if I get a few days to work on it). I didn't find the mentioned main/chart/templates/statefulset.yaml#L32 repo but only the main/charts/**/templates one.

This is an excellent issue to be worked on before diving into the actual vcluster's logic.

[matskiv]

@vsantos Oh, I would love to have automated tests for the charts. This has been on my todo list for a while, but it always gets pushed back 😂 I would try to first look for some tools tailored to Helm, to have sort of a "unit tests" for the Helm charts.
But this should be a separate issue for sure :)

[hiteshwani29]

@matskiv I would like to contribute to this issue, could you please assign it to me?

[matskiv]

@hiteshwani29 Thank you for your interest. It's assigned to you now.

[FabianKramm]

@jburnitz thanks for creating this issue! Yeah I guess the best solution is that we only define storageClassName if a name was defined in the values

[matskiv]

This is the same in the chart, so we should fix it.

As for this sentence from the description:

Define a default in values "Filesystem" is the inferred value when undefined per: https://github.com/kubernetes/api/blob/release-1.21/core/v1/types.go#L489

The default value of "Filesystem" is for the volumeMode field, which we don't set.

[matskiv]

@vsantos Yeah, absolutely! I'll assign this to you then. You can take your time, there is no rush with this issue.

---

I didn't find the mentioned main/chart/templates/statefulset.yaml#L32 repo but only the main/charts/**/templates one.

Yeah, we added more charts since this was reported 😄 I would say that the issue applies to all storageClassName references in all charts/**.

---

btw: a tip for quick validation. First time I was testing some chart changes I did in a way too complicated way, so sharing just in case.

helm template vcluster ./charts/k3s/ --set storage.className=test | grep storageClassName
helm template vcluster ./charts/k8s/ | grep storageClassName

I used a very similar command to this just the other day, it's very helpful for quickly checking the output of the chart.
The first one tests k3s chart with .Values.storage.className set. Second tests k8s chart without the .Values.storage.className value. And then I grep all outputted manifests... you get the idea :)

[vsantos]

@matskiv Do you guys think is a good idea to add some kind of "automated" mechanism to make it easier to test static YAML files as part of the scope of this PR? We could use bats, it's a very old tool but good in make assertions in bash scripts. Your suggested logic to test locally could be automated to ensure that a given values.yaml will return specific structures.

Or maybe we should just fix the broken logic and in the future start a new thread about automation alternatives?