helm: support setting storageclass volumeBindingMode in rook-ceph-cluster helm chart

[bdun1013]

Description of your changes:

Added the ability to configure the volumeBindingMode for the block, file, and object StoageClass resources in the rook-ceph-cluster helm chart.

Which issue is resolved by this Pull Request: Resolves #11011

Checklist:

• [x] Commit Message Formatting: Commit titles and messages follow guidelines in the developer guide).
• [ ] Skip Tests for Docs: If this is only a documentation change, add the label skip-ci on the PR.
• [x] Reviewed the developer guide on Submitting a Pull Request
• [ ] Pending release notes updated with breaking and/or notable changes for the next minor release.
• [ ] Documentation has been updated, if necessary.
• [ ] Unit tests have been added, if necessary.
• [ ] Integration tests have been added, if necessary.

[travisn]

Note:- this field is immutable and cannot be changed after creation. If someone is using other bindingMode than Immediate then need to take care of setting the right value.

Correct, if the storage class fields change, i'd expect the upgrades of the helm chart to fail. But since the helm upgrade tests are passing, perhaps there is not a problem since Immediate is already K8s' default value. @bdun1013 Could you run a quick helm upgrade test with this and see if changing the default causes an upgrade failure? I think that's fine since it will be a corner case for someone to set that policy and they might not even do it during an upgrade, we just need to make sure we understand upgrade behavior, thanks.

[travisn]

@Mergifyio rebase

[mergify[bot]]

rebase

✅ Branch has been successfully rebased

[travisn]

Note:- this field is immutable and cannot be changed after creation. If someone is using other bindingMode than Immediate then need to take care of setting the right value.

Correct, if the storage class fields change, i'd expect the upgrades of the helm chart to fail. But since the helm upgrade tests are passing, perhaps there is not a problem since Immediate is already K8s' default value. @bdun1013 Could you run a quick helm upgrade test with this and see if changing the default causes an upgrade failure? I think that's fine since it will be a corner case for someone to set that policy and they might not even do it during an upgrade, we just need to make sure we understand upgrade behavior, thanks.

@bdun1013 This would be good to get merged, were you just able to double check the upgrade behavior?

[bdun1013]

@travisn Sorry for the delay. I did verify that an upgrade works as long as the volumeBindingMode is not changed from Immediate. If the volumeBindingMode is set to something other than Immediate, an update will fail because volumeBindingMode is immutable as @Madhu-1 mentioned.

These changes will allow a fresh install to change the volumeBindingMode of the StorageClass resources to something other than Immediate.

[travisn]

@parth-gr Any findings on #11398 yet? It would be good to get this in soon with that fix.

[github-actions[bot]]

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

[subhamkrai]

@parth-gr Any findings on #11398 yet? It would be good to get this in soon with that fix.

@parth-gr any update?

[parth-gr]

@parth-gr Any findings on #11398 yet? It would be good to get this in soon with that fix.

@parth-gr any update?

Looking

[github-actions[bot]]

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

[galexrt]

If the volumeBindingMode is set to something other than Immediate, an update will fail because volumeBindingMode is immutable as @Madhu-1 mentioned.

In my eyes it is reasonable to allow users to change the option as Helm will tell them during an upgrade when they would change this option on an object. This would cause the update to fail but with a "good" error message informing the user. The default from Kubernetes is Immediate anyways so we should be good to go here with adding this to the Helm chart (as long as the docs state that it can't be changed later on, maybe even a link to the Kubernetes docs where it describes this behavior?).

@Madhu-1 Could you rebase the PR, I seem to be unable to restart the failed CI.

[travisn]

It's rebased now, sounds good to go ahead and merge since the default value isn't expected to cause any issues during upgrade.