From: Anderson Bravalheri Date: Sun, 16 Dec 2018 20:44:08 +0000 (+0000) Subject: Inject "wim_port_mappings" from "wim.config" X-Git-Tag: v5.0.3~4^2 X-Git-Url: https://osm.etsi.org/gitweb/?p=osm%2FRO.git;a=commitdiff_plain;h=fed47b0903b8e5ee93591ef53540432d3ecce796 Inject "wim_port_mappings" from "wim.config" Automatically create/update port mappings during WIM creation/update, as discussed in the WIM implementation meeting (12/Dec/2018). Additionally fix small related errors and ensure wan_port_mappings are returned by the HTTP server using the same keys forming the schema. Change-Id: Icc27ad85c1de826ed96cb42b377055ea1b0c2cab Signed-off-by: Anderson Bravalheri --- diff --git a/osm_ro/wim/engine.py b/osm_ro/wim/engine.py index fcb3477f..6ff2b4fc 100644 --- a/osm_ro/wim/engine.py +++ b/osm_ro/wim/engine.py @@ -48,12 +48,17 @@ import logging from contextlib import contextmanager from itertools import groupby from operator import itemgetter +from sys import exc_info from uuid import uuid4 +from six import reraise + from ..utils import remove_none_items from .actions import Action from .errors import ( + DbBaseException, NoWimConnectedToDatacenters, + UnexpectedDatabaseError, WimAccountNotActive ) from .wim_thread import WimThread @@ -76,10 +81,29 @@ class WimEngine(object): Please check the wim schema to have more information about ``properties``. + The ``config`` property might contain a ``wim_port_mapping`` dict, + In this case, the method ``create_wim_port_mappings`` will be + automatically invoked. + Returns: str: uuid of the newly created WIM record """ - return self.persist.create_wim(properties) + port_mapping = ((properties.get('config', {}) or {}) + .pop('wim_port_mapping', {})) + uuid = self.persist.create_wim(properties) + + if port_mapping: + try: + self.create_wim_port_mappings(uuid, port_mapping) + except DbBaseException: + # Rollback + self.delete_wim(uuid) + ex = UnexpectedDatabaseError('Failed to create port mappings' + 'Rolling back wim creation') + self.logger.exception(str(ex)) + reraise(ex.__class__, ex, exc_info()[2]) + + return uuid def get_wim(self, uuid_or_name, tenant_id=None): """Retrieve existing WIM record by name or id. @@ -95,8 +119,35 @@ class WimEngine(object): ``properties`` is a dictionary with the properties being changed, if a property is not present, the old value will be preserved + + Similarly to create_wim, the ``config`` property might contain a + ``wim_port_mapping`` dict, In this case, port mappings will be + automatically updated. """ - return self.persist.update_wim(uuid_or_name, properties) + port_mapping = ((properties.get('config', {}) or {}) + .pop('wim_port_mapping', {})) + orig_props = self.persist.get_by_name_or_uuid('wims', uuid_or_name) + uuid = orig_props['uuid'] + + response = self.persist.update_wim(uuid, properties) + + if port_mapping: + try: + # It is very complex to diff and update individually all the + # port mappings. Therefore a practical approach is just delete + # and create it again. + self.persist.delete_wim_port_mappings(uuid) + # ^ Calling from persistence avoid reloading twice the thread + self.create_wim_port_mappings(uuid, port_mapping) + except DbBaseException: + # Rollback + self.update_wim(uuid_or_name, orig_props) + ex = UnexpectedDatabaseError('Failed to update port mappings' + 'Rolling back wim updates\n') + self.logger.exception(str(ex)) + reraise(ex.__class__, ex, exc_info()[2]) + + return response def delete_wim(self, uuid_or_name): """Kill the corresponding wim threads and erase the WIM record""" diff --git a/osm_ro/wim/http_handler.py b/osm_ro/wim/http_handler.py index f5eeed99..b88dab3f 100644 --- a/osm_ro/wim/http_handler.py +++ b/osm_ro/wim/http_handler.py @@ -120,6 +120,9 @@ class WimHandler(BaseHandler): def http_get_wim(self, tenant_id, wim_id): tenant_id = None if tenant_id == 'any' else tenant_id wim = self.engine.get_wim(wim_id, tenant_id) + mappings = self.engine.get_wim_port_mappings(wim_id) + wim['config'] = utils.merge_dicts(wim.get('config', {}) or {}, + wim_port_mapping=mappings) return format_out({'wim': wim}) @route('POST', '/wims') diff --git a/osm_ro/wim/persistence.py b/osm_ro/wim/persistence.py index 8f95d563..b956965b 100644 --- a/osm_ro/wim/persistence.py +++ b/osm_ro/wim/persistence.py @@ -326,12 +326,11 @@ class WimPersistence(object): where = {'uuid': wim['uuid']} # unserialize config, edit and serialize it again - if wim_descriptor.get('config'): - new_config_dict = wim_descriptor["config"] - config_dict = remove_none_items(merge_dicts( - wim.get('config') or {}, new_config_dict)) - wim_descriptor['config'] = ( - _serialize(config_dict) if config_dict else None) + new_config_dict = wim_descriptor.get('config', {}) or {} + config_dict = remove_none_items(merge_dicts( + wim.get('config', {}) or {}, new_config_dict)) + wim_descriptor['config'] = ( + _serialize(config_dict) if config_dict else None) with self.lock: self.db.update_rows('wims', wim_descriptor, where) @@ -508,8 +507,12 @@ class WimPersistence(object): See :obj:`~.query` for additional keyword arguments. """ - kwargs.update(datacenter=datacenter, tenant=tenant) - return self.query(_DATACENTER_JOIN, **kwargs) + if tenant: + kwargs.update(datacenter=datacenter, tenant=tenant) + return self.query(_DATACENTER_JOIN, **kwargs) + else: + return [self.get_by_name_or_uuid('datacenters', + datacenter, **kwargs)] def get_datacenter_by(self, datacenter=None, tenant=None, **kwargs): """Similar to ``get_datacenters_by``, but ensuring just one result""" @@ -622,7 +625,7 @@ class WimPersistence(object): return [ {'wim_id': key[0], 'datacenter_id': key[1], - 'wan_pop_port_mappings': [ + 'pop_wan_mappings': [ filter_out_dict_keys(mapping, ( 'id', 'wim_id', 'datacenter_id', 'created_at', 'modified_at')) diff --git a/osm_ro/wim/schemas.py b/osm_ro/wim/schemas.py index f4be2166..fb65fdd3 100644 --- a/osm_ro/wim/schemas.py +++ b/osm_ro/wim/schemas.py @@ -41,6 +41,71 @@ from ..openmano_schemas import ( # WIM ------------------------------------------------------------------------- wim_types = ["tapi", "onos", "odl", "dynpac"] +dpid_type = { + "type": "string", + "pattern": + "^[0-9a-zA-Z]+(:[0-9a-zA-Z]+)*$" +} + +port_type = { + "oneOf": [ + {"type": "string", + "minLength": 1, + "maxLength": 5}, + {"type": "integer", + "minimum": 1, + "maximum": 65534} + ] +} + +wim_port_mapping_desc = { + "type": "array", + "items": { + "type": "object", + "properties": { + "datacenter_name": nameshort_schema, + "pop_wan_mappings": { + "type": "array", + "items": { + "type": "object", + "properties": { + "pop_switch_dpid": dpid_type, + "pop_switch_port": port_type, + "wan_service_endpoint_id": name_schema, + "wan_service_mapping_info": { + "type": "object", + "properties": { + "mapping_type": name_schema, + "wan_switch_dpid": dpid_type, + "wan_switch_port": port_type + }, + "additionalProperties": True, + "required": ["mapping_type"] + } + }, + "oneOf": [ + { + "required": [ + "pop_switch_dpid", + "pop_switch_port", + "wan_service_endpoint_id" + ] + }, + { + "required": [ + "pop_switch_dpid", + "pop_switch_port", + "wan_service_mapping_info" + ] + } + ] + } + } + }, + "required": ["datacenter_name", "pop_wan_mappings"] + } +} + wim_schema_properties = { "name": name_schema, "description": description_schema, @@ -49,7 +114,12 @@ wim_schema_properties = { "enum": ["tapi", "onos", "odl", "dynpac"] }, "wim_url": description_schema, - "config": {"type": "object"} + "config": { + "type": "object", + "properties": { + "wim_port_mapping": wim_port_mapping_desc + } + } } wim_schema = { @@ -103,75 +173,12 @@ wim_account_schema = { "additionalProperties": False } -dpid_type = { - "type": "string", - "pattern": - "^[0-9a-zA-Z]+(:[0-9a-zA-Z]+)*$" -} - -port_type = { - "oneOf": [ - {"type": "string", - "minLength": 1, - "maxLength": 5}, - {"type": "integer", - "minimum": 1, - "maximum": 65534} - ] -} - wim_port_mapping_schema = { "$schema": "http://json-schema.org/draft-04/schema#", "title": "wim mapping information schema", "type": "object", "properties": { - "wim_port_mapping": { - "type": "array", - "items": { - "type": "object", - "properties": { - "datacenter_name": nameshort_schema, - "pop_wan_mappings": { - "type": "array", - "items": { - "type": "object", - "properties": { - "pop_switch_dpid": dpid_type, - "pop_switch_port": port_type, - "wan_service_endpoint_id": name_schema, - "wan_service_mapping_info": { - "type": "object", - "properties": { - "mapping_type": name_schema, - "wan_switch_dpid": dpid_type, - "wan_switch_port": port_type - }, - "additionalProperties": True, - "required": ["mapping_type"] - } - }, - "oneOf": [ - { - "required": [ - "pop_switch_dpid", - "pop_switch_port", - "wan_service_endpoint_id" - ] - }, - { - "required": [ - "pop_switch_dpid", - "pop_switch_port", - "wan_service_mapping_info" - ] - } - ] - } - } - }, - "required": ["datacenter_name", "pop_wan_mappings"] - } - } + "wim_port_mapping": wim_port_mapping_desc }, "required": ["wim_port_mapping"] } diff --git a/osm_ro/wim/tests/fixtures.py b/osm_ro/wim/tests/fixtures.py index 1b0da4bc..cb662ab0 100644 --- a/osm_ro/wim/tests/fixtures.py +++ b/osm_ro/wim/tests/fixtures.py @@ -167,7 +167,7 @@ def processed_port_mapping(wim, datacenter, return { 'wim_id': uuid('wim%d' % wim), 'datacenter_id': uuid('dc%d' % datacenter), - 'wan_pop_port_mappings': [ + 'pop_wan_mappings': [ {'pop_switch_dpid': pop_dpid, 'pop_switch_port': wim + 1 + i, 'wan_service_endpoint_id': diff --git a/osm_ro/wim/tests/test_http_handler.py b/osm_ro/wim/tests/test_http_handler.py index 04577e46..428b1ce8 100644 --- a/osm_ro/wim/tests/test_http_handler.py +++ b/osm_ro/wim/tests/test_http_handler.py @@ -137,6 +137,44 @@ class TestHttpHandler(TestCaseWithDatabasePerTest): merge_dicts(eg.wim(1), name='My-New-Name'), response.json['wim']) + def test_edit_wim__port_mappings(self): + # Given a WIM exists in the database + self.populate() + # when a PUT /wims/ request arrives + wim_id = uuid('wim1') + response = self.app.put_json( + '/wims/{}'.format(wim_id), { + 'wim': dict( + name='My-New-Name', + config={'wim_port_mapping': [{ + 'datacenter_name': 'dc0', + 'pop_wan_mappings': [{ + 'pop_switch_dpid': '00:AA:11:BB:22:CC:33:DD', + 'pop_switch_port': 1, + 'wan_service_mapping_info': { + 'mapping_type': 'dpid-port', + 'wan_switch_dpid': 'BB:BB:BB:BB:BB:BB:BB:0A', + 'wan_switch_port': 1 + } + }]}] + } + ) + } + ) + + # then the request should be well succeeded + self.assertEqual(response.status_code, OK) + # and the registered wim (wim1) should be present + self.assertDictContainsSubset( + merge_dicts(eg.wim(1), name='My-New-Name'), + response.json['wim']) + # and the port mappings hould be updated + mappings = response.json['wim']['config']['wim_port_mapping'] + self.assertEqual(len(mappings), 1) + self.assertEqual( + mappings[0]['pop_wan_mappings'][0]['pop_switch_dpid'], + '00:AA:11:BB:22:CC:33:DD') + def test_delete_wim(self): # Given a WIM exists in the database self.populate() @@ -178,6 +216,35 @@ class TestHttpHandler(TestCaseWithDatabasePerTest): self.assertEqual(response.status_code, OK) self.assertEqual(response.json['wim']['name'], 'wim999') + def test_create_wim__port_mappings(self): + self.populate() + # when a POST /wims request arrives with the right payload + response = self.app.post_json( + '/wims', { + 'wim': merge_dicts( + eg.wim(999), + config={'wim_port_mapping': [{ + 'datacenter_name': 'dc0', + 'pop_wan_mappings': [{ + 'pop_switch_dpid': 'AA:AA:AA:AA:AA:AA:AA:01', + 'pop_switch_port': 1, + 'wan_service_mapping_info': { + 'mapping_type': 'dpid-port', + 'wan_switch_dpid': 'BB:BB:BB:BB:BB:BB:BB:01', + 'wan_switch_port': 1 + } + }]}] + } + ) + } + ) + + # then the request should be well succeeded + self.assertEqual(response.status_code, OK) + self.assertEqual(response.json['wim']['name'], 'wim999') + self.assertEqual( + len(response.json['wim']['config']['wim_port_mapping']), 1) + def test_create_wim_account(self): # Given a WIM and a NFVO tenant exist but are not associated self.populate([{'wims': [eg.wim(0)]}, diff --git a/osm_ro/wim/tests/test_persistence.py b/osm_ro/wim/tests/test_persistence.py index d09a1163..e3e6cf61 100644 --- a/osm_ro/wim/tests/test_persistence.py +++ b/osm_ro/wim/tests/test_persistence.py @@ -170,7 +170,7 @@ class TestWimPersistence(TestCaseWithDatabasePerTest): self.assertIsNot(wim, None) # and a array of pairs 'wan' <> 'pop' connections - pairs = chain(*(m['wan_pop_port_mappings'] for m in mappings)) + pairs = chain(*(m['pop_wan_mappings'] for m in mappings)) self.assertEqual(len(list(pairs)), 2 * eg.NUM_WIMS) def test_get_wim_port_mappings_multiple(self): @@ -198,14 +198,14 @@ class TestWimPersistence(TestCaseWithDatabasePerTest): self.assertEqual(mappings[0]['wim_id'], uuid('wim0')) self.assertEqual(mappings[0]['datacenter_id'], uuid('dc0')) - self.assertEqual(len(mappings[0]['wan_pop_port_mappings']), 3) + self.assertEqual(len(mappings[0]['pop_wan_mappings']), 3) # when we retreive the mappings for more then one wim/datacenter # the grouping should still work properly mappings = self.persist.get_wim_port_mappings( wim=['wim0', 'wim1'], datacenter=['dc0', 'dc1']) self.assertEqual(len(mappings), 4) - pairs = chain(*(m['wan_pop_port_mappings'] for m in mappings)) + pairs = chain(*(m['pop_wan_mappings'] for m in mappings)) self.assertEqual(len(list(pairs)), 6) def test_get_actions_in_group(self):