configuring statefulset UpdateStrategy

opensearch-project / opensearch-k8s-operator

OpenSearch Kubernetes Operator

Apache License 2.0

366 stars 192 forks source link

configuring statefulset UpdateStrategy #793

Closed wangshulei098 closed 2 months ago

wangshulei098 commented 2 months ago

Description

this PR add configuring statefulset UpdateStrategy to NodePool. if not set use "OnDelete" as default UpdateStrategy.

Check List

[ ] Commits are signed per the DCO using --signoff
[ ] Unittest added for the new/changed functionality and all unit tests are successful
[ ] Customer-visible features documented
[ ] No linter warnings (make lint)

If CRDs are changed:

[ ] CRD YAMLs updated (make manifests) and also copied into the helm chart
[ ] Changes to CRDs documented

Please refer to the PR guidelines before submitting this pull request.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. For more information on following Developer Certificate of Origin and signing off your commits, please check here.

swoehrl-mw commented 2 months ago

Hi @wangshulei098 . Can you please elaborate why you want to make this change? The updateStrategy is very specifically set the way it is so the operator can do a controlled rolling restart in case of changes/updates. Giving the user the option to change that would mess with operator logic.

wangshulei098 commented 2 months ago

Hi @wangshulei098 . Can you please elaborate why you want to make this change? The updateStrategy is very specifically set the way it is so the operator can do a controlled rolling restart in case of changes/updates. Giving the user the option to change that would mess with operator logic.

When NodePoll settings change, such as resources. The controller automatically synchronize the cluster state to the expected state without requiring manual intervention. The updateStrategy should be allowed to be modified.

swoehrl-mw commented 2 months ago

When NodePoll settings change, such as resources. The controller automatically synchronize the cluster state to the expected state without requiring manual intervention. The updateStrategy should be allowed to be modified.

And therein lies the problem, this is done by the operator so it can (among other things) keep cluster health and data replication in mind when doing the restarts (which is one of the reasons for having an operator in the first place, as the built-in k8s mechanisms can not correctly deal with this). As such it is not feasable to allow the user to configure this.

Sorry, but I'm closing this PR as it will not be merged.