From 19b1c9686bbe15e4735a6830163f8814329e4635 Mon Sep 17 00:00:00 2001 From: Brian Kroth Date: Mon, 12 Feb 2024 22:37:12 +0000 Subject: [PATCH 01/11] Allow use of Azure VM Service for only remote exec. i.e., don't require deploymentTemplatePath Additionally, rename some things to make it more clear that the base class is just for deployment services. --- ...> azure-deployment-service-subschema.json} | 13 ++++----- .../azure-network-service-subschema.json | 2 +- .../azure/azure-vm-service-subschema.json | 4 +-- ...rvices.py => azure_deployment_services.py} | 29 ++++++++++--------- .../remote/azure/azure_network_services.py | 4 +-- .../remote/azure/azure_vm_services.py | 4 +-- ...ure_vm_service-partial-no-deployment.jsonc | 5 ++++ .../schemas/services/test_services_schemas.py | 4 +-- 8 files changed, 34 insertions(+), 31 deletions(-) rename mlos_bench/mlos_bench/config/schemas/services/remote/azure/{azure-service-subschema.json => azure-deployment-service-subschema.json} (89%) rename mlos_bench/mlos_bench/services/remote/azure/{azure_services.py => azure_deployment_services.py} (94%) create mode 100644 mlos_bench/mlos_bench/tests/config/schemas/services/test-cases/good/partial/azure_vm_service-partial-no-deployment.jsonc 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-deployment-service-subschema.json similarity index 89% rename from mlos_bench/mlos_bench/config/schemas/services/remote/azure/azure-service-subschema.json rename to mlos_bench/mlos_bench/config/schemas/services/remote/azure/azure-deployment-service-subschema.json index bb28b2d1027..2b613c55b7c 100644 --- 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-deployment-service-subschema.json @@ -1,10 +1,10 @@ { "$schema": "https://json-schema.org/draft/2020-12/schema", - "$id": "https://raw.githubusercontent.com/microsoft/MLOS/main/mlos_bench/mlos_bench/config/schemas/services/remote/azure/azure-service-subschema.json", - "title": "mlos_bench Azure Base Service config", - "description": "Base config schema for an mlos_bench Azure Service", + "$id": "https://raw.githubusercontent.com/microsoft/MLOS/main/mlos_bench/mlos_bench/config/schemas/services/remote/azure/azure-deployment-service-subschema.json", + "title": "mlos_bench Azure Base Deployment Service config", + "description": "Base config schema for an mlos_bench Azure Deployment Services", "$defs": { - "azure_service_config": { + "azure_deployment_service_config": { "type": "object", "allOf": [ { @@ -49,10 +49,7 @@ }, "minProperties": 1 } - }, - "required": [ - "deploymentTemplatePath" - ] + } } ] } diff --git a/mlos_bench/mlos_bench/config/schemas/services/remote/azure/azure-network-service-subschema.json b/mlos_bench/mlos_bench/config/schemas/services/remote/azure/azure-network-service-subschema.json index d3c0c8a088c..5b8a4b083c5 100644 --- a/mlos_bench/mlos_bench/config/schemas/services/remote/azure/azure-network-service-subschema.json +++ b/mlos_bench/mlos_bench/config/schemas/services/remote/azure/azure-network-service-subschema.json @@ -15,7 +15,7 @@ "type": "object", "allOf": [ { - "$ref": "./azure-service-subschema.json#/$defs/azure_service_config" + "$ref": "./azure-deployment-service-subschema.json#/$defs/azure_deployment_service_config" } ], "unevaluatedProperties": false diff --git a/mlos_bench/mlos_bench/config/schemas/services/remote/azure/azure-vm-service-subschema.json b/mlos_bench/mlos_bench/config/schemas/services/remote/azure/azure-vm-service-subschema.json index d821bd2a24f..38991fcd3af 100644 --- a/mlos_bench/mlos_bench/config/schemas/services/remote/azure/azure-vm-service-subschema.json +++ b/mlos_bench/mlos_bench/config/schemas/services/remote/azure/azure-vm-service-subschema.json @@ -15,7 +15,7 @@ "type": "object", "allOf": [ { - "$ref": "./azure-service-subschema.json#/$defs/azure_service_config" + "$ref": "./azure-deployment-service-subschema.json#/$defs/azure_deployment_service_config" }, { "properties": { @@ -34,5 +34,5 @@ "unevaluatedProperties": false } }, - "required": ["class", "config"] + "required": ["class"] } diff --git a/mlos_bench/mlos_bench/services/remote/azure/azure_services.py b/mlos_bench/mlos_bench/services/remote/azure/azure_deployment_services.py similarity index 94% rename from mlos_bench/mlos_bench/services/remote/azure/azure_services.py rename to mlos_bench/mlos_bench/services/remote/azure/azure_deployment_services.py index 115fc36ff76..7a1becc73ed 100644 --- a/mlos_bench/mlos_bench/services/remote/azure/azure_services.py +++ b/mlos_bench/mlos_bench/services/remote/azure/azure_deployment_services.py @@ -3,7 +3,7 @@ # Licensed under the MIT License. # """ -Base class for certain Azure Services classes. +Base class for certain Azure Services classes that do deployments. """ import abc @@ -25,9 +25,9 @@ _LOG = logging.getLogger(__name__) -class AzureService(Service, metaclass=abc.ABCMeta): +class AzureDeploymentService(Service, metaclass=abc.ABCMeta): """ - Helper methods to manage Azure resources via REST APIs. + Helper methods to manage and deploy Azure resources via REST APIs. """ _POLL_INTERVAL = 4 # seconds @@ -73,8 +73,6 @@ def __init__(self, check_required_params(self.config, [ "subscription", "resourceGroup", - "deploymentTemplatePath", - "deploymentTemplateParameters", ]) # These parameters can come from command line as strings, so conversion is needed. @@ -84,15 +82,18 @@ def __init__(self, self._total_retries = int(self.config.get("requestTotalRetries", self._REQUEST_TOTAL_RETRIES)) self._backoff_factor = float(self.config.get("requestBackoffFactor", self._REQUEST_RETRY_BACKOFF_FACTOR)) - # TODO: Provide external schema validation? - template = self.config_loader_service.load_config( - self.config['deploymentTemplatePath'], schema_type=None) - assert template is not None and isinstance(template, dict) - self._deploy_template = template - - # Allow for recursive variable expansion as we do with global params and const_args. - deploy_params = DictTemplater(self.config['deploymentTemplateParameters']).expand_vars(extra_source_dict=global_config) - self._deploy_params = merge_parameters(dest=deploy_params, source=global_config) + self._deploy_template = {} + self._deploy_params = {} + if self.config.get("deploymentTemplatePath") is not None: + # TODO: Provide external schema validation? + template = self.config_loader_service.load_config( + self.config['deploymentTemplatePath'], schema_type=None) + assert template is not None and isinstance(template, dict) + self._deploy_template = template + + # Allow for recursive variable expansion as we do with global params and const_args. + deploy_params = DictTemplater(self.config['deploymentTemplateParameters']).expand_vars(extra_source_dict=global_config) + self._deploy_params = merge_parameters(dest=deploy_params, source=global_config) @property def deploy_params(self) -> dict: 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 index 905b594ee58..ca61ff330cd 100644 --- a/mlos_bench/mlos_bench/services/remote/azure/azure_network_services.py +++ b/mlos_bench/mlos_bench/services/remote/azure/azure_network_services.py @@ -12,14 +12,14 @@ from mlos_bench.environments.status import Status from mlos_bench.services.base_service import Service -from mlos_bench.services.remote.azure.azure_services import AzureService +from mlos_bench.services.remote.azure.azure_deployment_services import AzureDeploymentService from mlos_bench.services.types.network_provisioner_type import SupportsNetworkProvisioning from mlos_bench.util import merge_parameters _LOG = logging.getLogger(__name__) -class AzureNetworkService(AzureService, SupportsNetworkProvisioning): +class AzureNetworkService(AzureDeploymentService, SupportsNetworkProvisioning): """ Helper methods to manage Virtual Networks on Azure. """ 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 index 123b1bc4934..7fdbdc18dfb 100644 --- a/mlos_bench/mlos_bench/services/remote/azure/azure_vm_services.py +++ b/mlos_bench/mlos_bench/services/remote/azure/azure_vm_services.py @@ -15,7 +15,7 @@ from mlos_bench.environments.status import Status from mlos_bench.services.base_service import Service -from mlos_bench.services.remote.azure.azure_services import AzureService +from mlos_bench.services.remote.azure.azure_deployment_services import AzureDeploymentService from mlos_bench.services.types.remote_exec_type import SupportsRemoteExec from mlos_bench.services.types.host_provisioner_type import SupportsHostProvisioning from mlos_bench.services.types.host_ops_type import SupportsHostOps @@ -25,7 +25,7 @@ _LOG = logging.getLogger(__name__) -class AzureVMService(AzureService, SupportsHostProvisioning, SupportsHostOps, SupportsOSOps, SupportsRemoteExec): +class AzureVMService(AzureDeploymentService, SupportsHostProvisioning, SupportsHostOps, SupportsOSOps, SupportsRemoteExec): """ Helper methods to manage VMs on Azure. """ diff --git a/mlos_bench/mlos_bench/tests/config/schemas/services/test-cases/good/partial/azure_vm_service-partial-no-deployment.jsonc b/mlos_bench/mlos_bench/tests/config/schemas/services/test-cases/good/partial/azure_vm_service-partial-no-deployment.jsonc new file mode 100644 index 00000000000..19feac88843 --- /dev/null +++ b/mlos_bench/mlos_bench/tests/config/schemas/services/test-cases/good/partial/azure_vm_service-partial-no-deployment.jsonc @@ -0,0 +1,5 @@ +{ + "class": "mlos_bench.services.remote.azure.AzureVMService" + // This version can only support remote exec when subscription, resourceGroup, + // and vmName are in the parameters for ther environment. +} diff --git a/mlos_bench/mlos_bench/tests/config/schemas/services/test_services_schemas.py b/mlos_bench/mlos_bench/tests/config/schemas/services/test_services_schemas.py index dbf48269f08..bde267eab45 100644 --- a/mlos_bench/mlos_bench/tests/config/schemas/services/test_services_schemas.py +++ b/mlos_bench/mlos_bench/tests/config/schemas/services/test_services_schemas.py @@ -17,7 +17,7 @@ from mlos_bench.services.base_service import Service from mlos_bench.services.config_persistence import ConfigPersistenceService from mlos_bench.services.local.temp_dir_context import TempDirContextService -from mlos_bench.services.remote.azure.azure_services import AzureService +from mlos_bench.services.remote.azure.azure_deployment_services import AzureDeploymentService from mlos_bench.services.remote.ssh.ssh_service import SshService from mlos_bench.tests import try_resolve_class_name @@ -39,7 +39,7 @@ NON_CONFIG_SERVICE_CLASSES = { ConfigPersistenceService, # configured thru the launcher cli args TempDirContextService, # ABCMeta abstract class, but no good way to test that dynamically in Python. - AzureService, # ABCMeta abstract base class + AzureDeploymentService, # ABCMeta abstract base class SshService, # ABCMeta abstract base class } From 27cd782d5e468ff07bc5794a167c6c5d49e969da Mon Sep 17 00:00:00 2001 From: Brian Kroth Date: Mon, 12 Feb 2024 22:40:21 +0000 Subject: [PATCH 02/11] log a message when that happens --- .../services/remote/azure/azure_deployment_services.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mlos_bench/mlos_bench/services/remote/azure/azure_deployment_services.py b/mlos_bench/mlos_bench/services/remote/azure/azure_deployment_services.py index 7a1becc73ed..01ed9fb5e51 100644 --- a/mlos_bench/mlos_bench/services/remote/azure/azure_deployment_services.py +++ b/mlos_bench/mlos_bench/services/remote/azure/azure_deployment_services.py @@ -94,6 +94,8 @@ def __init__(self, # Allow for recursive variable expansion as we do with global params and const_args. deploy_params = DictTemplater(self.config['deploymentTemplateParameters']).expand_vars(extra_source_dict=global_config) self._deploy_params = merge_parameters(dest=deploy_params, source=global_config) + else: + _LOG.info("No deploymentTemplatePath provided. Deployment services will be unavailable.") @property def deploy_params(self) -> dict: From 72cb15b644e0dfd3329860552ec80326769d2fb2 Mon Sep 17 00:00:00 2001 From: Brian Kroth Date: Thu, 22 Feb 2024 22:19:16 +0000 Subject: [PATCH 03/11] fixups --- .../services/remote/azure/azure_network_services_test.py | 2 +- .../tests/services/remote/azure/azure_vm_services_test.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/mlos_bench/mlos_bench/tests/services/remote/azure/azure_network_services_test.py b/mlos_bench/mlos_bench/tests/services/remote/azure/azure_network_services_test.py index b255117321b..be6d7da359d 100644 --- a/mlos_bench/mlos_bench/tests/services/remote/azure/azure_network_services_test.py +++ b/mlos_bench/mlos_bench/tests/services/remote/azure/azure_network_services_test.py @@ -66,7 +66,7 @@ def test_wait_network_deployment_retry(mock_getconn: MagicMock, (401, Status.SUCCEEDED), (404, Status.SUCCEEDED), ]) -@patch("mlos_bench.services.remote.azure.azure_services.requests") +@patch("mlos_bench.services.remote.azure.azure_deployment_services.requests") # pylint: disable=too-many-arguments def test_network_operation_status(mock_requests: MagicMock, azure_network_service: AzureNetworkService, 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 index a724529a2c9..e1e578b8597 100644 --- 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 @@ -123,7 +123,7 @@ def test_azure_vm_service_custom_data(azure_auth_service: AzureAuthService) -> N (401, Status.FAILED), (404, Status.FAILED), ]) -@patch("mlos_bench.services.remote.azure.azure_services.requests") +@patch("mlos_bench.services.remote.azure.azure_deployment_services.requests") # pylint: disable=too-many-arguments def test_vm_operation_status(mock_requests: MagicMock, azure_vm_service: AzureVMService, @@ -146,8 +146,8 @@ def test_vm_operation_status(mock_requests: MagicMock, assert status == operation_status -@patch("mlos_bench.services.remote.azure.azure_services.time.sleep") -@patch("mlos_bench.services.remote.azure.azure_services.requests.Session") +@patch("mlos_bench.services.remote.azure.azure_deployment_services.time.sleep") +@patch("mlos_bench.services.remote.azure.azure_deployment_services.requests.Session") def test_wait_vm_operation_ready(mock_session: MagicMock, mock_sleep: MagicMock, azure_vm_service: AzureVMService) -> None: """ @@ -175,7 +175,7 @@ def test_wait_vm_operation_ready(mock_session: MagicMock, mock_sleep: MagicMock, assert status.is_succeeded() -@patch("mlos_bench.services.remote.azure.azure_services.requests.Session") +@patch("mlos_bench.services.remote.azure.azure_deployment_services.requests.Session") def test_wait_vm_operation_timeout(mock_session: MagicMock, azure_vm_service: AzureVMService) -> None: """ From d73b0c923a35cb6e0fd897bebcfe2ff50953c82c Mon Sep 17 00:00:00 2001 From: Brian Kroth Date: Thu, 22 Feb 2024 22:28:09 +0000 Subject: [PATCH 04/11] test fixups --- .../remote/azure/azure-network-service-subschema.json | 4 ++++ .../bad/invalid/azure_vm_service-config-missing.jsonc | 8 -------- 2 files changed, 4 insertions(+), 8 deletions(-) delete mode 100644 mlos_bench/mlos_bench/tests/config/schemas/services/test-cases/bad/invalid/azure_vm_service-config-missing.jsonc diff --git a/mlos_bench/mlos_bench/config/schemas/services/remote/azure/azure-network-service-subschema.json b/mlos_bench/mlos_bench/config/schemas/services/remote/azure/azure-network-service-subschema.json index 5b8a4b083c5..8aea02d2d1e 100644 --- a/mlos_bench/mlos_bench/config/schemas/services/remote/azure/azure-network-service-subschema.json +++ b/mlos_bench/mlos_bench/config/schemas/services/remote/azure/azure-network-service-subschema.json @@ -16,6 +16,10 @@ "allOf": [ { "$ref": "./azure-deployment-service-subschema.json#/$defs/azure_deployment_service_config" + }, + { + "$comment": "Unlike VM service, which can still do remote exec, network service needs a deployment template.", + "required": ["deploymentTemplatePath"] } ], "unevaluatedProperties": false diff --git a/mlos_bench/mlos_bench/tests/config/schemas/services/test-cases/bad/invalid/azure_vm_service-config-missing.jsonc b/mlos_bench/mlos_bench/tests/config/schemas/services/test-cases/bad/invalid/azure_vm_service-config-missing.jsonc deleted file mode 100644 index bbf1ed8b512..00000000000 --- a/mlos_bench/mlos_bench/tests/config/schemas/services/test-cases/bad/invalid/azure_vm_service-config-missing.jsonc +++ /dev/null @@ -1,8 +0,0 @@ -{ - "class": "mlos_bench.services.remote.azure.AzureVMService", - "config": { - // "deploymentTemplatePath": "some/path/to/deployment/template.jsonc", // <-- missing - "subscription": "subscription-id", - "resourceGroup": "rg" - } -} From 389d6d27bbc806b3e44ed3c195a01dfeb0f5cb57 Mon Sep 17 00:00:00 2001 From: Brian Kroth Date: Thu, 22 Feb 2024 22:37:43 +0000 Subject: [PATCH 05/11] improve tests to support remote exec only mode --- .../remote/azure/azure_vm_services_test.py | 14 +++++++------- .../tests/services/remote/azure/conftest.py | 15 +++++++++++++++ 2 files changed, 22 insertions(+), 7 deletions(-) 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 index e1e578b8597..57021967028 100644 --- 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 @@ -240,7 +240,7 @@ def test_wait_vm_operation_retry(mock_getconn: MagicMock, (404, Status.FAILED), ]) @patch("mlos_bench.services.remote.azure.azure_vm_services.requests") -def test_remote_exec_status(mock_requests: MagicMock, azure_vm_service: AzureVMService, +def test_remote_exec_status(mock_requests: MagicMock, azure_vm_service_remote_exec_only: AzureVMService, http_status_code: int, operation_status: Status) -> None: """ Test waiting for completion of the remote execution on Azure. @@ -254,14 +254,14 @@ def test_remote_exec_status(mock_requests: MagicMock, azure_vm_service: AzureVMS }) mock_requests.post.return_value = mock_response - status, _ = azure_vm_service.remote_exec(script, config={"vmName": "test-vm"}, env_params={}) + status, _ = azure_vm_service_remote_exec_only.remote_exec(script, config={"vmName": "test-vm"}, env_params={}) assert status == operation_status @patch("mlos_bench.services.remote.azure.azure_vm_services.requests") def test_remote_exec_headers_output(mock_requests: MagicMock, - azure_vm_service: AzureVMService) -> None: + azure_vm_service_remote_exec_only: AzureVMService) -> None: """ Check if HTTP headers from the remote execution on Azure are correct. """ @@ -279,7 +279,7 @@ def test_remote_exec_headers_output(mock_requests: MagicMock, }) mock_requests.post.return_value = mock_response - _, cmd_output = azure_vm_service.remote_exec(script, config={"vmName": "test-vm"}, env_params={ + _, cmd_output = azure_vm_service_remote_exec_only.remote_exec(script, config={"vmName": "test-vm"}, env_params={ "param_1": 123, "param_2": "abc", }) @@ -315,7 +315,7 @@ def test_remote_exec_headers_output(mock_requests: MagicMock, (Status.PENDING, {}, {}), (Status.FAILED, {}, {}), ]) -def test_get_remote_exec_results(azure_vm_service: AzureVMService, operation_status: Status, +def test_get_remote_exec_results(azure_vm_service_remote_exec_only: AzureVMService, operation_status: Status, wait_output: dict, results_output: dict) -> None: """ Test getting the results of the remote execution on Azure. @@ -325,9 +325,9 @@ def test_get_remote_exec_results(azure_vm_service: AzureVMService, operation_sta mock_wait_host_operation = MagicMock() mock_wait_host_operation.return_value = (operation_status, wait_output) # azure_vm_service.wait_host_operation = mock_wait_host_operation - setattr(azure_vm_service, "wait_host_operation", mock_wait_host_operation) + setattr(azure_vm_service_remote_exec_only, "wait_host_operation", mock_wait_host_operation) - status, cmd_output = azure_vm_service.get_remote_exec_results(params) + status, cmd_output = azure_vm_service_remote_exec_only.get_remote_exec_results(params) assert status == operation_status assert mock_wait_host_operation.call_args[0][0] == params diff --git a/mlos_bench/mlos_bench/tests/services/remote/azure/conftest.py b/mlos_bench/mlos_bench/tests/services/remote/azure/conftest.py index 13e520a527f..24f90c64202 100644 --- a/mlos_bench/mlos_bench/tests/services/remote/azure/conftest.py +++ b/mlos_bench/mlos_bench/tests/services/remote/azure/conftest.py @@ -80,6 +80,21 @@ def azure_vm_service(azure_auth_service: AzureAuthService) -> AzureVMService: }, parent=azure_auth_service) +@pytest.fixture +def azure_vm_service_remote_exec_only(azure_auth_service: AzureAuthService) -> AzureVMService: + """ + Creates a dummy Azure VM service with no deployment template. + """ + return AzureVMService(config={ + "subscription": "TEST_SUB", + "resourceGroup": "TEST_RG", + "pollInterval": 1, + "pollTimeout": 2 + }, global_config={ + "vmName": "test-vm", # Should come from the upper-level config + }, parent=azure_auth_service) + + @pytest.fixture def azure_fileshare(config_persistence_service: ConfigPersistenceService) -> AzureFileShareService: """ From 4b1d825ff1e1a88b0876f196ce8607ed0b6eabca Mon Sep 17 00:00:00 2001 From: Brian Kroth Date: Thu, 22 Feb 2024 22:56:57 +0000 Subject: [PATCH 06/11] additional tests --- .../remote/azure/azure_deployment_services.py | 2 ++ .../remote/azure/azure_vm_services_test.py | 14 ++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/mlos_bench/mlos_bench/services/remote/azure/azure_deployment_services.py b/mlos_bench/mlos_bench/services/remote/azure/azure_deployment_services.py index 01ed9fb5e51..187b7c055b5 100644 --- a/mlos_bench/mlos_bench/services/remote/azure/azure_deployment_services.py +++ b/mlos_bench/mlos_bench/services/remote/azure/azure_deployment_services.py @@ -410,6 +410,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} """ + if not self._deploy_template: + raise ValueError(f"Missing deployment template: {self}") 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) 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 index 57021967028..89da9dac53c 100644 --- 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 @@ -146,6 +146,20 @@ def test_vm_operation_status(mock_requests: MagicMock, assert status == operation_status +@pytest.mark.parametrize( + ("operation_name", "accepts_params"), [ + ("provision_host", True), + ]) +def test_vm_operation_invalid(azure_vm_service_remote_exec_only: AzureVMService, + operation_name: str, + accepts_params: bool) -> None: + """ + Test VM operation status for an incomplete service config. + """ + operation = getattr(azure_vm_service_remote_exec_only, operation_name) + with pytest.raises(ValueError): + (_, _) = operation({"vmName": "test-vm"}) if accepts_params else operation() + @patch("mlos_bench.services.remote.azure.azure_deployment_services.time.sleep") @patch("mlos_bench.services.remote.azure.azure_deployment_services.requests.Session") def test_wait_vm_operation_ready(mock_session: MagicMock, mock_sleep: MagicMock, From 1a93db5d7975b3cb9dc1a21a13feb1049d2b2a2c Mon Sep 17 00:00:00 2001 From: Brian Kroth Date: Fri, 23 Feb 2024 17:42:06 +0000 Subject: [PATCH 07/11] pylint --- .../tests/services/remote/azure/azure_vm_services_test.py | 1 + 1 file changed, 1 insertion(+) 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 index 89da9dac53c..97bf904a563 100644 --- 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 @@ -160,6 +160,7 @@ def test_vm_operation_invalid(azure_vm_service_remote_exec_only: AzureVMService, with pytest.raises(ValueError): (_, _) = operation({"vmName": "test-vm"}) if accepts_params else operation() + @patch("mlos_bench.services.remote.azure.azure_deployment_services.time.sleep") @patch("mlos_bench.services.remote.azure.azure_deployment_services.requests.Session") def test_wait_vm_operation_ready(mock_session: MagicMock, mock_sleep: MagicMock, From 506070035ed3ee9fc59d8e86158ddde522e6b061 Mon Sep 17 00:00:00 2001 From: Brian Kroth Date: Fri, 23 Feb 2024 20:13:59 +0000 Subject: [PATCH 08/11] Add additional checks and options to override --- .../azure-deployment-service-subschema.json | 4 ++-- .../remote/azure/azure_network_services.py | 2 ++ .../azure/azure_network_services_test.py | 23 +++++++++++++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/mlos_bench/mlos_bench/config/schemas/services/remote/azure/azure-deployment-service-subschema.json b/mlos_bench/mlos_bench/config/schemas/services/remote/azure/azure-deployment-service-subschema.json index 2b613c55b7c..8abf7dd6782 100644 --- a/mlos_bench/mlos_bench/config/schemas/services/remote/azure/azure-deployment-service-subschema.json +++ b/mlos_bench/mlos_bench/config/schemas/services/remote/azure/azure-deployment-service-subschema.json @@ -21,8 +21,8 @@ "type": "string" }, "deploymentTemplatePath": { - "description": "Path to an ARM template file.", - "type": "string", + "description": "Path to an ARM template file, or null if it should be skipped.", + "type": ["string", "null"], "pattern": "[.]json[c]?$" }, "deploymentTemplateParameters": { 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 index ca61ff330cd..34c2cd62709 100644 --- a/mlos_bench/mlos_bench/services/remote/azure/azure_network_services.py +++ b/mlos_bench/mlos_bench/services/remote/azure/azure_network_services.py @@ -67,6 +67,8 @@ def __init__(self, self.wait_network_deployment, ]) ) + if not self._deploy_template: + raise ValueError("AzureNetworkService requires a deployment template.") 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 diff --git a/mlos_bench/mlos_bench/tests/services/remote/azure/azure_network_services_test.py b/mlos_bench/mlos_bench/tests/services/remote/azure/azure_network_services_test.py index be6d7da359d..22fec74c74e 100644 --- a/mlos_bench/mlos_bench/tests/services/remote/azure/azure_network_services_test.py +++ b/mlos_bench/mlos_bench/tests/services/remote/azure/azure_network_services_test.py @@ -13,6 +13,7 @@ from mlos_bench.environments.status import Status +from mlos_bench.services.remote.azure.azure_auth import AzureAuthService from mlos_bench.services.remote.azure.azure_network_services import AzureNetworkService from mlos_bench.tests.services.remote.azure import make_httplib_json_response @@ -87,3 +88,25 @@ def test_network_operation_status(mock_requests: MagicMock, (status, _) = operation({}) if accepts_params else operation() (status, _) = operation({"vnetName": "test-vnet"}) if accepts_params else operation() assert status == operation_status + + +@pytest.fixture +def test_azure_network_service_no_deployment_template(azure_auth_service: AzureAuthService) -> None: + """ + Tests creating a network services without a deployment template (should fail). + """ + with pytest.raises(ValueError): + _ = AzureNetworkService(config={ + "deploymentTemplatePath": None, + "deploymentTemplateParameters": { + "location": "westus2", + }, + }, parent=azure_auth_service) + + with pytest.raises(ValueError): + _ = AzureNetworkService(config={ + # "deploymentTemplatePath": None, + "deploymentTemplateParameters": { + "location": "westus2", + }, + }, parent=azure_auth_service) From 4cb6f0ddd67262366d5699ca26e381b80e54a4df Mon Sep 17 00:00:00 2001 From: Brian Kroth Date: Fri, 23 Feb 2024 20:16:46 +0000 Subject: [PATCH 09/11] tweak --- mlos_bench/mlos_bench/tests/services/remote/azure/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mlos_bench/mlos_bench/tests/services/remote/azure/conftest.py b/mlos_bench/mlos_bench/tests/services/remote/azure/conftest.py index 24f90c64202..45feb1aa503 100644 --- a/mlos_bench/mlos_bench/tests/services/remote/azure/conftest.py +++ b/mlos_bench/mlos_bench/tests/services/remote/azure/conftest.py @@ -89,7 +89,7 @@ def azure_vm_service_remote_exec_only(azure_auth_service: AzureAuthService) -> A "subscription": "TEST_SUB", "resourceGroup": "TEST_RG", "pollInterval": 1, - "pollTimeout": 2 + "pollTimeout": 2, }, global_config={ "vmName": "test-vm", # Should come from the upper-level config }, parent=azure_auth_service) From b346d245aa8c3ce4c8b0e34236d70b92b34d9d78 Mon Sep 17 00:00:00 2001 From: Brian Kroth Date: Fri, 23 Feb 2024 20:18:09 +0000 Subject: [PATCH 10/11] whitespace --- .../tests/config/schemas/services/test_services_schemas.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mlos_bench/mlos_bench/tests/config/schemas/services/test_services_schemas.py b/mlos_bench/mlos_bench/tests/config/schemas/services/test_services_schemas.py index bde267eab45..c96346ad7b2 100644 --- a/mlos_bench/mlos_bench/tests/config/schemas/services/test_services_schemas.py +++ b/mlos_bench/mlos_bench/tests/config/schemas/services/test_services_schemas.py @@ -39,7 +39,7 @@ NON_CONFIG_SERVICE_CLASSES = { ConfigPersistenceService, # configured thru the launcher cli args TempDirContextService, # ABCMeta abstract class, but no good way to test that dynamically in Python. - AzureDeploymentService, # ABCMeta abstract base class + AzureDeploymentService, # ABCMeta abstract base class SshService, # ABCMeta abstract base class } From c5018bce0fc042cbda5009700bb404459db49535 Mon Sep 17 00:00:00 2001 From: Brian Kroth Date: Fri, 23 Feb 2024 20:20:46 +0000 Subject: [PATCH 11/11] more helpful error message --- .../mlos_bench/services/remote/azure/azure_network_services.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 index 34c2cd62709..081d5d842e3 100644 --- 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,7 +68,8 @@ def __init__(self, ]) ) if not self._deploy_template: - raise ValueError("AzureNetworkService requires a deployment template.") + raise ValueError("AzureNetworkService requires a deployment template:\n" + + f"config={config}\nglobal_config={global_config}") 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