qa/cephfs/cap_tester: simplify CapTester and its instantiation

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>
(cherry picked from commit f0ffade0525674b1f851797b1107060d8aa93d5c)

Conflicts:
 - qa/tasks/cephfs/caps_helper.py:
   This file went through changes due to merging of commit 59c9104c54d6.

diff --git a/qa/tasks/cephfs/caps_helper.py b/qa/tasks/cephfs/caps_helper.py
index 79baeb3b7463eaebd33219b99db71f03afe023e7..e53ac7ea88040db52b1948476b9841f6e5c877f9 100644 (file)
--- a/qa/tasks/cephfs/caps_helper.py
+++ b/qa/tasks/cephfs/caps_helper.py
@@ -163,19 +163,53 @@ def assert_in(member, container):
     assert member in container, msg
 
 
-class CapTester:
-    """
-    Test that MON and MDS caps are enforced.
+class MonCapTester:
+    '''
+    Tested that MON caps are enforced on client by checking if the name of
+    CephFS, which client is authorized for, is listed in output of commmand
+    "ceph fs ls".
+
+    USAGE: Create object of this class at the beginning of the test method
+    and when MON cap needs to be tested, call the method run_mon_cap_tests().
+    '''
+
+    def run_mon_cap_tests(self, fs, client_id):
+        """
+        Check that MON cap is enforced for a client by searching for a Ceph
+        FS name in output of cmd "fs ls" executed with that client's caps.
+
+        fs is any fs object so that ceph commands can be exceuted.
+        """
+        get_cluster_cmd_op = fs.mon_manager.raw_cluster_cmd
+        keyring = get_cluster_cmd_op(args=f'auth get client.{client_id}')
+        moncap = get_mon_cap_from_keyring(keyring)
+        keyring_path = fs.admin_remote.mktemp(data=keyring)
 
-    MDS caps are tested by exercising read-write permissions and MON caps are
-    tested using output of command "ceph fs ls". Besides, it provides
-    write_test_files() which creates test files at the given path on CephFS
-    mounts passed to it.
+        fsls = get_cluster_cmd_op(
+            args=f'fs ls --id {client_id} -k {keyring_path}')
+        log.info(f'output of fs ls cmd run by client.{client_id} -\n{fsls}')
 
-    USAGE: Call write_test_files() method at the beginning of the test and
-    once the caps that needs to be tested are assigned to the client and
-    CephFS be remount for caps to effective, call run_cap_tests(),
-    run_mon_cap_tests() or run_mds_cap_tests() as per the need.
+        fsnames = get_fsnames_from_moncap(moncap)
+        if fsnames == []:
+            log.info('no FS name is mentioned in moncap, client has '
+                     'permission to list all files. moncap -\n{moncap}')
+            return
+
+        log.info('FS names are mentioned in moncap. moncap -\n{moncap}')
+        log.info('testing for presence of these FS names in output of '
+                 '"fs ls" command run by client.')
+        for fsname in fsnames:
+            fsname_cap_str = f'name: {fsname}'
+            assert_in(fsname_cap_str, fsls)
+
+
+class MdsCapTester:
+    """
+    Test that MDS caps are enforced on the client by exercising read-write
+    permissions.
+
+    USAGE: Create object of this class at the beginning of the test method
+    and when MDS cap needs to be tested, call the method run_mds_cap_tests().
     """
 
     def __init__(self, mount=None, path=''):
@@ -233,39 +267,6 @@ class CapTester:
             cephfs_mntpt = {self.mount.cephfs_mntpt}
             hostfs_mntpt = {self.mount.hostfs_mntpt}''')
 
-    def run_cap_tests(self, fs, client_id, perm, mntpt=None):
-        self.run_mon_cap_tests(fs, client_id)
-        self.run_mds_cap_tests(perm, mntpt=mntpt)
-
-    def run_mon_cap_tests(self, fs, client_id):
-        """
-        Check that MON cap is enforced for a client by searching for a Ceph
-        FS name in output of cmd "fs ls" executed with that client's caps.
-
-        fs is any fs object so that ceph commands can be exceuted.
-        """
-        get_cluster_cmd_op = fs.mon_manager.raw_cluster_cmd
-        keyring = get_cluster_cmd_op(args=f'auth get client.{client_id}')
-        moncap = get_mon_cap_from_keyring(keyring)
-        keyring_path = fs.admin_remote.mktemp(data=keyring)
-
-        fsls = get_cluster_cmd_op(
-            args=f'fs ls --id {client_id} -k {keyring_path}')
-        log.info(f'output of fs ls cmd run by client.{client_id} -\n{fsls}')
-
-        fsnames = get_fsnames_from_moncap(moncap)
-        if fsnames == []:
-            log.info('no FS name is mentioned in moncap, client has '
-                     'permission to list all files. moncap -\n{moncap}')
-            return
-
-        log.info('FS names are mentioned in moncap. moncap -\n{moncap}')
-        log.info('testing for presence of these FS names in output of '
-                 '"fs ls" command run by client.')
-        for fsname in fsnames:
-            fsname_cap_str = f'name: {fsname}'
-            assert_in(fsname_cap_str, fsls)
-
     def run_mds_cap_tests(self, perm, mntpt=None):
         """
         Run test for read perm and, for write perm, run positive test if it
@@ -324,6 +325,25 @@ class CapTester:
         log.info('absence of write perm was tested successfully: '
                  f'failed to be write data to file {self.path}.')
 
+
+class CapTester(MonCapTester, MdsCapTester):
+    '''
+    Test MON caps as well as MDS caps. For usage see docstrings of class
+    MDSCapTester.
+
+    USAGE: Create object of this class at the beginning of the test method
+    and when MON and MDS cap needs to be tested, call the method
+    run_cap_tests().
+    '''
+
+    def __init__(self, mount=None, path='', create_test_files=True):
+        if create_test_files:
+            self._create_test_files(mount, path)
+
+    def run_cap_tests(self, fs, client_id, perm, mntpt=None):
+        self.run_mon_cap_tests(fs, client_id)
+        self.run_mds_cap_tests(perm, mntpt)
+
     def _conduct_neg_test_for_root_squash_caps(self, _cmdargs, sudo_write=False):
         possible_errmsgs = ('permission denied', 'operation not permitted')
         cmdargs = ['sudo'] if sudo_write else ['']
@@ -352,4 +372,3 @@ class CapTester:
 
     def conduct_pos_test_for_open_caps(self, sudo_read=True):
         self.conduct_pos_test_for_read_caps(sudo_read)
-

diff --git a/qa/tasks/cephfs/test_admin.py b/qa/tasks/cephfs/test_admin.py
index 986855fcb645dbd106208a6a407fc0891a2d0bee..85b6a9225930f8e96bd5ef67b2780fc57e942e12 100644 (file)
--- a/qa/tasks/cephfs/test_admin.py
+++ b/qa/tasks/cephfs/test_admin.py
@@ -1289,8 +1289,7 @@ class TestFsAuthorize(CephFSTestCase):
         keyring = self.fs.authorize(self.client_id, FS_AUTH_CAPS)
 
         self._remount(keyring)
-        self.captester.run_mon_cap_tests(self.fs, self.client_id)
-        self.captester.run_mds_cap_tests(PERM)
+        self.captester.run_cap_tests(self.fs, self.client_id, PERM)
 
     def test_single_path_rw(self):
         PERM = 'rw'
@@ -1299,8 +1298,7 @@ class TestFsAuthorize(CephFSTestCase):
         keyring = self.fs.authorize(self.client_id, FS_AUTH_CAPS)
 
         self._remount(keyring)
-        self.captester.run_mon_cap_tests(self.fs, self.client_id)
-        self.captester.run_mds_cap_tests(PERM)
+        self.captester.run_cap_tests(self.fs, self.client_id, PERM)
 
     def test_single_path_rootsquash(self):
         if not isinstance(self.mount_a, FuseMount):
@@ -1400,8 +1398,8 @@ class TestFsAuthorize(CephFSTestCase):
             PERM = c[1]
             self._remount(keyring, PATH)
             # actual tests...
-            self.captesters[i].run_mon_cap_tests(self.fs, self.client_id)
-            self.captesters[i].run_mds_cap_tests(PERM, PATH)
+            self.captesters[i].run_cap_tests(self.fs, self.client_id, PERM,
+                                             PATH)
 
     def tearDown(self):
         self.mount_a.umount_wait()

diff --git a/qa/tasks/cephfs/test_multifs_auth.py b/qa/tasks/cephfs/test_multifs_auth.py
index 99ff70e0dba08feacdd8386aa63ab04089544624..3d4acd58398ca29d4355ddb2e5d2d6975ece28b8 100644 (file)
--- a/qa/tasks/cephfs/test_multifs_auth.py
+++ b/qa/tasks/cephfs/test_multifs_auth.py
@@ -4,8 +4,9 @@ Test for Ceph clusters with multiple FSs.
 import logging
 
 from tasks.cephfs.cephfs_test_case import CephFSTestCase
-from tasks.cephfs.caps_helper import (CapTester, gen_mon_cap_str,
-                                      gen_osd_cap_str, gen_mds_cap_str)
+from tasks.cephfs.caps_helper import (MonCapTester, MdsCapTester,
+                                      gen_mon_cap_str, gen_osd_cap_str,
+                                      gen_mds_cap_str)
 
 from teuthology.exceptions import CommandFailedError
 
@@ -40,21 +41,21 @@ class TestMultiFS(CephFSTestCase):
 class TestMONCaps(TestMultiFS):
 
     def test_moncap_with_one_fs_names(self):
-        self.captester = CapTester()
+        self.captester = MonCapTester()
         moncap = gen_mon_cap_str((('r', self.fs1.name),))
         self.create_client(self.client_id, moncap)
 
         self.captester.run_mon_cap_tests(self.fs1, self.client_id)
 
     def test_moncap_with_multiple_fs_names(self):
-        self.captester = CapTester()
+        self.captester = MonCapTester()
         moncap = gen_mon_cap_str((('r', self.fs1.name), ('r', self.fs2.name)))
         self.create_client(self.client_id, moncap)
 
         self.captester.run_mon_cap_tests(self.fs1, self.client_id)
 
     def test_moncap_with_blanket_allow(self):
-        self.captester = CapTester()
+        self.captester = MonCapTester()
         moncap = 'allow r'
         self.create_client(self.client_id, moncap)
 
@@ -72,7 +73,8 @@ class TestMDSCaps(TestMultiFS):
     """
     def test_rw_with_fsname_and_no_path_in_cap(self):
         PERM = 'rw'
-        self.captesters = (CapTester(self.mount_a), CapTester(self.mount_b))
+        self.captesters = (MdsCapTester(self.mount_a),
+                           MdsCapTester(self.mount_b))
 
         moncap, osdcap, mdscap = self._gen_caps(PERM, both_fsnames=True)
         keyring = self.create_client(self.client_id, moncap, osdcap, mdscap)
@@ -83,7 +85,8 @@ class TestMDSCaps(TestMultiFS):
 
     def test_r_with_fsname_and_no_path_in_cap(self):
         PERM = 'r'
-        self.captesters = (CapTester(self.mount_a), CapTester(self.mount_b))
+        self.captesters = (MdsCapTester(self.mount_a),
+                           MdsCapTester(self.mount_b))
 
         moncap, osdcap, mdscap = self._gen_caps(PERM, both_fsnames=True)
         keyring = self.create_client(self.client_id, moncap, osdcap, mdscap)
@@ -96,8 +99,8 @@ class TestMDSCaps(TestMultiFS):
         PERM, CEPHFS_MNTPT = 'rw', 'dir1'
         self.mount_a.run_shell(f'mkdir {CEPHFS_MNTPT}')
         self.mount_b.run_shell(f'mkdir {CEPHFS_MNTPT}')
-        self.captesters = (CapTester(self.mount_a, CEPHFS_MNTPT),
-                           CapTester(self.mount_b, CEPHFS_MNTPT))
+        self.captesters = (MdsCapTester(self.mount_a, CEPHFS_MNTPT),
+                           MdsCapTester(self.mount_b, CEPHFS_MNTPT))
 
         moncap, osdcap, mdscap = self._gen_caps(PERM, True, CEPHFS_MNTPT)
         keyring = self.create_client(self.client_id, moncap, osdcap, mdscap)
@@ -110,8 +113,8 @@ class TestMDSCaps(TestMultiFS):
         PERM, CEPHFS_MNTPT = 'r', 'dir1'
         self.mount_a.run_shell(f'mkdir {CEPHFS_MNTPT}')
         self.mount_b.run_shell(f'mkdir {CEPHFS_MNTPT}')
-        self.captesters = (CapTester(self.mount_a, CEPHFS_MNTPT),
-                           CapTester(self.mount_b, CEPHFS_MNTPT))
+        self.captesters = (MdsCapTester(self.mount_a, CEPHFS_MNTPT),
+                           MdsCapTester(self.mount_b, CEPHFS_MNTPT))
 
         moncap, osdcap, mdscap = self._gen_caps(PERM, True, CEPHFS_MNTPT)
         keyring = self.create_client(self.client_id, moncap, osdcap, mdscap)
@@ -126,8 +129,8 @@ class TestMDSCaps(TestMultiFS):
         PERM, CEPHFS_MNTPT = 'rw', 'dir1'
         self.mount_a.run_shell(f'mkdir {CEPHFS_MNTPT}')
         self.mount_b.run_shell(f'mkdir {CEPHFS_MNTPT}')
-        self.captesters = (CapTester(self.mount_a, CEPHFS_MNTPT),
-                           CapTester(self.mount_b, CEPHFS_MNTPT))
+        self.captesters = (MdsCapTester(self.mount_a, CEPHFS_MNTPT),
+                           MdsCapTester(self.mount_b, CEPHFS_MNTPT))
 
         moncap, osdcap, mdscap = self._gen_caps(PERM, False, CEPHFS_MNTPT)
         keyring = self.create_client(self.client_id, moncap, osdcap, mdscap)
@@ -142,8 +145,8 @@ class TestMDSCaps(TestMultiFS):
         PERM, CEPHFS_MNTPT = 'r', 'dir1'
         self.mount_a.run_shell(f'mkdir {CEPHFS_MNTPT}')
         self.mount_b.run_shell(f'mkdir {CEPHFS_MNTPT}')
-        self.captesters = (CapTester(self.mount_a, CEPHFS_MNTPT),
-                           CapTester(self.mount_b, CEPHFS_MNTPT))
+        self.captesters = (MdsCapTester(self.mount_a, CEPHFS_MNTPT),
+                           MdsCapTester(self.mount_b, CEPHFS_MNTPT))
 
         moncap, osdcap, mdscap = self._gen_caps(PERM, False, CEPHFS_MNTPT)
         keyring = self.create_client(self.client_id, moncap, osdcap, mdscap)
@@ -154,7 +157,8 @@ class TestMDSCaps(TestMultiFS):
 
     def test_rw_with_no_fsname_and_no_path(self):
         PERM = 'rw'
-        self.captesters = (CapTester(self.mount_a), CapTester(self.mount_b))
+        self.captesters = (MdsCapTester(self.mount_a),
+                           MdsCapTester(self.mount_b))
 
         moncap, osdcap, mdscap = self._gen_caps(PERM)
         keyring = self.create_client(self.client_id, moncap, osdcap, mdscap)
@@ -165,7 +169,8 @@ class TestMDSCaps(TestMultiFS):
 
     def test_r_with_no_fsname_and_no_path(self):
         PERM = 'r'
-        self.captesters = (CapTester(self.mount_a), CapTester(self.mount_b))
+        self.captesters = (MdsCapTester(self.mount_a),
+                           MdsCapTester(self.mount_b))
 
         moncap, osdcap, mdscap = self._gen_caps(PERM)
         keyring = self.create_client(self.client_id, moncap, osdcap, mdscap)