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.

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

qa/workunits/rbd: merge journal and snapshot test scripts #48508

Merged
merged 1 commit into from Nov 3, 2023

Conversation

pkalever
Copy link

@pkalever pkalever commented Oct 17, 2022

The idea is to avoid the maintenance of duplicate code in both the journal and snapshot test scripts.
Usage:
rbd_mirror.sh [<mode>]

Available modes: snapshot or journal

You can also use environment variable RBD_MIRRIR_MODE to set the mode

Fixes: https://tracker.ceph.com/issues/54312
Signed-off-by: Prasanna Kumar Kalever prasanna.kalever@redhat.com

Checklist

  • Tracker (select at least one)
    • References tracker ticket
  • Component impact
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • No doc update is appropriate
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows

@pkalever
Copy link
Author

@idryomov will it be possible for you to schedule the lab tests for this PR, please? Thanks!

Copy link
Contributor

@chrisphoffman chrisphoffman left a comment

Choose a reason for hiding this comment

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

Overall great job, there was a bunch to get right and it looks like you did.

I have two additional improvements:
-As this change deletes rbd_mirror_snapshot.sh and rbd_mirror_journal.sh and combines into rbd_mirror.sh can you change the yaml files for integration testing to use this new script? For example: ./ceph/qa/suites/rbd/mirror/workloads/rbd-mirror-workunit-config-key.yaml

-rbd_mirror_helpers.sh comments reference one of the old scripts, I'd recommend changing it to reference the new one.

qa/workunits/rbd/rbd_mirror.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror.sh Outdated Show resolved Hide resolved
@pkalever
Copy link
Author

@idryomov @chrisphoffman thanks for the review, addressed your review comments now.

@pkalever
Copy link
Author

jenkins test windows

Copy link
Contributor

@trociny trociny left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM (if it passes qa).

qa/workunits/rbd/rbd_mirror_helpers.sh Outdated Show resolved Hide resolved
@pkalever pkalever force-pushed the rbd-tests branch 2 times, most recently from 23e6966 to 2f46ec2 Compare October 21, 2022 12:43
@pkalever
Copy link
Author

pkalever commented Dec 7, 2022

@idryomov isn't this ready for a merge?

Copy link
Contributor

@idryomov idryomov left a comment

Choose a reason for hiding this comment

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

Please squash everything into the first commit as that is where rbd_mirror_journal.sh and rbd_mirror_snapshot.sh cease to exist.

qa/workunits/rbd/rbd_mirror.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror_helpers.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror.sh Outdated Show resolved Hide resolved
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@github-actions github-actions bot added the stale label Jun 26, 2023
@github-actions
Copy link

This pull request has been automatically closed because there has been no activity for 90 days. Please feel free to reopen this pull request (or open a new one) if the proposed change is still appropriate. Thank you for your contribution!

@github-actions github-actions bot closed this Jul 26, 2023
@pkalever pkalever reopened this Aug 10, 2023
@pkalever
Copy link
Author

@idryomov anything pending on this?

@github-actions github-actions bot removed the stale label Aug 10, 2023
Copy link
Contributor

@trociny trociny left a comment

Choose a reason for hiding this comment

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

no issues noticed

qa/workunits/rbd/rbd_mirror_helpers.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror_helpers.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror.sh Outdated Show resolved Hide resolved
@pkalever
Copy link
Author

pkalever commented Oct 3, 2023

Latest version test run results:
test_journal.txt
test_snapshot.txt

qa/workunits/rbd/rbd_mirror_helpers.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror_helpers.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror_helpers.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror_helpers.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror.sh Outdated Show resolved Hide resolved
@idryomov
Copy link
Contributor

idryomov commented Nov 1, 2023

Please squash everything into the first commit: the other two commits don't stand on their own because rbd_mirror_journal.sh and rbd_mirror_snapshot.sh are renamed/removed in the first commit.

@pkalever
Copy link
Author

pkalever commented Nov 2, 2023

Latest version test run results:
test_snapshot.txt
test_journal.txt

qa/workunits/rbd/rbd_mirror.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror.sh Outdated Show resolved Hide resolved
qa/workunits/rbd/rbd_mirror.sh Outdated Show resolved Hide resolved
The idea is to avoid the maintenance of duplicate code in both the journal
and snapshot test scripts.

Usage:
   RBD_MIRROR_MODE=journal rbd_mirror.sh

Use environment variable RBD_MIRROR_MODE to set the mode
Available modes: snapshot | journal

Fixes: https://tracker.ceph.com/issues/54312
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
@pkalever
Copy link
Author

pkalever commented Nov 2, 2023

Latest version test run results:
test_snapshot2.txt
test_journal2.txt

@idryomov
Copy link
Contributor

idryomov commented Nov 3, 2023

@idryomov idryomov merged commit c93a53a into ceph:main Nov 3, 2023
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants