From f51a1231e24bfc7e64f898690b6c66d39c56cced Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Tue, 19 May 2026 13:55:19 +0000 Subject: [PATCH 1/4] 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 ) From 091020fb42a383b7c0e6cb43c081c055dceb88c7 Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Wed, 27 May 2026 07:04:21 +0000 Subject: [PATCH 2/4] Let argparse convert the args We'll pass type=comma_separated_kv_to_dict when defining user script args, letting it do the parsing for us. --- coriolisclient/cli/deployments.py | 2 + coriolisclient/cli/transfers.py | 4 ++ coriolisclient/cli/utils.py | 48 ++++++++++---------- coriolisclient/tests/cli/test_deployments.py | 7 ++- coriolisclient/tests/cli/test_utils.py | 37 ++++++++++++--- 5 files changed, 65 insertions(+), 33 deletions(-) diff --git a/coriolisclient/cli/deployments.py b/coriolisclient/cli/deployments.py index 0012047..281f588 100644 --- a/coriolisclient/cli/deployments.py +++ b/coriolisclient/cli/deployments.py @@ -186,6 +186,7 @@ def get_parser(self, prog_name): action='append', required=False, dest="global_scripts", + type=cli_utils.comma_separated_kv_to_dict, 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 ' @@ -197,6 +198,7 @@ def get_parser(self, prog_name): action='append', required=False, dest="instance_scripts", + type=cli_utils.comma_separated_kv_to_dict, help='A script that will run for a particular ' 'instance specified by the --instance option. ' 'This option can be used multiple times. ' diff --git a/coriolisclient/cli/transfers.py b/coriolisclient/cli/transfers.py index 7ff54ed..83e545a 100644 --- a/coriolisclient/cli/transfers.py +++ b/coriolisclient/cli/transfers.py @@ -198,6 +198,7 @@ def get_parser(self, prog_name): parser.add_argument('--user-script-global', action='append', required=False, dest="global_scripts", + type=cli_utils.comma_separated_kv_to_dict, 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 ' @@ -205,6 +206,7 @@ def get_parser(self, prog_name): parser.add_argument('--user-script-instance', action='append', required=False, dest="instance_scripts", + type=cli_utils.comma_separated_kv_to_dict, help='A script that will run for a particular ' 'instance specified by the --instance option. ' 'This option can be used multiple times. ' @@ -354,6 +356,7 @@ def get_parser(self, prog_name): parser.add_argument('--user-script-global', action='append', required=False, dest="global_scripts", + type=cli_utils.comma_separated_kv_to_dict, 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 ' @@ -361,6 +364,7 @@ def get_parser(self, prog_name): parser.add_argument('--user-script-instance', action='append', required=False, dest="instance_scripts", + type=cli_utils.comma_separated_kv_to_dict, help='A script that will run for a particular ' 'instance specified by the --instance option. ' 'This option can be used multiple times. ' diff --git a/coriolisclient/cli/utils.py b/coriolisclient/cli/utils.py index 011ae17..d078fdc 100644 --- a/coriolisclient/cli/utils.py +++ b/coriolisclient/cli/utils.py @@ -202,29 +202,39 @@ def comma_separated_kv_to_dict(input_string: str) -> dict: for kv_pair in kv_pairs: try: key, value = kv_pair.split("=") - except ValueError: - raise ValueError("Not a = pair: %s" % kv_pair) + except ValueError as err: + raise ValueError( + "Not a = pair: %s. " % kv_pair) from err out[key] = value return out -def compose_user_scripts(global_scripts, instance_scripts): +def compose_user_scripts( + global_scripts: list[dict], + instance_scripts: list[dict], +) -> dict: + """Process user script arguments. + + :param global_scripts: a list of dicts describing user scripts, the target + OS and the phase in which the scripts should be invoked. + The dicts are expected to contain the following keys: + * : one of the operating systems supported by Coriolis, + e.g. "windows" or "linux". The value will be a local script path. + * "phase": optional phase, defaults to "osmorphing_post_os_mount". + :param instance_scripts: a list of dicts describing user scripts, each + having a corresponding instance. + The dicts are expected to contain the following keys: + * : The name of the instance where the script must run. + * "phase": optional phase, defaults to "osmorphing_post_os_mount". + :returns: the processed list of scripts as expected by the Coriolis API. + """ ret = { "global": {}, "instances": {} } global_scripts = global_scripts or [] instance_scripts = instance_scripts or [] - 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) + for params in global_scripts: phase = params.pop("phase", constants.PHASE_OSMORPHING_POST_OS_MOUNT) if phase not in constants.USER_SCRIPT_PHASES: raise ValueError( @@ -264,17 +274,7 @@ def compose_user_scripts(global_scripts, instance_scripts): } 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) - + for params in instance_scripts: phase = params.pop("phase", constants.PHASE_OSMORPHING_POST_OS_MOUNT) if phase not in constants.USER_SCRIPT_PHASES: raise ValueError( diff --git a/coriolisclient/tests/cli/test_deployments.py b/coriolisclient/tests/cli/test_deployments.py index 0aefa8a..1685471 100644 --- a/coriolisclient/tests/cli/test_deployments.py +++ b/coriolisclient/tests/cli/test_deployments.py @@ -210,13 +210,16 @@ def test_get_parser(self): parser = self.cli.get_parser('coriolis') global_script = "linux=/linux/path" instance_script = "instance1=/instance1/path" + + processed_global_scripts = [{"linux": "/linux/path"}] + processed_instance_scripts = [{"instance1": "/instance1/path"}] args = parser.parse_args([ 'transfer_id', '--force', '--dont-clone-disks', '--skip-os-morphing', '--user-script-global', global_script, '--user-script-instance', instance_script]) self.assertEqual( - ('transfer_id', True, False, True, [global_script], - [instance_script]), + ('transfer_id', True, False, True, + processed_global_scripts, processed_instance_scripts), (args.transfer, args.force, args.clone_disks, args.skip_os_morphing, args.global_scripts, args.instance_scripts)) diff --git a/coriolisclient/tests/cli/test_utils.py b/coriolisclient/tests/cli/test_utils.py index a0218ef..7eadf9f 100644 --- a/coriolisclient/tests/cli/test_utils.py +++ b/coriolisclient/tests/cli/test_utils.py @@ -234,6 +234,17 @@ def test_get_option_value_from_args_json_value_error(self): "option-name" ) + @ddt.data( + "linux script", + "linux=script,phase", + ) + def test_comma_separated_kv_to_dict_invalid(self, params): + self.assertRaises( + ValueError, + utils.comma_separated_kv_to_dict, + params, + ) + @ddt.data( { "global_scripts": None, @@ -253,11 +264,6 @@ def test_get_option_value_from_args_json_value_error(self): "instances": {"instance_1": None} } }, - { - "global_scripts": ["linux script"], - "instance_scripts": ["linux script"], - "expected_result": None - }, { "global_scripts": ["invalid_os=scrips"], "instance_scripts": None, @@ -289,8 +295,14 @@ def test_get_option_value_from_args_json_value_error(self): } ) def test_compose_user_scripts(self, data): - global_scripts = data["global_scripts"] - instance_scripts = data["instance_scripts"] + global_scripts = [ + utils.comma_separated_kv_to_dict(params) + for params in data["global_scripts"] + ] if data["global_scripts"] else None + instance_scripts = [ + utils.comma_separated_kv_to_dict(params) + for params in data["instance_scripts"] + ] if data["instance_scripts"] else None expected_result = data["expected_result"] if expected_result: @@ -323,6 +335,17 @@ def test_compose_user_scripts_from_file(self): f"instance1={script_path},phase=osmorphing_pre_os_mount", # noqa ] + # We could've provided the dicts directly but we'll exercise + # comma_separated_kv_to_dict instead. + global_scripts = [ + utils.comma_separated_kv_to_dict(params) + for params in global_scripts + ] + instance_scripts = [ + utils.comma_separated_kv_to_dict(params) + for params in instance_scripts + ] + result = utils.compose_user_scripts(global_scripts, instance_scripts) payload = '"mock_script1"\n"mock_script2"\n' From b557fad8c4b60c441e7ae6594953706612bce0f3 Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Wed, 27 May 2026 08:32:49 +0000 Subject: [PATCH 3/4] Bump hacking dependency and remove noqa comments Older flake8/pycodestyle versions weren't able to properly handle f strings, so we had to add "noqa" comments. This issue was addressed in recent versions, all we have to do is bump the "hacking" dependency, which currently enforces older flake8 releases. https://github.com/PyCQA/pycodestyle/pull/1148 --- coriolisclient/tests/cli/test_deployments.py | 2 +- coriolisclient/tests/cli/test_utils.py | 10 +++++----- test-requirements.txt | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/coriolisclient/tests/cli/test_deployments.py b/coriolisclient/tests/cli/test_deployments.py index 1685471..f80f0ca 100644 --- a/coriolisclient/tests/cli/test_deployments.py +++ b/coriolisclient/tests/cli/test_deployments.py @@ -219,7 +219,7 @@ def test_get_parser(self): '--user-script-instance', instance_script]) self.assertEqual( ('transfer_id', True, False, True, - processed_global_scripts, processed_instance_scripts), + processed_global_scripts, processed_instance_scripts), (args.transfer, args.force, args.clone_disks, args.skip_os_morphing, args.global_scripts, args.instance_scripts)) diff --git a/coriolisclient/tests/cli/test_utils.py b/coriolisclient/tests/cli/test_utils.py index 7eadf9f..ec1a834 100644 --- a/coriolisclient/tests/cli/test_utils.py +++ b/coriolisclient/tests/cli/test_utils.py @@ -283,14 +283,14 @@ def test_comma_separated_kv_to_dict_invalid(self, params): "global_scripts": None, # Too many parameters. "instance_scripts": [ - f"linux={_user_script_path},windows={_user_script_path}"], # noqa + f"linux={_user_script_path},windows={_user_script_path}"], "expected_result": None }, { "global_scripts": None, # Invalid phase. "instance_scripts": [ - f"linux={_user_script_path},phase=invalid-phase"], # noqa + f"linux={_user_script_path},phase=invalid-phase"], "expected_result": None } ) @@ -326,13 +326,13 @@ def test_compose_user_scripts_from_file(self): script_path = os.path.join(script_path, 'data/user_scripts.yml') 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 + f"windows={script_path},phase=osmorphing_pre_os_mount", + f"windows={script_path},phase=osmorphing_post_os_mount", ] instance_scripts = [ f"instance0={script_path}", f"instance1={script_path}", - f"instance1={script_path},phase=osmorphing_pre_os_mount", # noqa + f"instance1={script_path},phase=osmorphing_pre_os_mount", ] # We could've provided the dicts directly but we'll exercise diff --git a/test-requirements.txt b/test-requirements.txt index fea111a..bd292a0 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -2,7 +2,7 @@ # of appearance. Changing the order has an impact on the overall integration # process, which may cause wedges in the gate later. -hacking>=6.0.1,<=6.0.1 # Apache-2.0 +hacking>=7.0.0,<7.1.0 # Apache-2.0 coverage!=4.4,>=4.0 # Apache-2.0 ddt>=1.2.1 # MIT oslotest>=3.8.0 # Apache-2.0 From 82cf920a9dd239ce59161ebd53b1fdecdb0e3d5b Mon Sep 17 00:00:00 2001 From: Lucian Petrut Date: Wed, 27 May 2026 09:13:42 +0000 Subject: [PATCH 4/4] Update user script help strings * specify the supported phases and which one is the default * mention the fact that when updating transfers, the script path can be omitted in order to unregister it --- coriolisclient/cli/deployments.py | 8 +- coriolisclient/cli/transfers.py | 167 ++++++++++++++++++------------ coriolisclient/cli/utils.py | 3 +- 3 files changed, 109 insertions(+), 69 deletions(-) diff --git a/coriolisclient/cli/deployments.py b/coriolisclient/cli/deployments.py index 281f588..8b8ec00 100644 --- a/coriolisclient/cli/deployments.py +++ b/coriolisclient/cli/deployments.py @@ -192,7 +192,9 @@ def get_parser(self, prog_name): '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.') + 'windows=/path/to/script.ps1,phase=osmorphing_pre_os_mount. ' + 'Supported phases: osmorphing_post_os_mount (default), ' + 'osmorphing_pre_os_mount.') parser.add_argument( '--user-script-instance', action='append', @@ -207,7 +209,9 @@ def get_parser(self, prog_name): '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.') + 'phase=osmorphing_pre_os_mount. ' + 'Supported phases: osmorphing_post_os_mount (default), ' + '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/transfers.py b/coriolisclient/cli/transfers.py index 83e545a..dacdce6 100644 --- a/coriolisclient/cli/transfers.py +++ b/coriolisclient/cli/transfers.py @@ -170,50 +170,72 @@ class CreateTransfer(show.ShowOne): """Create a new transfer""" def get_parser(self, prog_name): parser = super(CreateTransfer, self).get_parser(prog_name) - parser.add_argument('--origin-endpoint', required=True, - help='The origin endpoint id') - parser.add_argument('--destination-endpoint', required=True, - help='The destination endpoint id') - parser.add_argument('--instance', action='append', required=True, - dest="instances", metavar="INSTANCE_IDENTIFIER", - help='The identifier of a source instance to be ' - 'transferred. Can be specified multiple ' - 'times') - parser.add_argument('--scenario', - dest="scenario", metavar="SCENARIO", - choices=[ - TRANSFER_SCENARIO_REPLICA, - TRANSFER_SCENARIO_LIVE_MIGRATION], - default=TRANSFER_SCENARIO_REPLICA, - help='The type of scenario to use when creating ' - 'the Transfer. "replica" will create a ' - 'monthly-billed Replica which can be ' - 'executed and deployed as many times as ' - 'desired, while "live_migration" will ' - 'create a Transfer which can be synced ' - 'as many times as needed but only ' - 'deployed once.') - parser.add_argument('--notes', dest='notes', - help='Notes about the transfer') - parser.add_argument('--user-script-global', action='append', - required=False, - dest="global_scripts", - type=cli_utils.comma_separated_kv_to_dict, - 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", - type=cli_utils.comma_separated_kv_to_dict, - 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( + '--origin-endpoint', + required=True, + help='The origin endpoint id') + parser.add_argument( + '--destination-endpoint', + required=True, + help='The destination endpoint id') + parser.add_argument( + '--instance', + action='append', + required=True, + dest="instances", + metavar="INSTANCE_IDENTIFIER", + help='The identifier of a source instance to be ' + 'transferred. Can be specified multiple times') + parser.add_argument( + '--scenario', + dest="scenario", + metavar="SCENARIO", + choices=[ + TRANSFER_SCENARIO_REPLICA, + TRANSFER_SCENARIO_LIVE_MIGRATION], + default=TRANSFER_SCENARIO_REPLICA, + help='The type of scenario to use when creating ' + 'the Transfer. "replica" will create a ' + 'monthly-billed Replica which can be ' + 'executed and deployed as many times as ' + 'desired, while "live_migration" will ' + 'create a Transfer which can be synced ' + 'as many times as needed but only ' + 'deployed once.') + parser.add_argument( + '--notes', + dest='notes', + help='Notes about the transfer') + parser.add_argument( + '--user-script-global', + action='append', + required=False, + dest="global_scripts", + type=cli_utils.comma_separated_kv_to_dict, + 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. ' + 'Supported phases: osmorphing_post_os_mount (default), ' + 'osmorphing_pre_os_mount.') + parser.add_argument( + '--user-script-instance', action='append', + required=False, + dest="instance_scripts", + type=cli_utils.comma_separated_kv_to_dict, + 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. ' + 'Supported phases: osmorphing_post_os_mount (default), ' + 'osmorphing_pre_os_mount.') cli_utils.add_args_for_json_option_to_parser( parser, 'destination-environment') @@ -351,27 +373,42 @@ class UpdateTransfer(show.ShowOne): def get_parser(self, prog_name): parser = super(UpdateTransfer, self).get_parser(prog_name) parser.add_argument('id', help='The transfer\'s id') - parser.add_argument('--notes', dest='notes', - help='Notes about the transfer.') - parser.add_argument('--user-script-global', action='append', - required=False, - dest="global_scripts", - type=cli_utils.comma_separated_kv_to_dict, - 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", - type=cli_utils.comma_separated_kv_to_dict, - 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( + '--notes', + dest='notes', + help='Notes about the transfer.') + parser.add_argument( + '--user-script-global', + action='append', + required=False, + dest="global_scripts", + type=cli_utils.comma_separated_kv_to_dict, + 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. ' + 'Omit the path to unregister the script, e.g. "linux=".' + 'Can optionally include a script phase: ' + 'windows=/path/to/script.ps1,phase=osmorphing_pre_os_mount. ' + 'Supported phases: osmorphing_post_os_mount (default), ' + 'osmorphing_pre_os_mount.') + parser.add_argument( + '--user-script-instance', action='append', + required=False, + dest="instance_scripts", + type=cli_utils.comma_separated_kv_to_dict, + 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.' + 'Omit the path to unregister the script, e.g. "linux=".' + 'Can optionally include a script phase: ' + 'instance_name=/path/to/script.ps1,' + 'phase=osmorphing_pre_os_mount. ' + 'Supported phases: osmorphing_post_os_mount (default), ' + 'osmorphing_pre_os_mount.') cli_utils.add_args_for_json_option_to_parser( parser, 'destination-environment') diff --git a/coriolisclient/cli/utils.py b/coriolisclient/cli/utils.py index d078fdc..7ea37b0 100644 --- a/coriolisclient/cli/utils.py +++ b/coriolisclient/cli/utils.py @@ -246,8 +246,7 @@ def compose_user_scripts( "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.") + raise ValueError("Too many parameters.") os_type = list(params.keys())[0] script_path = params[os_type] if os_type not in constants.OS_LIST: