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 <david.garcia@canonical.com>

diff --git a/n2vc/libjuju.py b/n2vc/libjuju.py
index bf356f4..161370b 100644 (file)
--- a/n2vc/libjuju.py
+++ b/n2vc/libjuju.py
@@ -40,7 +40,6 @@ from n2vc.exceptions import (
     JujuApplicationNotFound,
     JujuLeaderUnitNotFound,
     JujuActionNotFound,
     JujuApplicationNotFound,
     JujuLeaderUnitNotFound,
     JujuActionNotFound,

-    JujuModelAlreadyExists,

     JujuControllerFailedConnecting,
     JujuApplicationExists,
     JujuInvalidK8sConfiguration,
     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.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()
         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:
         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)
 
             # 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:
             # 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,
                 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,
                 )
                     cloud_name=cloud_name,
                     credential_name=credential_name or cloud_name,
                 )

-                self.models.add(model_name)

         finally:
             if model:
                 await self.disconnect_model(model)
         finally:
             if model:
                 await self.disconnect_model(model)


@@ -855,6 +844,9 @@ class Libjuju:
         controller = await self.get_controller()
         model = None
         try:
         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
             model = await self.get_model(controller, model_name)
             self.log.debug("Destroying model {}".format(model_name))
             uuid = model.info.uuid


@@ -866,10 +858,6 @@ class Libjuju:
             # Disconnect model
             await self.disconnect_model(model)
 
             # 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
             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 (file)
--- 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,
 from n2vc.libjuju import Libjuju
 from n2vc.exceptions import (
     JujuControllerFailedConnecting,

-    JujuModelAlreadyExists,

     JujuMachineNotFound,
     JujuApplicationNotFound,
     JujuActionNotFound,
     JujuMachineNotFound,
     JujuApplicationNotFound,
     JujuActionNotFound,


@@ -238,10 +237,10 @@ class AddModelTest(LibjujuTestCase):
     ):
         mock_model_exists.return_value = True
 
     ):
         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()
 
 
         mock_disconnect_controller.assert_called()