Move deploymentName from the Service config to the Environment config

Needed so that we can support multiple Environment deployments under the same Service. Closes microsoft#610
bpkroth · Nov 30, 2023 · ae45996 · ae45996
1 parent bab3ca5
commit ae45996
Show file tree

Hide file tree

Showing 20 changed files with 57 additions and 29 deletions.
diff --git a/mlos_bench/mlos_bench/config/schemas/services/remote/azure/azure-service-subschema.json b/mlos_bench/mlos_bench/config/schemas/services/remote/azure/azure-service-subschema.json
@@ -20,11 +20,6 @@
                             "description": "The name of the resource group to place the deployment in (typically provided in the global config in order to omit from source control).",
                             "type": "string"
                         },
-                        "deploymentName": {
-                            "$comment": "TODO: provide templating strings to augment names via experiment_id/trial_id, for instance.",
-                            "description": "Name of the deployment to create for the experiment resources.",
-                            "type": "string"
-                        },
                         "deploymentTemplatePath": {
                             "description": "Path to an ARM template file.",
                             "type": "string",
@@ -56,7 +51,6 @@
                         }
                     },
                     "required": [
-                        "deploymentName",
                         "deploymentTemplatePath"
                     ]
                 }

diff --git a/mlos_bench/mlos_bench/config/services/remote/azure/service-linux-vm-ops.jsonc b/mlos_bench/mlos_bench/config/services/remote/azure/service-linux-vm-ops.jsonc
@@ -10,7 +10,6 @@
                 // can be overridden by the parameters pushed from the caller Environment.
                 "subscription": "PLACEHOLDER; AZURE SUBSCRIPTION ID",
                 "resourceGroup": "PLACEHOLDER; e.g., os-autotune",
-                "deploymentName": "PLACEHOLDER; e.g., mlos-2vms",
 
                 "deploymentTemplatePath": "services/remote/azure/arm-templates/azuredeploy-ubuntu-vm.jsonc",
 

diff --git a/mlos_bench/mlos_bench/services/remote/azure/azure_network_services.py b/mlos_bench/mlos_bench/services/remote/azure/azure_network_services.py
@@ -68,6 +68,15 @@ def __init__(self,
             ])
         )
 
+    def _set_default_params(self, params: dict) -> dict:    # pylint: disable=no-self-use
+        # Try and provide a semi sane default for the deploymentName if not provided
+        # since this is a common way to set the deploymentName and can same some
+        # config work for the caller.
+        if 'vnetName' not in params:
+            raise ValueError("Required param 'vnetName' is missing.")
+        params.setdefault(f"{params['vnetName']}-deployment")
+        return params
+
     def wait_network_deployment(self, params: dict, *, is_setup: bool) -> Tuple[Status, dict]:
         """
         Waits for a pending operation on an Azure VM to resolve to SUCCEEDED or FAILED.
@@ -127,6 +136,7 @@ def deprovision_network(self, params: dict, ignore_errors: bool = True) -> Tuple
             A pair of Status and result. The result is always {}.
             Status is one of {PENDING, SUCCEEDED, FAILED}
         """
+        params = self._set_default_params(params)
         config = merge_parameters(
             dest=self.config.copy(),
             source=params,

diff --git a/mlos_bench/mlos_bench/services/remote/azure/azure_services.py b/mlos_bench/mlos_bench/services/remote/azure/azure_services.py
@@ -73,7 +73,6 @@ def __init__(self,
         check_required_params(self.config, [
             "subscription",
             "resourceGroup",
-            "deploymentName",
             "deploymentTemplatePath",
             "deploymentTemplateParameters",
         ])
@@ -102,6 +101,23 @@ def deploy_params(self) -> dict:
         """
         return self._deploy_params
 
+    @abc.abstractmethod
+    def _set_default_params(self, params: dict) -> dict:
+        """
+        Optionally set some default parameters for the request.
+
+        Parameters
+        ----------
+        params : dict
+            The parameters.
+
+        Returns
+        -------
+        dict
+            The updated parameters.
+        """
+        raise NotImplementedError("Should be overridden by subclass.")
+
     def _get_session(self, params: dict) -> requests.Session:
         """
         Get a session object that includes automatic retries and headers for REST API calls.
@@ -250,7 +266,8 @@ def _wait_deployment(self, params: dict, *, is_setup: bool) -> Tuple[Status, dic
             Status is one of {PENDING, SUCCEEDED, FAILED, TIMED_OUT}
             Result is info on the operation runtime if SUCCEEDED, otherwise {}.
         """
-        _LOG.info("Wait for %s to %s", params.get("deploymentName") or self.config.get("deploymentName"),
+        params = self._set_default_params(params)
+        _LOG.info("Wait for %s to %s", params.get("deploymentName"),
                   "provision" if is_setup else "deprovision")
         return self._wait_while(self._check_deployment, Status.PENDING, params)
 
@@ -268,12 +285,14 @@ def _wait_while(self, func: Callable[[dict], Tuple[Status, dict]],
             Steady state status - keep polling `func` while it returns `loop_status`.
         params : dict
             Flat dictionary of (key, value) pairs of tunable parameters.
+            Requires deploymentName.
 
         Returns
         -------
         result : (Status, dict)
             A pair of Status and result.
         """
+        params = self._set_default_params(params)
         config = merge_parameters(
             dest=self.config.copy(), source=params, required_keys=["deploymentName"])
 
@@ -322,6 +341,7 @@ def _check_deployment(self, params: dict) -> Tuple[Status, dict]:   # pylint: di
             A pair of Status and result. The result is always {}.
             Status is one of {SUCCEEDED, PENDING, FAILED}
         """
+        params = self._set_default_params(params)
         config = merge_parameters(
             dest=self.config.copy(),
             source=params,
@@ -387,7 +407,8 @@ def _provision_resource(self, params: dict) -> Tuple[Status, dict]:
             parameters extracted from the response JSON, or {} if the status is FAILED.
             Status is one of {PENDING, SUCCEEDED, FAILED}
         """
-        config = merge_parameters(dest=self.config.copy(), source=params)
+        params = self._set_default_params(params)
+        config = merge_parameters(dest=self.config.copy(), source=params, required_keys=["deploymentName"])
         _LOG.info("Deploy: %s :: %s", config["deploymentName"], params)
 
         params = merge_parameters(dest=self._deploy_params.copy(), source=params)

diff --git a/mlos_bench/mlos_bench/services/remote/azure/azure_vm_services.py b/mlos_bench/mlos_bench/services/remote/azure/azure_vm_services.py
@@ -162,6 +162,15 @@ def __init__(self,
             with open(self._custom_data_file, 'r', encoding='utf-8') as custom_data_fh:
                 self._deploy_params["customData"] = custom_data_fh.read()
 
+    def _set_default_params(self, params: dict) -> dict:    # pylint: disable=no-self-use
+        # Try and provide a semi sane default for the deploymentName if not provided
+        # since this is a common way to set the deploymentName and can same some
+        # config work for the caller.
+        if 'vmName' not in params:
+            raise ValueError("Required param 'vmName' is missing.")
+        params.setdefault(f"{params['vmName']}-deployment")
+        return params
+
     def wait_host_deployment(self, params: dict, *, is_setup: bool) -> Tuple[Status, dict]:
         """
         Waits for a pending operation on an Azure VM to resolve to SUCCEEDED or FAILED.
@@ -203,6 +212,8 @@ def wait_host_operation(self, params: dict) -> Tuple[Status, dict]:
             Result is info on the operation runtime if SUCCEEDED, otherwise {}.
         """
         _LOG.info("Wait for operation on VM %s", params["vmName"])
+        # Try and provide a semi sane default for the deploymentName
+        params.setdefault(f"{params['vmName']}-deployment")
         return self._wait_while(self._check_operation_status, Status.RUNNING, params)
 
     def wait_os_operation(self, params: dict) -> Tuple["Status", dict]:
@@ -243,6 +254,7 @@ def deprovision_host(self, params: dict) -> Tuple[Status, dict]:
             A pair of Status and result. The result is always {}.
             Status is one of {PENDING, SUCCEEDED, FAILED}
         """
+        params = self._set_default_params(params)
         config = merge_parameters(
             dest=self.config.copy(),
             source=params,
@@ -280,6 +292,7 @@ def deallocate_host(self, params: dict) -> Tuple[Status, dict]:
             A pair of Status and result. The result is always {}.
             Status is one of {PENDING, SUCCEEDED, FAILED}
         """
+        params = self._set_default_params(params)
         config = merge_parameters(
             dest=self.config.copy(),
             source=params,
@@ -311,6 +324,7 @@ def start_host(self, params: dict) -> Tuple[Status, dict]:
             A pair of Status and result. The result is always {}.
             Status is one of {PENDING, SUCCEEDED, FAILED}
         """
+        params = self._set_default_params(params)
         config = merge_parameters(
             dest=self.config.copy(),
             source=params,
@@ -344,6 +358,7 @@ def stop_host(self, params: dict, force: bool = False) -> Tuple[Status, dict]:
             A pair of Status and result. The result is always {}.
             Status is one of {PENDING, SUCCEEDED, FAILED}
         """
+        params = self._set_default_params(params)
         config = merge_parameters(
             dest=self.config.copy(),
             source=params,
@@ -380,6 +395,7 @@ def restart_host(self, params: dict, force: bool = False) -> Tuple[Status, dict]
             A pair of Status and result. The result is always {}.
             Status is one of {PENDING, SUCCEEDED, FAILED}
         """
+        params = self._set_default_params(params)
         config = merge_parameters(
             dest=self.config.copy(),
             source=params,
@@ -422,6 +438,7 @@ def remote_exec(self, script: Iterable[str], config: dict,
             A pair of Status and result.
             Status is one of {PENDING, SUCCEEDED, FAILED}
         """
+        config = self._set_default_params(config)
         config = merge_parameters(
             dest=self.config.copy(),
             source=config,

diff --git a/...config/schemas/services/test-cases/bad/invalid/azure_network_service-config-missing.jsonc b/...config/schemas/services/test-cases/bad/invalid/azure_network_service-config-missing.jsonc
@@ -1,9 +1,8 @@
 {
     "class": "mlos_bench.services.remote.azure.AzureNetworkService",
     "config": {
-        "deploymentTemplatePath": "some/path/to/deployment/template.jsonc",
+        //"deploymentTemplatePath": "some/path/to/deployment/template.jsonc", // <-- missing
         "subscription": "subscription-id",
         "resourceGroup": "rg"
-        //"deploymentName": "deployment"    // <-- missing
     }
 }
diff --git a/...ces/test-cases/bad/invalid/azure_vm_service-config-arm-params-bad-type-array-object.jsonc b/...ces/test-cases/bad/invalid/azure_vm_service-config-arm-params-bad-type-array-object.jsonc
@@ -5,7 +5,6 @@
         "subscription": "subscription-id",
 
         "resourceGroup": "rg",
-        "deploymentName": "deployment",
         "deploymentTemplateParameters": {
             "array": [ { "invalid": "type"} ]
         }

diff --git a/...s/services/test-cases/bad/invalid/azure_vm_service-config-arm-params-bad-type-array.jsonc b/...s/services/test-cases/bad/invalid/azure_vm_service-config-arm-params-bad-type-array.jsonc
@@ -5,7 +5,6 @@
         "subscription": "subscription-id",
 
         "resourceGroup": "rg",
-        "deploymentName": "deployment",
         "deploymentTemplateParameters": {
             "array": [ ["nested", "array", "invalid"] ]
         }

diff --git a/.../services/test-cases/bad/invalid/azure_vm_service-config-arm-params-bad-type-object.jsonc b/.../services/test-cases/bad/invalid/azure_vm_service-config-arm-params-bad-type-object.jsonc
@@ -5,7 +5,6 @@
         "subscription": "subscription-id",
 
         "resourceGroup": "rg",
-        "deploymentName": "deployment",
         "deploymentTemplateParameters": {
             "object": { "invalid": "type" }
         }

diff --git a/...ig/schemas/services/test-cases/bad/invalid/azure_vm_service-config-empty-arm-params.jsonc b/...ig/schemas/services/test-cases/bad/invalid/azure_vm_service-config-empty-arm-params.jsonc
@@ -3,7 +3,6 @@
     "config": {
         "subscription": "subscription-id",
         "resourceGroup": "rg",
-        "deploymentName": "deployment",
         "deploymentTemplatePath": "some/path/to/deployment/template.jsonc",
         "deploymentTemplateParameters": {
             // "param": "value" // <-- need at least one parameter

diff --git a/...ests/config/schemas/services/test-cases/bad/invalid/azure_vm_service-config-missing.jsonc b/...ests/config/schemas/services/test-cases/bad/invalid/azure_vm_service-config-missing.jsonc
@@ -1,9 +1,8 @@
 {
     "class": "mlos_bench.services.remote.azure.AzureVMService",
     "config": {
-        "deploymentTemplatePath": "some/path/to/deployment/template.jsonc",
+        // "deploymentTemplatePath": "some/path/to/deployment/template.jsonc", // <-- missing
         "subscription": "subscription-id",
         "resourceGroup": "rg"
-        //"deploymentName": "deployment"    // <-- missing
     }
 }
diff --git a/...onfig/schemas/services/test-cases/bad/unhandled/azure_network_service-config-extras.jsonc b/...onfig/schemas/services/test-cases/bad/unhandled/azure_network_service-config-extras.jsonc
@@ -4,7 +4,6 @@
         "deploymentTemplatePath": "some/path/to/deployment/template.jsonc",
         "subscription": "subscription-id",
         "resourceGroup": "rg",
-        "deploymentName": "deployment",
         "vmName": "test-vm",
         "extra": "invalid"
     }

diff --git a/...sts/config/schemas/services/test-cases/bad/unhandled/azure_vm_service-config-extras.jsonc b/...sts/config/schemas/services/test-cases/bad/unhandled/azure_vm_service-config-extras.jsonc
@@ -4,7 +4,6 @@
         "deploymentTemplatePath": "some/path/to/deployment/template.jsonc",
         "subscription": "subscription-id",
         "resourceGroup": "rg",
-        "deploymentName": "deployment",
         "vmName": "test-vm",
         "extra": "invalid"
     }

diff --git a/...bench/tests/config/schemas/services/test-cases/good/full/azure_network_service-full.jsonc b/...bench/tests/config/schemas/services/test-cases/good/full/azure_network_service-full.jsonc
@@ -7,7 +7,6 @@
         "deploymentTemplatePath": "some/path/to/deployment/template.jsonc",
         "subscription": "subscription-id",
         "resourceGroup": "rg",
-        "deploymentName": "${experiment_id}-vnet-deployment",
 
         "deploymentTemplateParameters": {
             "location": "westus2",

diff --git a/...mlos_bench/tests/config/schemas/services/test-cases/good/full/azure_vm_service-full.jsonc b/...mlos_bench/tests/config/schemas/services/test-cases/good/full/azure_vm_service-full.jsonc
@@ -7,7 +7,6 @@
         "deploymentTemplatePath": "some/path/to/deployment/template.jsonc",
         "subscription": "subscription-id",
         "resourceGroup": "rg",
-        "deploymentName": "deployment",
 
         "customDataFile": "some/path/to/custom/data.yml",
         "deploymentTemplateParameters": {

diff --git a/...tests/config/schemas/services/test-cases/good/partial/azure_network_service-partial.jsonc b/...tests/config/schemas/services/test-cases/good/partial/azure_network_service-partial.jsonc
@@ -3,7 +3,6 @@
     "config": {
         "subscription": "subscription-id",
         "resourceGroup": "rg",
-        "deploymentName": "deployment",
         "deploymentTemplatePath": "some/path/to/deployment/template.jsonc",
         "deploymentTemplateParameters": {
             "param": "value"

diff --git a/...fig/schemas/services/test-cases/good/partial/azure_vm_service-partial-no-arm-params.jsonc b/...fig/schemas/services/test-cases/good/partial/azure_vm_service-partial-no-arm-params.jsonc
@@ -1,7 +1,6 @@
 {
     "class": "mlos_bench.services.remote.azure.AzureVMService",
     "config": {
-        "deploymentName": "deployment",
         "deploymentTemplatePath": "some/path/to/deployment/template.jsonc"
     }
 }
diff --git a/...ench/tests/config/schemas/services/test-cases/good/partial/azure_vm_service-partial.jsonc b/...ench/tests/config/schemas/services/test-cases/good/partial/azure_vm_service-partial.jsonc
@@ -3,7 +3,6 @@
     "config": {
         "subscription": "subscription-id",
         "resourceGroup": "rg",
-        "deploymentName": "deployment",
         "deploymentTemplatePath": "some/path/to/deployment/template.jsonc",
         "deploymentTemplateParameters": {
             "param": "value"

diff --git a/mlos_bench/mlos_bench/tests/services/remote/azure/azure_vm_services_test.py b/mlos_bench/mlos_bench/tests/services/remote/azure/azure_vm_services_test.py
@@ -62,7 +62,6 @@ def test_azure_vm_service_recursive_template_params(azure_auth_service: AzureAut
     """
     config = {
         "deploymentTemplatePath": "services/remote/azure/arm-templates/azuredeploy-ubuntu-vm.jsonc",
-        "deploymentName": "TEST_DEPLOYMENT1",
         "subscription": "TEST_SUB1",
         "resourceGroup": "TEST_RG1",
         "deploymentTemplateParameters": {
@@ -72,6 +71,7 @@ def test_azure_vm_service_recursive_template_params(azure_auth_service: AzureAut
         },
     }
     global_config = {
+        "deploymentName": "TEST_DEPLOYMENT1",
         "vmName": "test-vm",
         "location": "eastus",
     }
@@ -88,14 +88,14 @@ def test_azure_vm_service_custom_data(azure_auth_service: AzureAuthService) -> N
     config = {
         "customDataFile": "services/remote/azure/cloud-init/alt-ssh.yml",
         "deploymentTemplatePath": "services/remote/azure/arm-templates/azuredeploy-ubuntu-vm.jsonc",
-        "deploymentName": "TEST_DEPLOYMENT1",
         "subscription": "TEST_SUB1",
         "resourceGroup": "TEST_RG1",
         "deploymentTemplateParameters": {
             "location": "eastus2",
         },
     }
     global_config = {
+        "deploymentName": "TEST_DEPLOYMENT1",
         "vmName": "test-vm",
     }
     with pytest.raises(ValueError):

diff --git a/mlos_bench/mlos_bench/tests/services/remote/azure/conftest.py b/mlos_bench/mlos_bench/tests/services/remote/azure/conftest.py
@@ -47,7 +47,6 @@ def azure_network_service(azure_auth_service: AzureAuthService) -> AzureNetworkS
     """
     return AzureNetworkService(config={
         "deploymentTemplatePath": "services/remote/azure/arm-templates/azuredeploy-ubuntu-vm.jsonc",
-        "deploymentName": "TEST_DEPLOYMENT-VNET",
         "subscription": "TEST_SUB",
         "resourceGroup": "TEST_RG",
         "deploymentTemplateParameters": {
@@ -56,6 +55,7 @@ def azure_network_service(azure_auth_service: AzureAuthService) -> AzureNetworkS
         "pollInterval": 1,
         "pollTimeout": 2
     }, global_config={
+        "deploymentName": "TEST_DEPLOYMENT-VNET",
         "vnetName": "test-vnet",  # Should come from the upper-level config
     }, parent=azure_auth_service)
 
@@ -67,7 +67,6 @@ def azure_vm_service(azure_auth_service: AzureAuthService) -> AzureVMService:
     """
     return AzureVMService(config={
         "deploymentTemplatePath": "services/remote/azure/arm-templates/azuredeploy-ubuntu-vm.jsonc",
-        "deploymentName": "TEST_DEPLOYMENT-VM",
         "subscription": "TEST_SUB",
         "resourceGroup": "TEST_RG",
         "deploymentTemplateParameters": {
@@ -76,6 +75,7 @@ def azure_vm_service(azure_auth_service: AzureAuthService) -> AzureVMService:
         "pollInterval": 1,
         "pollTimeout": 2
     }, global_config={
+        "deploymentName": "TEST_DEPLOYMENT-VM",
         "vmName": "test-vm",  # Should come from the upper-level config
     }, parent=azure_auth_service)