Skip to content

Add an E2E test for a non-centralised multi-datacenter node replacement procedure #2942

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 3 commits into from
Oct 21, 2025
Merged

Add an E2E test for a non-centralised multi-datacenter node replacement procedure #2942

merged 3 commits into from
Oct 21, 2025

Conversation

@rzetelskik
Copy link
Member

@rzetelskik rzetelskik commented Sep 4, 2025 •
edited by mflendrich
Loading

Description of your changes: This PR introduces an E2E test for a "manual" (as in non-centralised, but triggered in a given worker cluster) multi-datacenter node replacement procedure. It also unifies the single and multi-dc tests.

Which issue is resolved by this Pull Request:
Part of #2861

/kind new-test
/priority important-soon

@scylla-operator-bot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@scylla-operator-bot scylla-operator-bot bot added kind/new-test Categorizes issue/PR as a newly created test case. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. 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 Sep 4, 2025
@rzetelskik
Copy link
Member Author

/test all

@scylla-operator-bot scylla-operator-bot bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 4, 2025
@rzetelskik rzetelskik requested a review from Copilot September 4, 2025 13:18
Copilot AI reviewed Sep 4, 2025
View reviewed changes
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds an E2E test for multi-datacenter node replacement procedures that are triggered directly in a datacenter. The test verifies that the ScyllaDB operator can properly handle node replacements initiated by setting a replace label on a service, ensuring the cluster maintains consistency and the replacement node is properly configured.

  • Adds a new helper function IsScyllaDBClusterProgressing to check cluster progression state
  • Implements a comprehensive E2E test that simulates node replacement in a multi-datacenter environment
  • Verifies proper configuration of replacement nodes and cluster state consistency

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
test/e2e/utils/helpers.go Adds helper function to check if ScyllaDBCluster is in progressing state
test/e2e/set/scylladbcluster/multidatacenter/scylladbcluster_replace.go Implements comprehensive E2E test for multi-datacenter node replacement procedure

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@rzetelskik
Copy link
Member Author

/retest

@rzetelskik rzetelskik marked this pull request as ready for review September 5, 2025 10:38
@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 Sep 5, 2025
@rzetelskik rzetelskik changed the title Add an E2E test for a multi-datacenter node replacement procedure triggered directly in a datacenter Add an E2E test for a non-centralised multi-datacenter node replacement procedure Sep 5, 2025
czeslavo
czeslavo reviewed Sep 9, 2025
View reviewed changes
Copy link
Contributor

@czeslavo czeslavo left a comment

Choose a reason for hiding this comment

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

Left a couple of nits, looks good overall.

@rzetelskik rzetelskik force-pushed the multi-dc-node-replacement branch from 358f2b3 to 3cc60d1 Compare September 9, 2025 10:42
@rzetelskik rzetelskik requested a review from czeslavo September 9, 2025 10:42
czeslavo
czeslavo approved these changes Sep 9, 2025
View reviewed changes
Copy link
Contributor

@czeslavo czeslavo left a comment

Choose a reason for hiding this comment

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

/lgtm

@scylla-operator-bot scylla-operator-bot bot added the lgtm Indicates that a PR is ready to be merged. label Sep 9, 2025
@scylla-operator-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: czeslavo, 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:

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

@rzetelskik
Copy link
Member Author

/retest

@rzetelskik
Copy link
Member Author

/test images
/retest

@rzetelskik
Copy link
Member Author

@rzetelskik: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-gke-multi-datacenter-parallel 3cc60d1 link true /test e2e-gke-multi-datacenter-parallel
Full PR test history. Your PR dashboard.

/test images
/retest

@rzetelskik
Copy link
Member Author

@rzetelskik: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-gke-multi-datacenter-parallel 3cc60d1 link true /test e2e-gke-multi-datacenter-parallel
Full PR test history. Your PR dashboard.

/test images
/retest

@rzetelskik
Copy link
Member Author

@rzetelskik: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-gke-multi-datacenter-parallel 3cc60d1 link unknown /test e2e-gke-multi-datacenter-parallel

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

/retest

@rzetelskik
Copy link
Member Author

/test images
/retest

@scylla-operator-bot scylla-operator-bot bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 16, 2025
@scylla-operator-bot
Copy link
Contributor

PR needs rebase.

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.

@rzetelskik rzetelskik force-pushed the multi-dc-node-replacement branch from 3cc60d1 to b61d2a9 Compare October 20, 2025 12:31
@rzetelskik rzetelskik removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 20, 2025
@rzetelskik rzetelskik requested a review from czeslavo October 20, 2025 12:33
@rzetelskik
Copy link
Member Author

/test all

@rzetelskik
Copy link
Member Author

/test images
/retest

@rzetelskik
Copy link
Member Author

/test images
/retest

1 similar comment
@rzetelskik
Copy link
Member Author

/test images
/retest

@rzetelskik
Copy link
Member Author

/test images
/retest

@scylla-operator-bot
Copy link
Contributor

@rzetelskik: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-gke-multi-datacenter-parallel b61d2a9 link false /test e2e-gke-multi-datacenter-parallel

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

@scylla-operator-bot scylla-operator-bot bot merged commit 52bdcd7 into scylladb:master Oct 21, 2025
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@czeslavo czeslavo Awaiting requested review from czeslavo

Assignees

@czeslavo czeslavo

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/new-test Categorizes issue/PR as a newly created test case. 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/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.

2 participants

@rzetelskik @czeslavo