diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f12d8b6e..f84caa45 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -43,3 +43,4 @@ repos: rev: v1.16.1 hooks: - id: typos + args: [--config=.typos.toml] diff --git a/.typos.toml b/.typos.toml index 7d956f99..33c47156 100644 --- a/.typos.toml +++ b/.typos.toml @@ -1,5 +1,5 @@ [files] -extend-exclude = ["*.json", "*.js", "*.ipynb", "LICENSE", "*.css"] +extend-exclude = ["*.json", "*.js", "*.ipynb", "LICENSE", "*.css", ".gitignore"] [default.extend-identifiers] HashiCorp = "HashiCorp" diff --git a/docs/azure-permissions.md b/docs/azure-permissions.md index 5e202a47..3a1ecfab 100644 --- a/docs/azure-permissions.md +++ b/docs/azure-permissions.md @@ -10,9 +10,9 @@ As a provisioning tool, Matcha interacts with Azure on your behalf, hiding away ## What permissions does Matcha require? -In its current form, the following Azure permissions are required: +Your account is required to have **either**: -1. Owner +1. Owner; _OR_ 2. A combination of: Contributor + User Access Administrator > Note: These are high level roles with a lot of privileges and we're actively working on introducing more granular permissions. diff --git a/docs/getting-started.md b/docs/getting-started.md index 6b0a879e..fbcbbf61 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -27,6 +27,7 @@ Next, you'll need to install a couple of things. * Python 3.8 or newer, along with Virtual Env and PIP. * The Azure command line tool. Instructions on installing this can be found [here](https://learn.microsoft.com/en-us/cli/azure/install-azure-cli). +* Docker. This is used to build images locally, before running them on Azure. Instructions for installing Docker can be found [here](https://www.docker.com/). The Docker daemon will need to be running on your system. * Terraform. We use this to provision services inside Azure. You'll find installation instructions for your platform [here](https://developer.hashicorp.com/terraform/downloads?product_intent=terraform). We recommend version 1.4 or newer. # The movie recommender @@ -73,7 +74,7 @@ az login When you run this command, you'll be taken to the Azure login screen in a web browser window, and you'll be asked if you want to allow the Azure CLI to access your Azure account. You'll need to grant this permission in order for Matcha to gain access to your Azure account when it provisions infrastructure. -> Note: you'll need certain permissions in order for Matcha to work. If you're unsure, you can just run `matcha` and it will tell you if you're missing any permissions. For specifics around permissions, please see our explainer on [Azure Permissions](azure-permissions.md). +> Note: you'll need certain permissions for Matcha to work. If you're unsure, you can run `matcha provision` and if your Azure account is missing the required permissions, the `provision` command will tell you. For specifics around permissions, please see our explainer on [Azure Permissions](azure-permissions.md). Next, let's provision: @@ -232,4 +233,6 @@ The final thing you'll want to do is decommission the infrastructure that Matcha matcha destroy ``` -> Note that this command is irreversible will remove all the resources deployed by `matcha provision` including the resource group, so make sure you save any data you wish to keep before running this command. +> Note: that this command is irreversible will remove all the resources deployed by `matcha provision` including the resource group, so make sure you save any data you wish to keep before running this command. +> +> You may also notice that an additional resource has appeared in Azure called 'NetworkWatcherRG' (if it wasn't already there). This is a resource that is automatically provisioned by Azure in each region when there is in-coming traffic to a provisioned resource and isn't controlled by Matcha. More information can be found [here](https://learn.microsoft.com/en-us/azure/network-watcher/network-watcher-monitoring-overview) on how to manage or remove this resource. diff --git a/docs/inside-matcha.md b/docs/inside-matcha.md index 10cc2c7e..6b5ce214 100644 --- a/docs/inside-matcha.md +++ b/docs/inside-matcha.md @@ -33,3 +33,5 @@ The user can use `get` to request information on specific resources or propertie ## `destroy` Once the user has finished with their provisioned environment, `destroy` enables them to tear down the resources. It works by calling the `destroy` Terraform command via the `python-terraform` library, which interacts with the configured Terraform files in the `.matcha/` directory. + +> You may also notice that an additional resource has appeared in Azure called 'NetworkWatcherRG' (if it wasn't already there). This is a resource that is automatically provisioned by Azure in each region when there is in-coming traffic to a provisioned resource and isn't controlled by Matcha. More information can be found [here](https://learn.microsoft.com/en-us/azure/network-watcher/network-watcher-monitoring-overview) on how to manage or remove this resource. diff --git a/docs/references.md b/docs/references.md index cc12e551..8548abcf 100644 --- a/docs/references.md +++ b/docs/references.md @@ -1,3 +1,3 @@ # API Reference Documentation -::: src.matcha_ml.core.core \ No newline at end of file +::: src.matcha_ml.core.core diff --git a/pyproject.toml b/pyproject.toml index 9e312034..276469bc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -155,5 +155,5 @@ module = [ ignore_missing_imports = true [tool.ruff.pylint] -max-branches = 13 +max-branches = 14 max-args = 6 diff --git a/src/matcha_ml/cli/constants.py b/src/matcha_ml/cli/constants.py index 55a0e3f2..800452c5 100644 --- a/src/matcha_ml/cli/constants.py +++ b/src/matcha_ml/cli/constants.py @@ -20,3 +20,14 @@ ("Azure Resource Group", "The resource group containing the provisioned resources"), ("Matcha State Container", "A storage container for tracking matcha state"), ] + +INFRA_FACTS = [ + "Did you know that Matcha tea was created by accident?", + "The brewing temperature of the water affects the taste of Matcha", + "Samurai's drank Matcha before battles", + "Matcha is provisioning Kubernetes which orchestrates tools", + "Seldon Core is used for model deployment", + "MLflow is used as an experiment tracker", + "Matcha is maintained by Fuzzy Labs", + "Everything being provisioned is fully open source", +] diff --git a/src/matcha_ml/cli/ui/spinner.py b/src/matcha_ml/cli/ui/spinner.py index b6de9dfc..4cc9f855 100644 --- a/src/matcha_ml/cli/ui/spinner.py +++ b/src/matcha_ml/cli/ui/spinner.py @@ -30,9 +30,14 @@ def __init__(self, status: str): ) self.progress.add_task(description=status, total=None) - def __enter__(self) -> None: - """Call when a spinner object is created using a `with` statement.""" + def __enter__(self) -> "Spinner": + """Call when a spinner object is created using a `with` statement. + + Returns: + Spinner: the instance for a context manager. + """ self.progress.start() + return self def __exit__( self, diff --git a/src/matcha_ml/cli/ui/status_message_builders.py b/src/matcha_ml/cli/ui/status_message_builders.py index 92fadac5..ec64362e 100644 --- a/src/matcha_ml/cli/ui/status_message_builders.py +++ b/src/matcha_ml/cli/ui/status_message_builders.py @@ -1,8 +1,13 @@ """UI status message builders.""" +import time +from random import shuffle from typing import List, Optional, Tuple from rich.console import Console +from matcha_ml.cli.constants import INFRA_FACTS +from matcha_ml.cli.ui.spinner import Spinner + err_console = Console(stderr=True) @@ -83,3 +88,20 @@ def build_warning_status(status: str) -> str: str: formatted message """ return f"[yellow]{status}[/yellow]" + + +def terraform_status_update(spinner: Spinner) -> None: + """Outputs some facts about the deployment and matcha tea while the terraform functions are running. + + Args: + spinner: The rich spinner that the messages are printed above. + """ + infra_facts_shuffled = list(range(len(INFRA_FACTS))) + shuffle(infra_facts_shuffled) + + time.sleep(10) # there should be a delay prior to spitting facts. + + while True: + fact = INFRA_FACTS[infra_facts_shuffled.pop()] + spinner.progress.console.print(build_status(fact)) + time.sleep(10) diff --git a/src/matcha_ml/core/core.py b/src/matcha_ml/core/core.py index 9c952e68..6c4e6986 100644 --- a/src/matcha_ml/core/core.py +++ b/src/matcha_ml/core/core.py @@ -1,6 +1,7 @@ """The core functionality for Matcha API.""" import os from typing import Optional +from warnings import warn from matcha_ml.cli._validation import get_command_validation from matcha_ml.cli.ui.print_messages import print_status @@ -15,6 +16,22 @@ from matcha_ml.templates.azure_template import AzureTemplate +MAJOR_MINOR_ZENML_VERSION = "0.36" + + +def zenml_version_is_supported() -> None: + """Check the zenml version of the local environment against the version matcha is expecting.""" + try: + import zenml + if zenml.__version__[:3] != MAJOR_MINOR_ZENML_VERSION: + warn( + f"Matcha expects ZenML version {MAJOR_MINOR_ZENML_VERSION}.x, but you have version {zenml.__version__}." + ) + except: + warn(f"No local installation of ZenMl found. Defaulting to version {MAJOR_MINOR_ZENML_VERSION} for remote " + f"resources.") + + @track(event_name=AnalyticsEvent.GET) def get( resource_name: Optional[str], @@ -117,7 +134,9 @@ def destroy() -> None: ) template_runner = AzureRunner() - with remote_state_manager.use_lock(), remote_state_manager.use_remote_state(): + with remote_state_manager.use_lock( + destroy=True + ), remote_state_manager.use_remote_state(destroy=True): template_runner.deprovision() remote_state_manager.deprovision_remote_state() @@ -184,6 +203,7 @@ def provision( MatchaError: If prefix is not valid. MatchaError: If region is not valid. """ + zenml_version_is_supported() remote_state_manager = RemoteStateManager() template_runner = AzureRunner() diff --git a/src/matcha_ml/errors.py b/src/matcha_ml/errors.py index 7da8f0d5..923672e2 100644 --- a/src/matcha_ml/errors.py +++ b/src/matcha_ml/errors.py @@ -81,7 +81,5 @@ def __init__(self, tf_error: str, *args: Any, **kwargs: Any): *args: args **kwargs: kwargs """ - message = ( - f"Terraform failed because of the following error: '{tf_error}'." - ) + message = f"Terraform failed because of the following error: '{tf_error}'." super().__init__(message, *args, **kwargs) diff --git a/src/matcha_ml/infrastructure/remote_state_storage/output.tf b/src/matcha_ml/infrastructure/remote_state_storage/output.tf index 6c50a7b5..d5516cba 100644 --- a/src/matcha_ml/infrastructure/remote_state_storage/output.tf +++ b/src/matcha_ml/infrastructure/remote_state_storage/output.tf @@ -22,3 +22,8 @@ output "cloud_azure_location"{ description = "The Azure location in which the resources are provisioned" value = var.location } + +output "cloud_azure_resource_group_name" { + description = "Name of the Azure resource group" + value = module.resource_group.name +} diff --git a/src/matcha_ml/runners/azure_runner.py b/src/matcha_ml/runners/azure_runner.py index 73cd7fd9..57c64afe 100644 --- a/src/matcha_ml/runners/azure_runner.py +++ b/src/matcha_ml/runners/azure_runner.py @@ -57,7 +57,7 @@ def provision(self) -> None: self._validate_terraform_config() self._validate_kubeconfig(base_path=".kube/config") self._initialize_terraform(msg="Matcha") - self._apply_terraform() + self._apply_terraform(msg="Matcha") tf_output = self.tfs.terraform_client.output() matcha_state_service = MatchaStateService(terraform_output=tf_output) self._show_terraform_outputs(matcha_state_service._state) diff --git a/src/matcha_ml/runners/base_runner.py b/src/matcha_ml/runners/base_runner.py index b0d650f8..f6e394c0 100644 --- a/src/matcha_ml/runners/base_runner.py +++ b/src/matcha_ml/runners/base_runner.py @@ -1,5 +1,6 @@ """Run terraform templates to provision and deprovision resources.""" import os +from multiprocessing.pool import ThreadPool from typing import Optional, Tuple import typer @@ -10,9 +11,13 @@ from matcha_ml.cli.ui.status_message_builders import ( build_status, build_substep_success_status, + terraform_status_update, ) from matcha_ml.errors import MatchaTerraformError -from matcha_ml.services.terraform_service import TerraformConfig, TerraformService +from matcha_ml.services.terraform_service import ( + TerraformConfig, + TerraformService, +) SPINNER = "dots" @@ -95,11 +100,11 @@ def _initialize_terraform(self, msg: str = "", destroy: bool = False) -> None: ) with Spinner("Initializing"): - ret_code, _, err = self.tfs.init() + tf_result = self.tfs.init() - if ret_code != 0: + if tf_result.return_code != 0: print_error("The command 'terraform init' failed.") - raise MatchaTerraformError(tf_error=err) + raise MatchaTerraformError(tf_error=tf_result.std_err) print_status( build_substep_success_status( @@ -126,22 +131,38 @@ def _check_matcha_directory_exists(self) -> None: ) raise typer.Exit() - def _apply_terraform(self) -> None: + def _apply_terraform(self, msg: str = "") -> None: """Run terraform apply to create resources on cloud. + Args: + msg (str) : Name of the type of resource (e.g. "Remote State" or "Matcha"). + Raises: MatchaTerraformError: if 'terraform apply' failed. """ - with Spinner("Applying"): - ret_code, _, err = self.tfs.apply() + with Spinner("Applying") as spinner: + pool = ThreadPool(processes=1) + _ = pool.apply_async(terraform_status_update, (spinner,)) - if ret_code != 0: - raise MatchaTerraformError(tf_error=err) - print_status( - build_substep_success_status( - f"{Emojis.CHECKMARK.value} Matcha resources have been provisioned!\n" + tf_result = self.tfs.apply() + + pool.terminate() + + if tf_result.return_code != 0: + raise MatchaTerraformError(tf_error=tf_result.std_err) + + if msg: + print_status( + build_substep_success_status( + f"{Emojis.CHECKMARK.value} {msg} resources have been provisioned!\n" + ) + ) + else: + print_status( + build_substep_success_status( + f"{Emojis.CHECKMARK.value} Resources have been provisioned!\n" + ) ) - ) def _destroy_terraform(self, msg: str = "") -> None: """Destroy the provisioned resources. @@ -156,10 +177,10 @@ def _destroy_terraform(self, msg: str = "") -> None: ) print() with Spinner("Destroying"): - ret_code, _, err = self.tfs.destroy() + tf_result = self.tfs.destroy() - if ret_code != 0: - raise MatchaTerraformError(tf_error=err) + if tf_result.return_code != 0: + raise MatchaTerraformError(tf_error=tf_result.std_err) def provision(self) -> Optional[Tuple[str, str, str]]: """Provision resources required for the deployment.""" diff --git a/src/matcha_ml/runners/remote_state_runner.py b/src/matcha_ml/runners/remote_state_runner.py index 1f7417f7..99b72dfc 100644 --- a/src/matcha_ml/runners/remote_state_runner.py +++ b/src/matcha_ml/runners/remote_state_runner.py @@ -38,9 +38,7 @@ def _get_terraform_output(self) -> Tuple[str, str, str]: prefix = "remote_state_storage" account_name = tf_outputs[f"{prefix}_account_name"]["value"] - resource_group_name = tf_outputs[f"{prefix}_resource_group_name"][ - "value" - ] + resource_group_name = tf_outputs[f"{prefix}_resource_group_name"]["value"] container_name = tf_outputs[f"{prefix}_container_name"]["value"] return account_name, container_name, resource_group_name @@ -65,7 +63,7 @@ def provision(self) -> Tuple[str, str, str]: self._validate_terraform_config() self._validate_kubeconfig(base_path=".kube/config") self._initialize_terraform(msg="Remote State") - self._apply_terraform() + self._apply_terraform(msg="Remote State") return self._get_terraform_output() def deprovision(self) -> None: diff --git a/src/matcha_ml/services/analytics_service.py b/src/matcha_ml/services/analytics_service.py index 5021c2d6..e659f402 100644 --- a/src/matcha_ml/services/analytics_service.py +++ b/src/matcha_ml/services/analytics_service.py @@ -3,6 +3,7 @@ This approach to collecting usage data was inspired by ZenML; source: https://github.com/zenml-io/zenml/blob/main/src/zenml/utils/analytics_utils.py """ import functools +import logging from enum import Enum from time import perf_counter from typing import Any, Callable, Optional, Tuple @@ -15,7 +16,10 @@ from matcha_ml.services.global_parameters_service import GlobalParameters from matcha_ml.state import MatchaState, MatchaStateService -analytics.write_key = "qwBKAvY6MEUvv5XIs4rE07ohf5neT3sx" +WRITE_KEY = "qwBKAvY6MEUvv5XIs4rE07ohf5neT3sx" + +# Suppress Segment warnings +logging.getLogger("segment").setLevel(logging.FATAL) class AnalyticsEvent(str, Enum): @@ -27,12 +31,14 @@ class AnalyticsEvent(str, Enum): def execute_analytics_event( - func: Callable, *args, **kwargs + func: Callable[..., Any], *args: Any, **kwargs: Any ) -> Tuple[Optional[MatchaState], Any]: """Exists to Temporarily fix misleading error messages coming from track decorator. Args: func (Callable): The function decorated by track. + *args (Any): arguments passed to the function. + **kwargs (Any): additional key word arguments passed to the function. Returns: The result of the call to func, the error code. @@ -57,6 +63,9 @@ def decorator(func: Callable[..., Any]) -> Callable[..., Any]: Args: func (Callable[..., Any]): The function that is being decorated + + Returns: + Callable[..., Any]: The function that is being decorated """ @functools.wraps(func) @@ -110,11 +119,13 @@ def inner(*args: Any, **kwargs: Any) -> Any: result, error_code = execute_analytics_event(func, *args, **kwargs) te = perf_counter() - analytics.track( + client = analytics.Client(WRITE_KEY, max_retries=1, debug=False) + + client.track( global_params.user_id, event_name.value, { - "time_taken": te - ts, + "time_taken": float(te) - float(ts), # type: ignore "error_type": f"{error_code.__class__}.{error_code.__class__.__name__}", "command_succeeded": error_code is None, "matcha_state_uuid": matcha_state_uuid, diff --git a/src/matcha_ml/services/terraform_service.py b/src/matcha_ml/services/terraform_service.py index 1c76c9f1..d4b6fafd 100644 --- a/src/matcha_ml/services/terraform_service.py +++ b/src/matcha_ml/services/terraform_service.py @@ -3,11 +3,20 @@ import glob import os from pathlib import Path -from typing import Optional, Tuple +from typing import Optional import python_terraform +@dataclasses.dataclass +class TerraformResult: + """A class to hold the result of the terraform commands.""" + + return_code: int + std_out: str + std_err: str + + @dataclasses.dataclass class TerraformConfig: """Configuration required for terraform.""" @@ -66,9 +75,7 @@ def check_installation(self) -> bool: return True - def verify_kubectl_config_file( - self, config_path: str = ".kube/config" - ) -> None: + def verify_kubectl_config_file(self, config_path: str = ".kube/config") -> None: """Checks if kubeconfig is present at location ~/.kube/config. Args: @@ -122,7 +129,7 @@ def get_tf_state_dir(self) -> Path: """ return Path(os.path.join(self.config.working_dir, ".terraform")) - def init(self) -> Tuple[int, str, str]: + def init(self) -> TerraformResult: """Run `terraform init` with the initialized Terraform client from the python_terraform module. Returns: @@ -133,9 +140,9 @@ def init(self) -> Tuple[int, str, str]: raise_on_error=False, ) - return ret_code, out, err + return TerraformResult(ret_code, out, err) - def apply(self) -> Tuple[int, str, str]: + def apply(self) -> TerraformResult: """Run `terraform apply` with the initialized Terraform client from the python_terraform module. Returns: @@ -150,9 +157,9 @@ def apply(self) -> Tuple[int, str, str]: auto_approve=True, ) - return ret_code, out, err + return TerraformResult(ret_code, out, err) - def destroy(self) -> Tuple[int, str, str]: + def destroy(self) -> TerraformResult: """Destroy the provisioned resources with the initialized Terraform client from the python_terraform module.. Returns: @@ -166,4 +173,4 @@ def destroy(self) -> Tuple[int, str, str]: auto_approve=True, ) - return ret_code, out, err + return TerraformResult(ret_code, out, err) diff --git a/src/matcha_ml/state/matcha_state.py b/src/matcha_ml/state/matcha_state.py index d31c7948..cd8f90ae 100644 --- a/src/matcha_ml/state/matcha_state.py +++ b/src/matcha_ml/state/matcha_state.py @@ -140,7 +140,7 @@ def __init__( Raises: MatchaError: if the state file does not exist. - MatchaError: if MatchaStateService is initialize with both 'matcha_state' and 'terraform_output' arguments. + MatchaError: if MatchaStateService is initialized with both 'matcha_state' and 'terraform_output' arguments. """ if matcha_state is not None and terraform_output is not None: raise MatchaError( diff --git a/src/matcha_ml/state/remote_state_manager.py b/src/matcha_ml/state/remote_state_manager.py index 030f19f1..8b3b94b1 100644 --- a/src/matcha_ml/state/remote_state_manager.py +++ b/src/matcha_ml/state/remote_state_manager.py @@ -292,17 +292,21 @@ def upload(self, local_folder_path: str) -> None: ) @contextlib.contextmanager - def use_remote_state(self) -> Iterator[None]: + def use_remote_state(self, destroy: bool = False) -> Iterator[None]: """Context manager to use remote state. Downloads the state before executing the code. Upload the state when context is finished. + + Args: + destroy (bool): Flag for whether the command being run is 'destroy' or not. """ self.download(os.getcwd()) yield None - self.upload(os.path.join(".matcha", "infrastructure")) + if not destroy: + self.upload(os.path.join(".matcha", "infrastructure")) def lock(self) -> None: """Lock remote state. @@ -335,10 +339,15 @@ def unlock(self) -> None: ) @contextlib.contextmanager - def use_lock(self) -> Iterator[None]: - """Context manager to lock state.""" + def use_lock(self, destroy: bool = False) -> Iterator[None]: + """Context manager to lock state. + + Args: + destroy (bool): Flag for whether the command being run is 'destroy' or not. + """ self.lock() try: yield finally: - self.unlock() + if not destroy: + self.unlock() diff --git a/src/matcha_ml/templates/azure_template.py b/src/matcha_ml/templates/azure_template.py index e7b2a4fe..3aeeba4c 100644 --- a/src/matcha_ml/templates/azure_template.py +++ b/src/matcha_ml/templates/azure_template.py @@ -53,6 +53,7 @@ def build_template( # Add matcha.state file one directory above the template config_dict = vars(config) _ = config_dict.pop("password", None) + config_dict["resource-group-name"] = f"{config_dict['prefix']}-resources" initial_state_file_dict = {"cloud": config_dict} matcha_state = MatchaState.from_dict(initial_state_file_dict) MatchaStateService(matcha_state=matcha_state) diff --git a/tests/conftest.py b/tests/conftest.py index 349135e0..a64b327d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -98,7 +98,7 @@ def mocked_segment_track_decorator(): MagicMock: Mocked segment track function. """ with patch( - "matcha_ml.services.analytics_service.analytics.track" + "matcha_ml.services.analytics_service.analytics.Client.track" ) as track_analytics: track_analytics.return_value = None diff --git a/tests/test_runners/test_base_runner.py b/tests/test_runners/test_base_runner.py index 29587a17..77d30656 100644 --- a/tests/test_runners/test_base_runner.py +++ b/tests/test_runners/test_base_runner.py @@ -9,6 +9,7 @@ from matcha_ml.errors import MatchaTerraformError from matcha_ml.runners.base_runner import BaseRunner +from matcha_ml.services.terraform_service import TerraformResult CLASS_STUB = "matcha_ml.runners.base_runner.BaseRunner" @@ -101,7 +102,7 @@ def test_initialize_terraform(capsys: SysCapture): assert expected in captured.out with mock.patch.object(template_runner.tf_state_dir, "exists", return_value=False): - template_runner.tfs.init = MagicMock(return_value=(0, "", "")) + template_runner.tfs.init = MagicMock(return_value=TerraformResult(0, "", "")) expected = " initialized!" template_runner._initialize_terraform() @@ -117,15 +118,17 @@ def test_apply_terraform(capsys: SysCapture): capsys (SysCapture): fixture to capture stdout and stderr """ template_runner = BaseRunner() - template_runner.tfs.apply = MagicMock(return_value=(0, "", "")) - expected = "Matcha resources have been provisioned!" + template_runner.tfs.apply = MagicMock(return_value=TerraformResult(0, "", "")) + expected = "Remote State resources have been provisioned!" - template_runner._apply_terraform() + template_runner._apply_terraform(msg="Remote State") captured = capsys.readouterr() assert expected in captured.out - template_runner.tfs.apply = MagicMock(return_value=(1, "", "Apply failed")) + template_runner.tfs.apply = MagicMock( + return_value=TerraformResult(1, "", "Apply failed") + ) with pytest.raises(MatchaTerraformError) as exc_info: template_runner._apply_terraform() @@ -143,7 +146,7 @@ def test_destroy_terraform(capsys: SysCapture): template_runner (AzureTemplateRunner): a AzureTemplateRunner object instance """ template_runner = BaseRunner() - template_runner.tfs.destroy = MagicMock(return_value=(0, "", "")) + template_runner.tfs.destroy = MagicMock(return_value=TerraformResult(0, "", "")) expected = "Destroying your resources" @@ -155,7 +158,9 @@ def test_destroy_terraform(capsys: SysCapture): assert expected in captured.out - template_runner.tfs.destroy = MagicMock(return_value=(1, "", "Init failed")) + template_runner.tfs.destroy = MagicMock( + return_value=TerraformResult(1, "", "Init failed") + ) with pytest.raises(MatchaTerraformError) as exc_info: template_runner._destroy_terraform() diff --git a/tests/test_services/test_terraform_service.py b/tests/test_services/test_terraform_service.py index 3d797c57..3cbcd074 100644 --- a/tests/test_services/test_terraform_service.py +++ b/tests/test_services/test_terraform_service.py @@ -6,7 +6,10 @@ import pytest from python_terraform import TerraformCommandError -from matcha_ml.services.terraform_service import TerraformConfig, TerraformService +from matcha_ml.services.terraform_service import ( + TerraformConfig, + TerraformService, +) @pytest.fixture @@ -218,7 +221,7 @@ def test_init(terraform_test_config: TerraformConfig): tfs.terraform_client.init = MagicMock(return_value=(0, "", "")) - _, _, _ = tfs.init() + _ = tfs.init() tfs.terraform_client.init.assert_called() @@ -233,7 +236,7 @@ def test_apply(terraform_test_config: TerraformConfig): tfs.terraform_client.apply = MagicMock(return_value=(0, "", "")) - _, _, _ = tfs.apply() + _ = tfs.apply() tfs.terraform_client.apply.assert_called() @@ -248,6 +251,6 @@ def test_destroy(terraform_test_config: TerraformConfig): tfs.terraform_client.destroy = MagicMock(return_value=(0, "", "")) - _, _, _ = tfs.destroy() + _ = tfs.destroy() tfs.terraform_client.destroy.assert_called() diff --git a/tests/test_state/test_remote_state_manager.py b/tests/test_state/test_remote_state_manager.py index 8948a589..9cc3eea0 100644 --- a/tests/test_state/test_remote_state_manager.py +++ b/tests/test_state/test_remote_state_manager.py @@ -424,6 +424,37 @@ def test_use_lock( assert mock_azure_storage_instance.delete_blob.call_count == 1 +def test_use_lock_on_destroy_command( + valid_config_testing_directory: str, mock_azure_storage_instance: MagicMock +): + """Test use_lock context manager when destroy is True. + + Args: + valid_config_testing_directory (str): temporary working directory path, with valid config file + mock_azure_storage_instance (MagicMock): mock of AzureStorage instance + """ + os.chdir(valid_config_testing_directory) + + mock_azure_storage_instance.blob_exists.return_value = False + + remote_state = RemoteStateManager() + with remote_state.use_lock(destroy=True): + # Test that the state was locked + mock_azure_storage_instance.create_empty.assert_called_with( + container_name="test-container", blob_name=LOCK_FILE_NAME + ) + assert mock_azure_storage_instance.create_empty.call_count == 1 + mock_azure_storage_instance.blob_exists.assert_not_called() + mock_azure_storage_instance.delete_blob.assert_not_called() + + # Set azure storage mock into a locked state + mock_azure_storage_instance.blob_exists.return_value = True + + # Test that the state was not unlocked + mock_azure_storage_instance.blob_exists.assert_not_called() + mock_azure_storage_instance.delete_blob.assert_not_called() + + def test_use_remote_state(): """Test use_remote_state context manager.""" remote_state_manager = RemoteStateManager() @@ -435,6 +466,17 @@ def test_use_remote_state(): mocked_upload.assert_called_once_with(os.path.join(".matcha", "infrastructure")) +def test_use_remote_state_on_destroy(): + """Test use_remote_state context manager and assert upload is not called when destroy is True.""" + remote_state_manager = RemoteStateManager() + with patch.object(remote_state_manager, "upload") as mocked_upload, patch.object( + remote_state_manager, "download" + ) as mocked_download: + with remote_state_manager.use_remote_state(destroy=True): + mocked_download.assert_called_once_with(os.getcwd()) + mocked_upload.assert_not_called() + + def test_is_state_provisioned_returns_false_when_resource_group_does_not_exist( valid_config_testing_directory: str, mock_azure_storage_instance: MagicMock ):