Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to Set PodManagementPolicy specific to Nodegroup from Specs #864

Closed

Conversation

iam404
Copy link

@iam404 iam404 commented Jul 23, 2024

Description

Add ability to Set PodManagementPolicy for Node groups. Default to OrderedReady.

PodManagementPolicy controls how pods are created during initial scale up, when replacing pods on nodes, or when scaling down. The default policy is OrderedReady. This PR will help to set Parallel mode where pods are created in parallel to match the desired scale without waiting.

OpenSearch Operator is already managing deletion ordering itself, it wont matter much to have Parallel.
If we look at the official helm chart of OpenSearch its already set to Parallel by default.

Also, for specs with PodManagementPolicy = Parallel, the recoverymode logic is bypassed.

Issues Resolved

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.

Signed-off-by: Prabuddha Chakraborty <[email protected]>
Signed-off-by: Prabuddha Chakrborty <[email protected]>
… to Parallel

Signed-off-by: Prabuddha Chakraborty <[email protected]>
Signed-off-by: Prabuddha Chakrborty <[email protected]>
@iam404 iam404 force-pushed the custom-podmanagementpolicy branch from 8848855 to dede5ec Compare July 23, 2024 06:55
Signed-off-by: Prabuddha Chakrborty <[email protected]>
@prudhvigodithi
Copy link
Member

Thanks @iam404 AFAIK there was a reason for using OrderedReady, adding @swoehrl-mw to provide some thoughts on this PR.
Thanks
@getsaurabh02 @salyh

@swoehrl-mw
Copy link
Collaborator

The way the operator is implemented right now it depends on the OrderedReady to bring up a new cluster one by one. What is valid to discuss is if we could also implement it in a way to bring up a cluster in parallel and have that stable (AFAIK bootstrapping a new cluster in parallel led to unstable behaviour so it was done with OrderedReady). But regardless, that should be an internal decision of the operator logic and not something the user can configure.

@prudhvigodithi IMO this PR should not be merged.

@swoehrl-mw
Copy link
Collaborator

Closing as I don't think this PR should be merged, see my last comment.

@swoehrl-mw swoehrl-mw closed this Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

3 participants