From 502fbd7f287eebd8c546b9933fef725a993fdf54 Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Mon, 18 May 2026 13:38:32 +0000 Subject: [PATCH 1/3] exec_ssh_cmd: only retry commands that failed due to SSH errors We used to retry all SSH commands that failed, even those that don't actually exist. We're still retrying commands that are expected to fail, wasting time unnecessarily during os morphing: coriolis.exception.CoriolisException: Command "readlink -en /dev/sda2" failed on host '172.17.0.3:22' with exit code: 1 2026-05-18 12:14:11.783 1667765 WARNING coriolis.osmorphing.osmount.base [-] Target not found for symlink: /dev/sda2. Original link path will be returned What makes things even worse is that the caller functions are often retried as well, having the "retry_on_error" decorator without any parameters. Commands that are expected to fail end up being retried 25 times. We'll update "exec_ssh_cmd" so that by default it will perform retries only in case of SSH errors. For convenience, we're exposing the retry helper arguments (number of retries, retry interval and terminal exceptions). In addition to that, we'll introduce a new argument for the list of retried exceptions. If the caller needs the actual commands to be retried as well, "SSHCommandFailed" may be added to the list of retried exceptions. --- coriolis/exception.py | 4 ++++ coriolis/tests/test_utils.py | 35 ++++++++++++++++++++----------- coriolis/utils.py | 40 +++++++++++++++++++++++++++++------- 3 files changed, 60 insertions(+), 19 deletions(-) diff --git a/coriolis/exception.py b/coriolis/exception.py index 73f38488..d228a7e7 100644 --- a/coriolis/exception.py +++ b/coriolis/exception.py @@ -509,6 +509,10 @@ class MinionMachineCommandTimeout(CoriolisException): pass +class SSHCommandFailed(CoriolisException): + pass + + class SSHCommandNotFoundException(CoriolisException): pass diff --git a/coriolis/tests/test_utils.py b/coriolis/tests/test_utils.py index 2bfc8cbf..2ad18cd1 100644 --- a/coriolis/tests/test_utils.py +++ b/coriolis/tests/test_utils.py @@ -83,6 +83,26 @@ def test_retry_on_error_exception_keyboard_interrupt(self): self.assertEqual(self.mock_func.call_count, 1) + def test_retry_on_error_not_in_list_of_retried(self): + self.mock_func.side_effect = ValueError + + self.assertRaises(ValueError, utils.retry_on_error( + max_attempts=5, sleep_seconds=0, + terminal_exceptions=[], + retried_exceptions=(CoriolisTestException, ))(self.mock_func)) + + self.assertEqual(self.mock_func.call_count, 1) + + def test_retry_on_error_in_list_of_retried(self): + self.mock_func.side_effect = CoriolisTestException + + self.assertRaises(CoriolisTestException, utils.retry_on_error( + max_attempts=5, sleep_seconds=0, + terminal_exceptions=[], + retried_exceptions=(CoriolisTestException, ))(self.mock_func)) + + self.assertEqual(self.mock_func.call_count, 5) + def test_retry_on_error_terminal_exception(self): self.mock_func.side_effect = CoriolisTestException @@ -383,10 +403,7 @@ def test_exec_ssh_cmd_exception(self): self.mock_ssh.exec_command.return_value = (None, self.mock_stdout, self.mock_stdout) - original_exec_ssh_cmd = testutils.get_wrapped_function( - utils.exec_ssh_cmd) - - self.assertRaises(exception.CoriolisException, original_exec_ssh_cmd, + self.assertRaises(exception.SSHCommandFailed, utils.exec_ssh_cmd, self.mock_ssh, "command") self.mock_ssh.exec_command.assert_called_once_with( @@ -398,11 +415,8 @@ def test_exec_ssh_cmd_command_not_found_in_stdout(self): self.mock_ssh.exec_command.return_value = (None, self.mock_stdout, self.mock_stdout) - original_exec_ssh_cmd = testutils.get_wrapped_function( - utils.exec_ssh_cmd) - self.assertRaises(exception.SSHCommandNotFoundException, - original_exec_ssh_cmd, self.mock_ssh, "command") + utils.exec_ssh_cmd, self.mock_ssh, "command") def test_exec_ssh_cmd_exit_code_127(self): self.mock_stdout.read.return_value = b'' @@ -410,11 +424,8 @@ def test_exec_ssh_cmd_exit_code_127(self): self.mock_ssh.exec_command.return_value = (None, self.mock_stdout, self.mock_stdout) - original_exec_ssh_cmd = testutils.get_wrapped_function( - utils.exec_ssh_cmd) - self.assertRaises(exception.SSHCommandNotFoundException, - original_exec_ssh_cmd, self.mock_ssh, "command") + utils.exec_ssh_cmd, self.mock_ssh, "command") def test_exec_ssh_cmd_chroot(self): self.mock_stdout.read.return_value = b'output\n' diff --git a/coriolis/utils.py b/coriolis/utils.py index 0655e05a..592e888e 100644 --- a/coriolis/utils.py +++ b/coriolis/utils.py @@ -168,7 +168,8 @@ def get_single_result(lis): def retry_on_error(max_attempts=5, sleep_seconds=1, - terminal_exceptions=[]): + terminal_exceptions=[], + retried_exceptions=(Exception, )): def _retry_on_error(func): @functools.wraps(func) def _exec_retry(*args, **kwargs): @@ -180,7 +181,7 @@ def _exec_retry(*args, **kwargs): LOG.debug("Got a KeyboardInterrupt, skip retrying") LOG.exception(ex) raise - except Exception as ex: + except retried_exceptions as ex: if any([isinstance(ex, tex) for tex in terminal_exceptions]): raise @@ -314,10 +315,7 @@ def list_ssh_dir(ssh, remote_path): return [f for f in output.splitlines() if f.strip()] -@retry_on_error(terminal_exceptions=[ - exception.MinionMachineCommandTimeout, - exception.SSHCommandNotFoundException]) -def exec_ssh_cmd(ssh, cmd, environment=None, get_pty=False, timeout=None): +def _exec_ssh_cmd(ssh, cmd, environment=None, get_pty=False, timeout=None): remote_str = "" if timeout is not None: timeout = float(timeout) @@ -350,7 +348,7 @@ def exec_ssh_cmd(ssh, cmd, environment=None, get_pty=False, timeout=None): "command not found" in stdout_str or "command not found" in stderr_str): raise exception.SSHCommandNotFoundException(msg) - raise exception.CoriolisException(msg) + raise exception.SSHCommandFailed(msg) # 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. @@ -360,6 +358,34 @@ def exec_ssh_cmd(ssh, cmd, environment=None, get_pty=False, timeout=None): return std_out.decode('utf-8', errors='replace') +def exec_ssh_cmd( + ssh, + cmd, + environment=None, + get_pty=False, + timeout=None, + max_attempts=5, + retry_interval=1, + terminal_exceptions=( + exception.MinionMachineCommandTimeout, + exception.SSHCommandNotFoundException, + ), + retried_exceptions=(paramiko.ssh_exception.SSHException,), +): + # By default, we'll perform retires only in case of SSH failures. + # + # Add "SSHCommandFailed" to the list of retried exceptions if failed + # commands should be retried as well. + + @retry_on_error(max_attempts=max_attempts, sleep_seconds=retry_interval, + terminal_exceptions=terminal_exceptions, + retried_exceptions=retried_exceptions) + def wrapper(): + return _exec_ssh_cmd(ssh, cmd, environment, get_pty, timeout) + + return wrapper() + + def exec_ssh_cmd_chroot(ssh, chroot_dir, cmd, environment=None, get_pty=False, timeout=None): return exec_ssh_cmd(ssh, "sudo -E chroot %s %s" % (chroot_dir, cmd), From 06f8ba0882f40414267ae391d65a827eda7ab1a0 Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Mon, 18 May 2026 08:02:34 +0000 Subject: [PATCH 2/3] Extend user scripts API, allowing the phase to be specified MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When deploying replicas, Coriolis users can specify scripts that will be executed during os-morphing, right after the OS partition is mounted on the minion instance. In some situations, this is too late since user scripts may be needed in order to be able to mount the OS disk, for example if it’s encrypted. In other situations it’s too early. Some scripts may need to be executed on the replica instance. This may require provider assistance. To accommodate these use cases, we’ll extend the deployment API, allowing the user to specify when a given script should be executed. Each script may specify one of the following execution phases: * osmorphing-pre-os-mount * osmorphing-post-os-mount (default) * replica-initial-boot (TBD) We'll continue to support the basic script format as well. See the API samples for a few examples. Note that we can't use the OS morphing tools to run pre-os-mount scripts. A few approaches have been considered: * initialize the OS morphing tools against the minion root partition * hacky * unnecessarily complicated and brittle * use the base Linux/Windows OS morphing classes * can't be instantiated since not all abstract methods are implemented by the base classes * extend the OS mount tools to allow running pre-os-mount user scripts * easiest and most natural approach, imlemented by this patch --- .../deployment-from-transfer-req.json | 14 +++ coriolis/api/v1/utils.py | 108 ++++++++++++++++- coriolis/conductor/rpc/server.py | 7 -- coriolis/constants.py | 17 +++ coriolis/osmorphing/manager.py | 26 ++++- coriolis/osmorphing/osmount/base.py | 33 ++++++ coriolis/osmorphing/osmount/windows.py | 26 +++++ coriolis/tasks/osmorphing_tasks.py | 19 ++- coriolis/tests/conductor/rpc/test_server.py | 12 +- .../deployments/test_osmorphing.py | 109 ++++++++++++++++-- coriolis/tests/integration/utils.py | 54 +++++++-- .../tests/osmorphing/osmount/test_base.py | 3 + coriolis/tests/osmorphing/test_manager.py | 20 +++- .../tests/tasks/data/osmorphing_task_run.yml | 28 +++-- coriolis/tests/tasks/test_osmorphing_tasks.py | 5 +- 15 files changed, 428 insertions(+), 53 deletions(-) diff --git a/coriolis/api-refs/api_samples/deployment/deployment-from-transfer-req.json b/coriolis/api-refs/api_samples/deployment/deployment-from-transfer-req.json index 9e955937..faaab4e8 100644 --- a/coriolis/api-refs/api_samples/deployment/deployment-from-transfer-req.json +++ b/coriolis/api-refs/api_samples/deployment/deployment-from-transfer-req.json @@ -6,6 +6,20 @@ "skip_os_morphing": false, "instance_osmorphing_minion_pool_mappings": { "instance1": "morphing_pool" + }, + "user_scripts": { + "global": { + "linux": [ + { + "phase": "osmorphing_pre_os_mount", + "payload": "echo 'unlocking encrypted OS partion'" + }, + { + "phase": "osmorphing_post_os_mount", + "payload": "echo 'modifying replica OS filesystem'" + } + ] + } } } } diff --git a/coriolis/api/v1/utils.py b/coriolis/api/v1/utils.py index 4daff9c5..b3ffdfca 100644 --- a/coriolis/api/v1/utils.py +++ b/coriolis/api/v1/utils.py @@ -86,7 +86,103 @@ def _build_keyerror_message(resource, method, key): return msg +def _sanitize_newlines(payload: str) -> str: + # Convert \r\n or \n\r to \n. + return payload.replace('\r\n', '\n').replace('\n\r', '\n') + + +def _process_user_scripts( + user_scripts: list[dict] | str | None, + sanitize_newlines: bool = False, +) -> list[dict]: + # Process the user script(s), returning an "extended format" + # list. See 'validate_user_scripts for more details. + if user_scripts is None: + return [] + elif isinstance(user_scripts, str): + # Basic script format. + payload = user_scripts + if sanitize_newlines: + payload = _sanitize_newlines(payload) + return [ + { + "phase": constants.PHASE_OSMORPHING_POST_OS_MOUNT, + "payload": payload, + }, + ] + elif isinstance(user_scripts, list): + # Extended format. + for script_item in user_scripts: + if not isinstance(script_item, dict): + raise exception.InvalidInput( + reason="Invalid user script list item, expecting dict.") + + allowed_keys = ["payload", "phase"] + for key in script_item: + if key not in allowed_keys: + raise exception.InvalidInput( + reason=(f"Invalid script item key: {key}, " + f"allowed keys: {allowed_keys}")) + + if not script_item.get("phase"): + script_item["phase"] = constants.PHASE_OSMORPHING_POST_OS_MOUNT + if script_item["phase"] not in constants.USER_SCRIPT_PHASES: + raise exception.InvalidInput( + reason=( + f"Unknown user script phase: {script_item['phase']}, " + f"supported phases: {constants.USER_SCRIPT_PHASES}.")) + if "payload" not in script_item: + raise exception.InvalidInput( + reason="Missing 'payload' field.") + if not isinstance(script_item["payload"], str): + raise exception.InvalidInput( + reason="Invalid payload type, expecting string.") + + if sanitize_newlines: + script_item["payload"] = _sanitize_newlines( + script_item["payload"]) + + return user_scripts + else: + raise exception.InvalidInput( + reason=("Invalid user script format. Expecting a string or a " + "list of dicts containing the payload and phase.")) + + def validate_user_scripts(user_scripts): + # Validate the top level user scripts dict. + # Example: + # { + # # Globally executed scripts, used if there are no per-instance + # # scripts. + # 'global': { + # # Basic format: single script in string format. + # # Executed + # 'windows': 'write-host "hello-world!"' + # # Extended format: a list of scripts, allowing the execution + # # phase to be specified. + # 'linux': [ + # { + # 'phase': 'osmorphing_pre_os_mount', + # 'payload': 'echo "configuring LUKS"', + # }, + # { + # 'phase': 'osmorphing_post_os_mount', + # 'payload': 'echo "modifying OS configuration"' + # } + # ] + # }, + # # Per instance scripts. + # 'instances': { + # 'instance-id': [ + # # Extended format without an explicit execution phase, + # # Defaulting to osmorphing_post_os_mount. + # { + # 'payload': 'echo "modifying OS configuration"' + # } + # ] + # } + # } if user_scripts is None: user_scripts = {} if not isinstance(user_scripts, dict): @@ -104,6 +200,9 @@ def validate_user_scripts(user_scripts): reason='The provided global user script os_type "%s" is ' 'invalid. Must be one of the ' 'following: %s' % (os_type, constants.VALID_OS_TYPES)) + global_scripts[os_type] = _process_user_scripts( + global_scripts[os_type], + sanitize_newlines=(os_type == constants.OS_TYPE_LINUX)) instance_scripts = user_scripts.get('instances', {}) if not isinstance(instance_scripts, dict): @@ -111,7 +210,14 @@ def validate_user_scripts(user_scripts): reason='"instances" must be a mapping between the identifiers of ' 'the instances in the Replica/Migration and their ' 'respective scripts.') - + for instance_id in instance_scripts: + # The conductor used to do this, sanitizing newlines regardless + # of the instance OS types. + # TODO(lpetrut): consider moving it to the OS morphing side, which + # has the OS type at hand. + instance_scripts[instance_id] = _process_user_scripts( + instance_scripts[instance_id], + sanitize_newlines=True) return user_scripts diff --git a/coriolis/conductor/rpc/server.py b/coriolis/conductor/rpc/server.py index 98bec5e1..b3c66dfd 100644 --- a/coriolis/conductor/rpc/server.py +++ b/coriolis/conductor/rpc/server.py @@ -1724,13 +1724,6 @@ def _normalize_user_scripts(self, user_scripts, instances): instance, instances) user_scripts['instances'].pop(instance, None) continue - user_scripts['instances'][instance] = ( - user_scripts['instances'][instance].replace('\r\n', '\n'). - replace('\n\r', '\n')) - linux_scripts = user_scripts.get('global', {}).get('linux') - if linux_scripts: - user_scripts['global']['linux'] = ( - linux_scripts.replace('\r\n', '\n').replace('\n\r', '\n')) return user_scripts @transfer_synchronized diff --git a/coriolis/constants.py b/coriolis/constants.py index 270bf47d..6225f12b 100644 --- a/coriolis/constants.py +++ b/coriolis/constants.py @@ -361,3 +361,20 @@ MINION_MACHINE_POWER_STATUS_POWERED_OFF = "POWERED_OFF" MINION_MACHINE_POWER_STATUS_POWERING_ON = "POWERING_ON" MINION_MACHINE_POWER_STATUS_POWERING_OFF = "POWERING_OFF" + +# User script execution phases. +# +# Scripts that must be executed before the OS partition is mounted, for +# example scripts that unlock encrypted partitions. +PHASE_OSMORPHING_PRE_OS_MOUNT = "osmorphing_pre_os_mount" +# Scripts that are executed after the OS partition is mounted (the default). +PHASE_OSMORPHING_POST_OS_MOUNT = "osmorphing_post_os_mount" +# We may eventually add "PHASE_REPLICA_FIRST_BOOT" for convenience, although +# the users can already achieve this by using os-morphing scripts to schedule +# scripts that will be executed at the next boot. This may require import +# provider support. + +USER_SCRIPT_PHASES = [ + PHASE_OSMORPHING_PRE_OS_MOUNT, + PHASE_OSMORPHING_POST_OS_MOUNT, +] diff --git a/coriolis/osmorphing/manager.py b/coriolis/osmorphing/manager.py index 50493b8a..e7fcfc31 100644 --- a/coriolis/osmorphing/manager.py +++ b/coriolis/osmorphing/manager.py @@ -6,6 +6,7 @@ from oslo_config import cfg from oslo_log import log as logging +from coriolis import constants from coriolis import events from coriolis import exception from coriolis.osmorphing import base as base_osmorphing @@ -126,11 +127,12 @@ def get_osmorphing_tools_class_for_provider( def morph_image(origin_provider, destination_provider, connection_info, - osmorphing_info, user_script, event_handler): + osmorphing_info, user_scripts, event_handler): event_manager = events.EventManager(event_handler) os_type = osmorphing_info.get('os_type') ignore_devices = osmorphing_info.get('ignore_devices', []) + user_scripts = user_scripts or [] # instantiate and run OSMount tools: os_mount_tools = osmount_factory.get_os_mount_tools( @@ -150,6 +152,19 @@ def morph_image(origin_provider, destination_provider, connection_info, "OSMorphing minion machine and the VM undergoing OSMorphing. " "Error was: %s" % str(err)) from err + pre_os_mount_user_scripts = [ + script["payload"] for script in user_scripts + if script["phase"] == constants.PHASE_OSMORPHING_PRE_OS_MOUNT] + if not pre_os_mount_user_scripts: + event_manager.progress_update( + 'No pre-os-mount OS morphing user script specified') + for user_script in pre_os_mount_user_scripts: + event_manager.progress_update( + 'Running pre-os-mount OS morphing user script') + # We haven't detected the morphed OS partition yet, we'll use + # the mount tools to run pre-mount user scripts. + os_mount_tools.run_user_script(user_script) + event_manager.progress_update("Discovering and mounting OS partitions") os_root_dir, os_root_dev = os_mount_tools.mount_os() @@ -210,13 +225,16 @@ def morph_image(origin_provider, destination_provider, connection_info, CONF.default_osmorphing_operation_timeout) import_os_morphing_tools.set_environment(environment) - if user_script: + post_os_mount_user_scripts = [ + script["payload"] for script in user_scripts + if script["phase"] == constants.PHASE_OSMORPHING_POST_OS_MOUNT] + for user_script in post_os_mount_user_scripts: event_manager.progress_update( 'Running OS morphing user script') import_os_morphing_tools.run_user_script(user_script) - else: + if not post_os_mount_user_scripts: event_manager.progress_update( - 'No OS morphing user script specified') + 'No post-os-mount OS morphing user script specified') event_manager.progress_update( 'OS being migrated: %s' % detected_os_info['friendly_release_name']) diff --git a/coriolis/osmorphing/osmount/base.py b/coriolis/osmorphing/osmount/base.py index b7a1a585..fec59bde 100644 --- a/coriolis/osmorphing/osmount/base.py +++ b/coriolis/osmorphing/osmount/base.py @@ -61,6 +61,11 @@ def set_proxy(self, proxy_settings): def get_environment(self): return self._environment + @abc.abstractmethod + def run_user_script(self, user_script): + """Run pre-os-mount user script.""" + pass + class BaseSSHOSMountTools(BaseOSMountTools): @utils.retry_on_error(max_attempts=5, sleep_seconds=3) @@ -674,3 +679,31 @@ def set_proxy(self, proxy_settings): if no_proxy: LOG.debug("Proxy exclusions: %s", no_proxy) self._environment["no_proxy"] = '.'.join(no_proxy) + + def run_user_script(self, user_script): + if len(user_script) == 0: + return + + script_path = "/tmp/coriolis_user_script" + try: + utils.write_ssh_file( + self._ssh, + script_path, + user_script) + except Exception as err: + raise exception.CoriolisException( + "Failed to copy user script to target system.") from err + + try: + utils.exec_ssh_cmd( + self._ssh, + "sudo chmod +x %s" % script_path, + get_pty=True) + + utils.exec_ssh_cmd( + self._ssh, + f'sudo "{script_path}"', + get_pty=True) + except Exception as err: + raise exception.CoriolisException( + "Failed to run user script.") from err diff --git a/coriolis/osmorphing/osmount/windows.py b/coriolis/osmorphing/osmount/windows.py index 66375e6e..825b5b27 100644 --- a/coriolis/osmorphing/osmount/windows.py +++ b/coriolis/osmorphing/osmount/windows.py @@ -239,3 +239,29 @@ def mount_os(self): def dismount_os(self, root_drive): self._bring_nonboot_disks_offline() + + def run_user_script(self, user_script): + if len(user_script) == 0: + return + + script_path = "$env:TMP\\coriolis_user_script.ps1" + try: + utils.write_winrm_file( + self._conn, + script_path, + user_script) + except Exception as err: + raise exception.CoriolisException( + "Failed to copy user script to target system.") from err + + cmd = ('$ErrorActionPreference = "Stop"; powershell.exe ' + '-NonInteractive -ExecutionPolicy RemoteSigned ' + '-File "%(script)s"') % { + "script": script_path, + } + try: + out = self._conn.exec_ps_command(cmd) + LOG.debug("User script output: %s" % out) + except Exception as err: + raise exception.CoriolisException( + "Failed to run user script.") from err diff --git a/coriolis/tasks/osmorphing_tasks.py b/coriolis/tasks/osmorphing_tasks.py index 3005a525..cfd8bbe4 100644 --- a/coriolis/tasks/osmorphing_tasks.py +++ b/coriolis/tasks/osmorphing_tasks.py @@ -55,21 +55,30 @@ def _run(self, ctxt, instance, origin, destination, task_info, osmorphing_info = task_info.get('osmorphing_info', {}) user_scripts = task_info.get("user_scripts") - instance_script = None + instance_scripts = None if user_scripts: - instance_script = user_scripts.get("instances", {}).get(instance) - if not instance_script: + instance_scripts = user_scripts.get("instances", {}).get(instance) + if not instance_scripts: os_type = osmorphing_info.get("os_type") if os_type: - instance_script = user_scripts.get( + instance_scripts = user_scripts.get( "global", {}).get(os_type) + if isinstance(instance_scripts, str): + # Legacy record, convert to extended format. + instance_scripts = [ + { + "phase": constants.PHASE_OSMORPHING_POST_OS_MOUNT, + "payload": instance_scripts, + } + ] + osmorphing_manager.morph_image( origin_provider, destination_provider, osmorphing_connection_info, osmorphing_info, - instance_script, + instance_scripts, event_handler) return {} diff --git a/coriolis/tests/conductor/rpc/test_server.py b/coriolis/tests/conductor/rpc/test_server.py index 096cd33a..0444ab37 100644 --- a/coriolis/tests/conductor/rpc/test_server.py +++ b/coriolis/tests/conductor/rpc/test_server.py @@ -2098,8 +2098,8 @@ def test__get_transfer_not_found(self, mock_get_transfer): ( { 'instances': { - "mock_instance_1": "mock_value_1\r\n", - "mock_instance_2": "mock_value_2\r\n" + "mock_instance_1": "mock_value_1\n", + "mock_instance_2": "mock_value_2\n" } }, ["mock_instance_2"], @@ -2112,11 +2112,11 @@ def test__get_transfer_not_found(self, mock_get_transfer): ( { 'global': { - "linux": "mock_value_1\r\n" + "linux": "mock_value_1\n" }, 'instances': { - "mock_instance_1": "mock_value_1\n\r", - "mock_instance_2": "mock_value_2\n\r" + "mock_instance_1": "mock_value_1\n", + "mock_instance_2": "mock_value_2\n" } }, ["mock_instance_2"], @@ -2132,7 +2132,7 @@ def test__get_transfer_not_found(self, mock_get_transfer): ( { 'global': { - "linux": "mock_value_1\n\r" + "linux": "mock_value_1\n" } }, [], diff --git a/coriolis/tests/integration/deployments/test_osmorphing.py b/coriolis/tests/integration/deployments/test_osmorphing.py index 54384b72..9c0ab98a 100644 --- a/coriolis/tests/integration/deployments/test_osmorphing.py +++ b/coriolis/tests/integration/deployments/test_osmorphing.py @@ -7,6 +7,8 @@ installation in the target OS. """ +import uuid + from coriolis.tests.integration import base as integration_base from coriolis.tests.integration import utils as test_utils @@ -21,22 +23,115 @@ def setUp(self): super().setUp() test_utils.write_os_image_to_disk(self._src_device, "ubuntu:24.04") - def test_deployment_with_os_morphing(self): - self.assertFalse( - test_utils.path_exists_on_device(self._src_device, "usr/bin/jq"), - "jq was found on the source device before OS morphing", - ) + def _execute_transfer_and_deployment(self, deployment_kwargs=None): + deployment_kwargs = deployment_kwargs or {} self._execute_and_wait(self._transfer.id) - deployment = self._client.deployments.create_from_transfer( self._transfer.id, skip_os_morphing=False, + **deployment_kwargs, ) self.addCleanup(self._client.deployments.delete, deployment.id) - self.assertDeploymentCompleted(deployment.id) + + def test_deployment_with_os_morphing(self): + self.assertFalse( + test_utils.path_exists_on_device(self._src_device, "usr/bin/jq"), + "jq was found on the source device before OS morphing", + ) + + self._execute_transfer_and_deployment() + self.assertTrue( test_utils.path_exists_on_device(self._dst_device, "usr/bin/jq"), "jq was not found on the destination device after OS morphing", ) + + def test_os_morphing_global_script_basic_format(self): + expected_string = str(uuid.uuid4()) + user_scripts = { + 'global': { + # Coriolis will pass the root path as the first argument. + # We'll use a Windows style line ending, expecting it to + # be sanitized. + 'linux': f"echo -n {expected_string} > $1/cookie\r\n", + 'windows': 'should-not-get-executed', + } + } + deployment_kwargs = { + "user_scripts": user_scripts, + } + self._execute_transfer_and_deployment(deployment_kwargs) + + file_contents = test_utils.read_file_from_device( + self._dst_device, + "cookie") + self.assertEqual(expected_string, file_contents) + + def test_os_morphing_instance_script_basic_format(self): + expected_string = str(uuid.uuid4()) + instance = self._src_device + user_scripts = { + 'instances': { + instance: f"echo -n {expected_string} > $1/cookie\n\r" + } + } + deployment_kwargs = { + "user_scripts": user_scripts, + } + self._execute_transfer_and_deployment(deployment_kwargs) + + file_contents = test_utils.read_file_from_device( + self._dst_device, + "cookie") + self.assertEqual(expected_string, file_contents) + + def test_os_morphing_global_script_extended_format(self): + user_scripts = { + 'global': { + 'linux': [ + { + "phase": "osmorphing_pre_os_mount", + # Write the mounts to the minion temp dir. + "payload": "mount > /tmp/pre_mounts\r\n", + }, + { + "phase": "osmorphing_post_os_mount", + # Copy the mounts file to the migrated OS drive. + "payload": "cp /tmp/pre_mounts $1/", + }, + { + "phase": "osmorphing_post_os_mount", + # Write the mounts to the migrated OS drive. + "payload": "mount > $1/post_mounts", + }, + ], + 'windows': [ + { + "phase": "osmorphing_pre_os_mount", + "payload": "should-not-get-executed", + }, + { + "phase": "osmorphing_post_os_mount", + "payload": "should-not-get-executed", + }, + ] + } + } + deployment_kwargs = { + "user_scripts": user_scripts, + } + self._execute_transfer_and_deployment(deployment_kwargs) + + pre_mounts = test_utils.read_file_from_device( + self._dst_device, + "pre_mounts") + post_mounts = test_utils.read_file_from_device( + self._dst_device, + "post_mounts") + + # Ensure that the "osmorphing_pre_os_mount" was executed before + # the replica OS disk was mounted. + self.assertNotIn(self._dst_device, pre_mounts) + self.assertIn(self._dst_device, post_mounts) diff --git a/coriolis/tests/integration/utils.py b/coriolis/tests/integration/utils.py index d954e53d..b5efc6d7 100644 --- a/coriolis/tests/integration/utils.py +++ b/coriolis/tests/integration/utils.py @@ -152,12 +152,20 @@ def devices_match(path_a, path_b): def _run(cmd, check=True): LOG.debug("Running: %s", " ".join(str(c) for c in cmd)) - return subprocess.run( - cmd, - stdout=subprocess.PIPE, - stderr=subprocess.DEVNULL, - check=check, - ) + try: + return subprocess.run( + cmd, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + check=check, + ) + except subprocess.CalledProcessError as ex: + LOG.error( + "Command failed: %s, return code: %s, stderr: %s", + " ".join(str(c) for c in cmd), + ex.returncode, + ex.stderr) + raise def wait_for_ssh(host, port, username, pkey_path, timeout=30): @@ -276,10 +284,20 @@ def get_container_ip(container_id): :param container_id: container ID or name :returns: IP address string """ - result = _run( - ["docker", "inspect", "--format", - "{{.NetworkSettings.IPAddress}}", - container_id]) + try: + result = _run( + ["docker", "inspect", "--format", + "{{range.NetworkSettings.Networks}}{{.IPAddress}}{{end}}", + container_id]) + except subprocess.CalledProcessError as ex: + if "template parsing error" in ex.stderr: + # Fallback to the old format. + result = _run( + ["docker", "inspect", "--format", + "{{.NetworkSettings.IPAddress}}", + container_id]) + else: + raise return result.stdout.decode().strip() @@ -366,3 +384,19 @@ def path_exists_on_device(device_path, rel_path): return os.path.exists(os.path.join(mount_point, rel_path)) finally: _run(["umount", mount_point]) + + +def read_file_from_device(device_path, rel_path): + """Retrieves the specified file from the filesystem of *device_path*. + + Mounts the device read-only into a temporary directory, reads the file, + then unmounts. + """ + with tempfile.TemporaryDirectory() as mount_point: + _run(["mount", "-o", "ro", device_path, mount_point]) + + try: + with open(os.path.join(mount_point, rel_path)) as f: + return f.read() + finally: + _run(["umount", mount_point]) diff --git a/coriolis/tests/osmorphing/osmount/test_base.py b/coriolis/tests/osmorphing/osmount/test_base.py index e4068439..3c0afac9 100644 --- a/coriolis/tests/osmorphing/osmount/test_base.py +++ b/coriolis/tests/osmorphing/osmount/test_base.py @@ -43,6 +43,9 @@ def dismount_os(self): def mount_os(self): pass + def run_user_script(self, user_script): + pass + class BaseSSHOSMountToolsTestCase(test_base.CoriolisBaseTestCase): """Test suite for the BaseSSHOSMountTools class.""" diff --git a/coriolis/tests/osmorphing/test_manager.py b/coriolis/tests/osmorphing/test_manager.py index 2012e413..0e7e932b 100644 --- a/coriolis/tests/osmorphing/test_manager.py +++ b/coriolis/tests/osmorphing/test_manager.py @@ -4,6 +4,7 @@ import logging from unittest import mock +from coriolis import constants from coriolis import exception from coriolis.osmorphing import base as base_osmorphing from coriolis.osmorphing import manager @@ -19,6 +20,17 @@ def setUp(self): 'os_type': 'linux', 'ignore_devices': [] } + self._mock_user_scripts = [ + { + "phase": constants.PHASE_OSMORPHING_PRE_OS_MOUNT, + "payload": "pre-os-mount-script", + }, + { + "phase": constants.PHASE_OSMORPHING_POST_OS_MOUNT, + "payload": "post-os-mount-script", + }, + ] + manager.CONF.proxy.url = "http://127.0.0.1:8080" manager.CONF.proxy.username = "admin" manager.CONF.proxy.password = "Random-Password-123!" @@ -211,7 +223,7 @@ def test_morph_image( manager.morph_image( mock.sentinel.origin_provider, mock.sentinel.destination_provider, mock.sentinel.connection_info, self.osmorphing_info, - mock.sentinel.user_script, self.event_handler) + self._mock_user_scripts, self.event_handler) expected_calls = [ mock.call( @@ -250,7 +262,7 @@ def test_morph_image_failed_os_mount_setup( manager.morph_image, mock.sentinel.origin_provider, mock.sentinel.destination_provider, mock.sentinel.connection_info, self.osmorphing_info, - mock.sentinel.user_script, + self._mock_user_scripts, self.event_handler) mock_get_os_mount_tools.assert_called_once_with( @@ -284,7 +296,7 @@ def test_morph_image_no_import_os_morphing_tools_cls( manager.morph_image, mock.sentinel.origin_provider, mock.sentinel.destination_provider, mock.sentinel.connection_info, self.osmorphing_info, - mock.sentinel.user_script, self.event_handler) + self._mock_user_scripts, self.event_handler) @mock.patch.object(manager.osmount_factory, 'get_os_mount_tools') @mock.patch.object(manager.events, 'EventManager') @@ -336,4 +348,4 @@ def test_morph_image_dismount_os_exception( manager.morph_image, mock.sentinel.origin_provider, mock.sentinel.destination_provider, mock.sentinel.connection_info, self.osmorphing_info, - mock.sentinel.user_script, self.event_handler) + self._mock_user_scripts, self.event_handler) diff --git a/coriolis/tests/tasks/data/osmorphing_task_run.yml b/coriolis/tests/tasks/data/osmorphing_task_run.yml index f1a050c3..79204b4f 100644 --- a/coriolis/tests/tasks/data/osmorphing_task_run.yml +++ b/coriolis/tests/tasks/data/osmorphing_task_run.yml @@ -5,7 +5,7 @@ osmorphing_connection_info: info1: value1 info2: value2 - expected_instance_script: ~ + expected_instance_scripts: ~ - config: instance: instance_1 @@ -17,7 +17,9 @@ user_scripts: instances: instance_1: instance_script1 - expected_instance_script: instance_script1 + expected_instance_scripts: + - phase: osmorphing_post_os_mount + payload: instance_script1 - config: instance: instance_1 @@ -29,8 +31,20 @@ info2: value2 user_scripts: global: - linux: linux_script1 - expected_instance_script: linux_script1 + linux: + - phase: osmorphing_pre_os_mount + payload: instance_script1 + - phase: osmorphing_pre_os_mount + payload: instance_script2 + - phase: osmorphing_post_os_mount + payload: instance_script3 + expected_instance_scripts: + - phase: osmorphing_pre_os_mount + payload: instance_script1 + - phase: osmorphing_pre_os_mount + payload: instance_script2 + - phase: osmorphing_post_os_mount + payload: instance_script3 - config: instance: instance_1 @@ -42,7 +56,7 @@ user_scripts: global: linux: linux_script1 - expected_instance_script: ~ + expected_instance_scripts: ~ - config: instance: instance_1 @@ -53,7 +67,7 @@ info1: value1 info2: value2 user_scripts: {} - expected_instance_script: ~ + expected_instance_scripts: ~ - config: instance: instance_1 @@ -66,4 +80,4 @@ user_scripts: global: windows: windows_script1 - expected_instance_script: ~ + expected_instance_scripts: ~ diff --git a/coriolis/tests/tasks/test_osmorphing_tasks.py b/coriolis/tests/tasks/test_osmorphing_tasks.py index 13402f45..3b8ceb30 100644 --- a/coriolis/tests/tasks/test_osmorphing_tasks.py +++ b/coriolis/tests/tasks/test_osmorphing_tasks.py @@ -25,7 +25,7 @@ def setUp(self): @ddt.file_data('data/osmorphing_task_run.yml') @ddt.unpack def test__run(self, mock_morph_image, mock_unmarshal, mock_get_provider, - config, expected_instance_script): + config, expected_instance_scripts): mock_get_provider.side_effect = [mock.sentinel.origin_provider, mock.sentinel.destination_provider] instance = config['instance'] @@ -48,10 +48,11 @@ def test__run(self, mock_morph_image, mock_unmarshal, mock_get_provider, mock_get_provider.assert_has_calls(expected_calls) mock_unmarshal.assert_called_once_with( task_info['osmorphing_connection_info']) + mock_morph_image.assert_called_once_with( mock.sentinel.origin_provider, mock.sentinel.destination_provider, mock_unmarshal.return_value, osmorphing_info, - expected_instance_script, mock.sentinel.event_handler) + expected_instance_scripts, mock.sentinel.event_handler) self.assertEqual(result, {}) From c35f99168247054a71ceb0e9fa5f21ac8383dd20 Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Wed, 20 May 2026 10:49:17 +0000 Subject: [PATCH 3/3] Propagate Windows user script errors `powershell.exe -File` doesn't propagate the exit code of the specified script. As an alternative, we'll use '& $script; exit $LASTEXITCODE', moving the execution policy and non-interactive parameters to the "exec_ps_command" method. --- coriolis/osmorphing/osmount/windows.py | 6 +----- coriolis/osmorphing/windows.py | 4 +--- coriolis/tests/osmorphing/test_windows.py | 8 +++----- coriolis/tests/test_wsman.py | 8 +++++++- coriolis/wsman.py | 9 ++++++++- 5 files changed, 20 insertions(+), 15 deletions(-) diff --git a/coriolis/osmorphing/osmount/windows.py b/coriolis/osmorphing/osmount/windows.py index 825b5b27..2d8236e2 100644 --- a/coriolis/osmorphing/osmount/windows.py +++ b/coriolis/osmorphing/osmount/windows.py @@ -254,11 +254,7 @@ def run_user_script(self, user_script): raise exception.CoriolisException( "Failed to copy user script to target system.") from err - cmd = ('$ErrorActionPreference = "Stop"; powershell.exe ' - '-NonInteractive -ExecutionPolicy RemoteSigned ' - '-File "%(script)s"') % { - "script": script_path, - } + cmd = f'& "{script_path}"; exit $LASTEXITCODE' try: out = self._conn.exec_ps_command(cmd) LOG.debug("User script output: %s" % out) diff --git a/coriolis/osmorphing/windows.py b/coriolis/osmorphing/windows.py index 0149d4e7..887a269d 100644 --- a/coriolis/osmorphing/windows.py +++ b/coriolis/osmorphing/windows.py @@ -406,9 +406,7 @@ def run_user_script(self, user_script): raise exception.CoriolisException( "Failed to copy user script to target system.") from err - cmd = ('$ErrorActionPreference = "Stop"; powershell.exe ' - '-NonInteractive -ExecutionPolicy RemoteSigned ' - '-File "%(script)s" "%(os_root_dir)s"') % { + cmd = ('& "%(script)s" "%(os_root_dir)s"; exit $LASTEXITCODE') % { "script": script_path, "os_root_dir": self._os_root_dir, } diff --git a/coriolis/tests/osmorphing/test_windows.py b/coriolis/tests/osmorphing/test_windows.py index f635561e..3feadf9a 100644 --- a/coriolis/tests/osmorphing/test_windows.py +++ b/coriolis/tests/osmorphing/test_windows.py @@ -338,11 +338,9 @@ def test_run_user_script(self, mock_write_winrm_file): mock_write_winrm_file.assert_called_once_with( self.conn, script_path, user_script) self.conn.exec_ps_command.assert_called_once_with( - ('$ErrorActionPreference = "Stop"; powershell.exe ' - '-NonInteractive -ExecutionPolicy RemoteSigned ' - '-File "%(script)s" "%(os_root_dir)s"') % { - "script": script_path, - "os_root_dir": self.os_root_dir, + ('& "%(script)s" "%(os_root_dir)s"; exit $LASTEXITCODE') % { + "script": script_path, + "os_root_dir": self.os_root_dir, } ) diff --git a/coriolis/tests/test_wsman.py b/coriolis/tests/test_wsman.py index 7a9dacb7..1683b3f5 100644 --- a/coriolis/tests/test_wsman.py +++ b/coriolis/tests/test_wsman.py @@ -148,7 +148,13 @@ def test_exec_ps_command(self): self.conn.exec_command.return_value = "std_out\n\n" result = self.conn.exec_ps_command(self.cmd) self.conn.exec_command.assert_called_once_with( - "powershell.exe", ["-EncodedCommand", 'dABlAHMAdABfAGMAbQBkAA=='], + "powershell.exe", + [ + "-EncodedCommand", + 'dABlAHMAdABfAGMAbQBkAA==', + '-NonInteractive', + '-ExecutionPolicy', 'RemoteSigned', + ], timeout=None) self.assertEqual(result, "std_out") diff --git a/coriolis/wsman.py b/coriolis/wsman.py index adb69cc2..02bba720 100644 --- a/coriolis/wsman.py +++ b/coriolis/wsman.py @@ -149,7 +149,14 @@ def exec_ps_command(self, cmd, ignore_stdout=False, timeout=None): LOG.debug("Executing PS command: %s", cmd) base64_cmd = base64.b64encode(cmd.encode('utf-16le')).decode() return self.exec_command( - "powershell.exe", ["-EncodedCommand", base64_cmd], + "powershell.exe", + [ + "-EncodedCommand", + base64_cmd, + "-NonInteractive", + "-ExecutionPolicy", + "RemoteSigned", + ], timeout=timeout)[:-2] def test_path(self, remote_path):