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.2] test(manager): Introduce the sanity test on mixed cluster (vnodes+tablets) #7748

Closed
wants to merge 4 commits into from
Closed

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

wants to merge 4 commits into from

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
c3a05d3 
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
db38a30 
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
dceaf54 
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)
48310e2 
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)

@mikliapko
Copy link
Contributor

mikliapko commented Jun 26, 2024 •
edited
Loading

What is related to unittest failure - that's the problem we've been living for some time now.
But since mergify was introduced we can't ignore it anymore.

I'll raise the PR to fix this failing test in manager-3.2 branch.

@mikliapko
Copy link
Contributor

I'll raise the PR to fix this failing test in manager-3.2 branch.

#7753

@mikliapko mikliapko closed this Jun 26, 2024
@mikliapko mikliapko reopened this Jun 26, 2024
@mikliapko
Copy link
Contributor

To backport the changes from here multiple conflicts must be resolved.
Considering the Manager 3.3 is about to be released I suggest to close this PR and don't backport the changes into manager-3.2.

@mikliapko mikliapko closed this Jun 27, 2024
@mergify mergify bot deleted the mergify/bp/manager-3.2/pr-7632 branch June 27, 2024 09:14
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