Helm chart should not upgrade Storage classes and snapshot classes

[travisn]

Is this a bug report or feature request?

• Bug Report

Deviation from expected behavior:
The cluster helm chart attempts to upgrade storage classes when their properties change, which causes a failure such as:

StorageClass.storage.k8s.io "ceph-filesystem" is invalid: parameters: 
Forbidden: updates to parameters are forbidden.

Expected behavior:
Storage classes and snapshot classes are immutable. If their properties change, they should not be updated with the helm chart upgrade.

We should be able to accomplish this with the lookup function. If the lookup of the storage class says it doesn't exist, then we create it. Otherwise, we skip the update. Here is an example of using the lookup to see if a resource exists.

How to reproduce it (minimal and precise):

1. Install the cluster helm chart with a storage class
2. Change a helm property that causes and update in the storage class
3. Upgrade the helm chart
4. See the helm chart failure since the storage class is immutable

[BlaineEXE]

Counterpoint: it seems to me that it's little beyond the appropriate scope of the Rook Helm charts to create storageclasses at all.

[parth-gr]

I can take it up if it's not urgent,
Would be a great start to get my hands dirty with Helm

[travisn]

Counterpoint: it seems to me that it's little beyond the appropriate scope of the Rook Helm charts to create storageclasses at all.

Just from the perspective that storage classes are immutable? Otherwise, it seems very nice to parameterize the storage classes with the few fields that anyone ever needs to change such as the namespace.

[parth-gr]

@travisn
The doc says Keep in mind that Helm is not supposed to contact the Kubernetes API Server during a helm template or a helm install|upgrade|delete|rollback --dry-run, so the lookup function will return an empty list (i.e. dict) in such a case.

We should not use it while upgrading but this sc setting would need to be checked while helm upgrade.right?

[parth-gr]

The lookup cmd example cronjob with helm https://www.youtube.com/watch?v=_oSrLtaE9Sw
https://mkdev.me/posts/lookup-kubernetes-resources-inside-helm-charts

[parth-gr]

Alternatives that can be used,

--reuse-values                               when upgrading, reuse the last release's values and merge in any overrides from the command line via --set and -f. If '--reset-values' is specified, this is ignored

While doing helm upgrade,

Or maybe using post-upgrae Hooks https://helm.sh/docs/topics/charts_hooks/#the-available-hooks

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.

[github-actions]

This issue has been automatically closed due to inactivity. Please re-open if this still requires investigation.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.

[github-actions]

This issue has been automatically closed due to inactivity. Please re-open if this still requires investigation.

[parth-gr]

Still good to investigate