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

rbd: don't default empty pool name unless namespace is specified #47106

Merged
merged 2 commits into from Jul 18, 2022
Merged

rbd: don't default empty pool name unless namespace is specified #47106

merged 2 commits into from Jul 18, 2022

Conversation

idryomov
Copy link
Contributor

Contribution Guidelines

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
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

@idryomov idryomov requested a review from a team as a code owner July 14, 2022 18:41
@github-actions github-actions bot added the tests label Jul 14, 2022
chrisphoffman
chrisphoffman approved these changes Jul 14, 2022
View reviewed changes
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.

What are your thoughts on keeping it as is and adding a -R or --recursive flag to get desired "all" output? That functionality would then be similar to "mirror snapshot schedule ls -R".

That being said, I see that the patch includes reverting back to functionality that was before regression and have no issues with that way either.

LGTM.

@idryomov
Copy link
Contributor Author

That being said, I see that the patch includes reverting back to functionality that was before regression and have no issues with that way either.

Breaking existing interfaces is a huge no-no, so I just resurrected the lost functionality. It's unfortunate that pool specs implementation here is inconsistent with the implementation of level specs in mirror snapshot and trash purge schedules, but they aren't really the same at a high level either, so I think it's fine.

@idryomov
Copy link
Contributor Author

jenkins test api

@idryomov
rbd: don't default empty pool name unless namespace is specified
b2137e2 
Commit 96f05a7 ("rbd: delay determination of default pool name")
broke "rbd perf image iostat" and "rbd perf image iotop" GLOBAL_POOL_KEY
support (the ability to blend all rbd pools together into a single
view).

Fixes: https://tracker.ceph.com/issues/56561
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
@idryomov
rbd: drop unused default_empty_pool_name argument
365300f 
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
@idryomov
Copy link
Contributor Author

idryomov commented Jul 16, 2022 •
edited

Tweaked the new test case to avoid separate data pools (e.g. due to pool/ec-data-pool facet):

https://pulpito.ceph.com/dis-2022-07-16_08:11:57-rbd-wip-dis-testing-2-distro-default-smithi/

@idryomov
Copy link
Contributor Author

jenkins test windows

@idryomov
Copy link
Contributor Author

idryomov commented Jul 18, 2022 •
edited

https://pulpito.ceph.com/dis-2022-07-17_14:22:10-rbd-wip-dis-testing-distro-default-smithi/
https://pulpito.ceph.com/dis-2022-07-17_22:13:58-rbd-wip-dis-testing-distro-default-smithi/
https://pulpito.ceph.com/dis-2022-07-18_06:38:41-rbd-wip-dis-testing-distro-default-smithi/

@idryomov idryomov merged commit 1a1fea5 into ceph:main Jul 18, 2022
@idryomov idryomov deleted the wip-56561 branch July 18, 2022 08:08
This was referenced Jul 18, 2022
Merged
Merged
Merged
VasishtaShastry pushed a commit to VasishtaShastry/cephci that referenced this pull request Aug 5, 2022
Adding jq package to suite calling cli_generec.sh scipt
70b5541 
Align to changes made in ceph/ceph#47106
VasishtaShastry pushed a commit to VasishtaShastry/cephci that referenced this pull request Aug 5, 2022
Adding jq package to suite calling cli_generec.sh scipt
829b176 
Align to changes made in ceph/ceph#47106

Signed-off-by: Vasishta <vashastr@redhat.com>
VasishtaShastry pushed a commit to VasishtaShastry/cephci that referenced this pull request Aug 8, 2022
Adding jq package to suite calling cli_generec.sh scipt
3de1f50 
Align to changes made in ceph/ceph#47106

Signed-off-by: Vasishta <vashastr@redhat.com>
VasishtaShastry pushed a commit to VasishtaShastry/cephci that referenced this pull request Aug 8, 2022
Tuning pacific/rbd suites - branch name, package
fbcc5cb 
Changes -

1) Align branch name for upstream scripts
2) Adding jq package to suite calling cli_generec.sh scipt
Align to changes made in ceph/ceph#47106

Signed-off-by: Vasishta <vashastr@redhat.com>
VasishtaShastry pushed a commit to VasishtaShastry/cephci that referenced this pull request Aug 8, 2022
Tuning pacific/rbd suites - branch name, package
b3479f9 
Changes -

1) Align branch name for upstream scripts
2) Adding jq package to suite calling cli_generec.sh scipt
Align to changes made in ceph/ceph#47106

Signed-off-by: Vasishta <vashastr@redhat.com>

Squashes:
Aug 8: modified branch name(tag) in suites/pacific/cephadm/tier-0.yaml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trociny trociny trociny approved these changes

@chrisphoffman chrisphoffman chrisphoffman approved these changes

Assignees
No one assigned
Labels
bug-fix rbd tests wip-dis-testing
Projects
None yet
Milestone
No milestone
3 participants
@idryomov @trociny @chrisphoffman