qa/cephfs: improve test_multifs_auth.py

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>

diff --git a/qa/tasks/cephfs/test_multifs_auth.py b/qa/tasks/cephfs/test_multifs_auth.py
index da9677a38fccfc6db6e8ed6a24bf372a87e8e5ec..c9ea5f52897f11b9caa3f0d21e48522c2ea4b45f 100644 (file)
--- a/qa/tasks/cephfs/test_multifs_auth.py
+++ b/qa/tasks/cephfs/test_multifs_auth.py
@@ -42,26 +42,23 @@ class TestMONCaps(TestMultiFS):
 
     def test_moncap_with_one_fs_names(self):
         moncap = f'allow r fsname={self.fs1.name}'
-        self.setup_test_env(moncap)
+        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):
         moncap = (f'allow r fsname={self.fs1.name}, '
                   f'allow r fsname={self.fs2.name}')
-        self.setup_test_env(moncap)
+        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):
         moncap = 'allow r'
-        self.setup_test_env(moncap)
+        self.create_client(self.client_id, moncap)
 
         self.captester.run_mon_cap_tests(self.fs1, self.client_id)
 
-    def setup_test_env(self, moncap):
-        self.create_client(self.client_id, moncap)
-
 
 #TODO: add tests for capsecs 'p' and 's'.
 class TestMDSCaps(TestMultiFS):
@@ -72,15 +69,23 @@ class TestMDSCaps(TestMultiFS):
     3. Remount the current mounts with this new client.
     4. Test read and write on both FSs.
     """
+    def setUp(self):
+        super(self.__class__, self).setUp()
+        self.mounts = (self.mount_a, self.mount_b)
+
     def test_rw_with_fsname_and_no_path_in_cap(self):
         PERM = 'rw'
-        self.setup_test_env(PERM, True)
+        self.captester.write_test_files(self.mounts)
+        keyring_paths = self._create_client(PERM, fsname=True)
+        self.remount_with_new_client(keyring_paths)
 
         self.captester.run_mds_cap_tests(PERM)
 
     def test_r_with_fsname_and_no_path_in_cap(self):
         PERM = 'r'
-        self.setup_test_env(PERM, True)
+        self.captester.write_test_files(self.mounts)
+        keyring_paths = self._create_client(PERM, fsname=True)
+        self.remount_with_new_client(keyring_paths)
 
         self.captester.run_mds_cap_tests(PERM)
 
@@ -88,7 +93,9 @@ 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.setup_test_env(PERM, True, CEPHFS_MNTPT)
+        self.captester.write_test_files(self.mounts, CEPHFS_MNTPT)
+        keyring_paths = self._create_client(PERM, fsname=True)
+        self.remount_with_new_client(keyring_paths, CEPHFS_MNTPT)
 
         self.captester.run_mds_cap_tests(PERM, CEPHFS_MNTPT)
 
@@ -96,7 +103,9 @@ 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.setup_test_env(PERM, True, CEPHFS_MNTPT)
+        self.captester.write_test_files(self.mounts, CEPHFS_MNTPT)
+        keyring_paths = self._create_client(PERM, fsname=True)
+        self.remount_with_new_client(keyring_paths, CEPHFS_MNTPT)
 
         self.captester.run_mds_cap_tests(PERM, CEPHFS_MNTPT)
 
@@ -106,7 +115,9 @@ 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.setup_test_env(PERM, False, CEPHFS_MNTPT)
+        self.captester.write_test_files(self.mounts, CEPHFS_MNTPT)
+        keyring_paths = self._create_client(PERM)
+        self.remount_with_new_client(keyring_paths, CEPHFS_MNTPT)
 
         self.captester.run_mds_cap_tests(PERM, CEPHFS_MNTPT)
 
@@ -116,19 +127,25 @@ 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.setup_test_env(PERM, False, CEPHFS_MNTPT)
+        self.captester.write_test_files(self.mounts, CEPHFS_MNTPT)
+        keyring_paths = self._create_client(PERM)
+        self.remount_with_new_client(keyring_paths, CEPHFS_MNTPT)
 
         self.captester.run_mds_cap_tests(PERM, CEPHFS_MNTPT)
 
     def test_rw_with_no_fsname_and_no_path(self):
         PERM = 'rw'
-        self.setup_test_env(PERM)
+        self.captester.write_test_files(self.mounts)
+        keyring_paths = self._create_client(PERM)
+        self.remount_with_new_client(keyring_paths)
 
         self.captester.run_mds_cap_tests(PERM)
 
     def test_r_with_no_fsname_and_no_path(self):
         PERM = 'r'
-        self.setup_test_env(PERM)
+        self.captester.write_test_files(self.mounts)
+        keyring_paths = self._create_client(PERM)
+        self.remount_with_new_client(keyring_paths)
 
         self.captester.run_mds_cap_tests(PERM)
 
@@ -160,31 +177,18 @@ class TestMDSCaps(TestMultiFS):
 
         return moncap, osdcap, mdscap
 
-    def setup_test_env(self, perm, fsname=False, cephfs_mntpt='/'):
-        """
-        Creates the cap string, files on both the FSs and then creates the
-        new client with the cap and remounts both the FSs with newly created
-        client.
-        """
-        self.captester.write_test_files((self.mount_a, self.mount_b),
-                                        cephfs_mntpt)
-
-        keyring_paths = self.create_client_and_keyring_file(perm, fsname,
-                                                            cephfs_mntpt)
-        self.remount_with_new_client(cephfs_mntpt, keyring_paths)
-
-    def create_client_and_keyring_file(self, perm, fsname, cephfs_mntpt):
+    def _create_client(self, perm, fsname=False, cephfs_mntpt='/'):
         moncap, osdcap, mdscap = self.generate_caps(perm, fsname,
                                                     cephfs_mntpt)
 
         keyring = self.create_client(self.client_id, moncap, osdcap, mdscap)
         keyring_paths = []
-        for mount_x in (self.mount_a, self.mount_b):
+        for mount_x in self.mounts:
             keyring_paths.append(mount_x.client_remote.mktemp(data=keyring))
 
         return keyring_paths
 
-    def remount_with_new_client(self, cephfs_mntpt, keyring_paths):
+    def remount_with_new_client(self, keyring_paths, cephfs_mntpt='/'):
         if isinstance(cephfs_mntpt, str) and cephfs_mntpt != '/' :
             cephfs_mntpt = '/' + cephfs_mntpt