Skip to content

qa/cephfs: upgrade caps_helper.py#46994

Merged
rishabh-d-dave merged 11 commits intoceph:mainfrom
rishabh-d-dave:qa-caps-helper-upgrade
Aug 26, 2022
Merged

qa/cephfs: upgrade caps_helper.py#46994
rishabh-d-dave merged 11 commits intoceph:mainfrom
rishabh-d-dave:qa-caps-helper-upgrade

Conversation

@rishabh-d-dave
Copy link
Contributor

@rishabh-d-dave rishabh-d-dave commented Jul 6, 2022

Related PR - PR #41779. Commits on this PR originally were located on PR #41779. These have been moved to a separate PR for ease of review, testing, merging and, more importantly to get these commits merged sooner so that this code can be reused to write other PRs.

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)
    • QA Improvements
  • 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 a review from a team July 6, 2022 11:57
@rishabh-d-dave
Copy link
Contributor Author

jenkins test make check

@rishabh-d-dave
Copy link
Contributor Author

jenkins test make check arm64

@rishabh-d-dave
Copy link
Contributor Author

jenkins test make check

@rishabh-d-dave rishabh-d-dave force-pushed the qa-caps-helper-upgrade branch 5 times, most recently from 646775e to e5e95c3 Compare July 7, 2022 15:52
@rishabh-d-dave
Copy link
Contributor Author

jenkins test make check

@rishabh-d-dave
Copy link
Contributor Author

jenkins test api

@rishabh-d-dave
Copy link
Contributor Author

@rishabh-d-dave rishabh-d-dave force-pushed the qa-caps-helper-upgrade branch 2 times, most recently from 790307c to 997d7e5 Compare July 8, 2022 07:49
@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Jul 8, 2022

@vshankar First 2 commits on this PR are from PR #46993 and PR #46991. Rest are as it is from PR #41779.

@github-actions
Copy link

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

@rishabh-d-dave rishabh-d-dave force-pushed the qa-caps-helper-upgrade branch 2 times, most recently from d074b8a to f477ebf Compare July 20, 2022 11:55
@rishabh-d-dave
Copy link
Contributor Author

jenkins test windows

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Jul 20, 2022

This helper method prevents repeating code to write test files in
test methods for caps.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
Concerned method: caps_helper.CapsHelper.write_test_file()

When CephFS mountpoint is changed inside a single test method, the test
files created at root are neither accessible nor useful. Therefore, add
an option to create the test files in the directory passed by the user.
This also increases general flexibility of the concerned method.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
Modify caps_helper.py such that calling code does't have to store
returned values just to pass those values again to a method in
caps_helper.py. This is a common pattern where write_test_files()
return 3 values and all 3 passed as it is to run_cap_tests().

The easy way to do it is to upgrade caps_helper so that it can be used
an object and not just as a class supplying helper methods. The return
values will be stored by the object internally and thus resued. In case
of testing multiple FSs inside a single test method, we'll now need
multiple instances of this class to keep those values separate.

And since CapsHelper is not just a class supplying helper methods
anymore, it's being renamed to CapTester.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
Simplify methods in CapsTester by adding a test set, which will be a
list of tuples. The first element in tuple will be the mount object,
the second will be the test file path and the third will be the test
file data. Thus instead of having three independent class variables
that are always used together now we have list of test sets making
management of multiple test sets simpler.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
Let's change the file data to include the file path, the CephFS name
and the host FS and CephFS mountpoint so that the bugs where, let's say,
"cephfs2" is mounted instead of "cephfs1" (where obviously both the
CephFSs lie on the same Ceph cluster) is caught by the tests due to
uniqueness of every test file's content.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
With this commit, the design of run_mon_cap_tests() is now aligned with
rest of CapTester -- it's not meant to be inherited by test class
anymore and it is to be called by using CapTester instance.

This commit also changes working of this method. Now instead of
obtaining FS names from Python objects representing FSs, it obtains
those names from MON cap. This removes the need to pass the FS objects
around.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
The intention is to make logs contain some information of what's being
done by caps_helper.py regardless of which test/method is calling it.
This should make logs more understandable and also add hints for
future debugging.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
caps_helper.py since caps_helper.py has been heavily modified in
previous commits.

setup_fs_contents() is being deleted since it is not used anymore.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
Upgrade test_admin.FsAuthorize to repair its compatibility with
caps_helper.py since caps_hepler.py has been heavily modified in
previous commits.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
test_single_path_rootsquash instead of using helper methods duplicates
the code from those methods. This commit fixes that and also upgrade
this method since caps_helper was upgraded in previous commits.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
Remove setup_test_env() because removing this methods removes an extra
layer of abstraction which makes tests more readable. Rename
_create_client_and_keyring_file() to just _create_client() and reverse
the order of parameters in remount_with_new_client() and set default
value of cephfs_mntpt to '/'.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
@rishabh-d-dave rishabh-d-dave force-pushed the qa-caps-helper-upgrade branch from f477ebf to 9bac23b Compare July 21, 2022 12:23
@rishabh-d-dave rishabh-d-dave requested a review from lxbsz July 21, 2022 12:25
Copy link
Member

@batrick batrick left a comment

Choose a reason for hiding this comment

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

nit: qa/cephfs: add a helper method in caps_helper.py contains an unrelated method, seemingly (run_cap_tests)

Looks fine otherwise if your testing is good. I did not go through it in depth.

@rishabh-d-dave rishabh-d-dave added needs-qa wip-rishabh-testing Rishabh's testing label and removed needs-review labels Aug 19, 2022
@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Aug 26, 2022

QA was successful - https://tracker.ceph.com/projects/cephfs/wiki/Main#2022-Aug-26.

test_multifs_auth.py failed for unrelated reason during (first) run[1] it passed in re-run[2].

@rishabh-d-dave rishabh-d-dave removed needs-qa wip-rishabh-testing Rishabh's testing label labels Aug 26, 2022
@rishabh-d-dave
Copy link
Contributor Author

test_admin.py was not executed during QA run. I ran it separately - http://pulpito.front.sepia.ceph.com/rishabh-2022-08-26_11:25:16-fs-wip-rishabh-testing-2022Aug19-distro-default-smithi/.

@rishabh-d-dave rishabh-d-dave merged commit e7b6c9d into ceph:main Aug 26, 2022
@rishabh-d-dave rishabh-d-dave added the wip-rishabh-testing Rishabh's testing label label Aug 26, 2022
@rishabh-d-dave rishabh-d-dave deleted the qa-caps-helper-upgrade branch December 18, 2023 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cephfs Ceph File System tests wip-rishabh-testing Rishabh's testing label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants