From: Adam Israel Date: Fri, 8 Mar 2019 23:33:35 +0000 (-0500) Subject: Fix bug 628 - Better handling of model management X-Git-Tag: v6.0.0~17 X-Git-Url: https://osm.etsi.org/gitweb/?a=commitdiff_plain;h=refs%2Fchanges%2F21%2F7321%2F1;p=osm%2FN2VC.git 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 --- diff --git a/n2vc/vnf.py b/n2vc/vnf.py index a079b97..6e4aaf3 100644 --- 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 --- 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 --- 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 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 --- a/tox.ini +++ b/tox.ini @@ -31,6 +31,7 @@ deps = pytest pytest-asyncio pytest-xdist + pytest-assume paramiko pylxd