From b96943565db0ae73314eed3439027191e05171a7 Mon Sep 17 00:00:00 2001 From: Patrick Donnelly Date: Mon, 28 Jun 2021 15:20:43 -0700 Subject: [PATCH 1/3] qa: use run_shell_payload to avoid sudo "run_shell" adds 'sudo' which runs afoul of new security protections on Ubuntu 20.04. Fixes: https://tracker.ceph.com/issues/51417 Signed-off-by: Patrick Donnelly --- qa/tasks/cephfs/test_cephfs_shell.py | 53 +++++++++++++--------------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/qa/tasks/cephfs/test_cephfs_shell.py b/qa/tasks/cephfs/test_cephfs_shell.py index 2833c3ea2d89a..174abd1fa3af6 100644 --- a/qa/tasks/cephfs/test_cephfs_shell.py +++ b/qa/tasks/cephfs/test_cephfs_shell.py @@ -137,7 +137,7 @@ def run_cephfs_shell_script(self, script, mount_x=None, scriptfile.write(script) # copy script to the machine running cephfs-shell. mount_x.client_remote.put_file(scriptpath, scriptpath) - mount_x.run_shell('chmod 755 ' + scriptpath) + mount_x.run_shell_payload(f"chmod 755 {scriptpath}") args = ["cephfs-shell", '-b', scriptpath] if shell_conf_path: @@ -321,8 +321,7 @@ def test_without_target_dir(self): size = i + 1 ofarg = 'of=' + path.join(tempdir, file_) bsarg = 'bs=' + str(size) + 'M' - self.mount_a.run_shell(['dd', 'if=/dev/urandom', ofarg, bsarg, - 'count=1']) + self.mount_a.run_shell_payload(f"dd if=/dev/urandom {ofarg} {bsarg} count=1") self.run_cephfs_shell_cmd('put ' + tempdir) for file_ in files: @@ -332,16 +331,15 @@ def test_without_target_dir(self): self.mount_a.stat(path.join(self.mount_a.mountpoint, tempdirname, file_)) - self.mount_a.run_shell(['rm', '-rf', tempdir]) + self.mount_a.run_shell_payload(f"rm -rf {tempdir}") self.run_cephfs_shell_cmd('get ' + tempdirname) pwd = self.get_cephfs_shell_cmd_output('!pwd') for file_ in files: if file_ == tempdirname: - self.mount_a.run_shell('stat ' + path.join(pwd, file_)) + self.mount_a.run_shell_payload(f"stat {path.join(pwd, file_)}") else: - self.mount_a.run_shell('stat ' + path.join(pwd, tempdirname, - file_)) + self.mount_a.run_shell_payload(f"stat {path.join(pwd, tempdirname, file_)}") def test_get_with_target_name(self): """ @@ -488,7 +486,7 @@ def test_cd_with_no_args(self): to root directory. """ path = 'dir1/dir2/dir3' - self.mount_a.run_shell('mkdir -p ' + path) + self.mount_a.run_shell_payload(f"mkdir -p {path}") expected_cwd = '/' script = 'cd {}\ncd\ncwd\n'.format(path) @@ -501,7 +499,7 @@ def test_cd_with_args(self): to the path passed in the argument. """ path = 'dir1/dir2/dir3' - self.mount_a.run_shell('mkdir -p ' + path) + self.mount_a.run_shell_payload(f"mkdir -p {path}") expected_cwd = '/dir1/dir2/dir3' script = 'cd {}\ncwd\n'.format(path) @@ -528,7 +526,7 @@ def test_du_works_for_non_empty_dirs(self): dir_abspath = path.join(self.mount_a.mountpoint, dirname) regfilename = 'some_regfile' regfile_abspath = path.join(dir_abspath, regfilename) - self.mount_a.run_shell('mkdir ' + dir_abspath) + self.mount_a.run_shell_payload(f"mkdir {dir_abspath}") self.mount_a.client_remote.write_file(regfile_abspath, 'somedata', sudo=True) @@ -544,7 +542,7 @@ def test_du_works_for_non_empty_dirs(self): def test_du_works_for_empty_dirs(self): dirname = 'some_directory' dir_abspath = path.join(self.mount_a.mountpoint, dirname) - self.mount_a.run_shell('mkdir ' + dir_abspath) + self.mount_a.run_shell_payload(f"mkdir {dir_abspath}") size = humansize(self.mount_a.stat(dir_abspath)['st_size']) expected_output = r'{}{}{}'.format(size, " +", dirname) @@ -559,8 +557,7 @@ def test_du_works_for_hardlinks(self): 'somedata', sudo=True) hlinkname = 'some_hardlink' hlink_abspath = path.join(self.mount_a.mountpoint, hlinkname) - self.mount_a.run_shell(['sudo', 'ln', regfile_abspath, - hlink_abspath], omit_sudo=False) + self.mount_a.run_shell_payload(f"ln {regfile_abspath} {hlink_abspath}") size = humansize(self.mount_a.stat(hlink_abspath)['st_size']) expected_output = r'{}{}{}'.format(size, " +", hlinkname) @@ -575,7 +572,7 @@ def test_du_works_for_softlinks_to_files(self): 'somedata', sudo=True) slinkname = 'some_softlink' slink_abspath = path.join(self.mount_a.mountpoint, slinkname) - self.mount_a.run_shell(['ln', '-s', regfile_abspath, slink_abspath]) + self.mount_a.run_shell_payload(f"ln -s {regfile_abspath} {slink_abspath}") size = humansize(self.mount_a.lstat(slink_abspath)['st_size']) expected_output = r'{}{}{}'.format((size), " +", slinkname) @@ -586,10 +583,10 @@ def test_du_works_for_softlinks_to_files(self): def test_du_works_for_softlinks_to_dirs(self): dirname = 'some_directory' dir_abspath = path.join(self.mount_a.mountpoint, dirname) - self.mount_a.run_shell('mkdir ' + dir_abspath) + self.mount_a.run_shell_payload(f"mkdir {dir_abspath}") slinkname = 'some_softlink' slink_abspath = path.join(self.mount_a.mountpoint, slinkname) - self.mount_a.run_shell(['ln', '-s', dir_abspath, slink_abspath]) + self.mount_a.run_shell_payload(f"ln -s {dir_abspath} {slink_abspath}") size = humansize(self.mount_a.lstat(slink_abspath)['st_size']) expected_output = r'{}{}{}'.format(size, " +", slinkname) @@ -612,11 +609,11 @@ def _setup_files(self, return_path_to_files=False, path_prefix='./'): slink_abspath = path.join(self.mount_a.mountpoint, slinkname) slink2_abspath = path.join(self.mount_a.mountpoint, slink2name) - self.mount_a.run_shell('mkdir ' + dir_abspath) - self.mount_a.run_shell('touch ' + regfile_abspath) - self.mount_a.run_shell(['ln', regfile_abspath, hlink_abspath]) - self.mount_a.run_shell(['ln', '-s', regfile_abspath, slink_abspath]) - self.mount_a.run_shell(['ln', '-s', dir_abspath, slink2_abspath]) + self.mount_a.run_shell_payload(f"mkdir {dir_abspath}") + self.mount_a.run_shell_payload(f"touch {regfile_abspath}") + self.mount_a.run_shell_payload(f"ln {regfile_abspath} {hlink_abspath}") + self.mount_a.run_shell_payload(f"ln -s {regfile_abspath} {slink_abspath}") + self.mount_a.run_shell_payload(f"ln -s {dir_abspath} {slink2_abspath}") dir2_name = 'dir2' dir21_name = 'dir21' @@ -624,8 +621,8 @@ def _setup_files(self, return_path_to_files=False, path_prefix='./'): dir2_abspath = path.join(self.mount_a.mountpoint, dir2_name) dir21_abspath = path.join(dir2_abspath, dir21_name) regfile121_abspath = path.join(dir21_abspath, regfile121_name) - self.mount_a.run_shell('mkdir -p ' + dir21_abspath) - self.mount_a.run_shell('touch ' + regfile121_abspath) + self.mount_a.run_shell_payload(f"mkdir -p {dir21_abspath}") + self.mount_a.run_shell_payload(f"touch {regfile121_abspath}") self.mount_a.client_remote.write_file(regfile_abspath, 'somedata', sudo=True) @@ -731,7 +728,7 @@ def test_df_with_no_args(self): def test_df_for_valid_directory(self): dir_name = 'dir1' - mount_output = self.mount_a.run_shell('mkdir ' + dir_name) + mount_output = self.mount_a.run_shell_payload(f"mkdir {dir_name}") log.info("cephfs-shell mount output:\n{}".format(mount_output)) self.validate_df(dir_name) @@ -799,10 +796,10 @@ def test_set_invalid_values(self): def test_exceed_file_limit(self): self.test_set() dir_abspath = path.join(self.mount_a.mountpoint, self.dir_name) - self.mount_a.run_shell('touch '+dir_abspath+'/file1') + self.mount_a.run_shell_payload(f"touch {dir_abspath}/file1") file2 = path.join(dir_abspath, "file2") try: - self.mount_a.run_shell('touch '+file2) + self.mount_a.run_shell_payload(f"touch {file2}") raise Exception("Something went wrong!! File creation should have failed") except CommandFailedError: # Test should pass as file quota set to 2 @@ -924,8 +921,8 @@ def test_ls_H_prints_human_readable_file_size(self): for (file_size, file_name) in zip(file_sizes, file_names): temp_file = self.mount_a.client_remote.mktemp(file_name) - self.mount_a.run_shell(f"fallocate -l {file_size} {temp_file}") - self.mount_a.run_shell(f'mv {temp_file} ./') + self.mount_a.run_shell_payload(f"fallocate -l {file_size} {temp_file}") + self.mount_a.run_shell_payload(f'mv {temp_file} ./') ls_H_output = self.get_cephfs_shell_cmd_output(['ls', '-lH']) From 648af46a0932ee8ea64c59cc6d113de07aee145e Mon Sep 17 00:00:00 2001 From: Patrick Donnelly Date: Tue, 29 Jun 2021 14:51:43 -0700 Subject: [PATCH 2/3] qa: convert mount calls to mount_wait These tests want to immediately use the mount anyway. But the main problem is, without waiting for the mount to complete, the command: chmod 1777 /path/to/mount is not run so the mount cannot be written to by normal users without sudo. Signed-off-by: Patrick Donnelly --- qa/tasks/cephfs/filesystem.py | 2 +- qa/tasks/cephfs/test_client_recovery.py | 4 ++-- qa/tasks/cephfs/test_failover.py | 4 ++-- qa/tasks/cephfs/test_fragment.py | 2 +- qa/tasks/cephfs/test_mirroring.py | 14 +++++++------- qa/tasks/cephfs/test_volumes.py | 10 +++++----- 6 files changed, 18 insertions(+), 18 deletions(-) diff --git a/qa/tasks/cephfs/filesystem.py b/qa/tasks/cephfs/filesystem.py index 34dfdc9525fbf..a84f2bbd57b17 100644 --- a/qa/tasks/cephfs/filesystem.py +++ b/qa/tasks/cephfs/filesystem.py @@ -666,7 +666,7 @@ def run_client_payload(self, cmd): from tasks.cephfs.fuse_mount import FuseMount d = misc.get_testdir(self._ctx) m = FuseMount(self._ctx, {}, d, "admin", self.client_remote, cephfs_name=self.name) - m.mount() + m.mount_wait() m.run_shell_payload(cmd) m.umount_wait(require_clean=True) diff --git a/qa/tasks/cephfs/test_client_recovery.py b/qa/tasks/cephfs/test_client_recovery.py index 3ae208a69925b..e02088278449c 100644 --- a/qa/tasks/cephfs/test_client_recovery.py +++ b/qa/tasks/cephfs/test_client_recovery.py @@ -615,10 +615,10 @@ def test_reconnect_after_blocklisted(self): self.mount_a.umount_wait() if isinstance(self.mount_a, FuseMount): - self.mount_a.mount(mntopts=['--client_reconnect_stale=1', '--fuse_disable_pagecache=1']) + self.mount_a.mount_wait(mntopts=['--client_reconnect_stale=1', '--fuse_disable_pagecache=1']) else: try: - self.mount_a.mount(mntopts=['recover_session=clean']) + self.mount_a.mount_wait(mntopts=['recover_session=clean']) except CommandFailedError: self.mount_a.kill_cleanup() self.skipTest("Not implemented in current kernel") diff --git a/qa/tasks/cephfs/test_failover.py b/qa/tasks/cephfs/test_failover.py index 45e343dcd2ae9..304d27c2c8cff 100644 --- a/qa/tasks/cephfs/test_failover.py +++ b/qa/tasks/cephfs/test_failover.py @@ -650,14 +650,14 @@ def test_clients(self): fs_a, fs_b = self._setup_two() # Mount a client on fs_a - self.mount_a.mount(cephfs_name=fs_a.name) + self.mount_a.mount_wait(cephfs_name=fs_a.name) self.mount_a.write_n_mb("pad.bin", 1) self.mount_a.write_n_mb("test.bin", 2) a_created_ino = self.mount_a.path_to_ino("test.bin") self.mount_a.create_files() # Mount a client on fs_b - self.mount_b.mount(cephfs_name=fs_b.name) + self.mount_b.mount_wait(cephfs_name=fs_b.name) self.mount_b.write_n_mb("test.bin", 1) b_created_ino = self.mount_b.path_to_ino("test.bin") self.mount_b.create_files() diff --git a/qa/tasks/cephfs/test_fragment.py b/qa/tasks/cephfs/test_fragment.py index 6e2823b4a2156..41977ca202836 100644 --- a/qa/tasks/cephfs/test_fragment.py +++ b/qa/tasks/cephfs/test_fragment.py @@ -297,7 +297,7 @@ def _count_fragmented(): self.mount_a.run_shell(["ln", "testdir1/{0}".format(i), "testdir2/"]) self.mount_a.umount_wait() - self.mount_a.mount() + self.mount_a.mount_wait() self.mount_a.wait_until_mounted() # flush journal and restart mds. after restart, testdir2 is not in mds' cache diff --git a/qa/tasks/cephfs/test_mirroring.py b/qa/tasks/cephfs/test_mirroring.py index 6ea3d5d302255..ef2672c84e791 100644 --- a/qa/tasks/cephfs/test_mirroring.py +++ b/qa/tasks/cephfs/test_mirroring.py @@ -480,7 +480,7 @@ def test_cephfs_mirror_stats(self): log.debug(f'mounting filesystem {self.secondary_fs_name}') self.mount_b.umount_wait() - self.mount_b.mount(cephfs_name=self.secondary_fs_name) + self.mount_b.mount_wait(cephfs_name=self.secondary_fs_name) # create a bunch of files in a directory to snap self.mount_a.run_shell(["mkdir", "d0"]) @@ -546,7 +546,7 @@ def test_cephfs_mirror_cancel_sync(self): log.debug(f'mounting filesystem {self.secondary_fs_name}') self.mount_b.umount_wait() - self.mount_b.mount(cephfs_name=self.secondary_fs_name) + self.mount_b.mount_wait(cephfs_name=self.secondary_fs_name) # create a bunch of files in a directory to snap self.mount_a.run_shell(["mkdir", "d0"]) @@ -582,7 +582,7 @@ def test_cephfs_mirror_restart_sync_on_blocklist(self): log.debug(f'mounting filesystem {self.secondary_fs_name}') self.mount_b.umount_wait() - self.mount_b.mount(cephfs_name=self.secondary_fs_name) + self.mount_b.mount_wait(cephfs_name=self.secondary_fs_name) # create a bunch of files in a directory to snap self.mount_a.run_shell(["mkdir", "d0"]) @@ -818,7 +818,7 @@ def test_cephfs_mirror_symlink_sync(self): log.debug(f'mounting filesystem {self.secondary_fs_name}') self.mount_b.umount_wait() - self.mount_b.mount(cephfs_name=self.secondary_fs_name) + self.mount_b.mount_wait(cephfs_name=self.secondary_fs_name) # create a bunch of files w/ symbolic links in a directory to snap self.mount_a.run_shell(["mkdir", "d0"]) @@ -955,7 +955,7 @@ def test_cephfs_mirror_incremental_sync(self): self.backup_fs.get_data_pool_name(), self.backup_fs.get_data_pool_name())) log.debug(f'mounting filesystem {self.secondary_fs_name}') self.mount_b.umount_wait() - self.mount_b.mount(cephfs_name=self.secondary_fs_name) + self.mount_b.mount_wait(cephfs_name=self.secondary_fs_name) repo = 'ceph-qa-suite' repo_dir = 'ceph_repo' @@ -1033,7 +1033,7 @@ def test_cephfs_mirror_incremental_sync_with_type_mixup(self): self.backup_fs.get_data_pool_name(), self.backup_fs.get_data_pool_name())) log.debug(f'mounting filesystem {self.secondary_fs_name}') self.mount_b.umount_wait() - self.mount_b.mount(cephfs_name=self.secondary_fs_name) + self.mount_b.mount_wait(cephfs_name=self.secondary_fs_name) typs = deque(['reg', 'dir', 'sym']) def cleanup_and_create_with_type(dirname, fnames): @@ -1104,7 +1104,7 @@ def test_cephfs_mirror_sync_with_purged_snapshot(self): self.backup_fs.get_data_pool_name(), self.backup_fs.get_data_pool_name())) log.debug(f'mounting filesystem {self.secondary_fs_name}') self.mount_b.umount_wait() - self.mount_b.mount(cephfs_name=self.secondary_fs_name) + self.mount_b.mount_wait(cephfs_name=self.secondary_fs_name) repo = 'ceph-qa-suite' repo_dir = 'ceph_repo' diff --git a/qa/tasks/cephfs/test_volumes.py b/qa/tasks/cephfs/test_volumes.py index 8f015106cff62..573d988e1b294 100644 --- a/qa/tasks/cephfs/test_volumes.py +++ b/qa/tasks/cephfs/test_volumes.py @@ -1216,7 +1216,7 @@ def test_authorize_deauthorize_legacy_subvolume(self): self._configure_guest_auth(guest_mount, authid, key) # mount the subvolume, and write to it - guest_mount.mount(cephfs_mntpt=mount_path) + guest_mount.mount_wait(cephfs_mntpt=mount_path) guest_mount.write_n_mb("data.bin", 1) # authorize guest authID read access to subvolume @@ -1226,7 +1226,7 @@ def test_authorize_deauthorize_legacy_subvolume(self): # guest client sees the change in access level to read only after a # remount of the subvolume. guest_mount.umount_wait() - guest_mount.mount(cephfs_mntpt=mount_path) + guest_mount.mount_wait(cephfs_mntpt=mount_path) # read existing content of the subvolume self.assertListEqual(guest_mount.ls(guest_mount.mountpoint), ["data.bin"]) @@ -1272,7 +1272,7 @@ def test_authorize_deauthorize_subvolume(self): self._configure_guest_auth(guest_mount, authid, key) # mount the subvolume, and write to it - guest_mount.mount(cephfs_mntpt=mount_path) + guest_mount.mount_wait(cephfs_mntpt=mount_path) guest_mount.write_n_mb("data.bin", 1) # authorize guest authID read access to subvolume @@ -1282,7 +1282,7 @@ def test_authorize_deauthorize_subvolume(self): # guest client sees the change in access level to read only after a # remount of the subvolume. guest_mount.umount_wait() - guest_mount.mount(cephfs_mntpt=mount_path) + guest_mount.mount_wait(cephfs_mntpt=mount_path) # read existing content of the subvolume self.assertListEqual(guest_mount.ls(guest_mount.mountpoint), ["data.bin"]) @@ -1887,7 +1887,7 @@ def test_subvolume_evict_client(self): self._configure_guest_auth(guest_mounts[i], auth_id, key) # mount the subvolume, and write to it - guest_mounts[i].mount(cephfs_mntpt=mount_path) + guest_mounts[i].mount_wait(cephfs_mntpt=mount_path) guest_mounts[i].write_n_mb("data.bin", 1) # Evict client, guest_mounts[0], using auth ID 'guest' and has mounted From 433b4b6e29cad2e9bf34a1e0445897dc1caa0a6a Mon Sep 17 00:00:00 2001 From: Patrick Donnelly Date: Tue, 29 Jun 2021 09:47:21 -0700 Subject: [PATCH 3/3] qa: avoid using sudo for regular test artifacts Signed-off-by: Patrick Donnelly --- qa/tasks/cephfs/caps_helper.py | 2 +- qa/tasks/cephfs/fuse_mount.py | 2 +- qa/tasks/cephfs/mount.py | 44 +++++++++++++------------ qa/tasks/cephfs/test_acls.py | 2 +- qa/tasks/cephfs/test_cephfs_shell.py | 21 ++++-------- qa/tasks/cephfs/test_client_recovery.py | 7 ++-- qa/tasks/cephfs/test_data_scan.py | 3 +- qa/tasks/cephfs/test_nfs.py | 6 ++-- qa/tasks/cephfs/test_scrub_checks.py | 8 ++--- qa/tasks/cephfs/test_volumes.py | 13 ++++---- 10 files changed, 52 insertions(+), 56 deletions(-) diff --git a/qa/tasks/cephfs/caps_helper.py b/qa/tasks/cephfs/caps_helper.py index e27a3e1b35c67..f3bc14b074205 100644 --- a/qa/tasks/cephfs/caps_helper.py +++ b/qa/tasks/cephfs/caps_helper.py @@ -60,7 +60,7 @@ def conduct_pos_test_for_write_caps(self, filepaths, mounts): self.assertEqual(data, contents1) def conduct_neg_test_for_write_caps(self, filepaths, mounts): - cmdargs = ['echo', 'some random data', Raw('|'), 'sudo', 'tee'] + cmdargs = ['echo', 'some random data', Raw('|'), 'tee'] for mount in mounts: for path in filepaths: diff --git a/qa/tasks/cephfs/fuse_mount.py b/qa/tasks/cephfs/fuse_mount.py index 4ced0df23ae50..5f4978e57155d 100644 --- a/qa/tasks/cephfs/fuse_mount.py +++ b/qa/tasks/cephfs/fuse_mount.py @@ -459,7 +459,7 @@ def _find_admin_socket(client_name): client_name="client.{0}".format(self.client_id), mountpoint=self.mountpoint) - asok_path = self.run_python(pyscript) + asok_path = self.run_python(pyscript, sudo=True) log.info("Found client admin socket at {0}".format(asok_path)) return asok_path diff --git a/qa/tasks/cephfs/mount.py b/qa/tasks/cephfs/mount.py index 3cd07f194786d..eb4f5c8913d1e 100644 --- a/qa/tasks/cephfs/mount.py +++ b/qa/tasks/cephfs/mount.py @@ -12,7 +12,7 @@ from IPy import IP from teuthology.contextutil import safe_while -from teuthology.misc import get_file, sudo_write_file +from teuthology.misc import get_file, write_file from teuthology.orchestra import run from teuthology.orchestra.run import CommandFailedError, ConnectionLostError, Raw @@ -625,7 +625,7 @@ def create_files(self): for suffix in self.test_files: log.info("Creating file {0}".format(suffix)) self.client_remote.run(args=[ - 'sudo', 'touch', os.path.join(self.hostfs_mntpt, suffix) + 'touch', os.path.join(self.hostfs_mntpt, suffix) ]) def test_create_file(self, filename='testfile', dirname=None, user=None, @@ -639,7 +639,7 @@ def check_files(self): for suffix in self.test_files: log.info("Checking file {0}".format(suffix)) r = self.client_remote.run(args=[ - 'sudo', 'ls', os.path.join(self.hostfs_mntpt, suffix) + 'ls', os.path.join(self.hostfs_mntpt, suffix) ], check_status=False) if r.exitstatus != 0: raise RuntimeError("Expected file {0} not found".format(suffix)) @@ -652,7 +652,7 @@ def write_file(self, path, data, perms=None): if path.find(self.hostfs_mntpt) == -1: path = os.path.join(self.hostfs_mntpt, path) - sudo_write_file(self.client_remote, path, data) + write_file(self.client_remote, path, data) if perms: self.run_shell(args=f'chmod {perms} {path}') @@ -664,7 +664,7 @@ def read_file(self, path): if path.find(self.hostfs_mntpt) == -1: path = os.path.join(self.hostfs_mntpt, path) - return self.run_shell(args=['sudo', 'cat', path], omit_sudo=False).\ + return self.run_shell(args=['cat', path]).\ stdout.getvalue().strip() def create_destroy(self): @@ -673,34 +673,36 @@ def create_destroy(self): filename = "{0} {1}".format(datetime.datetime.now(), self.client_id) log.debug("Creating test file {0}".format(filename)) self.client_remote.run(args=[ - 'sudo', 'touch', os.path.join(self.hostfs_mntpt, filename) + 'touch', os.path.join(self.hostfs_mntpt, filename) ]) log.debug("Deleting test file {0}".format(filename)) self.client_remote.run(args=[ - 'sudo', 'rm', '-f', os.path.join(self.hostfs_mntpt, filename) + 'rm', '-f', os.path.join(self.hostfs_mntpt, filename) ]) - def _run_python(self, pyscript, py_version='python3'): - return self.client_remote.run( - args=['sudo', 'adjust-ulimits', 'daemon-helper', 'kill', - py_version, '-c', pyscript], wait=False, stdin=run.PIPE, - stdout=StringIO()) + def _run_python(self, pyscript, py_version='python3', sudo=False): + args = [] + if sudo: + args.append('sudo') + args += ['adjust-ulimits', 'daemon-helper', 'kill', py_version, '-c', pyscript] + return self.client_remote.run(args=args, wait=False, stdin=run.PIPE, stdout=StringIO()) - def run_python(self, pyscript, py_version='python3'): - p = self._run_python(pyscript, py_version) + def run_python(self, pyscript, py_version='python3', sudo=False): + p = self._run_python(pyscript, py_version, sudo=sudo) p.wait() return p.stdout.getvalue().strip() - def run_shell(self, args, omit_sudo=True, timeout=900, **kwargs): + def run_shell(self, args, timeout=900, **kwargs): args = args.split() if isinstance(args, str) else args - # XXX: all commands ran with CephFS mount as CWD must be executed with - # superuser privileges when tests are being run using teuthology. - if args[0] != 'sudo': - args.insert(0, 'sudo') + kwargs.pop('omit_sudo', False) + sudo = kwargs.pop('sudo', False) cwd = kwargs.pop('cwd', self.mountpoint) stdout = kwargs.pop('stdout', StringIO()) stderr = kwargs.pop('stderr', StringIO()) + if sudo: + args.insert(0, 'sudo') + return self.client_remote.run(args=args, cwd=cwd, timeout=timeout, stdout=stdout, stderr=stderr, **kwargs) def run_shell_payload(self, payload, **kwargs): @@ -845,7 +847,7 @@ def wait_for_visible(self, basename="background_file", timeout=30): i = 0 while i < timeout: r = self.client_remote.run(args=[ - 'sudo', 'ls', os.path.join(self.hostfs_mntpt, basename) + 'stat', os.path.join(self.hostfs_mntpt, basename) ], check_status=False) if r.exitstatus == 0: log.debug("File {0} became visible from {1} after {2}s".format( @@ -943,7 +945,7 @@ def check_filelock(self, basename="background_file", do_flock=True): log.info("check lock on file {0}".format(basename)) self.client_remote.run(args=[ - 'sudo', 'python3', '-c', pyscript + 'python3', '-c', pyscript ]) def write_background(self, basename="background_file", loop=False): diff --git a/qa/tasks/cephfs/test_acls.py b/qa/tasks/cephfs/test_acls.py index 4f704c0767acd..654cb73abcfd2 100644 --- a/qa/tasks/cephfs/test_acls.py +++ b/qa/tasks/cephfs/test_acls.py @@ -21,7 +21,7 @@ def test_acls(self): elif isinstance(self.mount_a, KernelMount): log.info('client is kernel mounted') - self.mount_a.client_remote.run(args=['sudo', './check', + self.mount_a.client_remote.run(args=['./check', 'generic/099'], cwd=self.repo_path, stdout=BytesIO(), stderr=BytesIO(), timeout=30, check_status=True, label='running tests for ACLs from xfstests-dev') diff --git a/qa/tasks/cephfs/test_cephfs_shell.py b/qa/tasks/cephfs/test_cephfs_shell.py index 174abd1fa3af6..83ee3991196e0 100644 --- a/qa/tasks/cephfs/test_cephfs_shell.py +++ b/qa/tasks/cephfs/test_cephfs_shell.py @@ -512,8 +512,7 @@ class TestDU(TestCephFSShell): def test_du_works_for_regfiles(self): regfilename = 'some_regfile' regfile_abspath = path.join(self.mount_a.mountpoint, regfilename) - self.mount_a.client_remote.write_file(regfile_abspath, - 'somedata', sudo=True) + self.mount_a.client_remote.write_file(regfile_abspath, 'somedata') size = humansize(self.mount_a.stat(regfile_abspath)['st_size']) expected_output = r'{}{}{}'.format(size, " +", regfilename) @@ -527,8 +526,7 @@ def test_du_works_for_non_empty_dirs(self): regfilename = 'some_regfile' regfile_abspath = path.join(dir_abspath, regfilename) self.mount_a.run_shell_payload(f"mkdir {dir_abspath}") - self.mount_a.client_remote.write_file(regfile_abspath, - 'somedata', sudo=True) + self.mount_a.client_remote.write_file(regfile_abspath, 'somedata') # XXX: we stat `regfile_abspath` here because ceph du reports a non-empty # directory's size as sum of sizes of all files under it. @@ -553,8 +551,7 @@ def test_du_works_for_empty_dirs(self): def test_du_works_for_hardlinks(self): regfilename = 'some_regfile' regfile_abspath = path.join(self.mount_a.mountpoint, regfilename) - self.mount_a.client_remote.write_file(regfile_abspath, - 'somedata', sudo=True) + self.mount_a.client_remote.write_file(regfile_abspath, 'somedata') hlinkname = 'some_hardlink' hlink_abspath = path.join(self.mount_a.mountpoint, hlinkname) self.mount_a.run_shell_payload(f"ln {regfile_abspath} {hlink_abspath}") @@ -568,8 +565,7 @@ def test_du_works_for_hardlinks(self): def test_du_works_for_softlinks_to_files(self): regfilename = 'some_regfile' regfile_abspath = path.join(self.mount_a.mountpoint, regfilename) - self.mount_a.client_remote.write_file(regfile_abspath, - 'somedata', sudo=True) + self.mount_a.client_remote.write_file(regfile_abspath, 'somedata') slinkname = 'some_softlink' slink_abspath = path.join(self.mount_a.mountpoint, slinkname) self.mount_a.run_shell_payload(f"ln -s {regfile_abspath} {slink_abspath}") @@ -624,10 +620,8 @@ def _setup_files(self, return_path_to_files=False, path_prefix='./'): self.mount_a.run_shell_payload(f"mkdir -p {dir21_abspath}") self.mount_a.run_shell_payload(f"touch {regfile121_abspath}") - self.mount_a.client_remote.write_file(regfile_abspath, - 'somedata', sudo=True) - self.mount_a.client_remote.write_file(regfile121_abspath, - 'somemoredata', sudo=True) + self.mount_a.client_remote.write_file(regfile_abspath, 'somedata') + self.mount_a.client_remote.write_file(regfile121_abspath, 'somemoredata') # TODO: is there a way to trigger/force update ceph.dir.rbytes? # wait so that attr ceph.dir.rbytes gets a chance to be updated. @@ -815,8 +809,7 @@ def test_exceed_write_limit(self): file_abspath = path.join(dir_abspath, filename) try: # Write should fail as bytes quota is set to 6 - self.mount_a.client_remote.write_file(file_abspath, - 'Disk raise Exception', sudo=True) + self.mount_a.client_remote.write_file(file_abspath, 'Disk raise Exception') raise Exception("Write should have failed") except CommandFailedError: # Test should pass only when write command fails diff --git a/qa/tasks/cephfs/test_client_recovery.py b/qa/tasks/cephfs/test_client_recovery.py index e02088278449c..c067e8ac933bc 100644 --- a/qa/tasks/cephfs/test_client_recovery.py +++ b/qa/tasks/cephfs/test_client_recovery.py @@ -508,9 +508,8 @@ def test_stale_renew(self): self.assertEqual(current_readdirs, initial_readdirs); mount_b_gid = self.mount_b.get_global_id() - mount_b_pid = self.mount_b.get_client_pid() # stop ceph-fuse process of mount_b - self.mount_b.client_remote.run(args=["sudo", "kill", "-STOP", mount_b_pid]) + self.mount_b.suspend_netns() self.assert_session_state(mount_b_gid, "open") time.sleep(session_timeout * 1.5) # Long enough for MDS to consider session stale @@ -519,7 +518,7 @@ def test_stale_renew(self): self.assert_session_state(mount_b_gid, "stale") # resume ceph-fuse process of mount_b - self.mount_b.client_remote.run(args=["sudo", "kill", "-CONT", mount_b_pid]) + self.mount_b.resume_netns() # Is the new file visible from mount_b? (caps become invalid after session stale) self.mount_b.run_shell(["ls", "testdir/file2"]) @@ -682,7 +681,7 @@ def test_reconnect_after_blocklisted(self): raise RuntimeError("read() failed to raise error") """).format(path=path) rproc = self.mount_a.client_remote.run( - args=['sudo', 'python3', '-c', pyscript], + args=['python3', '-c', pyscript], wait=False, stdin=run.PIPE, stdout=run.PIPE) rproc.stdout.readline() diff --git a/qa/tasks/cephfs/test_data_scan.py b/qa/tasks/cephfs/test_data_scan.py index 933a7f67d7605..101fc804b43e0 100644 --- a/qa/tasks/cephfs/test_data_scan.py +++ b/qa/tasks/cephfs/test_data_scan.py @@ -471,7 +471,7 @@ def test_fragmented_injection(self): self.fs.set_joinable() self.fs.wait_for_daemons() self.mount_a.mount_wait() - files = self.mount_a.run_shell(["ls", "subdir/"]).stdout.getvalue().strip().split("\n") + files = self.mount_a.run_shell(["ls", "-l", ".", "subdir/"]).stdout.getvalue().strip().split("\n") self.assertListEqual(sorted(files), sorted(list(set(file_names) - set([victim_dentry])))) # Stop the filesystem @@ -490,6 +490,7 @@ def test_fragmented_injection(self): self.fs.set_joinable() self.fs.wait_for_daemons() self.mount_a.mount_wait() + self.mount_a.run_shell(["ls", "-l", "subdir/"]).stdout.getvalue().strip().split("\n") out = self.mount_a.run_shell(["cat", "subdir/{0}".format(victim_dentry)]).stdout.getvalue().strip() self.assertEqual(out, victim_dentry) diff --git a/qa/tasks/cephfs/test_nfs.py b/qa/tasks/cephfs/test_nfs.py index bc52bdc3f22e7..3013dffe1e540 100644 --- a/qa/tasks/cephfs/test_nfs.py +++ b/qa/tasks/cephfs/test_nfs.py @@ -263,9 +263,11 @@ def _test_mnt(self, pseudo_path, port, ip, check=True): return raise + self.ctx.cluster.run(args=['sudo', 'chmod', '1777', '/mnt']) + try: - self.ctx.cluster.run(args=['sudo', 'touch', '/mnt/test']) - out_mnt = self._sys_cmd(['sudo', 'ls', '/mnt']) + self.ctx.cluster.run(args=['touch', '/mnt/test']) + out_mnt = self._sys_cmd(['ls', '/mnt']) self.assertEqual(out_mnt, b'test\n') finally: self.ctx.cluster.run(args=['sudo', 'umount', '/mnt']) diff --git a/qa/tasks/cephfs/test_scrub_checks.py b/qa/tasks/cephfs/test_scrub_checks.py index f1af604802866..bcfc2fc9a3a1f 100644 --- a/qa/tasks/cephfs/test_scrub_checks.py +++ b/qa/tasks/cephfs/test_scrub_checks.py @@ -303,8 +303,8 @@ def test_scrub_repair(self): mds_rank = 0 test_dir = "scrub_repair_path" - self.mount_a.run_shell(["sudo", "mkdir", test_dir]) - self.mount_a.run_shell(["sudo", "touch", "{0}/file".format(test_dir)]) + self.mount_a.run_shell(["mkdir", test_dir]) + self.mount_a.run_shell(["touch", "{0}/file".format(test_dir)]) dir_objname = "{:x}.00000000".format(self.mount_a.path_to_ino(test_dir)) self.mount_a.umount_wait() @@ -323,7 +323,7 @@ def test_scrub_repair(self): # fragstat indicates the directory is not empty, rmdir should fail with self.assertRaises(CommandFailedError) as ar: - self.mount_a.run_shell(["sudo", "rmdir", test_dir]) + self.mount_a.run_shell(["rmdir", test_dir]) self.assertEqual(ar.exception.exitstatus, 1) self.tell_command(mds_rank, "scrub start /{0} repair".format(test_dir), @@ -333,7 +333,7 @@ def test_scrub_repair(self): time.sleep(10) # fragstat should be fixed - self.mount_a.run_shell(["sudo", "rmdir", test_dir]) + self.mount_a.run_shell(["rmdir", test_dir]) @staticmethod def json_validator(json_out, rc, element, expected_value): diff --git a/qa/tasks/cephfs/test_volumes.py b/qa/tasks/cephfs/test_volumes.py index 573d988e1b294..9049986523df2 100644 --- a/qa/tasks/cephfs/test_volumes.py +++ b/qa/tasks/cephfs/test_volumes.py @@ -262,7 +262,7 @@ def _do_subvolume_io(self, subvolume, subvolume_group=None, create_dir=None, io_path = subvolpath if create_dir: io_path = os.path.join(subvolpath, create_dir) - self.mount_a.run_shell(["mkdir", "-p", io_path]) + self.mount_a.run_shell_payload(f"mkdir -p {io_path}", sudo=True) log.debug("filling subvolume {0} with {1} files each {2}MB size under directory {3}".format(subvolume, number_of_files, file_size, io_path)) for i in range(number_of_files): @@ -278,9 +278,9 @@ def _do_subvolume_io_mixed(self, subvolume, subvolume_group=None): # this symlink's ownership would be changed sym_path2 = os.path.join(dir_path, "sym.0") - self.mount_a.run_shell(["sudo", "mkdir", dir_path], omit_sudo=False) - self.mount_a.run_shell(["sudo", "ln", "-s", "./{}".format(reg_file), sym_path1], omit_sudo=False) - self.mount_a.run_shell(["sudo", "ln", "-s", "./{}".format(reg_file), sym_path2], omit_sudo=False) + self.mount_a.run_shell(["mkdir", dir_path]) + self.mount_a.run_shell(["ln", "-s", "./{}".format(reg_file), sym_path1]) + self.mount_a.run_shell(["ln", "-s", "./{}".format(reg_file), sym_path2]) # flip ownership to nobody. assumption: nobody's id is 65534 self.mount_a.run_shell(["sudo", "chown", "-h", "65534:65534", sym_path2], omit_sudo=False) @@ -301,7 +301,7 @@ def _assert_meta_location_and_version(self, vol_name, subvol_name, subvol_group= group = subvol_group if subvol_group is not None else '_nogroup' metapath = os.path.join(".", "volumes", group, subvol_name, ".meta") - out = self.mount_a.run_shell(['cat', metapath]) + out = self.mount_a.run_shell(['sudo', 'cat', metapath]) lines = out.stdout.getvalue().strip().split('\n') sv_version = -1 for line in lines: @@ -333,8 +333,7 @@ def _create_v1_subvolume(self, subvol_name, subvol_group=None, has_snapshot=True # add a fake clone source meta_contents = meta_contents + '[source]\nvolume = fake\nsubvolume = fake\nsnapshot = fake\n' meta_filepath1 = os.path.join(self.mount_a.mountpoint, basepath, ".meta") - self.mount_a.client_remote.write_file(meta_filepath1, - meta_contents, sudo=True) + self.mount_a.client_remote.write_file(meta_filepath1, meta_contents, sudo=True) return createpath def _update_fake_trash(self, subvol_name, subvol_group=None, trash_name='fake', create=True):