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.

Sign up for GitHub

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

Jump to bottom

Document manual procedure of storage resizing (2nd take) #2243

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Document manual procedure of storage resizing (2nd take) #2243

wants to merge 2 commits into from

Conversation

gdubicki
Copy link
Contributor

@gdubicki gdubicki commented Dec 5, 2024 •
edited
Loading

Description of your changes:

Based on #1053 by @Choraden, expanded to include both storage increase and decrease cases.

I have tested the decreasing case on ScyllaDB v6.1.2 and Scylla Operator v1.14.0.

Which issue is resolved by this Pull Request:

Resolves #825

@scylla-operator-bot scylla-operator-bot bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 5, 2024
@scylla-operator-bot scylla-operator-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: gdubicki
Once this PR has been reviewed and has the lgtm label, please assign zimnx for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@scylla-operator-bot scylla-operator-bot bot added do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Dec 5, 2024
@scylla-operator-bot scylla-operator-bot
Copy link
Contributor

Hi @gdubicki. Thanks for your PR.

I'm waiting for a scylladb member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@scylla-operator-bot scylla-operator-bot bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 5, 2024
@gdubicki gdubicki marked this pull request as ready for review December 5, 2024 12:55
@scylla-operator-bot scylla-operator-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 5, 2024
@gdubicki gdubicki marked this pull request as draft December 5, 2024 12:59
@scylla-operator-bot scylla-operator-bot bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 5, 2024
@gdubicki
Copy link
Contributor Author

gdubicki commented Dec 5, 2024

Sorry, it's not ready yet. I need to combine it with the new index.

@gdubicki
Add storage resize guide
b01b42e
@gdubicki
Add storage decrease instructions
a9e1edb 
and expand the instructions with mention of Helm-managed
clusters and a more detailed instructions for node replacement
procedure. Fix some typos and make the values more consistent.
@scylla-operator-bot scylla-operator-bot bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 5, 2024
@gdubicki gdubicki marked this pull request as ready for review December 5, 2024 14:50
@scylla-operator-bot scylla-operator-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 5, 2024
@gdubicki gdubicki changed the title Add storage resize guide Document manual procedure of storage resizing (2nd take) Dec 5, 2024
zimnx
zimnx reviewed Dec 11, 2024
View reviewed changes
Copy link
Collaborator

@zimnx zimnx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for contirbution, I think it requires slightly more work to explain procedure better.

docs/source/resources/scyllaclusters/storage-resizing.md

## Increasing the storage size

1. Orphan delete ScyllaCluster
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's important to mention that existing scyllacluster spec needs to be saved somewhere and aligned to new storage requirements. Otherwise someone executing very first step of a procedure may loose the spec and find out later it's no longer available.

docs/source/resources/scyllaclusters/storage-resizing.md

Due to limitations of the underlying StatefulSet, ScyllaClusters don't allow storage changes. The following procedure describes how to adjust the storage manually.

Let's use the following pods and StatefulSet in the resize examples below.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be generic enough to explain it needs to be done for every rack (StatefulSet) that requires changes.

docs/source/resources/scyllaclusters/storage-resizing.md
@@ -0,0 +1,99 @@
# Resizing storage in ScyllaCluster

Due to limitations of the underlying StatefulSet, ScyllaClusters don't allow storage changes. The following procedure describes how to adjust the storage manually.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the requirements is that StorageClass used supports resising. It should be mentioned here and explained how to verify that.

docs/source/resources/scyllaclusters/storage-resizing.md
```
The StatefulSet will be recreated by the Scylla Operator with the new storage size.

5. (Only if PVC does not support volume expansion) You need to replace every Scylla node, one by one, following the procedure described in [replacing nodes](/resources/scyllaclusters/nodeoperations/replace-node). Start with the last node in the StatefulSet and move towards the first node. This will recreate each node and their PVCs with the new size.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be separate procedure for those StorageClasses, not embedded in this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zimnx zimnx zimnx left review comments

@tnozicka tnozicka Awaiting requested review from tnozicka

@rzetelskik rzetelskik Awaiting requested review from rzetelskik

Assignees
No one assigned
Labels
do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Document and test storage resizing on the same nodes
2 participants
@gdubicki @zimnx