Skip to content

qa/cephfs: improvements in caps_helper#50882

Merged
rishabh-d-dave merged 7 commits intoceph:mainfrom
rishabh-d-dave:fs-qa-CapTester
Apr 20, 2023
Merged

qa/cephfs: improvements in caps_helper#50882
rishabh-d-dave merged 7 commits intoceph:mainfrom
rishabh-d-dave:fs-qa-CapTester

Conversation

@rishabh-d-dave
Copy link
Contributor

@rishabh-d-dave rishabh-d-dave commented Apr 5, 2023

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)
    • Change in 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 a review from a team April 5, 2023 16:13
@rishabh-d-dave rishabh-d-dave changed the title qa/cephfs: improvements in caps_helper.CapTester qa/cephfs: improvements in caps_helper Apr 7, 2023
@rishabh-d-dave rishabh-d-dave mentioned this pull request Apr 7, 2023
Merged
4 tasks
vshankar
vshankar requested changes Apr 10, 2023
View reviewed changes
vshankar
vshankar reviewed Apr 10, 2023
View reviewed changes
@rishabh-d-dave
qa/cephfs: minor improvement in caps_helper
1a7ca48 
Use Python type list instead of tuple since it get's necessary to modify
members of this sequence.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
@rishabh-d-dave rishabh-d-dave force-pushed the fs-qa-CapTester branch 2 times, most recently from c809357 to 340395f Compare April 11, 2023 10:28
rishabh-d-dave added a commit to rishabh-d-dave/ceph that referenced this pull request Apr 11, 2023
@rishabh-d-dave
DNM: qa/cephfs: changes to test_admin.TestFsAuthorizerUpdate after...
c9ad6e8 
changes to caps_helper.CapTester.run_cap_tests().

TO BE SQUASHED when PR ceph#50882 merges.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
rishabh-d-dave added a commit to rishabh-d-dave/ceph that referenced this pull request Apr 11, 2023
@rishabh-d-dave
DNM: qa/cephfs: changes to test_admin.TestFsAuthorizerUpdate after...
3007b1f 
changes to caps_helper.CapTester.run_cap_tests().

TO BE SQUASHED when PR ceph#50882 merges.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
@rishabh-d-dave rishabh-d-dave force-pushed the fs-qa-CapTester branch 2 times, most recently from b9b099e to 5ef46dd Compare April 12, 2023 09:13
@vshankar vshankar requested a review from a team April 13, 2023 12:24
rishabh-d-dave added a commit to rishabh-d-dave/ceph that referenced this pull request Apr 13, 2023
@rishabh-d-dave
qa/cephfs: don't inherit CephFSTestCase in CapTester
a84dc64 
Inheritting CephFSTestCase in CapTester just for methods assertEqual()
and assertIn() from class unittest.TestCase is odd and heavy-weight.
Don't inherit CephFSTestCase and use simple assert instead.

Reference: ceph#50882 (comment).

To avoid code duplication, a couple of similar methods have been added
instead.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Apr 13, 2023

jenkins test make check arm64

rishabh-d-dave added a commit to rishabh-d-dave/ceph that referenced this pull request Apr 13, 2023
@rishabh-d-dave
qa/cephfs: don't inherit CephFSTestCase in CapTester
01a943d 
Inheritting CephFSTestCase in CapTester just for methods assertEqual()
and assertIn() from class unittest.TestCase is odd and heavy-weight.
Don't inherit CephFSTestCase and use simple assert instead.

Reference: ceph#50882 (comment).

To avoid code duplication, a couple of similar methods have been added
instead.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
vshankar
vshankar reviewed Apr 14, 2023
View reviewed changes
rishabh-d-dave added a commit to rishabh-d-dave/ceph that referenced this pull request Apr 14, 2023
@rishabh-d-dave
qa/cephfs: don't inherit CephFSTestCase in CapTester
58ad625 
Inheritting CephFSTestCase in CapTester just for methods assertEqual()
and assertIn() from class unittest.TestCase is odd and heavy-weight.
Don't inherit CephFSTestCase and use simple assert instead.

Reference: ceph#50882 (comment).

To avoid code duplication, a couple of similar methods have been added
instead.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
vshankar
vshankar requested changes Apr 14, 2023
View reviewed changes
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.

Also fix the make check failure:

flake8: commands[0] /home/jenkins-build/build/workspace/ceph-pull-requests/qa> flake8 --select=F,E9 --exclude=venv,.tox
./tasks/cephfs/caps_helper.py:8:1: F401 'tasks.cephfs.cephfs_test_case.CephFSTestCase' imported but unused

qa/tasks/cephfs/caps_helper.py Outdated

if len(caps) == 1:
return _gen_mon_cap_str(caps[0])
return _gen_mon_cap_str(*caps[0])
Copy link
Contributor

@vshankar vshankar Apr 14, 2023

Choose a reason for hiding this comment

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

explain a bit in the commit message about the bug.

Copy link
Contributor Author

@rishabh-d-dave rishabh-d-dave Apr 14, 2023

Choose a reason for hiding this comment

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

I avoided it because this meant explaining a Python concept.

Copy link
Contributor Author

@rishabh-d-dave rishabh-d-dave Apr 14, 2023

Choose a reason for hiding this comment

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

I've elaborated on the Python concept in the commit message now.

Copy link
Contributor

@vshankar vshankar Apr 14, 2023

Choose a reason for hiding this comment

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

OK, thx. So, the caps didn't get generated as expected? Didn't that cause a test case failure?

Copy link
Contributor Author

@rishabh-d-dave rishabh-d-dave Apr 14, 2023

Choose a reason for hiding this comment

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

It does. I've spotted this once in integration test runs.

Copy link
Contributor Author

@rishabh-d-dave rishabh-d-dave Apr 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This is where the bug was introduced - 969a93d. Here's the teuthology runs - #50497 (comment) And the testing included PR #50212 along with PR #50497.

Copy link
Contributor

@vshankar vshankar Apr 18, 2023

Choose a reason for hiding this comment

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

This is where the bug was introduced - 969a93d.

I don't think so. This commit uses unpack_tuple which does not have the bug - callers for _gen_mds_cap_str() are:

    if len(caps) == 1:
       return _gen_mds_cap_str(caps[0])

and

    for i, c in enumerate(caps):
       mds_cap +=  _gen_mds_cap_str(c)
       if i != len(caps) - 1:
           mds_cap += ', '

which seems fine.

Here's the teuthology runs - #50497 (comment) And the testing included PR #41779 along with PR #50497.

Isn't 87025d1 (PR #50664) where the bug got introduced? The test run for this is: http://pulpito.front.sepia.ceph.com/rishabh-2023-04-05_11:02:03-fs:functional-wip-rishabh-fs-auth-subcmd-distro-default-smithi/ - branch: wip-rishabh-fs-auth-subcmd which has the following code:

87025d15858a (Rishabh Dave 2023-03-29 20:48:53 +0530  71)     def _gen_mds_cap_str(perm, fsname=None, cephfs_mntpt='/'):
969a93d0dcf1 (Rishabh Dave 2023-03-13 16:16:13 +0530  72)         mds_cap = f'allow {perm}'
969a93d0dcf1 (Rishabh Dave 2023-03-13 16:16:13 +0530  73)         if fsname:
969a93d0dcf1 (Rishabh Dave 2023-03-13 16:16:13 +0530  74)             mds_cap += f' fsname={fsname}'
969a93d0dcf1 (Rishabh Dave 2023-03-13 16:16:13 +0530  75)         if cephfs_mntpt != '/':
969a93d0dcf1 (Rishabh Dave 2023-03-13 16:16:13 +0530  76)             if cephfs_mntpt[0] == '/':
969a93d0dcf1 (Rishabh Dave 2023-03-13 16:16:13 +0530  77)                 cephfs_mntpt = cephfs_mntpt[1:]
969a93d0dcf1 (Rishabh Dave 2023-03-13 16:16:13 +0530  78)             mds_cap += f' path={cephfs_mntpt}'
969a93d0dcf1 (Rishabh Dave 2023-03-13 16:16:13 +0530  79)         return mds_cap
969a93d0dcf1 (Rishabh Dave 2023-03-13 16:16:13 +0530  80)
969a93d0dcf1 (Rishabh Dave 2023-03-13 16:16:13 +0530  81)     if len(caps) == 1:
c8c29aaeb643 (Rishabh Dave 2023-04-07 20:43:46 +0530  82)         return _gen_mds_cap_str(*caps[0])
969a93d0dcf1 (Rishabh Dave 2023-03-13 16:16:13 +0530  83)
969a93d0dcf1 (Rishabh Dave 2023-03-13 16:16:13 +0530  84)     mds_cap = ''
969a93d0dcf1 (Rishabh Dave 2023-03-13 16:16:13 +0530  85)     for i, c in enumerate(caps):
87025d15858a (Rishabh Dave 2023-03-29 20:48:53 +0530  86)         mds_cap +=  _gen_mds_cap_str(*c)
969a93d0dcf1 (Rishabh Dave 2023-03-13 16:16:13 +0530  87)         if i != len(caps) - 1:
969a93d0dcf1 (Rishabh Dave 2023-03-13 16:16:13 +0530  88)             mds_cap += ', '
969a93d0dcf1 (Rishabh Dave 2023-03-13 16:16:13 +0530  89)
969a93d0dcf1 (Rishabh Dave 2023-03-13 16:16:13 +0530  90)     return mds_cap

where:

c8c29aa (Rishabh Dave 2023-04-07 20:43:46 +0530 82) return _gen_mds_cap_str(*caps[0])

is part of https://github.com/ceph/ceph/pull/41779/commits

and"

87025d1 (Rishabh Dave 2023-03-29 20:48:53 +0530 86) mds_cap += _gen_mds_cap_str(*c)

is a part of https://github.com/ceph/ceph/pull/50664/commits

It looks like the above PRs (#41779 and #50664) were included in the test branch, however, only #50664 was merged. @rishabh-d-dave ?

Copy link
Contributor Author

@rishabh-d-dave rishabh-d-dave Apr 18, 2023

Choose a reason for hiding this comment

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

Isn't 87025d1 (PR #50664) where the bug got introduced? The test run for this is: http://pulpito.front.sepia.ceph.com/rishabh-2023-04-05_11:02:03-fs:functional-wip-rishabh-fs-auth-subcmd-distro-default-smithi/ - branch: wip-rishabh-fs-auth-subcmd which has the following code:

Right. I had forgotten this commit when I replied to you.

where:

c8c29aa (Rishabh Dave 2023-04-07 20:43:46 +0530 82) return _gen_mds_cap_str(*caps[0])

is part of https://github.com/ceph/ceph/pull/41779/commits

c8c29aa was not present on PR #41779. It was added to only recently when this PR was created.

and"

87025d1 (Rishabh Dave 2023-03-29 20:48:53 +0530 86) mds_cap += _gen_mds_cap_str(*c)

is a part of https://github.com/ceph/ceph/pull/50664/commits

It looks like the above PRs (#41779 and #50664) were included in the test branch, however, only #50664 was merged. @rishabh-d-dave ?

Yes.

Copy link
Contributor

@vshankar vshankar Apr 18, 2023

Choose a reason for hiding this comment

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

c8c29aa was not present on PR #41779. It was added to only recently when this PR was created.

commit 39ce51a53cf9c8602adc89354214146e6ac505c3 (HEAD -> wip-rishabh-fs-auth-subcmd, rishabh-d-dave/fs-auth-subcmd, ci/wip-rishabh-fs-auth-subcmd)
Author: Rishabh Dave <ridave@redhat.com>
Date:   Fri Apr 7 23:51:26 2023 +0530

    mon/AuthMonitor: log when parsing caps fails

    Signed-off-by: Rishabh Dave <ridave@redhat.com>

commit c8c29aaeb643ca4dbf66ea20b23ed3b81e0bc08f
Author: Rishabh Dave <ridave@redhat.com>
Date:   Fri Apr 7 20:43:46 2023 +0530

    qa/cephfs/caps_helper: fix a bug in methods that generate cap string

    Signed-off-by: Rishabh Dave <ridave@redhat.com>

The commit was a part of the test branch - might not have been in the PR, but surely in the test branch, which is a bit more concerning then (cherry-picked?).

Anyway - please add Introduced-by: <commit> in the commit message.

Copy link
Contributor Author

@rishabh-d-dave rishabh-d-dave Apr 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The commit was a part of the test branch - might not have been in the PR, but surely in the test branch, which is a bit more concerning then (cherry-picked?).

The testing branch was updated after the testing took place.

The commit has been updated - 64e3dd7.

As per our conversation on IRC I've moved this commit to separate PR - #51127.

rishabh-d-dave added a commit to rishabh-d-dave/ceph that referenced this pull request Apr 14, 2023
@rishabh-d-dave
qa/cephfs: don't inherit CephFSTestCase in CapTester
5363582 
Inheritting CephFSTestCase in CapTester just for methods assertEqual()
and assertIn() from class unittest.TestCase is odd and heavy-weight.
Don't inherit CephFSTestCase and use simple assert instead.

Reference: ceph#50882 (comment).

To avoid code duplication, a couple of similar methods have been added
instead.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
@rishabh-d-dave
qa/cephfs: update method caps_helper.CapTester.run_cap_tests()
95c6daa 
Signed-off-by: Rishabh Dave <ridave@redhat.com>
@rishabh-d-dave
qa/cephfs/cap_tester: simplify CapTester and its instantiation
f0ffade 
Class CapTester contains two distinct immiscible group of methods: one
that tests MON caps and other that tests MDS caps. When using CapTester
for the former reason the instantiation neither needs mount object and
the path where files for testing will be created nor it needs to run the
method that creates files for testing rw permissions. When using
this class for latter the case is the exact opposite.

Create 2 separate classes for each of these purpose and class that
inherits both of these classes so that instantiating the class becomes
as simple as it can be.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Apr 18, 2023

jenkins test make check arm64

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Apr 18, 2023

@vshankar Tests passed - http://pulpito.front.sepia.ceph.com/rishabh-2023-04-18_15:04:40-fs-wip-rishabh-fs-qa-CapTester-distro-default-smithi/.

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Apr 18, 2023

jenkins test make check

@vshankar
Copy link
Contributor

vshankar commented Apr 19, 2023

@vshankar Tests passed - http://pulpito.front.sepia.ceph.com/rishabh-2023-04-18_15:04:40-fs-wip-rishabh-fs-qa-CapTester-distro-default-smithi/.

Looks fine. And the PRs in this test branch are this change and #51127? Just confirmings.

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Apr 19, 2023

@vshankar

@vshankar Tests passed - http://pulpito.front.sepia.ceph.com/rishabh-2023-04-18_15:04:40-fs-wip-rishabh-fs-qa-CapTester-distro-default-smithi/.

Looks fine. And the PRs in this test branch are this change and #51127? Just confirmings.

Yes.

@rishabh-d-dave rishabh-d-dave removed the wip-vshankar-backlog Backlog of CephFS PRs to pick for testing label Apr 19, 2023
rishabh-d-dave added a commit to rishabh-d-dave/ceph that referenced this pull request Apr 20, 2023
@rishabh-d-dave
DNM: qa/cephfs: changes to test_admin.TestFsAuthorizerUpdate after...
376f4e1 
changes to caps_helper.CapTester.run_cap_tests().

TO BE SQUASHED when PR ceph#50882 merges.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
@rishabh-d-dave rishabh-d-dave merged commit 9be6de9 into ceph:main Apr 20, 2023
@rishabh-d-dave rishabh-d-dave deleted the fs-qa-CapTester branch April 20, 2023 14:28
zdover23 pushed a commit to zdover23/ceph that referenced this pull request Apr 26, 2023
@rishabh-d-dave @zdover23
qa/cephfs: don't inherit CephFSTestCase in CapTester
48a2e0d 
Inheritting CephFSTestCase in CapTester just for methods assertEqual()
and assertIn() from class unittest.TestCase is odd and heavy-weight.
Don't inherit CephFSTestCase and use simple assert instead.

Reference: ceph#50882 (comment).

To avoid code duplication, a couple of similar methods have been added
instead.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
yuvalif pushed a commit to yuvalif/ceph that referenced this pull request Apr 30, 2023
@rishabh-d-dave @yuvalif
qa/cephfs: don't inherit CephFSTestCase in CapTester
adbf11e 
Inheritting CephFSTestCase in CapTester just for methods assertEqual()
and assertIn() from class unittest.TestCase is odd and heavy-weight.
Don't inherit CephFSTestCase and use simple assert instead.

Reference: ceph#50882 (comment).

To avoid code duplication, a couple of similar methods have been added
instead.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
yuvalif pushed a commit to yuvalif/ceph that referenced this pull request Apr 30, 2023
@rishabh-d-dave @yuvalif
Merge pull request ceph#50882 from rishabh-d-dave/fs-qa-CapTester
ce8cc22 
qa/cephfs: improvements in caps_helper

Reviewed-by: Venky Shankar <vshankar@redhat.com>
@rishabh-d-dave rishabh-d-dave restored the fs-qa-CapTester branch December 18, 2023 08:56
rishabh-d-dave added a commit to rishabh-d-dave/ceph that referenced this pull request Dec 18, 2023
@rishabh-d-dave
qa/cephfs: don't inherit CephFSTestCase in CapTester
974ba54 
Inheritting CephFSTestCase in CapTester just for methods assertEqual()
and assertIn() from class unittest.TestCase is odd and heavy-weight.
Don't inherit CephFSTestCase and use simple assert instead.

Reference: ceph#50882 (comment).

To avoid code duplication, a couple of similar methods have been added
instead.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit e8bdf94)
rishabh-d-dave added a commit to rishabh-d-dave/ceph that referenced this pull request Dec 18, 2023
@rishabh-d-dave
qa/cephfs: don't inherit CephFSTestCase in CapTester
48d8621 
Inheritting CephFSTestCase in CapTester just for methods assertEqual()
and assertIn() from class unittest.TestCase is odd and heavy-weight.
Don't inherit CephFSTestCase and use simple assert instead.

Reference: ceph#50882 (comment).

To avoid code duplication, a couple of similar methods have been added
instead.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit e8bdf94)
This was referenced Dec 18, 2023
Closed
Merged
rishabh-d-dave added a commit to rishabh-d-dave/ceph that referenced this pull request Jan 29, 2024
@rishabh-d-dave
qa/cephfs: don't inherit CephFSTestCase in CapTester
8f8d104 
Inheritting CephFSTestCase in CapTester just for methods assertEqual()
and assertIn() from class unittest.TestCase is odd and heavy-weight.
Don't inherit CephFSTestCase and use simple assert instead.

Reference: ceph#50882 (comment).

To avoid code duplication, a couple of similar methods have been added
instead.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit e8bdf94)
rishabh-d-dave added a commit to rishabh-d-dave/ceph that referenced this pull request Jan 29, 2024
@rishabh-d-dave
qa/cephfs: don't inherit CephFSTestCase in CapTester
9526701 
Inheritting CephFSTestCase in CapTester just for methods assertEqual()
and assertIn() from class unittest.TestCase is odd and heavy-weight.
Don't inherit CephFSTestCase and use simple assert instead.

Reference: ceph#50882 (comment).

To avoid code duplication, a couple of similar methods have been added
instead.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit e8bdf94)
rishabh-d-dave added a commit to rishabh-d-dave/ceph that referenced this pull request Feb 13, 2024
@rishabh-d-dave
qa/cephfs: don't inherit CephFSTestCase in CapTester
f1039f1 
Inheritting CephFSTestCase in CapTester just for methods assertEqual()
and assertIn() from class unittest.TestCase is odd and heavy-weight.
Don't inherit CephFSTestCase and use simple assert instead.

Reference: ceph#50882 (comment).

To avoid code duplication, a couple of similar methods have been added
instead.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit e8bdf94)
rishabh-d-dave added a commit to rishabh-d-dave/ceph that referenced this pull request Feb 22, 2024
@rishabh-d-dave
qa/cephfs: don't inherit CephFSTestCase in CapTester
b7fbd7d 
Inheritting CephFSTestCase in CapTester just for methods assertEqual()
and assertIn() from class unittest.TestCase is odd and heavy-weight.
Don't inherit CephFSTestCase and use simple assert instead.

Reference: ceph#50882 (comment).

To avoid code duplication, a couple of similar methods have been added
instead.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit e8bdf94)
rishabh-d-dave added a commit to rishabh-d-dave/ceph that referenced this pull request Mar 22, 2024
@rishabh-d-dave
qa/cephfs: don't inherit CephFSTestCase in CapTester
425807a 
Inheritting CephFSTestCase in CapTester just for methods assertEqual()
and assertIn() from class unittest.TestCase is odd and heavy-weight.
Don't inherit CephFSTestCase and use simple assert instead.

Reference: ceph#50882 (comment).

To avoid code duplication, a couple of similar methods have been added
instead.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit e8bdf94)
rishabh-d-dave added a commit to rishabh-d-dave/ceph that referenced this pull request Mar 22, 2024
@rishabh-d-dave
qa/cephfs: don't inherit CephFSTestCase in CapTester
ed4386a 
Inheritting CephFSTestCase in CapTester just for methods assertEqual()
and assertIn() from class unittest.TestCase is odd and heavy-weight.
Don't inherit CephFSTestCase and use simple assert instead.

Reference: ceph#50882 (comment).

To avoid code duplication, a couple of similar methods have been added
instead.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit e8bdf94)
rishabh-d-dave added a commit to rishabh-d-dave/ceph that referenced this pull request Apr 4, 2024
@rishabh-d-dave
qa/cephfs: don't inherit CephFSTestCase in CapTester
a120169 
Inheritting CephFSTestCase in CapTester just for methods assertEqual()
and assertIn() from class unittest.TestCase is odd and heavy-weight.
Don't inherit CephFSTestCase and use simple assert instead.

Reference: ceph#50882 (comment).

To avoid code duplication, a couple of similar methods have been added
instead.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit e8bdf94)

Conflicts:
  qa/tasks/cephfs/caps_helper.py
  - Conflict occured because code on Reef branch changed due to merging of
    commit 59c9104.
rishabh-d-dave added a commit to rishabh-d-dave/ceph that referenced this pull request Apr 4, 2024
@rishabh-d-dave
qa/cephfs: don't inherit CephFSTestCase in CapTester
2f85621 
Inheritting CephFSTestCase in CapTester just for methods assertEqual()
and assertIn() from class unittest.TestCase is odd and heavy-weight.
Don't inherit CephFSTestCase and use simple assert instead.

Reference: ceph#50882 (comment).

To avoid code duplication, a couple of similar methods have been added
instead.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit e8bdf94)

Conflicts:
  qa/tasks/cephfs/caps_helper.py
  - Conflict occured because code on Reef branch changed due to merging of
    commit 59c9104.
mkogan1 pushed a commit to mkogan1/ceph that referenced this pull request Jul 18, 2024
@rishabh-d-dave @tserlin
qa/cephfs: don't inherit CephFSTestCase in CapTester
a14c43a 
Inheritting CephFSTestCase in CapTester just for methods assertEqual()
and assertIn() from class unittest.TestCase is odd and heavy-weight.
Don't inherit CephFSTestCase and use simple assert instead.

Reference: ceph#50882 (comment).

To avoid code duplication, a couple of similar methods have been added
instead.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit e8bdf94)

Conflicts:
  qa/tasks/cephfs/caps_helper.py
  - Conflict occured because code on Reef branch changed due to merging of
    commit 59c9104.
(cherry picked from commit 2f85621)

Conflicts:
qa/tasks/cephfs/caps_helper.py
- CephFSMount.read_file() doesn't accept sudo_read in downstream yet

Resolves: rhbz#2149717
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vshankar vshankar vshankar approved these changes

Assignees

No one assigned

Labels

cephfs Ceph File System needs-review tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@rishabh-d-dave @vshankar