From b0a8f409e149715bf37d30c414474888c8a499f3 Mon Sep 17 00:00:00 2001 From: David Garcia Date: Mon, 15 Mar 2021 18:41:34 +0100 Subject: [PATCH] Fix bug 1467 Do not raise exceptions in n2vc.libjuju.add_model() or n2vc.libjuju.destroy_model() functions In general, functions should be as idempotent as possible. In this particular case, executing the add_model() function several times with the same arguments should just work. Same thing for destroy_model(). If the model has already been destroyed, it should return and not give any errors saying that the model doesn't exist. Change-Id: I87e11ea0fe1b4063b2f89900fcc2bbf1f915b953 Signed-off-by: David Garcia --- n2vc/libjuju.py | 22 +++++----------------- n2vc/tests/unit/test_libjuju.py | 9 ++++----- 2 files changed, 9 insertions(+), 22 deletions(-) diff --git a/n2vc/libjuju.py b/n2vc/libjuju.py index 191821a..7a73033 100644 --- a/n2vc/libjuju.py +++ b/n2vc/libjuju.py @@ -40,7 +40,6 @@ from n2vc.exceptions import ( JujuApplicationNotFound, JujuLeaderUnitNotFound, JujuActionNotFound, - JujuModelAlreadyExists, JujuControllerFailedConnecting, JujuApplicationExists, JujuInvalidK8sConfiguration, @@ -109,7 +108,6 @@ class Libjuju: self.loop.set_exception_handler(self.handle_exception) self.creating_model = asyncio.Lock(loop=self.loop) - self.models = set() self.log.debug("Libjuju initialized!") self.health_check_task = self._create_health_check_task() @@ -187,22 +185,14 @@ class Libjuju: controller = await self.get_controller() model = None try: - # Raise exception if model already exists - if await self.model_exists(model_name, controller=controller): - raise JujuModelAlreadyExists( - "Model {} already exists.".format(model_name) - ) - # Block until other workers have finished model creation while self.creating_model.locked(): await asyncio.sleep(0.1) - # If the model exists, return it from the controller - if model_name in self.models: - return - # Create the model async with self.creating_model: + if await self.model_exists(model_name, controller=controller): + return self.log.debug("Creating model {}".format(model_name)) model = await controller.add_model( model_name, @@ -210,7 +200,6 @@ class Libjuju: cloud_name=cloud_name, credential_name=credential_name or cloud_name, ) - self.models.add(model_name) finally: if model: await self.disconnect_model(model) @@ -856,6 +845,9 @@ class Libjuju: controller = await self.get_controller() model = None try: + if not await self.model_exists(model_name, controller=controller): + return + model = await self.get_model(controller, model_name) self.log.debug("Destroying model {}".format(model_name)) uuid = model.info.uuid @@ -867,10 +859,6 @@ class Libjuju: # Disconnect model await self.disconnect_model(model) - # Destroy model - if model_name in self.models: - self.models.remove(model_name) - await controller.destroy_model(uuid, force=True, max_wait=0) # Wait until model is destroyed diff --git a/n2vc/tests/unit/test_libjuju.py b/n2vc/tests/unit/test_libjuju.py index b7c7901..9ab6ddb 100644 --- a/n2vc/tests/unit/test_libjuju.py +++ b/n2vc/tests/unit/test_libjuju.py @@ -24,7 +24,6 @@ from .utils import FakeN2VC, FakeMachine, FakeApplication from n2vc.libjuju import Libjuju from n2vc.exceptions import ( JujuControllerFailedConnecting, - JujuModelAlreadyExists, JujuMachineNotFound, JujuApplicationNotFound, JujuActionNotFound, @@ -238,10 +237,10 @@ class AddModelTest(LibjujuTestCase): ): mock_model_exists.return_value = True - with self.assertRaises(JujuModelAlreadyExists): - self.loop.run_until_complete( - self.libjuju.add_model("existing_model", "cloud") - ) + # This should not raise an exception + self.loop.run_until_complete( + self.libjuju.add_model("existing_model", "cloud") + ) mock_disconnect_controller.assert_called() -- 2.25.1