Alow specifying the os-morphing user script phase

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

Author: petrutlucian94

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,

coriolisclient/cli/utils.py

@@ -191,42 +191,121 @@ 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 <key>=<value> pair: %s" % kv_pair)
+        out[key] = value
+    return out
+
+
 def compose_user_scripts(global_scripts, instance_scripts):
     ret = {
         "global": {},
         "instances": {}
     }
     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 "
+                "<os_type>=<script_path>. Can optionally include a comma "
+                "separated phase parameter, "
+                "e.g. <os_type>=<script_path>,phase=<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 "
+                "<instance>=<script_path>. Can optionally include a comma "
+                "separated phase parameter, "
+                "e.g. <instance>=<script_path>,phase=<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
 
 

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,
+]

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
         )