Skip to content

Add an enhancement proposal for initial ScyllaDBCluster integration with ScyllaDB Manager #2558

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
Merged
merged 2 commits into from
Apr 10, 2025
Merged

Add an enhancement proposal for initial ScyllaDBCluster integration with ScyllaDB Manager #2558

merged 2 commits into from
Apr 10, 2025

Conversation

rzetelskik
Copy link
Member

Description of your changes: Following the internal discussions, this PR introduces an enhancement proposal for initial ScyllaDBCluster integration with ScyllaDB Manager, including cluster registration, backup and repair scheduling.

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

/cc
/priority important-soon
/kind design

@scylla-operator-bot scylla-operator-bot bot added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 19, 2025
@scylla-operator-bot scylla-operator-bot
Copy link
Contributor

@rzetelskik: GitHub didn't allow me to request PR reviews from the following users: rzetelskik.

Note that only scylladb members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

Description of your changes: Following the internal discussions, this PR introduces an enhancement proposal for initial ScyllaDBCluster integration with ScyllaDB Manager, including cluster registration, backup and repair scheduling.

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

/cc
/priority important-soon
/kind design

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 kind/design Categorizes issue or PR as related to design. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Mar 19, 2025
@rzetelskik rzetelskik force-pushed the multidc-manager-enhancement-proposal branch 2 times, most recently from 8d7eda8 to 8f434b5 Compare March 19, 2025 16:18
@rzetelskik
Copy link
Member Author

/cc zimnx mflendrich
@Michal-Leszczynski could you also have a look at the proposed API changes?

@rzetelskik rzetelskik force-pushed the multidc-manager-enhancement-proposal branch from 8f434b5 to 3dabaeb Compare March 20, 2025 09:34
@rzetelskik
Copy link
Member Author

Updated cron comment in the proposed API and added an alternative approach to auth token propagation to Alternatives section.

@rzetelskik rzetelskik force-pushed the multidc-manager-enhancement-proposal branch from 3dabaeb to 4bcca44 Compare March 20, 2025 09:40
@rzetelskik rzetelskik force-pushed the multidc-manager-enhancement-proposal branch from 4bcca44 to 3945425 Compare March 20, 2025 09:48
Michal-Leszczynski
Michal-Leszczynski approved these changes Mar 20, 2025
View reviewed changes
Copy link

@Michal-Leszczynski Michal-Leszczynski left a comment

Choose a reason for hiding this comment

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

LGTM

@rzetelskik rzetelskik force-pushed the multidc-manager-enhancement-proposal branch from 3945425 to 23b68e5 Compare March 20, 2025 11:39
@rzetelskik
Copy link
Member Author

Changed StartDate type to *metav1.Time and added a json tag to ScyllaDBCluster reference.

@rzetelskik rzetelskik force-pushed the multidc-manager-enhancement-proposal branch from 23b68e5 to bda79bb Compare March 20, 2025 13:59
@rzetelskik rzetelskik force-pushed the multidc-manager-enhancement-proposal branch 3 times, most recently from 9db7811 to ea84456 Compare March 21, 2025 07:16
@rzetelskik rzetelskik changed the title Add an enhancement proposal for initial ScyllaDBCluster integration with ScyllaDB Manager [WIP] Add an enhancement proposal for initial ScyllaDBCluster integration with ScyllaDB Manager Mar 21, 2025
@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 Mar 21, 2025
@rzetelskik rzetelskik force-pushed the multidc-manager-enhancement-proposal branch from ea84456 to fa8393a Compare March 21, 2025 08:39
@rzetelskik rzetelskik changed the title [WIP] Add an enhancement proposal for initial ScyllaDBCluster integration with ScyllaDB Manager Add an enhancement proposal for initial ScyllaDBCluster integration with ScyllaDB Manager Mar 21, 2025
@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 Mar 21, 2025
@rzetelskik rzetelskik force-pushed the multidc-manager-enhancement-proposal branch from fa8393a to eda766a Compare March 21, 2025 09:24
@rzetelskik
Copy link
Member Author

Added naming convention for ScyllaDBManagerTasks created by ScyllaCluster translation controller.

mflendrich
mflendrich reviewed Mar 31, 2025
View reviewed changes
Copy link
Collaborator

@mflendrich mflendrich left a comment

Choose a reason for hiding this comment

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

The most important issue to me is the migration path for existing users - this may be lost in transit but it feels that upgrading versions of Operator will require changing the deployment pattern, which under semver is not acceptable between minor versions.

@rzetelskik rzetelskik force-pushed the multidc-manager-enhancement-proposal branch 2 times, most recently from 6a0298a to fda4abb Compare April 2, 2025 13:53
@rzetelskik rzetelskik force-pushed the multidc-manager-enhancement-proposal branch from fda4abb to 12d61e8 Compare April 2, 2025 13:57
@rzetelskik rzetelskik changed the title [WIP] Add an enhancement proposal for initial ScyllaDBCluster integration with ScyllaDB Manager Add an enhancement proposal for initial ScyllaDBCluster integration with ScyllaDB Manager Apr 2, 2025
@rzetelskik rzetelskik requested a review from mflendrich April 2, 2025 14:02
@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 Apr 2, 2025
@rzetelskik
Copy link
Member Author

@mflendrich I amended the proposal to remove the preemptively introduced ScyllaDBManager API. Please have a look.

mflendrich
mflendrich approved these changes Apr 2, 2025
View reviewed changes
Copy link
Collaborator

@mflendrich mflendrich left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks solid.

Comments should not be an issue, they're:

  • sanity checks (to ensure that we didn't end up too far away in our assumptions)
  • minor tightening of requirements - choosing the "stricter" variants in some places
  • nits

I think that (assuming that there's nothing devastating in the discussions attached) we can get going with an implementation.

LGTM 👏

/assign zimnx

@scylla-operator-bot scylla-operator-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mflendrich, Michal-Leszczynski, rzetelskik

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

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [mflendrich,rzetelskik]

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

@rzetelskik rzetelskik force-pushed the multidc-manager-enhancement-proposal branch from 5bd7cbe to 43334a1 Compare April 7, 2025 08:46
@rzetelskik rzetelskik requested a review from zimnx April 7, 2025 09:02
@rzetelskik rzetelskik changed the title Add an enhancement proposal for initial ScyllaDBCluster integration with ScyllaDB Manager [WIP] Add an enhancement proposal for initial ScyllaDBCluster integration with ScyllaDB Manager Apr 7, 2025
@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 Apr 7, 2025
@rzetelskik rzetelskik force-pushed the multidc-manager-enhancement-proposal branch 3 times, most recently from c11f026 to d77048b Compare April 8, 2025 14:35
@rzetelskik
Extend "ScyllaDBCluster integration with ScyllaDB Manager" proposal w…
7867a0b 
…ith ScyllaDBManagerClusterRegistration internal API
@rzetelskik rzetelskik force-pushed the multidc-manager-enhancement-proposal branch from d77048b to 7867a0b Compare April 10, 2025 11:53
@rzetelskik rzetelskik changed the title [WIP] Add an enhancement proposal for initial ScyllaDBCluster integration with ScyllaDB Manager Add an enhancement proposal for initial ScyllaDBCluster integration with ScyllaDB Manager Apr 10, 2025
@rzetelskik rzetelskik requested a review from mflendrich April 10, 2025 11:54
@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 Apr 10, 2025
@rzetelskik rzetelskik requested a review from zimnx April 10, 2025 11:54
@zimnx
Copy link
Collaborator

zimnx commented Apr 10, 2025

Thanks for updates, good job!
/lgtm

@scylla-operator-bot scylla-operator-bot bot added the lgtm Indicates that a PR is ready to be merged. label Apr 10, 2025
@scylla-operator-bot scylla-operator-bot bot merged commit f27471d into scylladb:master Apr 10, 2025
8 checks passed
@rzetelskik rzetelskik deleted the multidc-manager-enhancement-proposal branch April 10, 2025 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Michal-Leszczynski Michal-Leszczynski Michal-Leszczynski approved these changes

@mflendrich mflendrich Awaiting requested review from mflendrich

@zimnx zimnx Awaiting requested review from zimnx

Assignees

@zimnx zimnx

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/design Categorizes issue or PR as related to design. lgtm Indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Backup & repair in ScyllaDBCluster design doc
4 participants
@rzetelskik @zimnx @mflendrich @Michal-Leszczynski