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

Support different regions in disrupt_mgmt_restore nemesis #9492

Merged
merged 5 commits into from
Jan 7, 2025
Merged

Support different regions in disrupt_mgmt_restore nemesis #9492

merged 5 commits into from
Jan 7, 2025

Conversation

mikliapko
Copy link
Contributor

@mikliapko mikliapko commented Dec 6, 2024 •
edited
Loading

Closes scylladb/scylla-manager#4041

Changes:

  • All backup snapshots used in disrupt_mgmt_restore nemesis has been duplicated into eu-west-1 region to make test functional for this region as well.
  • Adjusted yaml file with snapshots configuration and the flow nemesis uses to choose the snapshot for the test.
  • Get rid of backup_bucket_region and just use region_name instead.
  • Fixed the region of ubuntu22-backup-test to us-east-1 to work with backups from us-east-1 s3 bucket.

Testing

  • MgmtRestore only test
  • longevity-mv-si-4days-streaming-test (duration decreased to 24 hours)
  • Manager sanity test (verify backup-bucket-region removal regression)

PR pre-checks (self review)

  • I added the relevant backport labels
  • I didn't leave commented-out/debugging code

@mikliapko mikliapko self-assigned this Dec 6, 2024
@mikliapko mikliapko force-pushed the nemesis-restore-region-fix branch 2 times, most recently from 1f7af20 to dfcfc38 Compare December 6, 2024 13:18
fruch
fruch reviewed Dec 9, 2024
View reviewed changes
test-cases/longevity/longevity-mv-si-4days.yaml Outdated
@@ -22,7 +22,7 @@ instance_type_monitor: 't3.xlarge'

root_disk_size_runner: 120

nemesis_class_name: 'SisyphusMonkey'
nemesis_class_name: 'MgmtRestore'
Copy link
Contributor

Choose a reason for hiding this comment

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

reminder to put this back

fruch
fruch reviewed Dec 9, 2024
View reviewed changes
jenkins-pipelines/manager/ubuntu22-manager-backup.jenkinsfile
@@ -5,7 +5,7 @@ def lib = library identifier: 'sct@snapshot', retriever: legacySCM(scm)

managerPipeline(
backend: 'aws',
region: 'us-west-2',
Copy link
Contributor

Choose a reason for hiding this comment

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

this is for testing purposes ? or by mistake ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's by purpose, otherwise the test would fail because of region and backup bucket misconfiguration.
Before, the test was passing as backup_bucket_region parameter was set to us-east-1 (means that manager agent had this region in configuration) and we used backups from the bucket at the same region.

@mikliapko
Copy link
Contributor Author

@fruch I'm not done with this PR yet.
Faced one more issue scylladb/scylla-manager#4041 (comment) that should be addressed.

@mikliapko mikliapko force-pushed the nemesis-restore-region-fix branch from dfcfc38 to 8722d45 Compare December 30, 2024 13:59
@mikliapko
refactor(mgr_nemesis): remove condition for old Manager versions < 3.2
a4d36d1 
Manager versions lower than 3.2 are outdated, not supported and not
tested currently. I'd never expect them to be tested again.
@mikliapko
fix(manager): get rid of backup_bucket_region parameter
2c574e7 
For now, we can't set different region_name and backup_bucket_region.
Thus, it makes sense to get rid of backup_bucket_region and just use
region_name instead.

Additionally, fix the region of ubuntu22-backup-test to us-east-1 to
work with backups from us-east-1 s3 bucket.
@mikliapko mikliapko force-pushed the nemesis-restore-region-fix branch from 8722d45 to 36704a2 Compare December 30, 2024 16:32
@mikliapko mikliapko added backport/2024.1 Need backport to 2024.1 backport/2024.2 Need backport to 2024.2 backport/6.2 backport/6.1 Need backport to 6.1 and removed backport/2024.1 Need backport to 2024.1 labels Dec 31, 2024
@mikliapko mikliapko marked this pull request as ready for review January 1, 2025 17:14
@mikliapko mikliapko requested a review from rayakurl as a code owner January 1, 2025 17:14
@mikliapko mikliapko requested review from fruch and a team and removed request for rayakurl January 1, 2025 17:14
fruch
fruch reviewed Jan 1, 2025
View reviewed changes
sdcm/nemesis.py Outdated Show resolved Hide resolved
fruch
fruch requested changes Jan 1, 2025
View reviewed changes
Copy link
Contributor

@fruch fruch left a comment

Choose a reason for hiding this comment

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

LGTM, but we should test this with a multidc case

@mikliapko
fix(manager): support different regions in disrupt_mgmt_restore nemesis
8709988 
All backup snapshots has been duplicated into eu-west-1 region to make
disrupt_mgmt_restore test functional for this region as well.

This fix adjusts yaml file with snapshots configuration and the flow
nemesis uses to choose the snapshot for the test.
@mikliapko
fix(manager): skip set ks strategy and rf for disrupt_mgmt_restore
33594d2 
According to comment (1), set ks strategy and rf is not needed if
restoring the schema within one DC.

It should be brought back after implementation of (2) which will
unblock schema restore into a different DC. For now, it's possible
to restore schema only within one DC.

Refs:
#1: scylladb/scylla-manager#4041
issuecomment-2565489699
#2: scylladb/scylla-manager#4049
@mikliapko
fix(nemesis): skip disrupt_mgmt_restore if multiDC cluster
0bc578b 
disrupt_mgmt_restore doesn't support multiDC cluster configuration due
to the issues:
- scylladb/scylla-manager#3829
- scylladb/scylla-manager#4049

Thus, it should be skipped if both issues are opened and cluster
configuration is multiDC.
@mikliapko mikliapko force-pushed the nemesis-restore-region-fix branch from e546534 to 0bc578b Compare January 2, 2025 12:27
fruch
fruch approved these changes Jan 2, 2025
View reviewed changes
Copy link
Contributor

@fruch fruch left a comment

Choose a reason for hiding this comment

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

LGTM

@fruch fruch requested a review from dimakr January 2, 2025 14:22
@mikliapko
Copy link
Contributor Author

@dimakr Could you please take a look?

dimakr
dimakr approved these changes Jan 7, 2025
View reviewed changes
Copy link
Contributor

@dimakr dimakr left a comment

Choose a reason for hiding this comment

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

LGTM

mergify bot pushed a commit that referenced this pull request Jan 8, 2025
@fruch @mergify
fix(region_name): handle case of region_name doesn't exist
5e05038 
in #9492 some code changed to assume `region_name` always exists
and when not using AWS, it doesn't exist (or it's empty)

switch to retriving it in a safer way

Ref: #9492
(cherry picked from commit 7d51938)

# Conflicts:
#	mgmt_cli_test.py
#	mgmt_upgrade_test.py
#	sdcm/cluster.py
#	sdcm/cluster_k8s/eks.py
This was referenced Jan 8, 2025
Merged
Merged
Merged
@scylladbbot scylladbbot added backport/6.2-done backport/2024.2-done Commit backported to 2024.2 backport/6.1-done Commit backported to 6.1 and removed backport/6.2 backport/2024.2 Need backport to 2024.2 backport/6.1 Need backport to 6.1 labels Jan 8, 2025
fruch added a commit that referenced this pull request Jan 8, 2025
@fruch
fix(region_name): handle case of region_name doesn't exist
942d973 
in #9492 some code changed to assume `region_name` always exists
and when not using AWS, it doesn't exist (or it's empty)

switch to retriving it in a safer way

Ref: #9492
(cherry picked from commit 7d51938)
fruch added a commit that referenced this pull request Jan 8, 2025
@fruch
fix(region_name): handle case of region_name doesn't exist
8b231d9 
in #9492 some code changed to assume `region_name` always exists
and when not using AWS, it doesn't exist (or it's empty)

switch to retriving it in a safer way

Ref: #9492
(cherry picked from commit 7d51938)
fruch added a commit that referenced this pull request Jan 8, 2025
@fruch
fix(region_name): handle case of region_name doesn't exist
1ce55c9 
in #9492 some code changed to assume `region_name` always exists
and when not using AWS, it doesn't exist (or it's empty)

switch to retriving it in a safer way

Ref: #9492
(cherry picked from commit 7d51938)
fruch added a commit that referenced this pull request Jan 8, 2025
@fruch
fix(region_name): handle case of region_name doesn't exist
4257cd9 
in #9492 some code changed to assume `region_name` always exists
and when not using AWS, it doesn't exist (or it's empty)

switch to retriving it in a safer way

Ref: #9492
(cherry picked from commit 7d51938)
fruch added a commit that referenced this pull request Jan 8, 2025
@fruch
fix(region_name): handle case of region_name doesn't exist
64b7a97 
in #9492 some code changed to assume `region_name` always exists
and when not using AWS, it doesn't exist (or it's empty)

switch to retriving it in a safer way

Ref: #9492
(cherry picked from commit 7d51938)
fruch added a commit that referenced this pull request Jan 8, 2025
@fruch
fix(region_name): handle case of region_name doesn't exist
339e461 
in #9492 some code changed to assume `region_name` always exists
and when not using AWS, it doesn't exist (or it's empty)

switch to retriving it in a safer way

Ref: #9492
(cherry picked from commit 7d51938)
@vponomaryov
Copy link
Contributor

It's backport to the 2024.2 caused the following bug: #9775

Haven't checked the 6.2 and 6.1 branches, but they probably are affected too.

cezarmoise added a commit to cezarmoise/scylla-cluster-tests that referenced this pull request Jan 13, 2025
@cezarmoise
fix(region_name): improvre retreving region_name in nemesis
a6b3cb4 
Retrieve `region_name` in a safer way, since in AWS it might not exist or is empty.

Follow-up to scylladb#9698.

refs: scylladb#9492, scylladb#9698
@cezarmoise cezarmoise mentioned this pull request Jan 13, 2025
Merged
2 tasks
cezarmoise added a commit to cezarmoise/scylla-cluster-tests that referenced this pull request Jan 13, 2025
@cezarmoise
fix(region_name): improvre retreving region_name in nemesis
a8ad7e9 
Retrieve `region_name` in a safer way, since in AWS it might not exist or is empty.

Follow-up to scylladb#9698.

refs: scylladb#9492, scylladb#9698
vponomaryov pushed a commit that referenced this pull request Jan 13, 2025
@cezarmoise @vponomaryov
fix(region_name): improvre retreving region_name in nemesis
36976cc 
Retrieve `region_name` in a safer way, since in AWS it might not exist or is empty.

Follow-up to #9698.

refs: #9492, #9698
scylladbbot pushed a commit to scylladbbot/scylla-cluster-tests that referenced this pull request Jan 13, 2025
@cezarmoise @actions-user
fix(region_name): improvre retreving region_name in nemesis
0517851 
Retrieve `region_name` in a safer way, since in AWS it might not exist or is empty.

Follow-up to scylladb#9698.

refs: scylladb#9492, scylladb#9698
(cherry picked from commit 36976cc)
@scylladbbot scylladbbot mentioned this pull request Jan 13, 2025
Merged
2 tasks
scylladbbot pushed a commit to scylladbbot/scylla-cluster-tests that referenced this pull request Jan 13, 2025
@cezarmoise @actions-user
fix(region_name): improvre retreving region_name in nemesis
ec82942 
Retrieve `region_name` in a safer way, since in AWS it might not exist or is empty.

Follow-up to scylladb#9698.

refs: scylladb#9492, scylladb#9698
(cherry picked from commit 36976cc)
@scylladbbot scylladbbot mentioned this pull request Jan 13, 2025
Merged
2 tasks
scylladbbot pushed a commit to scylladbbot/scylla-cluster-tests that referenced this pull request Jan 13, 2025
@cezarmoise @actions-user
fix(region_name): improvre retreving region_name in nemesis
e7f58b2 
Retrieve `region_name` in a safer way, since in AWS it might not exist or is empty.

Follow-up to scylladb#9698.

refs: scylladb#9492, scylladb#9698
(cherry picked from commit 36976cc)
@scylladbbot scylladbbot mentioned this pull request Jan 13, 2025
Merged
2 tasks
fruch pushed a commit that referenced this pull request Jan 13, 2025
@cezarmoise @fruch
fix(region_name): improvre retreving region_name in nemesis
a60301c 
Retrieve `region_name` in a safer way, since in AWS it might not exist or is empty.

Follow-up to #9698.

refs: #9492, #9698
(cherry picked from commit 36976cc)
fruch pushed a commit that referenced this pull request Jan 13, 2025
@cezarmoise @fruch
fix(region_name): improvre retreving region_name in nemesis
51a97c3 
Retrieve `region_name` in a safer way, since in AWS it might not exist or is empty.

Follow-up to #9698.

refs: #9492, #9698
(cherry picked from commit 36976cc)
fruch pushed a commit that referenced this pull request Jan 13, 2025
@cezarmoise @fruch
fix(region_name): improvre retreving region_name in nemesis
77d1f4d 
Retrieve `region_name` in a safer way, since in AWS it might not exist or is empty.

Follow-up to #9698.

refs: #9492, #9698
(cherry picked from commit 36976cc)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dimakr dimakr dimakr approved these changes

@fruch fruch fruch approved these changes

Assignees

@mikliapko mikliapko

Labels
backport/6.1-done Commit backported to 6.1 backport/6.2-done backport/2024.2-done Commit backported to 2024.2 promoted-to-master
Projects
None yet
Milestone
No milestone
5 participants
@mikliapko @vponomaryov @fruch @dimakr @scylladbbot