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

[Backport manager-3.3] test(manager): Introduce the sanity test on mixed cluster (vnodes+tablets) #7749

Merged
merged 4 commits into from
Jun 27, 2024
Merged

[Backport manager-3.3] test(manager): Introduce the sanity test on mixed cluster (vnodes+tablets) #7749

merged 4 commits into from
Jun 27, 2024

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented Jun 25, 2024

The PR partially addresses scylladb/scylla-manager#3853.

The PR consists of:

  • New test that performs sanity checks on cluster with mixed keyspaces - 1 vnodes keyspace + 1 tablets keyspace;
  • .jenkinsfile that handles test execution in CI;
  • A few fixes/extensions for some load/stress methods to handle non-default or several keyspace names;
  • Fix for system_auth keyspace repair test.

Testing

PR pre-checks (self review)

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

Reminders

  • Add New configuration option and document them (in sdcm/sct_config.py)
  • Add unit tests to cover my changes (under unit-test/ folder)
  • Update the Readme/doc folder relevant to this change (if needed)

(cherry picked from commit b2b76a4)

(cherry picked from commit 5681a09)

(cherry picked from commit 486b5a8)

(cherry picked from commit b2cd1ca)

Parent PR: #7632

mikliapko added 4 commits June 25, 2024 18:58
@mikliapko @mergify
fix(manager): extend some load/stress methods to handle keyspace names
dba0ba8 
Manager's _generate_load method has been worked to take keyspace_name
as a parameter instead of keyspace_name_to_replace. That allows to
generate the load for specific keyspace and makes the code more clear
and understandable.

assemble_and_run_all_stress_cmd_by_ks_names method was introduced
to run stress commands for keyspaces with provided names. It was
decided to keep it as separate method to not overcomplicate the
existing one (assemble_and_run_all_stress_cmd).

(cherry picked from commit b2b76a4)
@mikliapko @mergify
feature(tester.py): allow to specify tablets config when creating ks
63e0aba 
Extend create_keyspace method to include tablets configuration to
disable tablets (currently enabled by default) and provide initial
number of tablets.

(cherry picked from commit 5681a09)
@mikliapko @mergify
feature(manager): check system_auth ks repair only for raft topology
4836c65 
There is no system_auth ks in Scylla with enabled raft topology. Thus,
the test should check whether consistent topology changes are enabled
or not to form the valid list of repaired keyspaces.

(cherry picked from commit 486b5a8)
@mikliapko @mergify
test(manager): introduce test for mixed db (vnodes + tablets)
84bb3ad 
The new test performs sanity checks for cluster consisting of vnodes
and tablets simultaneously.

(cherry picked from commit b2cd1ca)
@fruch
Copy link
Contributor

fruch commented Jun 26, 2024

@mikliapko can you look into why it's failing the checks ? (are those are not broken on master ? after #7632 merged ?)

@mikliapko
Copy link
Contributor

@mikliapko can you look into why it's failing the checks ? (are those are not broken on master ? after #7632 merged ?)

So, we have a different behavior in master and release branches - the pre-commit checks are passing for master, but failing for manager-3.3 and manager-3.2.

@fruch I suspect this change might be a reason (#5799)

@fruch
Copy link
Contributor

fruch commented Jun 26, 2024

@mikliapko can you look into why it's failing the checks ? (are those are not broken on master ? after #7632 merged ?)

So, we have a different behavior in master and release branches - the pre-commit checks are passing for master, but failing for manager-3.3 and manager-3.2.

@fruch I suspect this change might be a reason (#5799)

if it was just per-commit... but how's he unittest is broken on those branches ?

@mikliapko
Copy link
Contributor

@mikliapko can you look into why it's failing the checks ? (are those are not broken on master ? after #7632 merged ?)

So, we have a different behavior in master and release branches - the pre-commit checks are passing for master, but failing for manager-3.3 and manager-3.2.
@fruch I suspect this change might be a reason (#5799)

if it was just per-commit... but how's he unittest is broken on those branches ?

unittests are fine in this manager-3.3 branch. There was a problem with unittests only in manager-3.2 branch.

@fruch
Copy link
Contributor

fruch commented Jun 26, 2024

@mikliapko can you look into why it's failing the checks ? (are those are not broken on master ? after #7632 merged ?)

So, we have a different behavior in master and release branches - the pre-commit checks are passing for master, but failing for manager-3.3 and manager-3.2.
@fruch I suspect this change might be a reason (#5799)

if it was just per-commit... but how's he unittest is broken on those branches ?

unittests are fine in this manager-3.3 branch. There was a problem with unittests only in manager-3.2 branch.

o.k

@mikliapko mikliapko mentioned this pull request Jun 27, 2024
Merged
2 tasks
@mikliapko
Copy link
Contributor

@mikliapko can you look into why it's failing the checks ? (are those are not broken on master ? after #7632 merged ?)

So, we have a different behavior in master and release branches - the pre-commit checks are passing for master, but failing for manager-3.3 and manager-3.2.
@fruch I suspect this change might be a reason (#5799)

if it was just per-commit... but how's he unittest is broken on those branches ?

unittests are fine in this manager-3.3 branch. There was a problem with unittests only in manager-3.2 branch.

o.k

Issued the PR #7762 to resolve this inconsistency between branches.

@mikliapko
Copy link
Contributor

Since all checks are green after inconsistency issues resolution, merging this PR

@mikliapko mikliapko merged commit d7ca65d into manager-3.3 Jun 27, 2024
7 checks passed
@mergify mergify bot deleted the mergify/bp/manager-3.3/pr-7632 branch June 27, 2024 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@mikliapko mikliapko

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@fruch @mikliapko