Fix bug 628 - Better handling of model management

In order to address the issue raised in bug 628, we've improved the
handling of how N2VC creates and destroys models. Previously this was
done transparently, but now they are explicit operations that will need
to be performed by the LCM.

The LCM will be required to call `CreateNetworkService()` prior to
calling `DeployCharms`. This will return True if successful, or raise an
exception if the model can't be created.

After LCM has called `RemoveCharms()`, it will need to call
`DestroyNetworkService()` to remove the model from the Juju controller.

- Fix lint errors
- Implement the `DestroyNetworkService` method, which will destroy the
model per Network Service, and any applications deployed to it.
- Adds a new test for creating and deleting models
- Add `CreateNetworkService` to explicitly create a new model for a
Network Service.
- Add proper return values from `logout()` so we can assert against it
during tests
- Add dependency on pytest-assume so we can test multiple failures per
test. This allows us to properly assert and still tear down the juju
client.
- Use pytest.assume. If a method asserts, we can continue with the
teardown of the client.

Change-Id: I3031b008f4ed79f978ecd84ade72e125153729a7
Signed-off-by: Adam Israel <adam.israel@canonical.com>

diff --git a/n2vc/vnf.py b/n2vc/vnf.py
index a079b97..6e4aaf3 100644 (file)
--- a/n2vc/vnf.py
+++ b/n2vc/vnf.py
@@ -19,7 +19,7 @@ if path not in sys.path:
 
 from juju.controller import Controller
 from juju.model import ModelObserver
-from juju.errors import JujuAPIError
+from juju.errors import JujuAPIError, JujuError
 
 # We might need this to connect to the websocket securely, but test and verify.
 try:
@@ -43,6 +43,10 @@ class N2VCPrimitiveExecutionFailed(Exception):
     """Something failed while attempting to execute a primitive."""
 
 
+class NetworkServiceDoesNotExist(Exception):
+    """The Network Service being acted against does not exist."""
+
+
 # Quiet the debug logging
 logging.getLogger('websockets.protocol').setLevel(logging.INFO)
 logging.getLogger('juju.client.connection').setLevel(logging.WARN)
@@ -779,8 +783,61 @@ class N2VC:
             self.log.debug(e)
             raise e
 
-    async def DestroyNetworkService(self, nsd):
-        raise NotImplementedError()
+    async def CreateNetworkService(self, ns_uuid):
+        """Create a new Juju model for the Network Service.
+
+        Creates a new Model in the Juju Controller.
+
+        :param str ns_uuid: A unique id representing an instaance of a
+            Network Service.
+
+        :returns: True if the model was created. Raises JujuError on failure.
+        """
+        if not self.authenticated:
+            await self.login()
+
+        models = await self.controller.list_models()
+        if ns_uuid not in models:
+            try:
+                self.models[ns_uuid] = await self.controller.add_model(
+                    ns_uuid
+                )
+            except JujuError as e:
+                if "already exists" not in e.message:
+                    raise e
+        return True
+
+    async def DestroyNetworkService(self, ns_uuid):
+        """Destroy a Network Service.
+
+        Destroy the Network Service and any deployed charms.
+
+        :param ns_uuid The unique id of the Network Service
+
+        :returns: True if the model was created. Raises JujuError on failure.
+        """
+
+        # Do not delete the default model. The default model was used by all
+        # Network Services, prior to the implementation of a model per NS.
+        if ns_uuid.lower() is "default":
+            return False
+
+        if not self.authenticated:
+            self.log.debug("Authenticating with Juju")
+            await self.login()
+
+        # Disconnect from the Model
+        if ns_uuid in self.models:
+            await self.disconnect_model(self.models[ns_uuid])
+
+        try:
+            await self.controller.destroy_models(ns_uuid)
+        except JujuError as e:
+            raise NetworkServiceDoesNotExist(
+                "The Network Service '{}' does not exist".format(ns_uuid)
+            )
+
+        return True
 
     async def GetMetrics(self, model_name, application_name):
         """Get the metrics collected by the VCA.
@@ -936,7 +993,7 @@ class N2VC:
             elif not c.isalpha():
                 c = "-"
             appname += c
-        return re.sub('\-+', '-', appname.lower())
+        return re.sub('-+', '-', appname.lower())
 
     # def format_application_name(self, nsd_name, vnfr_name, member_vnf_index=0):
     #     """Format the name of the application
@@ -989,9 +1046,13 @@ class N2VC:
             models = await self.controller.list_models()
 
             if model_name not in models:
-                self.models[model_name] = await self.controller.add_model(
-                    model_name
-                )
+                try:
+                    self.models[model_name] = await self.controller.add_model(
+                        model_name
+                    )
+                except JujuError as e:
+                    if "already exists" not in e.message:
+                        raise e
             else:
                 self.models[model_name] = await self.controller.get_model(
                     model_name
@@ -1051,7 +1112,7 @@ class N2VC:
     async def logout(self):
         """Logout of the Juju controller."""
         if not self.authenticated:
-            return
+            return False
 
         try:
             for model in self.models:
@@ -1074,15 +1135,11 @@ class N2VC:
                 "Fatal error logging out of Juju Controller: {}".format(e)
             )
             raise e
+        return True
 
     async def disconnect_model(self, model):
         self.log.debug("Disconnecting model {}".format(model))
         if model in self.models:
-            print(self.models[model].applications)
-            if len(self.models[model].applications) == 0:
-                print("Destroying empty model")
-                await self.controller.destroy_models(model)
-
             print("Disconnecting model")
             await self.models[model].disconnect()
             self.refcount['model'] -= 1

diff --git a/tests/base.py b/tests/base.py
index 8b61461..e4111eb 100644 (file)
--- a/tests/base.py
+++ b/tests/base.py
@@ -25,8 +25,7 @@ here = os.path.dirname(os.path.realpath(__file__))
 def is_bootstrapped():
     result = subprocess.run(['juju', 'switch'], stdout=subprocess.PIPE)
     return (
-        result.returncode == 0 and
-        len(result.stdout.decode().strip()) > 0)
+        result.returncode == 0 and len(result.stdout.decode().strip()) > 0)
 
 
 bootstrapped = pytest.mark.skipif(

diff --git a/tests/charms/layers/simple/reactive/simple.py b/tests/charms/layers/simple/reactive/simple.py
index 228be3c..802d60c 100644 (file)
--- a/tests/charms/layers/simple/reactive/simple.py
+++ b/tests/charms/layers/simple/reactive/simple.py
@@ -36,7 +36,7 @@ def touch():
         filename = action_get('filename')
         cmd = ['touch {}'.format(filename)]
         result, err = charms.sshproxy._run(cmd)
-    except:
+    except Exception:
         action_fail('command failed:' + err)
     else:
         action_set({'output': result})

diff --git a/tests/test_model.py b/tests/test_model.py
new file mode 100644 (file)
index 0000000..b54a7bb
--- /dev/null
+++ b/tests/test_model.py
@@ -0,0 +1,57 @@
+"""
+Test N2VC's ssh key generation
+"""
+import n2vc
+import os
+import pytest
+from . import base
+import tempfile
+import uuid
+
+
+@pytest.mark.asyncio
+async def test_model_create():
+    """Test the creation of a new model."""
+    client = base.get_n2vc()
+
+    model_name = "test-{}".format(
+        uuid.uuid4().hex[-4:],
+    )
+
+    pytest.assume(await client.CreateNetworkService(model_name))
+    pytest.assume(await client.DestroyNetworkService(model_name))
+    pytest.assume(await client.logout())
+
+
+@pytest.mark.asyncio
+async def test_destroy_non_existing_network_service():
+    """Destroy a model that doesn't exist."""
+
+    client = base.get_n2vc()
+
+    model_name = "test-{}".format(
+        uuid.uuid4().hex[-4:],
+    )
+
+    with pytest.raises(n2vc.vnf.NetworkServiceDoesNotExist):
+        pytest.assume(await client.DestroyNetworkService(model_name))
+
+    pytest.assume(await client.logout())
+
+
+@pytest.mark.asyncio
+async def test_model_create_duplicate():
+    """Create a new model, and try to create the same model."""
+    client = base.get_n2vc()
+
+    model_name = "test-{}".format(
+        uuid.uuid4().hex[-4:],
+    )
+
+    # Try to recreate bug 628
+    for x in range(0, 1000):
+        model = await client.get_model(model_name)
+        pytest.assume(model)
+
+    pytest.assume(await client.DestroyNetworkService(model_name))
+    pytest.assume(await client.logout())

diff --git a/tox.ini b/tox.ini
index a568d3f..b353ce8 100644 (file)
--- a/tox.ini
+++ b/tox.ini
@@ -31,6 +31,7 @@ deps =
     pytest
     pytest-asyncio
     pytest-xdist
+    pytest-assume
     paramiko
     pylxd