From f51a1231e24bfc7e64f898690b6c66d39c56cced Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Tue, 19 May 2026 13:55:19 +0000 Subject: [PATCH] Alow specifying the os-morphing user script phase 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’ve extended 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) Samples: * Explicit phase --user-script-global linux=/some/script.sh,phase=osmorphing-pre-os-mount * Implicit phase, defaults to osmorphing-post-os-mount --user-script-global linux=/some/script.sh * Repeating the flag, specifying multiple scripts: --user-script-instance some-instance=/some/script.sh,phase=osmorphing-pre-os-mount --user-script-instance some-instance=/other/script.sh,phase=osmorphing-pre-os-mount --user-script-instance some-instance=/another/script.sh,phase=osmorphing-post-os-mount --- coriolisclient/cli/deployments.py | 76 +++++++++------ coriolisclient/cli/utils.py | 129 ++++++++++++++++++++----- coriolisclient/constants.py | 17 ++++ coriolisclient/tests/cli/test_utils.py | 75 ++++++++++++-- 4 files changed, 232 insertions(+), 65 deletions(-) diff --git a/coriolisclient/cli/deployments.py b/coriolisclient/cli/deployments.py index 848adc9..0012047 100644 --- a/coriolisclient/cli/deployments.py +++ b/coriolisclient/cli/deployments.py @@ -161,37 +161,51 @@ class CreateDeployment(show.ShowOne): """Start a new deployment from an existing transfer""" def get_parser(self, prog_name): parser = super(CreateDeployment, self).get_parser(prog_name) - parser.add_argument('transfer', - help='The ID of the transfer to migrate') - parser.add_argument('--force', - help='Force the deployment in case of a transfer ' - 'with failed executions', action='store_true', - default=False) - parser.add_argument('--dont-clone-disks', - help='Retain the transfer disks by cloning them', - action='store_false', dest="clone_disks", - default=True) - parser.add_argument('--skip-os-morphing', - help='Skip the OS morphing process', - action='store_true', - default=False) - parser.add_argument('--user-script-global', action='append', - required=False, - dest="global_scripts", - help='A script that will run for a particular ' - 'os_type. This option can be used multiple ' - 'times. Use: linux=/path/to/script.sh or ' - 'windows=/path/to/script.ps1') - parser.add_argument('--user-script-instance', action='append', - required=False, - dest="instance_scripts", - help='A script that will run for a particular ' - 'instance specified by the --instance option. ' - 'This option can be used multiple times. ' - 'Use: "instance_name"=/path/to/script.sh.' - ' This option overwrites any OS specific script ' - 'specified in --user-script-global for this ' - 'instance') + parser.add_argument( + 'transfer', + help='The ID of the transfer to migrate') + parser.add_argument( + '--force', + help='Force the deployment in case of a transfer ' + 'with failed executions', + action='store_true', + default=False) + parser.add_argument( + '--dont-clone-disks', + help='Retain the transfer disks by cloning them', + action='store_false', + dest="clone_disks", + default=True) + parser.add_argument( + '--skip-os-morphing', + help='Skip the OS morphing process', + action='store_true', + default=False) + parser.add_argument( + '--user-script-global', + action='append', + required=False, + dest="global_scripts", + help='A script that will run for a particular os_type. This ' + 'option can be used multiple times. ' + 'Use: linux=/path/to/script.sh or ' + 'windows=/path/to/script.ps1. ' + 'Can optionally include a script phase: ' + 'windows=/path/to/script.ps1,phase=osmorphing_pre_os_mount.') + parser.add_argument( + '--user-script-instance', + action='append', + required=False, + dest="instance_scripts", + help='A script that will run for a particular ' + 'instance specified by the --instance option. ' + 'This option can be used multiple times. ' + 'Use: "instance_name"=/path/to/script.sh.' + ' This option overwrites any OS specific script ' + 'specified in --user-script-global for this ' + 'instance. Can optionally include a script phase: ' + 'instance_name=/path/to/script.ps1,' + 'phase=osmorphing_pre_os_mount.') cli_utils.add_minion_pool_args_to_parser( parser, include_origin_pool_arg=False, include_destination_pool_arg=False, diff --git a/coriolisclient/cli/utils.py b/coriolisclient/cli/utils.py index 18c1f1d..011ae17 100644 --- a/coriolisclient/cli/utils.py +++ b/coriolisclient/cli/utils.py @@ -191,6 +191,23 @@ def get_option_value_from_args(args, option_name, error_on_no_value=True): return value +def comma_separated_kv_to_dict(input_string: str) -> dict: + """Convert a comma separated list of key=value pairs to dict. + + Example: some_key=some_val,some_other_key=some_other_val + -> {"some_key": "some_val", "some_other_key": "some_other_val"} + """ + out = {} + kv_pairs = input_string.split(",") + for kv_pair in kv_pairs: + try: + key, value = kv_pair.split("=") + except ValueError: + raise ValueError("Not a = pair: %s" % kv_pair) + out[key] = value + return out + + def compose_user_scripts(global_scripts, instance_scripts): ret = { "global": {}, @@ -198,35 +215,97 @@ def compose_user_scripts(global_scripts, instance_scripts): } global_scripts = global_scripts or [] instance_scripts = instance_scripts or [] - for glb in global_scripts: - split = glb.split("=", 1) - if len(split) != 2: - continue - if split[0] not in constants.OS_LIST: + for global_script_str_params in global_scripts: + try: + params = comma_separated_kv_to_dict(global_script_str_params) + except ValueError: + raise ValueError( + "Invalid global user script parameter: %s. Expecting " + "=. Can optionally include a comma " + "separated phase parameter, " + "e.g. =,phase=" % + global_script_str_params) + phase = params.pop("phase", constants.PHASE_OSMORPHING_POST_OS_MOUNT) + if phase not in constants.USER_SCRIPT_PHASES: + raise ValueError( + f"Invalid user script phase: {phase}. " + "Available options are: " + f"{', '.join(constants.USER_SCRIPT_PHASES)}.") + if not params: + raise ValueError( + "OS type not specified. " + "Available options are: %s" % ", ".join(constants.OS_LIST)) + if len(params.keys()) > 1: + raise ValueError( + "Too many parameters. Expecting just the OS type.") + os_type = list(params.keys())[0] + script_path = params[os_type] + if os_type not in constants.OS_LIST: raise ValueError( "Invalid OS %s. Available options are: %s" % ( - split[0], ", ".join(constants.OS_LIST))) - if not split[1]: - # removing script - ret["global"][split[0]] = None - continue - if os.path.isfile(split[1]) is False: - raise ValueError("Could not find %s" % split[1]) - with open(split[1]) as sc: - ret["global"][split[0]] = sc.read() - - for inst in instance_scripts: - split = inst.split("=", 1) - if len(split) != 2: + os_type, ", ".join(constants.OS_LIST))) + + payload = None + # The user may omit the script path in order to remove all script + # records. + if not script_path: + ret["global"][os_type] = None continue - if not split[1]: - # removing script - ret['instances'][split[0]] = None + + if not os.path.isfile(script_path): + raise ValueError("Could not find %s" % script_path) + with open(script_path) as sc: + payload = sc.read() + if os_type not in ret["global"]: + ret["global"][os_type] = [] + script_entry = { + "phase": phase, + "payload": payload, + } + ret["global"][os_type].append(script_entry) + + for instance_scripts_str_params in instance_scripts: + try: + params = comma_separated_kv_to_dict(instance_scripts_str_params) + except ValueError: + raise ValueError( + "Invalid instance user script parameter: %s. Expecting " + "=. Can optionally include a comma " + "separated phase parameter, " + "e.g. =,phase=" % + instance_scripts_str_params) + + phase = params.pop("phase", constants.PHASE_OSMORPHING_POST_OS_MOUNT) + if phase not in constants.USER_SCRIPT_PHASES: + raise ValueError( + f"Invalid user script phase: {phase}. " + "Available options are: " + f"{', '.join(constants.USER_SCRIPT_PHASES)}.") + if not params: + raise ValueError("Instance not specified.") + if len(params.keys()) > 1: + raise ValueError( + "Too many parameters. Expecting just one instance.") + instance = list(params.keys())[0] + script_path = params[instance] + payload = None + # The user may omit the script path in order to remove all script + # records. + if not script_path: + ret["instances"][instance] = None continue - if os.path.isfile(split[1]) is False: - raise ValueError("Could not find %s" % split[1]) - with open(split[1]) as sc: - ret["instances"][split[0]] = sc.read() + + if not os.path.isfile(script_path): + raise ValueError("Could not find %s" % script_path) + with open(script_path) as sc: + payload = sc.read() + if instance not in ret["instances"]: + ret["instances"][instance] = [] + script_entry = { + "phase": phase, + "payload": payload, + } + ret["instances"][instance].append(script_entry) return ret diff --git a/coriolisclient/constants.py b/coriolisclient/constants.py index 8ba47d2..10c1981 100644 --- a/coriolisclient/constants.py +++ b/coriolisclient/constants.py @@ -48,3 +48,20 @@ OS_TYPE_OTHER, OS_TYPE_UNKNOWN, ] + +# 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/coriolisclient/tests/cli/test_utils.py b/coriolisclient/tests/cli/test_utils.py index 4dd6d63..a0218ef 100644 --- a/coriolisclient/tests/cli/test_utils.py +++ b/coriolisclient/tests/cli/test_utils.py @@ -9,6 +9,10 @@ from coriolisclient.cli import utils from coriolisclient.tests import test_base +_user_script_path = os.path.join( + os.path.dirname(os.path.realpath(__file__)), + 'data/user_scripts.yml') + @ddt.ddt class UtilsTestCase(test_base.CoriolisBaseTestCase): @@ -252,11 +256,7 @@ def test_get_option_value_from_args_json_value_error(self): { "global_scripts": ["linux script"], "instance_scripts": ["linux script"], - "expected_result": - { - "global": {}, - "instances": {} - } + "expected_result": None }, { "global_scripts": ["invalid_os=scrips"], @@ -273,6 +273,20 @@ def test_get_option_value_from_args_json_value_error(self): "instance_scripts": ["linux='invalid/file/path'"], "expected_result": None }, + { + "global_scripts": None, + # Too many parameters. + "instance_scripts": [ + f"linux={_user_script_path},windows={_user_script_path}"], # noqa + "expected_result": None + }, + { + "global_scripts": None, + # Invalid phase. + "instance_scripts": [ + f"linux={_user_script_path},phase=invalid-phase"], # noqa + "expected_result": None + } ) def test_compose_user_scripts(self, data): global_scripts = data["global_scripts"] @@ -298,15 +312,58 @@ def test_compose_user_scripts(self, data): def test_compose_user_scripts_from_file(self): script_path = os.path.dirname(os.path.realpath(__file__)) script_path = os.path.join(script_path, 'data/user_scripts.yml') - global_scripts = ["linux=%s" % script_path] - instance_scripts = ["linux=%s" % script_path] + global_scripts = [ + f"linux={script_path}", + f"windows={script_path},phase=osmorphing_pre_os_mount", # noqa + f"windows={script_path},phase=osmorphing_post_os_mount", # noqa + ] + instance_scripts = [ + f"instance0={script_path}", + f"instance1={script_path}", + f"instance1={script_path},phase=osmorphing_pre_os_mount", # noqa + ] result = utils.compose_user_scripts(global_scripts, instance_scripts) + payload = '"mock_script1"\n"mock_script2"\n' self.assertEqual( { - 'global': {'linux': '"mock_script1"\n"mock_script2"\n'}, - 'instances': {'linux': '"mock_script1"\n"mock_script2"\n'} + 'global': { + 'linux': [ + { + 'phase': "osmorphing_post_os_mount", + 'payload': payload, + }, + ], + 'windows': [ + { + 'phase': "osmorphing_pre_os_mount", + 'payload': payload, + }, + { + 'phase': "osmorphing_post_os_mount", + 'payload': payload, + }, + ], + }, + 'instances': { + 'instance0': [ + { + 'phase': "osmorphing_post_os_mount", + 'payload': payload, + }, + ], + 'instance1': [ + { + 'phase': "osmorphing_post_os_mount", + 'payload': payload, + }, + { + 'phase': "osmorphing_pre_os_mount", + 'payload': payload, + }, + ], + }, }, result )