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/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/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..2d8236e2 100644 --- a/coriolis/osmorphing/osmount/windows.py +++ b/coriolis/osmorphing/osmount/windows.py @@ -239,3 +239,25 @@ 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 = f'& "{script_path}"; exit $LASTEXITCODE' + 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/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/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/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/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, {}) 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/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/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), 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):