Skip to content

qa/cephfs: add more helper methods to caps_helper.py#50497

Merged
rishabh-d-dave merged 1 commit intoceph:mainfrom
rishabh-d-dave:fs-qa-caps-helper
Mar 24, 2023
Merged

qa/cephfs: add more helper methods to caps_helper.py#50497
rishabh-d-dave merged 1 commit intoceph:mainfrom
rishabh-d-dave:fs-qa-caps-helper

Conversation

@rishabh-d-dave
Copy link
Contributor

@rishabh-d-dave rishabh-d-dave commented Mar 13, 2023

The code on this PR first appeared on fs auth PR (PR #41779) since that PR needs more time to be reviewed and merged. This code is needed by fs swap PR (PR #50212) too, I am creating a new PR for this code hoping this will merge faster and thereby this code will become available to both the PRs.


Add methods that will accept read/write permissions, CephFS names and
CephFS mount point and in return will generate string form of MON, OSD
and MDS caps exactly as it is reported in Ceph keyrings.

Replace similar code in test_multifs_auth.py with calls to these helper
methods.

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)
    • Test code
  • 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

@rishabh-d-dave rishabh-d-dave requested review from a team and vshankar March 13, 2023 12:07
@rishabh-d-dave rishabh-d-dave force-pushed the fs-qa-caps-helper branch 5 times, most recently from a44d476 to 880b810 Compare March 14, 2023 14:23
@rishabh-d-dave
Copy link
Contributor Author

jenkins test make check

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Mar 14, 2023

self.test_set]

self.conduct_pos_test_for_read_caps()
self.run_pos_test_for_read_caps()
Copy link
Contributor

Choose a reason for hiding this comment

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

This commit says "Give these this methods a smaller name"

Does this change really help in any way?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it would more helpful if it could be contracted further.

Copy link
Contributor

Choose a reason for hiding this comment

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

I hate to nit pick of these things, but how small is small? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Umm, true. I should remove this change I guess...

We usually name our methods as run_abcd(). At least this change aligns this method name with general manner of naming. If this doesn't seem like a good reason, I'll remove this change. :)

Copy link
Contributor

Choose a reason for hiding this comment

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

sure

@rishabh-d-dave rishabh-d-dave force-pushed the fs-qa-caps-helper branch 2 times, most recently from e05a25e to 493eaff Compare March 20, 2023 08:21
Copy link
Contributor

@vshankar vshankar left a comment

Choose a reason for hiding this comment

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

Minor comment - otherwise LGTM.

@rishabh-d-dave Please run this through QA.

@rishabh-d-dave
Copy link
Contributor Author

Minor comment - otherwise LGTM.

Replied - #50497 (comment). Let me know whatever you think is appropriate and I'll make the change accordingly.

@rishabh-d-dave Please run this through QA.

This has been tested before but I'll give this one more run - #50497 (comment)

Add methods that will accept read/write permissions, CephFS names and
CephFS mount point and in return will generate string form of MON, OSD
and MDS caps exactly as it is reported in Ceph keyrings.

Replace similar code in test_multifs_auth.py with calls to these helper
methods.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
@rishabh-d-dave rishabh-d-dave added wip-rishabh-testing Rishabh's testing label and removed needs-review labels Mar 24, 2023
@rishabh-d-dave rishabh-d-dave removed the wip-rishabh-testing Rishabh's testing label label Mar 24, 2023
@rishabh-d-dave rishabh-d-dave merged commit 76177ab into ceph:main Mar 24, 2023
@rishabh-d-dave rishabh-d-dave deleted the fs-qa-caps-helper branch March 24, 2023 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cephfs Ceph File System tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants