From ae1307034567f1a579af349cb6b476a07872769c Mon Sep 17 00:00:00 2001 From: Fabian Fulga Date: Tue, 27 Jan 2026 15:03:32 +0200 Subject: [PATCH] Fix issue with files containing non-utf8 characters --- coriolis/osmorphing/base.py | 10 +- coriolis/osmorphing/debian.py | 5 +- coriolis/osmorphing/osmount/base.py | 37 +++--- coriolis/osmorphing/redhat.py | 5 +- coriolis/osmorphing/suse.py | 7 +- coriolis/providers/backup_writers.py | 2 +- .../tests/osmorphing/osmount/test_base.py | 120 +++++++++--------- coriolis/tests/osmorphing/test_base.py | 14 +- coriolis/tests/osmorphing/test_debian.py | 5 +- coriolis/tests/osmorphing/test_redhat.py | 7 +- coriolis/tests/osmorphing/test_suse.py | 9 +- .../tests/providers/test_backup_writers.py | 2 +- coriolis/tests/test_utils.py | 26 ++-- coriolis/utils.py | 32 +++-- 14 files changed, 144 insertions(+), 137 deletions(-) diff --git a/coriolis/osmorphing/base.py b/coriolis/osmorphing/base.py index 775646809..09917b9c3 100644 --- a/coriolis/osmorphing/base.py +++ b/coriolis/osmorphing/base.py @@ -335,7 +335,7 @@ def _read_config_file_sudo(self, chroot_path, check_exists=False): if check_exists: raise IOError("could not find %s" % chroot_path) return {} - content = self._read_file_sudo(chroot_path).decode() + content = self._read_file_sudo(chroot_path) config = utils.parse_ini_config(content) return config @@ -484,7 +484,7 @@ def _test_path_chroot(self, path): if path.startswith('/') is False: path = "/%s" % path exists = self._exec_cmd_chroot( - '[ -f "%s" ] && echo 1 || echo 0' % path).decode().rstrip('\n') + '[ -f "%s" ] && echo 1 || echo 0' % path).rstrip('\n') return exists == "1" def _read_file_sudo(self, chroot_path): @@ -497,7 +497,7 @@ def _read_file_sudo(self, chroot_path): def _read_grub_config(self, config): if self._test_path_chroot(config) is False: raise IOError("could not find %s" % config) - contents = self._read_file_sudo(config).decode() + contents = self._read_file_sudo(config) ret = {} for line in contents.split('\n'): if line.startswith("#"): @@ -512,7 +512,7 @@ def _get_grub_config_obj(self, grub_conf=None): grub_conf = grub_conf or "/etc/default/grub" if self._test_path_chroot(grub_conf) is False: raise IOError("could not find %s" % grub_conf) - tmp_file = self._exec_cmd_chroot("mktemp").decode().rstrip('\n') + tmp_file = self._exec_cmd_chroot("mktemp").rstrip('\n') self._exec_cmd_chroot( "/bin/cp -fp %s %s" % (grub_conf, tmp_file)) config_file = self._read_grub_config(tmp_file) @@ -561,7 +561,7 @@ def replace_in_cfg(opt, val): replace_in_cfg(option, value) else: append_to_cfg(option, value) - cfg = self._read_file_sudo(config_obj["location"]).decode() + cfg = self._read_file_sudo(config_obj["location"]) LOG.warning("TEMP CONFIG IS: %r" % cfg) def _set_grub2_cmdline(self, config_obj, options, clobber=False): diff --git a/coriolis/osmorphing/debian.py b/coriolis/osmorphing/debian.py index e500e1a3d..f713a819f 100644 --- a/coriolis/osmorphing/debian.py +++ b/coriolis/osmorphing/debian.py @@ -42,7 +42,7 @@ def disable_predictable_nic_names(self): grub_cfg = "etc/default/grub" if self._test_path_chroot(grub_cfg) is False: return - contents = self._read_file_sudo(grub_cfg).decode() + contents = self._read_file_sudo(grub_cfg) cfg = utils.Grub2ConfigEditor(contents) cfg.append_to_option( "GRUB_CMDLINE_LINUX_DEFAULT", @@ -127,8 +127,7 @@ def set_net_config(self, nics_info, dhcp): def get_installed_packages(self): cmd = "dpkg-query -f '${binary:Package}\\n' -W" try: - self.installed_packages = self._exec_cmd_chroot( - cmd).decode('utf-8').splitlines() + self.installed_packages = self._exec_cmd_chroot(cmd).splitlines() except exception.CoriolisException: self.installed_packages = [] diff --git a/coriolis/osmorphing/osmount/base.py b/coriolis/osmorphing/osmount/base.py index 926342c92..9b0a745bd 100644 --- a/coriolis/osmorphing/osmount/base.py +++ b/coriolis/osmorphing/osmount/base.py @@ -122,7 +122,7 @@ def get_connection(self): class BaseLinuxOSMountTools(BaseSSHOSMountTools): def _get_pvs(self): - out = self._exec_cmd("sudo pvdisplay -c").decode().splitlines() + out = self._exec_cmd("sudo pvdisplay -c").splitlines() LOG.debug("Output of 'pvdisplay -c' command: %s", out) pvs = {} for line in out: @@ -150,7 +150,7 @@ def _get_vgs(self): """ vgs_cmd = ( "sudo vgs -o vg_name,pv_name,vg_uuid, --noheadings --separator :") - out = self._exec_cmd(vgs_cmd).decode().splitlines() + out = self._exec_cmd(vgs_cmd).splitlines() LOG.debug("Output of %s command: %s", vgs_cmd, out) vgs_uuid_map = {} for line in out: @@ -197,8 +197,7 @@ def _check_vgs(self): def _get_vgnames(self): vg_names = [] - vgscan_out_lines = self._exec_cmd( - "sudo vgscan").decode().splitlines() + vgscan_out_lines = self._exec_cmd("sudo vgscan").splitlines() LOG.debug("Output of vgscan commnad: %s", vgscan_out_lines) for vgscan_out_line in vgscan_out_lines: m = re.match( @@ -212,7 +211,7 @@ def _get_vgnames(self): def _get_lv_paths(self): """ Returns list with paths of available LVM volumes. """ lvm_paths = [] - out = self._exec_cmd("sudo lvdisplay -c").decode().strip() + out = self._exec_cmd("sudo lvdisplay -c").strip() if out: LOG.debug("Decoded `lvdisplay` output data: %s", out) out_lines = out.splitlines() @@ -353,7 +352,7 @@ def _check_mount_fstab_partitions( def _get_symlink_target(self, symlink): target = symlink try: - target = self._exec_cmd('readlink -en %s' % symlink).decode() + target = self._exec_cmd('readlink -en %s' % symlink) LOG.debug("readlink %s returned: %s" % (symlink, target)) except Exception: LOG.warn('Target not found for symlink: %s. Original link path ' @@ -375,8 +374,7 @@ def _get_device_file_paths(self, symlink_list): return dev_file_paths def _get_mounted_devices(self): - mounts = self._exec_cmd( - "cat /proc/mounts").decode().splitlines() + mounts = self._exec_cmd("cat /proc/mounts").splitlines() ret = [] mounted_device_numbers = [] dev_nmb_cmd = "mountpoint -x %s" @@ -390,10 +388,9 @@ def _get_mounted_devices(self): continue ret.append(dev_name) mounted_device_numbers.append( - self._exec_cmd(dev_nmb_cmd % dev_name).decode().rstrip()) + self._exec_cmd(dev_nmb_cmd % dev_name).rstrip()) - block_devs = self._exec_cmd( - "ls -al /dev | grep ^b").decode().splitlines() + block_devs = self._exec_cmd("ls -al /dev | grep ^b").splitlines() for dev_line in block_devs: dev = dev_line.split() major_minor = "%s:%s" % ( @@ -409,8 +406,7 @@ def _get_mounted_devices(self): return ret def _get_mount_destinations(self): - mounts = self._exec_cmd( - "cat /proc/mounts").decode().splitlines() + mounts = self._exec_cmd("cat /proc/mounts").splitlines() ret = set() for line in mounts: colls = line.split() @@ -424,8 +420,7 @@ def _get_volume_block_devices(self): # where 'ln -s /dev/dm-N /dev//' # Querying for the kernel device name (KNAME) should ensure we get the # device names we desire both for physical and logical volumes. - volume_devs = self._exec_cmd( - "lsblk -lnao KNAME").decode().splitlines() + volume_devs = self._exec_cmd("lsblk -lnao KNAME").splitlines() LOG.debug("All block devices: %s", str(volume_devs)) volume_devs = ["/dev/%s" % d for d in volume_devs if @@ -446,7 +441,7 @@ def _find_dev_with_contents(self, devices, all_files=None, dev_name = None for dev_path in devices: dirs = None - tmp_dir = self._exec_cmd('mktemp -d').decode().splitlines()[0] + tmp_dir = self._exec_cmd('mktemp -d').splitlines()[0] try: self._exec_cmd('sudo mount %s %s' % (dev_path, tmp_dir)) # NOTE: it's possible that the device was mounted successfully @@ -508,7 +503,7 @@ def _find_and_mount_root(self, devices): "being migrated.") try: - tmp_dir = self._exec_cmd('mktemp -d').decode().splitlines()[0] + tmp_dir = self._exec_cmd('mktemp -d').splitlines()[0] self._exec_cmd('sudo mount %s %s' % (os_root_device, tmp_dir)) os_root_dir = tmp_dir except Exception as ex: @@ -560,7 +555,7 @@ def mount_os(self): for volume_dev in volume_devs: self._exec_cmd("sudo partx -v -a %s || true" % volume_dev) dev_paths += self._exec_cmd( - "sudo ls -1 %s*" % volume_dev).decode().splitlines() + "sudo ls -1 %s*" % volume_dev).splitlines() LOG.debug("All simple devices to scan: %s", dev_paths) lvm_dev_paths = [] @@ -573,7 +568,7 @@ def mount_os(self): self._exec_cmd("sudo vgchange -ay -S vg_uuid=%s" % vg_uuid) self._exec_cmd("sudo vgchange --refresh") dev_paths_for_group = self._exec_cmd( - f"sudo ls -1 /dev/{vg_props['name']}/*").decode().splitlines() + f"sudo ls -1 /dev/{vg_props['name']}/*").splitlines() lvm_dev_paths.extend(dev_paths_for_group) LOG.debug("All LVM vols to scan: %s", lvm_dev_paths) @@ -592,8 +587,8 @@ def mount_os(self): dev_path, dev_target) continue fs_type = self._exec_cmd( - "sudo blkid -o value -s TYPE %s || true" % - dev_path).decode().splitlines() + "sudo blkid -o value -s TYPE %s || true" % dev_path + ).splitlines() LOG.debug('Device %s filesystem types: %s', dev_path, fs_type) if fs_type and fs_type[0] in valid_filesystems: if fs_type[0] == "xfs": diff --git a/coriolis/osmorphing/redhat.py b/coriolis/osmorphing/redhat.py index a8369e274..9308e0887 100644 --- a/coriolis/osmorphing/redhat.py +++ b/coriolis/osmorphing/redhat.py @@ -172,8 +172,7 @@ def set_net_config(self, nics_info, dhcp): def get_installed_packages(self): cmd = 'rpm -qa --qf "%{NAME}\\n"' try: - self.installed_packages = self._exec_cmd_chroot( - cmd).decode('utf-8').splitlines() + self.installed_packages = self._exec_cmd_chroot(cmd).splitlines() except exception.CoriolisException: LOG.warning("Failed to get installed packages") LOG.trace(utils.get_exception_details()) @@ -231,7 +230,7 @@ def _find_yum_repos(self, repos_to_enable=[]): for file in repofiles: path = os.path.join(reposdir_path, file) try: - content = self._read_file_sudo(path).decode() + content = self._read_file_sudo(path) except Exception as e: LOG.warning( "Could not read yum repository file %s: %s", path, e) diff --git a/coriolis/osmorphing/suse.py b/coriolis/osmorphing/suse.py index 3331367ab..fa3a59491 100644 --- a/coriolis/osmorphing/suse.py +++ b/coriolis/osmorphing/suse.py @@ -68,8 +68,7 @@ def set_net_config(self, nics_info, dhcp): def get_installed_packages(self): cmd = 'rpm -qa --qf "%{NAME}\\n"' try: - self.installed_packages = self._exec_cmd_chroot( - cmd).decode('utf-8').splitlines() + self.installed_packages = self._exec_cmd_chroot(cmd).splitlines() except exception.CoriolisException: LOG.warning("Failed to get installed packages") LOG.trace(utils.get_exception_details()) @@ -134,7 +133,7 @@ def post_packages_install(self, package_names): def _enable_sles_module(self, module): available_modules = self._exec_cmd_chroot( - "SUSEConnect --list-extensions").decode() + "SUSEConnect --list-extensions") module_match = re.search("%s.*" % module, available_modules) try: module_path = module_match.group(0) @@ -171,7 +170,7 @@ def _get_repos(self): repos = {} repos_list = self._exec_cmd_chroot( "zypper repos -u | awk -F '|' '/^\s[0-9]+/ {print $2 $7}'" - ).decode() + ) for repo in repos_list.splitlines(): alias, uri = repo.strip().split() repos[alias] = uri diff --git a/coriolis/providers/backup_writers.py b/coriolis/providers/backup_writers.py index d746a160f..e17a3e8ba 100644 --- a/coriolis/providers/backup_writers.py +++ b/coriolis/providers/backup_writers.py @@ -912,7 +912,7 @@ def _setup_certificates(self, ssh): def _read_remote_file_sudo(self, remote_path): contents = utils.exec_ssh_cmd( self._ssh, "sudo cat %s" % remote_path, get_pty=True) - return contents.decode() + return contents def _init_writer(self, ssh, cert_paths): cmdline = ("%(cmd)s run -ca-cert %(ca_cert)s -key " diff --git a/coriolis/tests/osmorphing/osmount/test_base.py b/coriolis/tests/osmorphing/osmount/test_base.py index 9f3e9343c..3982c80cb 100644 --- a/coriolis/tests/osmorphing/osmount/test_base.py +++ b/coriolis/tests/osmorphing/osmount/test_base.py @@ -166,7 +166,7 @@ def setUp(self, mock_connect): @mock.patch.object(base.BaseSSHOSMountTools, '_exec_cmd') def test__get_pvs(self, mock_exec_cmd): mock_exec_cmd.return_value = ( - b"pv1:vg1\nimproper_line\npv2:vg1\n\npv3:vg2") + "pv1:vg1\nimproper_line\npv2:vg1\n\npv3:vg2") with self.assertLogs('coriolis.osmorphing.osmount.base', level=logging.WARN): @@ -179,7 +179,7 @@ def test__get_pvs(self, mock_exec_cmd): @mock.patch.object(base.BaseSSHOSMountTools, '_exec_cmd') def test__get_pvs_improper_output(self, mock_exec_cmd): - mock_exec_cmd.return_value = b"improper_line" + mock_exec_cmd.return_value = "improper_line" with self.assertLogs('coriolis.osmorphing.osmount.base', level=logging.WARN): @@ -192,7 +192,7 @@ def test__get_pvs_improper_output(self, mock_exec_cmd): @mock.patch.object(base.BaseSSHOSMountTools, '_exec_cmd') def test__get_vgs(self, mock_exec_cmd): mock_exec_cmd.return_value = ( - b"vg1:pv1:uuid1\nimproper_line\n\n\nvg2:pv3:uuid2") + "vg1:pv1:uuid1\nimproper_line\n\n\nvg2:pv3:uuid2") with self.assertLogs('coriolis.osmorphing.osmount.base', level=logging.WARN): @@ -214,7 +214,7 @@ def test__get_vgs(self, mock_exec_cmd): @mock.patch.object(base.uuid, 'uuid4') @mock.patch.object(base.BaseSSHOSMountTools, '_exec_cmd') def test__get_vgs_duplicate_vg_names(self, mock_exec_cmd, mock_uuid4): - mock_exec_cmd.return_value = b"vg1:pv1:uuid1\nvg1:pv2:uuid2" + mock_exec_cmd.return_value = "vg1:pv1:uuid1\nvg1:pv2:uuid2" mock_uuid4.return_value = "random_uuid" with self.assertLogs('coriolis.osmorphing.osmount.base', @@ -244,7 +244,7 @@ def test__get_vgs_duplicate_vg_names(self, mock_exec_cmd, mock_uuid4): @mock.patch.object(base.BaseSSHOSMountTools, '_exec_cmd') def test__get_vgs_improper_output(self, mock_exec_cmd): - mock_exec_cmd.return_value = b"improper_line" + mock_exec_cmd.return_value = "improper_line" with self.assertLogs('coriolis.osmorphing.osmount.base', level=logging.WARN): @@ -272,8 +272,8 @@ def test__check_vgs_with_exception(self, mock_exec_cmd): @mock.patch.object(base.BaseSSHOSMountTools, '_exec_cmd') def test__get_vgnames(self, mock_exec_cmd): mock_exec_cmd.return_value = ( - b" Found volume group \"vg1\" using metadata type lvm2\n" - b" Found volume group \"vg2\" using metadata type lvm2" + " Found volume group \"vg1\" using metadata type lvm2\n" + " Found volume group \"vg2\" using metadata type lvm2" ) result = self.base_os_mount_tools._get_vgnames() @@ -286,8 +286,8 @@ def test__get_vgnames(self, mock_exec_cmd): @mock.patch.object(base.BaseSSHOSMountTools, '_exec_cmd') def test__get_lv_paths(self, mock_exec_cmd): mock_exec_cmd.return_value = ( - b"/dev/vg1/lv1:vg1:lv1:-wi-ao----100.00g\n" - b"/dev/vg2/lv2:vg2:lv2:-wi-a-----50.00g" + "/dev/vg1/lv1:vg1:lv1:-wi-ao----100.00g\n" + "/dev/vg2/lv2:vg2:lv2:-wi-a-----50.00g" ) result = self.base_os_mount_tools._get_lv_paths() @@ -546,7 +546,7 @@ def test__check_mount_fstab_partitions_mountcmd_with_exception( @mock.patch.object(base.BaseSSHOSMountTools, '_exec_cmd') def test__get_symlink_target(self, mock_exec_cmd): - mock_exec_cmd.return_value = b"/dev/sda1" + mock_exec_cmd.return_value = "/dev/sda1" result = self.base_os_mount_tools._get_symlink_target( "/dev/sda1") @@ -585,11 +585,11 @@ def test__get_device_file_paths(self, mock_get_symlink_target): @mock.patch.object(base.BaseSSHOSMountTools, '_exec_cmd') def test__get_mounted_devices(self, mock_exec_cmd): mock_exec_cmd.side_effect = [ - b"/dev/sda1 on / type ext4 (rw,relatime,errors=remount-ro)\n", - b"/dev/sda1", - b"8:1", - b"brw-rw---- 1 root disk 8, 1 Jan 1 00:00 sda1 sda2", - b"" + "/dev/sda1 on / type ext4 (rw,relatime,errors=remount-ro)\n", + "/dev/sda1", + "8:1", + "brw-rw---- 1 root disk 8, 1 Jan 1 00:00 sda1 sda2", + "" ] result = self.base_os_mount_tools._get_mounted_devices() @@ -607,9 +607,9 @@ def test__get_mounted_devices(self, mock_exec_cmd): def test__get_mounted_devices_not_found(self, mock_test_ssh_path, mock_exec_cmd): mock_exec_cmd.side_effect = [ - b"/dev/sda1 on / type ext4 (rw,relatime,errors=remount-ro)\n", - b"/dev/sda1", - b"" + "/dev/sda1 on / type ext4 (rw,relatime,errors=remount-ro)\n", + "/dev/sda1", + "" ] mock_test_ssh_path.return_value = False @@ -631,9 +631,9 @@ def test__get_mounted_devices_not_found(self, mock_test_ssh_path, @mock.patch.object(base.BaseSSHOSMountTools, '_exec_cmd') def test__get_mount_destinations(self, mock_exec_cmd): mock_exec_cmd.return_value = ( - b"/dev/sda1 / ext4 rw,relatime 0 0\n" - b"/dev/sda2 /mnt ext4 rw,relatime 0 0\n" - b"/dev/sda3 /mnt1 ext4 rw,relatime 0 0\n" + "/dev/sda1 / ext4 rw,relatime 0 0\n" + "/dev/sda2 /mnt ext4 rw,relatime 0 0\n" + "/dev/sda3 /mnt1 ext4 rw,relatime 0 0\n" ) result = self.base_os_mount_tools._get_mount_destinations() @@ -645,7 +645,7 @@ def test__get_mount_destinations(self, mock_exec_cmd): @mock.patch.object(base.BaseSSHOSMountTools, '_exec_cmd') def test__get_volume_block_devices(self, mock_exec_cmd): - mock_exec_cmd.return_value = b"sda\nsda1\nsda2\nsdb\nsdb1\nsdb2" + mock_exec_cmd.return_value = "sda\nsda1\nsda2\nsdb\nsdb1\nsdb2" self.base_os_mount_tools._ignore_devices = ["/dev/sda1"] result = self.base_os_mount_tools._get_volume_block_devices() @@ -658,7 +658,7 @@ def test__get_volume_block_devices(self, mock_exec_cmd): @mock.patch.object(base.BaseSSHOSMountTools, '_exec_cmd') @mock.patch.object(base.utils, 'list_ssh_dir') def test__find_dev_with_contents(self, mock_list_ssh_dir, mock_exec_cmd): - mock_exec_cmd.return_value = b"/tmp/tmp_dir" + mock_exec_cmd.return_value = "/tmp/tmp_dir" mock_list_ssh_dir.return_value = ["etc", "bin", "sbin", "boot"] result = self.base_os_mount_tools._find_dev_with_contents( @@ -692,7 +692,7 @@ def test__find_dev_with_contents_both_all_and_one_of_files( @mock.patch.object(base.utils, 'list_ssh_dir') def test__find_dev_with_contents_one_of_files(self, mock_list_ssh_dir, mock_exec_cmd): - mock_exec_cmd.return_value = b"/tmp/tmp_dir" + mock_exec_cmd.return_value = "/tmp/tmp_dir" mock_list_ssh_dir.return_value = ["etc", "bin", "sbin", "boot"] result = self.base_os_mount_tools._find_dev_with_contents( @@ -713,7 +713,7 @@ def test__find_dev_with_contents_one_of_files(self, mock_list_ssh_dir, @mock.patch.object(base.utils, 'list_ssh_dir') def test__find_dev_with_contents_missing_all_files(self, mock_list_ssh_dir, mock_exec_cmd): - mock_exec_cmd.return_value = b"/tmp/tmp_dir" + mock_exec_cmd.return_value = "/tmp/tmp_dir" mock_list_ssh_dir.return_value = ["etc", "bin", "sbin", "boot"] # Append a missing file to the list of all_files @@ -759,7 +759,7 @@ def test__find_dev_with_contents_with_exception(self, mock_list_ssh_dir, def test__find_and_mount_root(self, mock_find_dev_with_contents, mock_exec_cmd, mock_test_ssh_path): devices = ["/dev/sda", "/dev/sdb"] - mock_exec_cmd.return_value = b"/tmp/tmp_dir" + mock_exec_cmd.return_value = "/tmp/tmp_dir" mock_find_dev_with_contents.return_value = "/dev/sda" mock_test_ssh_path.side_effect = [True, False, True, True] @@ -792,7 +792,7 @@ def test__find_and_mount_root_with_exception( self, mock_find_dev_with_contents, mock_exec_cmd, mock_test_ssh_path): devices = ["/dev/sda", "/dev/sdb"] - mock_exec_cmd.return_value = b"/tmp/tmp_dir" + mock_exec_cmd.return_value = "/tmp/tmp_dir" mock_find_dev_with_contents.return_value = None self.assertRaises(exception.OperatingSystemNotFound, @@ -811,7 +811,7 @@ def test__find_and_mount_root_exec_cmd_exception( self, mock_find_dev_with_contents, mock_exec_cmd, mock_test_ssh_path): devices = ["/dev/sda", "/dev/sdb"] - mock_exec_cmd.side_effect = [b"/tmp/tmp_dir", CoriolisTestException()] + mock_exec_cmd.side_effect = ["/tmp/tmp_dir", CoriolisTestException()] mock_find_dev_with_contents.return_value = "/dev/sda" mock_test_ssh_path.return_value = True @@ -859,21 +859,21 @@ def test_mount_os(self, mock_check_mount_fstab_partitions, mock_get_mounted_devices.return_value = ["/dev/sda1"] mock_find_dev_with_contents.return_value = "/dev/sdb1" mock_exec_cmd.side_effect = [ - b"", - b"/dev/sda1", - b"", - b"/dev/sdb1", - b"", - b"", - b"", - b"/dev/vg1/lv1", - b"/dev/sda1", - b"/dev/sdb1", - b"ext4", - b"/dev/vg1/lv1", - b"ext4", - b"/dev/vg1/random-lv", - b"ext4", + "", + "/dev/sda1", + "", + "/dev/sdb1", + "", + "", + "", + "/dev/vg1/lv1", + "/dev/sda1", + "/dev/sdb1", + "ext4", + "/dev/vg1/lv1", + "ext4", + "/dev/vg1/random-lv", + "ext4", ] result = self.base_os_mount_tools.mount_os() @@ -935,20 +935,20 @@ def test_mount_os_run_xfs(self, mock_check_mount_fstab_partitions, } mock_get_mounted_devices.return_value = ["/dev/sda1"] mock_exec_cmd.side_effect = [ - b"", - b"xfs\n", - b"", - b"xfs\n", - b"", - b"", - b"", - b"xfs\n", - b"xfs\n", - b"xfs\n", - b"ext4\n", - b"xfs\n", - b"ext4\n", - b"xfs\n", + "", + "xfs\n", + "", + "xfs\n", + "", + "", + "", + "xfs\n", + "xfs\n", + "xfs\n", + "ext4\n", + "xfs\n", + "ext4\n", + "xfs\n", ] result = self.base_os_mount_tools.mount_os() @@ -983,9 +983,9 @@ def test_dismount_os(self, mock_exec_cmd): root_dir = "/mnt/root_dir" mock_exec_cmd.side_effect = [ None, - (b"/dev/sda1 /mnt/root_dir/sub_dir type ext4\n" - b"/dev/sda2 /mnt/root_dir/dev type ext4\n" - b"/dev/sda3 /mnt/root_dir type ext4\n"), + ("/dev/sda1 /mnt/root_dir/sub_dir type ext4\n" + "/dev/sda2 /mnt/root_dir/dev type ext4\n" + "/dev/sda3 /mnt/root_dir type ext4\n"), None, None, None, diff --git a/coriolis/tests/osmorphing/test_base.py b/coriolis/tests/osmorphing/test_base.py index af4dd1f09..408a43f0f 100644 --- a/coriolis/tests/osmorphing/test_base.py +++ b/coriolis/tests/osmorphing/test_base.py @@ -462,7 +462,7 @@ def test__read_config_file(self, mock_read_ssh_ini): def test__read_config_file_sudo( self, mock_read_file_sudo, mock_test_path_chroot): mock_test_path_chroot.return_value = True - mock_read_file_sudo.return_value = b'[connection]\ntype=ethernet' + mock_read_file_sudo.return_value = '[connection]\ntype=ethernet' result = self.os_morphing_tools._read_config_file_sudo( self.chroot_path) @@ -783,7 +783,7 @@ def test__configure_cloud_init( @mock.patch.object(base.BaseLinuxOSMorphingTools, '_exec_cmd_chroot') def test__test_path_chroot(self, mock_exec_cmd_chroot): path = "/tmp/test_path" - mock_exec_cmd_chroot.return_value = b"1\n" + mock_exec_cmd_chroot.return_value = "1\n" result = self.os_morphing_tools._test_path_chroot(path) @@ -794,7 +794,7 @@ def test__test_path_chroot(self, mock_exec_cmd_chroot): @mock.patch.object(base.BaseLinuxOSMorphingTools, '_exec_cmd_chroot') def test__test_path_chroot_no_leading_slash(self, mock_exec_cmd_chroot): path = "tmp/test_path" - mock_exec_cmd_chroot.return_value = b"1\n" + mock_exec_cmd_chroot.return_value = "1\n" result = self.os_morphing_tools._test_path_chroot(path) @@ -825,7 +825,7 @@ def test__read_file_sudo_no_leading_slash(self, mock_exec_cmd_chroot): def test__read_grub_config_file_exists(self, mock_test_path_chroot, mock_read_file_sudo): config = mock.sentinel.config - file_contents = b'key1="value1"\n#comment\nkey2="value2"\ninvalid_line' + file_contents = 'key1="value1"\n#comment\nkey2="value2"\ninvalid_line' mock_test_path_chroot.return_value = True mock_read_file_sudo.return_value = file_contents @@ -857,7 +857,7 @@ def test__get_grub_config_obj_file_exists( tmp_file = "/tmp/tmp_file" mock_test_path_chroot.return_value = True - mock_exec_cmd_chroot.side_effect = [tmp_file.encode(), None] + mock_exec_cmd_chroot.side_effect = [tmp_file, None] mock_read_grub_config.return_value = ( mock_exec_cmd_chroot.return_value) @@ -933,7 +933,7 @@ def test_set_grub_value_append(self, mock_exec_cmd_chroot, } cfg = 'cfg' - mock_read_file_sudo.return_value = cfg.encode() + mock_read_file_sudo.return_value = cfg self.os_morphing_tools.set_grub_value(option, value, config_obj) @@ -956,7 +956,7 @@ def test_set_grub_value_replace(self, mock_exec_cmd_chroot, } cfg = 'cfg' - mock_read_file_sudo.return_value = cfg.encode() + mock_read_file_sudo.return_value = cfg self.os_morphing_tools.set_grub_value(option, value, config_obj) diff --git a/coriolis/tests/osmorphing/test_debian.py b/coriolis/tests/osmorphing/test_debian.py index f18a0498b..b9ce53940 100644 --- a/coriolis/tests/osmorphing/test_debian.py +++ b/coriolis/tests/osmorphing/test_debian.py @@ -59,7 +59,7 @@ def test_disable_predictable_nic_names( mock_test_path_chroot.assert_called_once_with('etc/default/grub') mock_grub2_cfg_editor.assert_called_once_with( - mock_read_file_sudo.return_value.decode.return_value) + mock_read_file_sudo.return_value) mock_grub2_cfg_editor.return_value.append_to_option.assert_has_calls( [ mock.call( @@ -215,8 +215,7 @@ def test_set_net_config_no_dhcp( @mock.patch.object(base.BaseLinuxOSMorphingTools, '_exec_cmd_chroot') def test_get_installed_packages(self, mock_exec_cmd_chroot): - mock_exec_cmd_chroot.return_value = \ - "package1\npackage2".encode('utf-8') + mock_exec_cmd_chroot.return_value = "package1\npackage2" self.morpher.get_installed_packages() diff --git a/coriolis/tests/osmorphing/test_redhat.py b/coriolis/tests/osmorphing/test_redhat.py index c92697c75..7c9d3fd63 100644 --- a/coriolis/tests/osmorphing/test_redhat.py +++ b/coriolis/tests/osmorphing/test_redhat.py @@ -296,8 +296,7 @@ def test_set_net_config_no_dhcp( @mock.patch.object(base.BaseLinuxOSMorphingTools, '_exec_cmd_chroot') def test_get_installed_packages(self, mock_exec_cmd_chroot): - mock_exec_cmd_chroot.return_value = \ - "package1\npackage2".encode('utf-8') + mock_exec_cmd_chroot.return_value = "package1\npackage2" self.morphing_tools.get_installed_packages() @@ -387,7 +386,7 @@ def test__yum_clean_all_path_not_exists(self, mock_test_path, @mock.patch.object(base.BaseLinuxOSMorphingTools, '_read_file_sudo') def test__find_yum_repos_found(self, mock_read_file_sudo, mock_list_dir): mock_list_dir.return_value = ['file1.repo', 'file2.repo'] - mock_read_file_sudo.return_value = b'[repo1]\n[repo2]' + mock_read_file_sudo.return_value = '[repo1]\n[repo2]' repos_to_enable = ['repo1'] result = self.morphing_tools._find_yum_repos(repos_to_enable) @@ -404,7 +403,7 @@ def test__find_yum_repos_found(self, mock_read_file_sudo, mock_list_dir): def test__find_yum_repos_not_found(self, mock_read_file_sudo, mock_list_dir): mock_list_dir.return_value = ['file1.repo', 'file2.repo'] - mock_read_file_sudo.return_value = b'[repo1]\n[repo2]' + mock_read_file_sudo.return_value = '[repo1]\n[repo2]' repos_to_enable = ['repo3'] with self.assertLogs('coriolis.osmorphing.redhat', level=logging.WARN): diff --git a/coriolis/tests/osmorphing/test_suse.py b/coriolis/tests/osmorphing/test_suse.py index eef4bb964..3ab5e2913 100644 --- a/coriolis/tests/osmorphing/test_suse.py +++ b/coriolis/tests/osmorphing/test_suse.py @@ -78,8 +78,7 @@ def test_check_os_not_supported(self): @mock.patch.object(base.BaseLinuxOSMorphingTools, '_exec_cmd_chroot') def test_get_installed_packages(self, mock_exec_cmd_chroot): - mock_exec_cmd_chroot.return_value = \ - "package1\npackage2".encode('utf-8') + mock_exec_cmd_chroot.return_value = "package1\npackage2" self.morphing_tools.get_installed_packages() @@ -241,7 +240,7 @@ def test_post_packages_install( @mock.patch.object(base.BaseLinuxOSMorphingTools, '_exec_cmd_chroot') def test__enable_sles_module(self, mock_exec_cmd_chroot): - mock_exec_cmd_chroot.return_value = b"module1\nmodule2\nmodule3" + mock_exec_cmd_chroot.return_value = "module1\nmodule2\nmodule3" self.morphing_tools._enable_sles_module("module2") @@ -262,7 +261,7 @@ def test__enable_sles_module(self, mock_exec_cmd_chroot): @mock.patch.object(base.BaseLinuxOSMorphingTools, '_exec_cmd_chroot') def test__enable_sles_module_with_exception(self, mock_exec_cmd_chroot): mock_exec_cmd_chroot.side_effect = [ - b"module output", None, None, Exception()] + "module output", None, None, Exception()] self.assertRaises(exception.CoriolisException, self.morphing_tools._enable_sles_module, @@ -289,7 +288,7 @@ def test_add_cloud_tools_repo_with_tumbleweed_version(self, mock_add_repo): @mock.patch.object(base.BaseLinuxOSMorphingTools, '_exec_cmd_chroot') def test__get_repos(self, mock_exec_cmd_chroot): mock_exec_cmd_chroot.return_value = ( - b"repo1 http://repo1.com\nrepo2 http://repo2.com") + "repo1 http://repo1.com\nrepo2 http://repo2.com") result = self.morphing_tools._get_repos() diff --git a/coriolis/tests/providers/test_backup_writers.py b/coriolis/tests/providers/test_backup_writers.py index e7a6353a4..1459758c3 100644 --- a/coriolis/tests/providers/test_backup_writers.py +++ b/coriolis/tests/providers/test_backup_writers.py @@ -1327,7 +1327,7 @@ def test__read_remote_file_sudo(self, mock_exec_ssh_cmd): mock_exec_ssh_cmd.assert_called_once_with( self._ssh, "sudo cat %s" % mock.sentinel.remote_path, get_pty=True) self.assertEqual( - result, mock_exec_ssh_cmd.return_value.decode.return_value) + result, mock_exec_ssh_cmd.return_value) @mock.patch.object( backup_writers.HTTPBackupWriterBootstrapper, diff --git a/coriolis/tests/test_utils.py b/coriolis/tests/test_utils.py index 26a319782..d1dde64c1 100644 --- a/coriolis/tests/test_utils.py +++ b/coriolis/tests/test_utils.py @@ -118,7 +118,7 @@ def test_get_udev_net_rules(self): @mock.patch.object(utils, 'exec_ssh_cmd') def test_parse_os_release(self, mock_ssh_cmd): - mock_ssh_cmd.return_value = b'ID=ubuntu\nVERSION_ID="20.04"\n' + mock_ssh_cmd.return_value = 'ID=ubuntu\nVERSION_ID="20.04"\n' result = utils.parse_os_release(self.mock_ssh) @@ -130,7 +130,7 @@ def test_parse_os_release(self, mock_ssh_cmd): @mock.patch.object(utils, 'exec_ssh_cmd') def test_parse_os_release_no_equal(self, mock_ssh_cmd): - mock_ssh_cmd.return_value = b'ID=ubuntu\nVERSION_ID="20.04"\nNOEQUAL\n' + mock_ssh_cmd.return_value = 'ID=ubuntu\nVERSION_ID="20.04"\nNOEQUAL\n' result = utils.parse_os_release(self.mock_ssh) @@ -142,7 +142,7 @@ def test_parse_os_release_no_equal(self, mock_ssh_cmd): @mock.patch.object(utils, 'exec_ssh_cmd') def test_parse_os_release_missing_fields(self, mock_ssh_cmd): - mock_ssh_cmd.return_value = b'NO_ID\nNO_VERSION_ID\n' + mock_ssh_cmd.return_value = 'NO_ID\nNO_VERSION_ID\n' result = utils.parse_os_release(self.mock_ssh) @@ -154,7 +154,7 @@ def test_parse_os_release_missing_fields(self, mock_ssh_cmd): @mock.patch.object(utils, 'exec_ssh_cmd') def test_parse_lsb_release(self, mock_ssh_cmd): - mock_ssh_cmd.return_value = b'Distributor ID: Ubuntu\nRelease: 20.04\n' + mock_ssh_cmd.return_value = 'Distributor ID: Ubuntu\nRelease: 20.04\n' result = utils.parse_lsb_release(self.mock_ssh) @@ -165,7 +165,7 @@ def test_parse_lsb_release(self, mock_ssh_cmd): @mock.patch.object(utils, 'exec_ssh_cmd') def test_parse_lsb_release_missing_fields(self, mock_ssh_cmd): - mock_ssh_cmd.return_value = b'No Distributor ID\nNo Release\n' + mock_ssh_cmd.return_value = 'No Distributor ID\nNo Release\n' result = utils.parse_lsb_release(self.mock_ssh) @@ -330,7 +330,7 @@ def test_exec_ssh_cmd(self): self.mock_ssh.exec_command.assert_called_once_with( "command", environment=None, get_pty=False, timeout=None) - self.assertEqual(result, b'output\n') + self.assertEqual(result, 'output\n') def test_exec_ssh_cmd_timeout_with_timeout(self): self.mock_stdout.read.return_value = b'output\n' @@ -343,7 +343,9 @@ def test_exec_ssh_cmd_timeout_with_timeout(self): self.mock_ssh.exec_command.assert_called_once_with( "command", environment=None, get_pty=False, timeout=10.0) - self.assertEqual(result, self.mock_stdout.read.return_value) + expected = self.mock_stdout.read.return_value.decode( + 'utf-8', errors='replace') + self.assertEqual(result, expected) def test_exec_ssh_cmd_getpeername_value_error(self): self.mock_stdout.read.return_value = b'output\n' @@ -360,7 +362,9 @@ def test_exec_ssh_cmd_getpeername_value_error(self): self.mock_ssh.exec_command.assert_called_once_with( "command", environment=None, get_pty=False, timeout=None) - self.assertEqual(output, self.mock_stdout.read.return_value) + expected = self.mock_stdout.read.return_value.decode( + 'utf-8', errors='replace') + self.assertEqual(output, expected) def test_exec_ssh_cmd_timeout(self): self.mock_stdout.read.side_effect = socket.timeout @@ -400,7 +404,9 @@ def test_exec_ssh_cmd_chroot(self): "sudo -E chroot /chroot /bin/bash -c command", environment=None, get_pty=False, timeout=None) - self.assertEqual(result, self.mock_stdout.read.return_value) + expected = self.mock_stdout.read.return_value.decode( + 'utf-8', errors='replace') + self.assertEqual(result, expected) def test_check_fs(self): self.mock_stdout.read.return_value.replace.return_value = \ @@ -427,7 +433,7 @@ def test_check_fs_exception(self, mock_exec_ssh_cmd): @mock.patch.object(utils, 'exec_ssh_cmd') def test_run_xfs_repair(self, mock_exec_ssh_cmd): - mock_exec_ssh_cmd.return_value = b"/tmp/tmp_dir\n" + mock_exec_ssh_cmd.return_value = "/tmp/tmp_dir\n" utils.run_xfs_repair(self.mock_ssh, "/dev/sda1") diff --git a/coriolis/utils.py b/coriolis/utils.py index 097ce52e4..8fe1016a5 100644 --- a/coriolis/utils.py +++ b/coriolis/utils.py @@ -208,7 +208,7 @@ def get_udev_net_rules(net_ifaces_info): def parse_os_release(ssh): os_release_info = exec_ssh_cmd( ssh, - "[ -f '/etc/os-release' ] && cat /etc/os-release || true").decode() + "[ -f '/etc/os-release' ] && cat /etc/os-release || true") info = {} for line in os_release_info.splitlines(): if "=" not in line: @@ -222,7 +222,7 @@ def parse_os_release(ssh): def parse_lsb_release(ssh): - out = exec_ssh_cmd(ssh, "lsb_release -a || true").decode() + out = exec_ssh_cmd(ssh, "lsb_release -a || true") dist_id = re.findall('^Distributor ID:\s(.*)$', out, re.MULTILINE) release = re.findall('^Release:\s(.*)$', out, re.MULTILINE) if dist_id and release: @@ -298,8 +298,17 @@ def write_winrm_file(conn, remote_path, content, overwrite=True): @retry_on_error() def list_ssh_dir(ssh, remote_path): - sftp = ssh.open_sftp() - return sftp.listdir(remote_path) + """List directory contents via SFTP, falling back to ls on + encoding errors.""" + try: + sftp = ssh.open_sftp() + return sftp.listdir(remote_path) + except UnicodeDecodeError as ex: + LOG.warning( + "SFTP listdir failed due to non-UTF-8 filename, " + "falling back to shell command. Error: %s", str(ex)) + output = exec_ssh_cmd(ssh, "ls -1 %s" % remote_path, get_pty=True) + return [f for f in output.splitlines() if f.strip()] @retry_on_error(terminal_exceptions=[exception.MinionMachineCommandTimeout]) @@ -335,7 +344,10 @@ def exec_ssh_cmd(ssh, cmd, environment=None, get_pty=False, timeout=None): # Most of the commands will use pseudo-terminal which unfortunately will # include a '\r' to every newline. This will affect all plugins too, so # best we can do now is replace them. - return std_out.replace(b'\r\n', b'\n').replace(b'\n\r', b'\n') + std_out = std_out.replace(b'\r\n', b'\n').replace(b'\n\r', b'\n') + + # Decode output with error handling for non-UTF-8 characters + return std_out.decode('utf-8', errors='replace') def exec_ssh_cmd_chroot(ssh, chroot_dir, cmd, environment=None, get_pty=False, @@ -349,7 +361,7 @@ def check_fs(ssh, fs_type, dev_path): try: out = exec_ssh_cmd( ssh, "sudo fsck -p -t %s %s" % (fs_type, dev_path), - get_pty=True).decode() + get_pty=True) LOG.debug("File system checked:\n%s", out) except Exception: LOG.warn("Checking file system returned an error:\n%s" % ( @@ -360,18 +372,18 @@ def check_fs(ssh, fs_type, dev_path): def run_xfs_repair(ssh, dev_path): try: tmp_dir = exec_ssh_cmd( - ssh, "mktemp -d").decode().rstrip("\n") + ssh, "mktemp -d").rstrip("\n") LOG.debug("mounting %s on %s" % (dev_path, tmp_dir)) mount_out = exec_ssh_cmd( ssh, "sudo mount %s %s" % (dev_path, tmp_dir), - get_pty=True).decode() + get_pty=True) LOG.debug("mount returned: %s" % mount_out) LOG.debug("Umounting %s" % tmp_dir) umount_out = exec_ssh_cmd( - ssh, "sudo umount %s" % tmp_dir, get_pty=True).decode() + ssh, "sudo umount %s" % tmp_dir, get_pty=True) LOG.debug("umounting returned: %s" % umount_out) out = exec_ssh_cmd( - ssh, "sudo xfs_repair %s" % dev_path, get_pty=True).decode() + ssh, "sudo xfs_repair %s" % dev_path, get_pty=True) LOG.debug("File system repaired:\n%s", out) except Exception as ex: LOG.warn("xfs_repair returned an error:\n%s", str(ex))