From 3fb8b009019a322111cd54a33b18f6d1ada4931f Mon Sep 17 00:00:00 2001 From: mulhern Date: Wed, 14 Feb 2024 20:42:12 -0500 Subject: [PATCH 1/2] Add some post-test symlink checking Scan for filesystem links in "/dev/mapper" calculate Stratis links, and verify that those links exist and match a link announced on the D-Bus. Use stratis-decode-dm to calculate the Stratis filesystem links from the "/dev/mapper" links. Signed-off-by: mulhern --- stratis_cli_cert.py | 6 ++++ stratisd_cert.py | 6 ++++ testlib/infra.py | 76 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+) diff --git a/stratis_cli_cert.py b/stratis_cli_cert.py index 04e2345..eafccfa 100644 --- a/stratis_cli_cert.py +++ b/stratis_cli_cert.py @@ -27,6 +27,7 @@ from testlib.dbus import StratisDbus, fs_n, p_n from testlib.infra import ( DbusMonitor, + FilesystemSymlinkMonitor, KernelKey, PostTestCheck, StratisdSystemdStart, @@ -198,6 +199,8 @@ def tearDown(self): DbusMonitor.run_check(self, stop_time) + FilesystemSymlinkMonitor.run_check(self, stop_time) + def _test_permissions(self, command_line, permissions, exp_stdout_empty): """ Test running CLI commands with and without root permissions. @@ -1243,6 +1246,9 @@ def main(): PostTestCheck.PRIVATE_SYMLINKS in parsed_args.post_test_check or parsed_args.verify_devices ) + FilesystemSymlinkMonitor.verify_devices = ( + PostTestCheck.FILESYSTEM_SYMLINKS in parsed_args.post_test_check + ) StratisCertify.maxDiff = None DbusMonitor.highest_revision_number = parsed_args.highest_revision_number diff --git a/stratisd_cert.py b/stratisd_cert.py index 94076ba..0cbbcbb 100644 --- a/stratisd_cert.py +++ b/stratisd_cert.py @@ -32,6 +32,7 @@ from testlib.dbus import StratisDbus, fs_n, p_n from testlib.infra import ( DbusMonitor, + FilesystemSymlinkMonitor, KernelKey, PostTestCheck, StratisdSystemdStart, @@ -229,6 +230,8 @@ def tearDown(self): DbusMonitor.run_check(self, stop_time) + FilesystemSymlinkMonitor.run_check(self, stop_time) + def _unittest_set_property( self, object_path, param_iface, dbus_param, dbus_value, exception_name ): # pylint: disable=too-many-arguments @@ -1362,6 +1365,9 @@ def main(): PostTestCheck.PRIVATE_SYMLINKS in parsed_args.post_test_check or parsed_args.verify_devices ) + FilesystemSymlinkMonitor.verify_devices = ( + PostTestCheck.FILESYSTEM_SYMLINKS in parsed_args.post_test_check + ) StratisCertify.maxDiff = None DbusMonitor.highest_revision_number = parsed_args.highest_revision_number print(f"Using block device(s) for tests: {StratisCertify.DISKS}") diff --git a/testlib/infra.py b/testlib/infra.py index 19e9d80..aea3c28 100644 --- a/testlib/infra.py +++ b/testlib/infra.py @@ -245,6 +245,81 @@ def run_check(self): pass +class FilesystemSymlinkMonitor(unittest.TestCase): + """ + Verify that devicmapper devices for filesystems have corresponding symlinks. + """ + + _DECODE_DM = "stratis-decode-dm" + + def run_check(self, stop_time): + """ + Check that the filesystem links on the D-Bus and the filesystem links + expected from looking at filesystem devicemapper paths match exactly. + + :param int stop_time: the time the test completed + """ + + if not FilesystemSymlinkMonitor.verify_devices: # pylint: disable=no-member + pass + + time.sleep(sleep_time(stop_time, 16)) + + managed_objects = StratisDbus.get_managed_objects() + + filesystems = frozenset( + [ + obj_data[StratisDbus.FS_IFACE]["Devnode"] + for obj_data in managed_objects.values() + if StratisDbus.FS_IFACE in obj_data + ] + ) + + found = 0 + + try: + for dev in os.listdir("/dev/mapper"): + if fnmatch.fnmatch(dev, "stratis-1-*-thin-fs-*"): + found += 1 + command = [ + FilesystemSymlinkMonitor._DECODE_DM, + os.path.join("/dev/mapper", dev), + "--output=symlink", + ] + try: + with subprocess.Popen( + command, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) as proc: + (stdoutdata, stderrdata) = proc.communicate() + if proc.returncode == 0: + symlink = stdoutdata.decode("utf-8").strip() + self.assertTrue(os.path.exists(symlink)) + self.assertIn(symlink, filesystems) + else: + raise RuntimeError( + f"{FilesystemSymlinkMonitor._DECODE_DM} " + f"invocation failed: {stderrdata.decode('utf-8')}" + ) + except FileNotFoundError as err: + raise RuntimeError( + f"Script '{FilesystemSymlinkMonitor._DECODE_DM}' " + "missing, test could not be run" + ) from err + + if found != len(filesystems): + raise RuntimeError( + f"{len(filesystems)} Stratis filesystems were created by " + f"this test but {found} '/dev/mapper' links were found." + ) + + except FileNotFoundError as err: + raise RuntimeError( + "Missing directory '/dev/mapper', test could not be run" + ) from err + + class DbusMonitor(unittest.TestCase): """ Manage starting and stopping the D-Bus monitor script. @@ -372,6 +447,7 @@ class PostTestCheck(Enum): DBUS_MONITOR = "monitor-dbus" SYSFS = "verify-sysfs" PRIVATE_SYMLINKS = "verify-private-symlinks" + FILESYSTEM_SYMLINKS = "verify-filesystem-symlinks" def __str__(self): return self.value From a451847650d12dfa756d68e1cd5451872637284e Mon Sep 17 00:00:00 2001 From: mulhern Date: Thu, 15 Feb 2024 09:34:15 -0500 Subject: [PATCH 2/2] Remove symlink-specific tests If we can do generic test at the end of the run, that should work better. Signed-off-by: mulhern --- stratisd_cert.py | 131 ----------------------------------------------- testlib/utils.py | 9 ---- 2 files changed, 140 deletions(-) diff --git a/stratisd_cert.py b/stratisd_cert.py index 0cbbcbb..634e0b9 100644 --- a/stratisd_cert.py +++ b/stratisd_cert.py @@ -43,7 +43,6 @@ create_relative_device_path, exec_command, exec_test_command, - resolve_symlink, revision_number_type, skip, ) @@ -126,40 +125,6 @@ def make_test_filesystem(pool_path, fs_name): return array_of_tuples_with_obj_paths_and_names[0][0] -def acquire_filesystem_symlink_targets( - pool_name, filesystem_name, pool_path, filesystem_path -): - """ - Acquire the symlink targets of the "/dev/stratis" symlink, - and the equivalent device-mapper "/dev/mapper" link, generated - via the info from get_managed_objects(). - NOTE: This may require a preceding "udevadm settle" call, to - ensure that up-to-date pool and filesystem information is being - collected. - :param str pool_name: pool name - :param str filesystem_name: filesystem name - :param str pool_path: pool path - :param str filesystem_path: filesystem path - :return: str fsdevdest, str fsdevmapperlinkdest - """ - objects = StratisDbus.get_managed_objects() - - pool_gmodata = objects[pool_path] - pool_uuid = pool_gmodata[StratisDbus.POOL_IFACE]["Uuid"] - filesystem_gmodata = objects[filesystem_path] - filesystem_uuid = filesystem_gmodata[StratisDbus.FS_IFACE]["Uuid"] - - filesystem_devnode = "/dev/stratis/" + pool_name + "/" + filesystem_name - - fs_devmapperlinkstr = ( - "/dev/mapper/stratis-1-" + pool_uuid + "-thin-fs-" + filesystem_uuid - ) - - fsdevdest = resolve_symlink(filesystem_devnode) - fsdevmapperlinkdest = resolve_symlink(fs_devmapperlinkstr) - return fsdevdest, fsdevmapperlinkdest - - class StratisCertify(unittest.TestCase): """ Unit tests for the stratisd package. @@ -899,102 +864,6 @@ def test_filesystem_create_permissions(self): self._test_permissions(StratisDbus.fs_create, [pool_path, fs_name], True) - @skip(_skip_condition(1)) - def test_filesystem_udev_symlink_create(self): - """ - Test the udev symlink creation for filesystem devices. - """ - pool_name = p_n() - pool_path, _ = make_test_pool(pool_name, StratisCertify.DISKS[0:1]) - - fs_name = fs_n() - filesystem_path = make_test_filesystem(pool_path, fs_name) - - fsdevdest, fsdevmapperlinkdest = acquire_filesystem_symlink_targets( - pool_name, fs_name, pool_path, filesystem_path - ) - self.assertEqual(fsdevdest, fsdevmapperlinkdest) - - @skip(_skip_condition(1)) - def test_filesystem_udev_symlink_fsrename(self): - """ - Test the udev symlink creation for filesystem devices after fs rename. - """ - pool_name = p_n() - pool_path, _ = make_test_pool(pool_name, StratisCertify.DISKS[0:1]) - - fs_name = fs_n() - filesystem_path = make_test_filesystem(pool_path, fs_name) - - fs_name_rename = fs_n() - - self._unittest_command( - StratisDbus.fs_rename(pool_name, fs_name, fs_name_rename), dbus.UInt16(0) - ) - # Settle after rename, to allow udev to recognize the fs rename - exec_command(["udevadm", "settle"]) - - fsdevdest, fsdevmapperlinkdest = acquire_filesystem_symlink_targets( - pool_name, fs_name_rename, pool_path, filesystem_path - ) - self.assertEqual(fsdevdest, fsdevmapperlinkdest) - - @skip(_skip_condition(1)) - def test_filesystem_udev_symlink_poolrename(self): - """ - Test the udev symlink creation for filesystem devices after pool rename. - """ - pool_name = p_n() - pool_path, _ = make_test_pool(pool_name, StratisCertify.DISKS[0:1]) - - fs_name = fs_n() - filesystem_path = make_test_filesystem(pool_path, fs_name) - - pool_name_rename = p_n() - - self._unittest_command( - StratisDbus.pool_rename(pool_name, pool_name_rename), dbus.UInt16(0) - ) - # Settle after rename, to allow udev to recognize the fs rename - exec_command(["udevadm", "settle"]) - - fsdevdest, fsdevmapperlinkdest = acquire_filesystem_symlink_targets( - pool_name_rename, fs_name, pool_path, filesystem_path - ) - self.assertEqual(fsdevdest, fsdevmapperlinkdest) - - @skip(_skip_condition(1)) - def test_filesystem_udev_symlink_fsrename_poolrename(self): - """ - Test the udev symlink creation for filesystem devices after fs and pool rename. - """ - pool_name = p_n() - pool_path, _ = make_test_pool(pool_name, StratisCertify.DISKS[0:1]) - - fs_name = fs_n() - filesystem_path = make_test_filesystem(pool_path, fs_name) - - fs_name_rename = fs_n() - - self._unittest_command( - StratisDbus.fs_rename(pool_name, fs_name, fs_name_rename), dbus.UInt16(0) - ) - # Settle after rename, to allow udev to recognize the filesystem rename - exec_command(["udevadm", "settle"]) - - pool_name_rename = p_n() - - self._unittest_command( - StratisDbus.pool_rename(pool_name, pool_name_rename), dbus.UInt16(0) - ) - # Settle after rename, to allow udev to recognize the pool rename - exec_command(["udevadm", "settle"]) - - fsdevdest, fsdevmapperlinkdest = acquire_filesystem_symlink_targets( - pool_name_rename, fs_name_rename, pool_path, filesystem_path - ) - self.assertEqual(fsdevdest, fsdevmapperlinkdest) - @skip(_skip_condition(1)) def test_filesystem_rename(self): """ diff --git a/testlib/utils.py b/testlib/utils.py index 35b69aa..30a9b61 100644 --- a/testlib/utils.py +++ b/testlib/utils.py @@ -35,15 +35,6 @@ def random_string(length=4): return "".join(random.choice(string.ascii_uppercase) for _ in range(length)) -def resolve_symlink(link): - """ - Resolves the destination of a symlink - :param link: filename of the link - :return: String - """ - return os.path.abspath(os.path.join(os.path.dirname(link), os.readlink(link))) - - def revision_number_type(revision_number): """ Raise value error if revision number is not valid.