Re-organize environment variables on node prep

- Resolve #252

CHANGELOG.md

@@ -4,8 +4,12 @@
 
 ### Added
 - Sample Windows container recipes
-- Added environment variables to pool configuration, which allows users to
-setup Batch environment variables for the start task.
+- Support environment variables on additional node prep commands
+
+### Changed
+- **Breaking Change:** the `additional_node_prep_commands` property has
+been migrated under the new `additional_node_prep` property as
+`commands`.
 
 ### Fixed
 - Some commands were incorrectly failing due to nodeid conflicts with

config_templates/pool.yaml

@@ -109,12 +109,16 @@ pool_specification:
       - node_prep
       - remote_user
       - task
+  additional_node_prep:
+    commands:
+      pre: []
+      post: []
+    environment_variables:
+      abc: xyz
+    environment_variables_keyvault_secret_id: https://myvault.vault.azure.net/secrets/nodeprepenv
   gpu:
     nvidia_driver:
       source: https://some.url
-  additional_node_prep_commands:
-    pre: []
-    post: []
   prometheus:
     node_exporter:
       enabled: false
@@ -129,6 +133,3 @@ pool_specification:
       - kata_containers
       - singularity
     default: null
-  environment_variables:
-    abc: 'xyz'
-  environment_variables_keyvault_secret_id: https://myvault.vault.azure.net/secrets/mypoolenv

convoy/fleet.py

@@ -1358,7 +1358,7 @@ def _construct_pool_object(
     # get container registries
     docker_registries = settings.docker_registries(config)
     # set additional start task commands (pre version)
-    start_task = pool_settings.additional_node_prep_commands_pre
+    start_task = pool_settings.additional_node_prep.commands_pre
     # set vm configuration
     if native:
         if util.is_not_empty(custom_image_na):
@@ -1498,7 +1498,7 @@ def _construct_pool_object(
         start_task.append(addlcmds)
     del addlcmds
     # add additional start task commands (post version)
-    start_task.extend(pool_settings.additional_node_prep_commands_post)
+    start_task.extend(pool_settings.additional_node_prep.commands_post)
     # create pool param
     pool = batchmodels.PoolAddParameter(
         id=pool_settings.id,
@@ -1681,16 +1681,19 @@ def _construct_pool_object(
         )
     # add custom env vars to the batch start task
     if util.is_not_empty(
-            pool_settings.environment_variables_keyvault_secret_id):
+            pool_settings.additional_node_prep.
+            environment_variables_keyvault_secret_id):
         _check_keyvault_client(keyvault_client)
         env_vars = keyvault.get_secret(
             keyvault_client,
-            pool_settings.environment_variables_keyvault_secret_id,
+            pool_settings.additional_node_prep.
+            environment_variables_keyvault_secret_id,
             value_is_json=True)
         env_vars = util.merge_dict(
-            pool_settings.environment_variables, env_vars or {})
+            pool_settings.additional_node_prep.environment_variables,
+            env_vars or {})
     else:
-        env_vars = pool_settings.environment_variables
+        env_vars = pool_settings.additional_node_prep.environment_variables
     if util.is_not_empty(env_vars):
         for key in env_vars:
             pool.start_task.environment_settings.append(

convoy/settings.py

@@ -175,20 +175,26 @@
         'ca_options',
     ]
 )
+AdditionalNodePrepSettings = collections.namedtuple(
+    'AdditionalNodePrepSettings', [
+        'commands_pre',
+        'commands_post',
+        'environment_variables',
+        'environment_variables_keyvault_secret_id',
+    ]
+)
 PoolSettings = collections.namedtuple(
     'PoolSettings', [
         'id', 'vm_size', 'vm_count', 'resize_timeout', 'max_tasks_per_node',
         'inter_node_communication_enabled', 'vm_configuration',
         'reboot_on_start_task_failed', 'attempt_recovery_on_unusable',
         'block_until_all_global_resources_loaded',
         'transfer_files_on_pool_creation', 'input_data', 'resource_files',
-        'gpu_driver', 'ssh', 'rdp', 'additional_node_prep_commands_pre',
-        'additional_node_prep_commands_post', 'virtual_network',
+        'gpu_driver', 'ssh', 'rdp', 'additional_node_prep', 'virtual_network',
         'autoscale', 'node_fill_type', 'remote_access_control',
         'certificates', 'prometheus', 'upload_diagnostics_logs_on_unusable',
         'container_runtimes_install', 'container_runtimes_default',
-        'per_job_auto_scratch', 'environment_variables',
-        'environment_variables_keyvault_secret_id',
+        'per_job_auto_scratch',
     ]
 )
 SSHSettings = collections.namedtuple(
@@ -1246,12 +1252,19 @@ def pool_settings(config):
     except KeyError:
         gpu_driver = None
     # additional node prep
-    addl_node_prep = _kv_read_checked(
-        conf, 'additional_node_prep_commands', default={})
-    additional_node_prep_commands_pre = _kv_read_checked(
-        addl_node_prep, 'pre', default=[])
-    additional_node_prep_commands_post = _kv_read_checked(
-        addl_node_prep, 'post', default=[])
+    addl_np = _kv_read_checked(
+        conf, 'additional_node_prep', default={})
+    addl_np_cmds = _kv_read_checked(addl_np, 'commands', default={})
+    additional_node_prep = AdditionalNodePrepSettings(
+        commands_pre=_kv_read_checked(addl_np_cmds, 'pre', default=[]),
+        commands_post=_kv_read_checked(addl_np_cmds, 'post', default=[]),
+        environment_variables=_kv_read_checked(
+            addl_np, 'environment_variables', default={}),
+        environment_variables_keyvault_secret_id=_kv_read_checked(
+            addl_np, 'environment_variables_keyvault_secret_id'),
+    )
+    del addl_np_cmds
+    del addl_np
     # certificates
     certdict = _kv_read_checked(conf, 'certificates', default={})
     certs = []
@@ -1311,8 +1324,7 @@ def pool_settings(config):
             password=rdp_password,
         ),
         gpu_driver=gpu_driver,
-        additional_node_prep_commands_pre=additional_node_prep_commands_pre,
-        additional_node_prep_commands_post=additional_node_prep_commands_post,
+        additional_node_prep=additional_node_prep,
         virtual_network=virtual_network_settings(
             conf,
             default_existing_ok=True,
@@ -1329,10 +1341,6 @@ def pool_settings(config):
         container_runtimes_default=cr_default,
         per_job_auto_scratch=_kv_read(
             conf, 'per_job_auto_scratch', default=False),
-        environment_variables=_kv_read_checked(
-            conf, 'environment_variables', default={}),
-        environment_variables_keyvault_secret_id=_kv_read_checked(
-            conf, 'environment_variables_keyvault_secret_id'),
     )
 
 

docs/13-batch-shipyard-configuration-pool.md

@@ -117,12 +117,16 @@ pool_specification:
       - task
       - start_task
       - remote_user
+  additional_node_prep:
+    commands:
+      pre: []
+      post: []
+    environment_variables:
+      abc: xyz
+    environment_variables_keyvault_secret_id: https://myvault.vault.azure.net/secrets/nodeprepenv
   gpu:
     nvidia_driver:
       source: https://some.url
-  additional_node_prep_commands:
-    pre: []
-    post: []
   prometheus:
     node_exporter:
       enabled: false
@@ -137,9 +141,6 @@ pool_specification:
       - kata_containers
       - singularity
     default: null
-  environment_variables:
-    abc: 'xyz'
-  environment_variables_keyvault_secret_id: https://myvault.vault.azure.net/secrets/mypoolenv
 ```
 
 The `pool_specification` property has the following members:
@@ -521,20 +522,30 @@ account and are only applied to new pool allocations.
         * (required) `visibility` is a list of visibility settings to apply
           to the certificate. Valid values are `node_prep`, `remote_user`,
           and `task`.
+* (optional) `additional_node_prep` defines any additional node preparation
+commands to execute on node start.
+    * (optional) `commands` are the commands to execute
+        * (optional) `pre` is an array of additional commands to execute
+          on the compute node host as part of node preparation which occur
+          prior to the Batch Shipyard node preparation steps. This is
+          particularly useful for preparing platform images with software
+          for custom Linux mounts.
+        * (optional) `post` is an array of additional commands to execute
+          on the compute node host as part of node preparation which occur
+          after the Batch Shipyard node preparation steps.
+    * (optional) `environment_variables` that are set on the Azure Batch
+      start task. Note that environment variables are not expanded and
+      are passed as-is.
+    * (optional) `environment_variables_keyvault_secret_id` are any
+      additional environment variables that should be applied to the start
+      task but are stored in KeyVault. The secret stored in KeyVault must be
+      a valid YAML/JSON string, e.g., `{ "env_var_name": "env_var_value" }`.
 * (optional) `gpu` property defines additional information for NVIDIA
 GPU-enabled VMs. If not specified, Batch Shipyard will automatically download
 the driver for the `vm_size` specified.
     * `nvidia_driver` property contains the following required members:
         * `source` is the source url to download the driver. This should be
           the silent-installable driver package.
-* (optional) `additional_node_prep_commands` contains the following members:
-    * (optional) `pre` is an array of additional commands to execute on the
-      compute node host as part of node preparation which occur prior to
-      the Batch Shipyard node preparation steps. This is particularly useful
-      for preparing platform images with software for custom Linux mounts.
-    * (optional) `post` is an array of additional commands to execute on the
-      compute node host as part of node preparation which occur after the
-      Batch Shipyard node preparation steps.
 * (optional) `prometheus` properties are to control if collectors for metrics
 to export to [Prometheus](https://prometheus.io/) monitoring are enabled.
 Note that all exporters do not have their ports mapped (NAT) on the load
@@ -575,13 +586,6 @@ behavior on the pool compute nodes.
     * (optional) `default` is the default container runtime to use for
       running Docker containers. This option has no effect on `singularity`
       containers.
-* (optional) `environment_variables` that are set on the Azure Batch start
-task. Note that environment variables are not expanded and are passed
-as-is.
-* (optional) `environment_variables_keyvault_secret_id` are any additional
-environment variables that should be applied to the start task but are
-stored in KeyVault. The secret stored in KeyVault must be a valid YAML/JSON
-string, e.g., `{ "env_var_name": "env_var_value" }`.
 
 ## Full template
 A full template of a credentials file can be found

schemas/pool.yaml

@@ -319,15 +319,25 @@ mapping:
             mapping:
               source:
                 type: str
-      additional_node_prep_commands:
+      additional_node_prep:
         type: map
         mapping:
-          pre:
-            sequence:
-              - type: str
-          post:
-            sequence:
-              - type: str
+          commands:
+            type: map
+            mapping:
+              pre:
+                sequence:
+                  - type: str
+              post:
+                sequence:
+                  - type: str
+          environment_variables:
+            type: map
+            mapping:
+              regex;(.+):
+                type: text
+          environment_variables_keyvault_secret_id:
+            type: str
       prometheus:
         type: map
         mapping:
@@ -364,10 +374,3 @@ mapping:
           default:
             type: str
             enum: ['kata_containers', 'runc']
-      environment_variables:
-        type: map
-        mapping:
-          regex;(.+):
-            type: text
-      environment_variables_keyvault_secret_id:
-        type: str