diff --git a/docs/reference.md b/docs/reference.md index e4dc990..b586d7e 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -28,6 +28,10 @@ on the specified controller. Keep any automatically created models. +### `--destroy-storage` + +Destroy storage allocated on the created models. + ### `--model-config` Path to a yaml file which will be applied to the model on creation. @@ -368,7 +372,7 @@ applied if the marked test method fails or errors. Log a summary of the status of the model. This is automatically called before the model is cleaned up. -#### `async def track_model(self, alias: str, model_name: Optional[str] = None, cloud_name: Optional[str] = None, use_existing: Optional[bool] = None, keep: Optional[ModelKeep, str, bool, None] = ModelKeep.IF_EXISTS, **kwargs,) -> Model` +#### `async def track_model(self, alias: str, model_name: Optional[str] = None, cloud_name: Optional[str] = None, use_existing: Optional[bool] = None, keep: Optional[ModelKeep, str, bool, None] = ModelKeep.IF_EXISTS, destroy_storage: bool = False, **kwargs,) -> Model` Indicate to `ops_test` to track a new model which is automatically created in juju or an existing juju model referenced by model_name. This allows `ops_test` to track multiple models on various clouds by a unique alias name. @@ -391,6 +395,7 @@ This allows `ops_test` to track multiple models on various clouds by a unique al * `None` (default): inherit boolean value of `use_existing` * `False`: `ops_test` will destroy at the end of testing * `True`: `ops_test` won't destroy at the end of testing +* `destroy_storage`: Wether the storage should be destroyed with the model, defaults to `False`. ##### Examples: diff --git a/pytest_operator/plugin.py b/pytest_operator/plugin.py index a11e674..311bcdc 100644 --- a/pytest_operator/plugin.py +++ b/pytest_operator/plugin.py @@ -9,14 +9,14 @@ import logging import os import re -import shutil import shlex +import shutil import subprocess import sys import textwrap from collections import OrderedDict -from functools import cached_property from fnmatch import fnmatch +from functools import cached_property from pathlib import Path from random import choices from string import ascii_lowercase, digits, hexdigits @@ -26,16 +26,16 @@ Iterable, List, Literal, - MutableMapping, Mapping, + MutableMapping, Optional, - TypeVar, Tuple, + TypeVar, Union, ) -from urllib.request import urlretrieve, urlopen -from urllib.parse import urlencode from urllib.error import HTTPError +from urllib.parse import urlencode +from urllib.request import urlopen, urlretrieve from zipfile import Path as ZipPath import jinja2 @@ -45,13 +45,13 @@ import yaml from _pytest.config import Config from _pytest.config.argparsing import Parser -from kubernetes import client as k8s_client -from kubernetes.client import Configuration as K8sConfiguration from juju.client import client from juju.client.jujudata import FileJujuData -from juju.exceptions import DeadEntityException from juju.errors import JujuError -from juju.model import Model, Controller, websockets +from juju.exceptions import DeadEntityException +from juju.model import Controller, Model, websockets +from kubernetes import client as k8s_client +from kubernetes.client import Configuration as K8sConfiguration log = logging.getLogger(__name__) @@ -86,6 +86,12 @@ def pytest_addoption(parser: Parser): action="store_true", help="Keep models handled by opstest, can be overriden in track_model", ) + parser.addoption( + "--destroy-storage", + action="store_true", + help="Destroy storage created in models handled by opstest," + "can be overriden in track_model", + ) parser.addoption( "--destructive-mode", action="store_true", @@ -412,6 +418,7 @@ class ModelInUseError(Exception): class ModelState: model: Model keep: bool + destroy_storage: bool controller_name: str cloud_name: Optional[str] model_name: str @@ -502,6 +509,7 @@ def __init__(self, request, tmp_path_factory): self._init_cloud_name: Optional[str] = request.config.option.cloud self._init_model_name: Optional[str] = request.config.option.model self._init_keep_model: bool = request.config.option.keep_models + self._init_destroy_storage: bool = request.config.option.destroy_storage # These may be modified by _setup_model self.controller_name = request.config.option.controller @@ -611,6 +619,18 @@ def keep_model(self) -> bool: current_state = self.current_alias and self._models.get(self.current_alias) return current_state.keep if current_state else self._init_keep_model + @property + def destroy_storage(self) -> bool: + """ + Represents whether the current model storage should be destroyed after tests. + """ + current_state = self.current_alias and self._models.get(self.current_alias) + return ( + current_state.destroy_storage + if current_state + else self._init_destroy_storage + ) + def _generate_name(self, kind: str) -> str: module_name = self.request.module.__name__.rpartition(".")[-1] suffix = "".join(choices(ascii_lowercase + digits, k=4)) @@ -679,7 +699,9 @@ async def juju(self, *args, **kwargs): return await self.run("juju", *args, **kwargs) - async def _add_model(self, cloud_name, model_name, keep=False, **kwargs): + async def _add_model( + self, cloud_name, model_name, keep=False, destroy_storage=False, **kwargs + ): """ Creates a model used by the test framework which would normally be destroyed after the tests are run in the module. @@ -711,7 +733,13 @@ async def _add_model(self, cloud_name, model_name, keep=False, **kwargs): # update the cache from the controller. await self.juju("models") state = ModelState( - model, keep, controller_name, cloud_name, model_name, timeout=timeout + model, + keep, + destroy_storage, + controller_name, + cloud_name, + model_name, + timeout=timeout, ) state.config = await model.get_config() return state @@ -724,13 +752,17 @@ async def _model_exists(self, model_name: str) -> bool: return model_name in all_models @staticmethod - async def _connect_to_model(controller_name, model_name, keep=True): + async def _connect_to_model( + controller_name, model_name, keep=True, destroy_storage=False + ): """ Makes a reference to an existing model used by the test framework which will not be destroyed after the tests are run in the module. """ model = Model() - state = ModelState(model, keep, controller_name, None, model_name) + state = ModelState( + model, keep, destroy_storage, controller_name, None, model_name + ) log.info( "Connecting to existing model %s on unspecified cloud", state.full_name ) @@ -771,6 +803,7 @@ async def _setup_model(self): model_name=self._init_model_name or self.default_model_name, cloud_name=self._init_cloud_name, keep=self._init_model_name is not None, + destroy_storage=self._init_destroy_storage, config=self.read_model_config(self._init_model_config), ) @@ -782,6 +815,7 @@ async def track_model( model_name: Optional[str] = None, cloud_name: Optional[str] = None, use_existing: Optional[bool] = None, + destroy_storage: Optional[bool] = None, keep: Union[ModelKeep, str, bool, None] = ModelKeep.IF_EXISTS, **kwargs, ) -> Model: @@ -794,6 +828,9 @@ async def track_model( None will craft a unique name @param Optional[str] cloud_name: cloud name in which to add a new model, None will use current cloud + @param Optional[bool] destroy_storage: wether the storage should be destroyed + with the model, None defaults to the + pytest config flag `--destroy-storage` @param Optional[bool] use_existing: None: True if model_name exists on this controller False: create a new model and keep=False, unless keep=True explicitly set @@ -836,6 +873,10 @@ async def track_model( elif keep is None: keep_val = self._init_keep_model or bool(use_existing) + destroy_storage_val = ( + self._init_destroy_storage if destroy_storage is None else destroy_storage + ) + if use_existing: if not model_name: raise NotImplementedError( @@ -848,7 +889,7 @@ async def track_model( cloud_name = cloud_name or self.cloud_name model_name = model_name or self._generate_name(kind="model") model_state = await self._add_model( - cloud_name, model_name, keep_val, **kwargs + cloud_name, model_name, keep_val, destroy_storage_val, **kwargs ) self._models[alias] = model_state if ops_cloud := self._clouds.get(cloud_name): @@ -889,8 +930,8 @@ async def forget_model( self, alias: Optional[str] = None, timeout: Optional[Timeout] = None, + destroy_storage: Optional[bool] = None, allow_failure: bool = True, - destroy_storage: bool = False, ): """ Forget a model and wait for it to be removed from the controller. @@ -930,6 +971,9 @@ async def is_model_alive(): if not self.keep_model: await self._reset(model, allow_failure, timeout=timeout) + destroy_storage = ( + self.destroy_storage if destroy_storage is None else destroy_storage + ) await self._controller.destroy_model( model_name, force=True, diff --git a/tests/unit/test_pytest_operator.py b/tests/unit/test_pytest_operator.py index 9182058..971f8a8 100644 --- a/tests/unit/test_pytest_operator.py +++ b/tests/unit/test_pytest_operator.py @@ -382,7 +382,7 @@ async def test_crash_dump_mode( ops_test._current_alias = "main" ops_test._models = { ops_test.current_alias: plugin.ModelState( - model, keep_models, "test", "local", "model" + model, keep_models, False, "test", "local", "model" ) } ops_test.crash_dump_output = None @@ -503,6 +503,7 @@ def setup_request(request, mock_juju): mock_request.config.option.model_alias = "main" mock_request.config.option.model_config = None mock_request.config.option.keep_models = False + mock_request.config.option.destroy_storage = False yield mock_request @@ -521,6 +522,9 @@ async def test_fixture_set_up_existing_model( assert ops_test.cloud_name is None assert ops_test.model_name == "this-model" assert ops_test.keep_model is True, "Model should be kept if it already exists" + assert ( + ops_test.destroy_storage is False + ), "Storage should not be destroyed by default" assert len(ops_test.models) == 1 @@ -582,6 +586,37 @@ async def test_model_keep_options( ), f"{ops_test.model_full_name} should follow configured keep" +@patch("pytest_operator.plugin.OpsTest.forget_model", AsyncMock()) +@patch("pytest_operator.plugin.OpsTest.run", AsyncMock()) +@pytest.mark.parametrize( + "global_flag, destroy_storage, expected", + [ + (False, None, False), + (False, True, True), + (False, False, False), + (True, None, True), + (True, True, True), + (True, False, False), + ], +) +async def test_destroy_storage_options( + global_flag, destroy_storage, expected, setup_request, tmp_path_factory +): + setup_request.config.option.destroy_storage = global_flag + ops_test = plugin.OpsTest(setup_request, tmp_path_factory) + await ops_test._setup_model() + with ops_test.model_context("main"): + assert ops_test.destroy_storage is bool( + global_flag + ), "main model should follow global flag" + + await ops_test.track_model("secondary", destroy_storage=destroy_storage) + with ops_test.model_context("secondary"): + assert ( + ops_test.destroy_storage is expected + ), f"{ops_test.model_full_name} should follow configured destroy_storage" + + @patch("pytest_operator.plugin.OpsTest.default_model_name", new_callable=PropertyMock) @patch("pytest_operator.plugin.OpsTest.juju", autospec=True) async def test_fixture_set_up_automatic_model(