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

Allow changing storageclass after the opensearch cluster is created. #873

Open
vinodhreddyg opened this issue Sep 25, 2024 · 6 comments
Open
Labels
documentation Improvements or additions to documentation enhancement New feature or request good first issue Good for newcomers

Comments

@vinodhreddyg
Copy link

Is your feature request related to a problem?

Once opensearch cluster is created the storageClass can't be changed.

What solution would you like?

I understand that changing the storageClass isn't allowed by sts validation but when teams are ok for a rolling restart with data migrated to the new pvc operator can facilitated this feature. In elasticsearch this is achieved by changing the nodeset name and storageClassName at once. So the operator creates new nodeset with new pvc's and migrate the data from older nodeset to new nodeset. Please refer below discussion

https://discuss.elastic.co/t/change-storageclass-of-an-already-running-cluster/267573/2

What alternatives have you considered?

no alternatives. Looking for operator support

Do you have any additional context?

Please ref this discussion https://discuss.elastic.co/t/change-storageclass-of-an-already-running-cluster/267573/2

@vinodhreddyg vinodhreddyg added enhancement New feature or request untriaged Issues that have not yet been triaged labels Sep 25, 2024
@vinodhreddyg vinodhreddyg changed the title Allow creating changing storageclass after the opensearch cluster is created. Allow changing storageclass after the opensearch cluster is created. Sep 25, 2024
@gaiksaya gaiksaya removed the untriaged Issues that have not yet been triaged label Sep 26, 2024
@gaiksaya
Copy link
Member

[Triage] Hi @swoehrl-mw @prudhvigodithi
Do we plan to support this? Any plans?
Thanks!

@swoehrl-mw
Copy link
Collaborator

Hi @vinodhreddyg. Without having tried it out, a similar approach should be possible using the opensarch-operator: Add a new nodePool with the new storageclass, reduce the replicas of the old nodepool to 0, then remove it completely.

I don't see us implementing this as a separate feature. If someone wants to contribute it, we would probably accept it (unless it has a too high impact on the existing code), but I see this as a nieche feature that does not have priority.

cc @gaiksaya

@peterzhuamazon peterzhuamazon added good first issue Good for newcomers documentation Improvements or additions to documentation labels Sep 30, 2024
@vinodhreddyg
Copy link
Author

@swoehrl-mw @peterzhuamazon

I tried you're suggested of adding another nodepool and set old nodepool to 0. This doesn't work indices are red now.

red   open elasticsearch-index          dHpoJviqSHi6MFiTxmZF9w 1 1

Also one more caveat if we change the name of any nodepool instead of changing the name it creates one more nodepool. I don't want to divert the topic but an observation/bug on it's own.

@swoehrl-mw
Copy link
Collaborator

@vinodhreddyg Can you provide more details why the cluster is red? Are indices completely missing because it was scaled down too fast?
Did you enable the smartscaler via spec.confMgmt.smartScaler? Because AFAIK otherwise the operator will not explicitly drain the nodes it removes.

@vinodhreddyg
Copy link
Author

@swoehrl-mw : Thanks for your response. It works with smartScaler enabled. Adding this to documentation would be helpful.

  1. One observation when I remove the old nodeset it's statefulset, service and pvcs are still available. I was expecting those to be cleanup.
  2. smartScaler is awesome but wondering why there is an option to toggle it why can't this enabled by default?

@swoehrl-mw
Copy link
Collaborator

@vinodhreddyg

One observation when I remove the old nodeset it's statefulset, service and pvcs are still available. I was expecting those to be cleanup

That is a known limitation (I thought we had an open issue for that but can't find it at the moment), the operator cannot detect a removed nodepool. The workaround is to first scale down to 0 and then remove it.

smartScaler is awesome but wondering why there is an option to toggle it why can't this enabled by default?

Basically historical reasons. When first added it was an unstable feature, and to avoid changing behaviour it was never enabled by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request good first issue Good for newcomers
Projects
Status: 📦 Backlog
Development

No branches or pull requests

4 participants