HDDS-11597. Fix persistent feature in helm chart StatefulSet

[pyttel]

What changes were proposed in this pull request?

HDDS-11597. Fixed the helm chart persistent feature of helm chart.

• What changes are proposed in the PR? and Why? It would be better if it is written from third person's
  perspective not just for the reviewer.

  There was a bug in the helm chart of PVC. Only one PVC for all replicas of each component (datanode, om, scm, s3g) was created if persistent mode was enabled. In addition the acceessModes where introduced because they are mandatory configuration properties.

• Provide as much context and rationale for the pull request as possible. It could be copy-paste from
  the Jira's description if the jira is well defined.

The following error showed up installing the helm chart in persistent mode PersistentVolumeClaim "YXZ-datanode" is invalid: spec.accessModes: Required value: at least 1 access mode is required. While testing the wrong PVC configuration was discovered and fixed. Both modes persistent and non-persistent are still supported.

What is the link to the Apache JIRA

HDDS-11597 Invalid accessModes for ozone helm chart in persistent mode

How was this patch tested?

This was tested manually with Rancher cluster setup on Azure

[adoroszlai]

Thanks @pyttel for the patch. Can you please enable Test Charts workflow in your fork?

@dnskr Can you please review? Also, can Test Charts be improved to exercise with both values for persistence?

[pyttel]

@adoroszlai I have enabled the Test Chart on the fork. Sorry for the confusing commits and reverts. I'm used to other git implementation handling things a bit differently and so I made a mistake while enabling. Finally I pulled from fix branch to main branch in my fork and the test passed. So effectively no changes to the test yaml are made. I did not introduced myself to the content of the test^^

[dnskr]

Could you please the change title to follow common pattern?
HDDS-11597. Fix persistent feature in helm chart StatefulSet

Looks good. Let me test, I'll get back in the upcoming days.

[dnskr]

This change triggers new version release.
@adoroszlai WDYT, should we release a new version of the chart after each PR or should we accumulate few changes and release 0.2.0?

[adoroszlai]

I think we can release 0.1.1 with this fix, and 0.2.0 with support for HA, which is likely going to be a bigger change (this comment mentions "Using more than 1 replicas for OM crashes").

[adoroszlai]

But we should increment version separately, not as part of the fix PR. Changing it in each PR would not scale (would cause conflicts if multiple PRs are in flight).

[pyttel]

Yes this is true. Should I change the version back with simple commit?

[adoroszlai]

I guess we can leave it now, unless some other fix comes before this is merged.

[dnskr]

Why did you changed ConfigMap name?

[pyttel]

I changed it because I used ozone helm chart as dependency chart. When using plain release name, I got a conflict with an already existing configMap of the main chart with {{ .Release.Name }}. This could avoid such conflicts and, in my opinion, this can help to identify the belonging of the configMap faster in such aggregated charts. But it is definitely not needed. I can change this back if you prefer it :)

[dnskr]

Got it. Let's leave {{ .Release.Name }}-ozone name for now.

[dnskr]

Tested pyttel@49b6e74 on local minikube instance:

helm install ozone charts/ozone --set datanode.persistence.enabled=true --set om.persistence.enabled=true --set s3g.persistence.enabled=true --set scm.persistence.enabled=true

Pods created:

kubectl get pod
NAME               READY   STATUS    RESTARTS   AGE
ozone-datanode-0   1/1     Running   0          55s
ozone-datanode-1   1/1     Running   0          52s
ozone-datanode-2   1/1     Running   0          48s
ozone-om-0         1/1     Running   0          55s
ozone-s3g-0        1/1     Running   0          55s
ozone-scm-0        1/1     Running   0          55s

PVCs created:

kubectl get pvc
NAME                              STATUS   VOLUME                                     CAPACITY   ACCESS MODES   STORAGECLASS   VOLUMEATTRIBUTESCLASS   AGE
ozone-datanode-ozone-datanode-0   Bound    pvc-0bd02b04-c61d-43e5-b304-1154d6fa46d9   10Gi       RWO            standard       <unset>                 89s
ozone-datanode-ozone-datanode-1   Bound    pvc-8b6eccea-9962-4387-8568-1e0b417c41f3   10Gi       RWO            standard       <unset>                 86s
ozone-datanode-ozone-datanode-2   Bound    pvc-0199a82f-17d8-4fad-acd8-a0f249c6ebbf   10Gi       RWO            standard       <unset>                 82s
ozone-om-ozone-om-0               Bound    pvc-d05e9aff-defb-45a8-abd4-fd5bb1670aa9   10Gi       RWO            standard       <unset>                 89s
ozone-s3g-ozone-s3g-0             Bound    pvc-e0dcae98-bdc4-4527-82ef-37dca6e5e141   10Gi       RWO            standard       <unset>                 89s
ozone-scm-ozone-scm-0             Bound    pvc-cc0ec139-2335-4c84-bcea-7ff09e4cdbd5   10Gi       RWO            standard       <unset>                 89

[adoroszlai]

Thanks @pyttel for the fix, @dnskr for testing and reviewing it.