Add destroy-storage command line parameter

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:
 

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,

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(