From 6e816cd16f11eabea6eed9b6f17ce2adda7cf4b2 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Fri, 7 Dec 2018 12:51:45 +0000 Subject: [PATCH] Change WAN Actions to use VIM config rules Extract switch ID and port by default from vim.config.external_connections, as discussed in the WIM implementation meeting (21/Nov/2018). Change-Id: I95cc5fb0d6ad2822ef4bd1b80e7652bfabf34120 Signed-off-by: Anderson Bravalheri --- osm_ro/wim/errors.py | 9 ++ osm_ro/wim/tests/fixtures.py | 47 ++++++-- osm_ro/wim/tests/test_actions.py | 180 ++++++++++++++++++++++-------- osm_ro/wim/wan_link_actions.py | 183 ++++++++++++++++++++++++------- 4 files changed, 324 insertions(+), 95 deletions(-) diff --git a/osm_ro/wim/errors.py b/osm_ro/wim/errors.py index 16c53b55..ca8c2b73 100644 --- a/osm_ro/wim/errors.py +++ b/osm_ro/wim/errors.py @@ -178,3 +178,12 @@ class WimAccountNotActive(HttpMappedError, KeyError): message += ('\nThe thread responsible for processing the actions have ' 'suddenly stopped, or have never being spawned') super(WimAccountNotActive, self).__init__(message, http_code) + + +class NoExternalPortFound(HttpMappedError): + """No external port associated to the instance_net""" + + def __init__(self, instance_net): + super(NoExternalPortFound, self).__init__( + '{} uuid({})'.format(self.__class__.__doc__, instance_net['uuid']), + http_code=Not_Found) diff --git a/osm_ro/wim/tests/fixtures.py b/osm_ro/wim/tests/fixtures.py index 1b52e497..1b0da4bc 100644 --- a/osm_ro/wim/tests/fixtures.py +++ b/osm_ro/wim/tests/fixtures.py @@ -36,7 +36,9 @@ from __future__ import unicode_literals import json +from itertools import izip from time import time +from textwrap import wrap from six.moves import range @@ -86,11 +88,28 @@ def wim_set(identifier=0, tenant=0): ] -def datacenter(identifier): +def _datacenter_to_switch_port(dc_id, port=None): + digits = 16 + switch = ':'.join(wrap(('%0' + str(digits) + 'x') % int(dc_id), 2)) + return (switch, str((port or int(dc_id)) + 1)) + + +def datacenter(identifier, external_ports_config=False): + config = '' if not external_ports_config else json.dumps({ + 'external_connections': [ + {'condition': { + 'provider:physical_network': 'provider', + 'encapsulation_type': 'vlan'}, + 'vim_external_port': + dict(izip(('switch', 'port'), + _datacenter_to_switch_port(identifier)))} + ]}) + return {'uuid': uuid('dc%d' % identifier), 'name': 'dc%d' % identifier, 'type': 'openvim', - 'vim_url': 'localhost'} + 'vim_url': 'localhost', + 'config': config} def datacenter_account(datacenter, tenant): @@ -107,7 +126,7 @@ def datacenter_tenant_association(datacenter, tenant): uuid('dc-account%d%d' % (tenant, datacenter))} -def datacenter_set(identifier, tenant): +def datacenter_set(identifier=0, tenant=0): """Records necessary to create a datacenter and connect it to a tenant""" return [ {'datacenters': [datacenter(identifier)]}, @@ -119,17 +138,19 @@ def datacenter_set(identifier, tenant): def wim_port_mapping(wim, datacenter, - pop_dpid='AA:AA:AA:AA:AA:AA:AA:AA', pop_port=0, - wan_dpid='BB:BB:BB:BB:BB:BB:BB:BB', wan_port=0): + pop_dpid='AA:AA:AA:AA:AA:AA:AA:AA', pop_port=None, + wan_dpid='BB:BB:BB:BB:BB:BB:BB:BB', wan_port=None): mapping_info = {'mapping_type': 'dpid-port', 'wan_switch_dpid': wan_dpid, - 'wan_switch_port': wan_port + datacenter + 1} + 'wan_switch_port': (str(wan_port) if wan_port else + str(int(datacenter) + int(wim) + 1))} id_ = 'dpid-port|' + sha1(json.dumps(mapping_info, sort_keys=True)) return {'wim_id': uuid('wim%d' % wim), 'datacenter_id': uuid('dc%d' % datacenter), 'pop_switch_dpid': pop_dpid, - 'pop_switch_port': pop_port + wim + 1, + 'pop_switch_port': (str(pop_port) if pop_port else + str(int(datacenter) + int(wim) + 1)), # ^ Datacenter router have one port managed by each WIM 'wan_service_endpoint_id': id_, # ^ WIM managed router have one port connected to each DC @@ -161,7 +182,8 @@ def processed_port_mapping(wim, datacenter, def consistent_set(num_wims=NUM_WIMS, num_tenants=NUM_TENANTS, - num_datacenters=NUM_DATACENTERS): + num_datacenters=NUM_DATACENTERS, + external_ports_config=False): return [ {'nfvo_tenants': [tenant(i) for i in range(num_tenants)]}, {'wims': [wim(j) for j in range(num_wims)]}, @@ -176,7 +198,7 @@ def consistent_set(num_wims=NUM_WIMS, num_tenants=NUM_TENANTS, for j in range(num_wims) ]}, {'datacenters': [ - datacenter(k) + datacenter(k, external_ports_config) for k in range(num_datacenters) ]}, {'datacenter_tenants': [ @@ -190,14 +212,15 @@ def consistent_set(num_wims=NUM_WIMS, num_tenants=NUM_TENANTS, for k in range(num_datacenters) ]}, {'wim_port_mappings': [ - wim_port_mapping(j, k) + (wim_port_mapping(j, k, *_datacenter_to_switch_port(k)) + if external_ports_config else wim_port_mapping(j, k)) for j in range(num_wims) for k in range(num_datacenters) ]}, ] -def instance_nets(num_datacenters=2, num_links=2): +def instance_nets(num_datacenters=2, num_links=2, status='BUILD'): """Example of multi-site deploy with N datacenters and M WAN links between them (e.g M = 2 -> back and forth) """ @@ -209,7 +232,7 @@ def instance_nets(num_datacenters=2, num_links=2): # ^ instance_scenario_id == NS Record id 'sce_net_id': uuid('vld%d' % l), # ^ scenario net id == VLD id - 'status': 'BUILD', + 'status': status, 'vim_net_id': None, 'created': True} for k in range(num_datacenters) diff --git a/osm_ro/wim/tests/test_actions.py b/osm_ro/wim/tests/test_actions.py index 920182bd..cee3c96f 100644 --- a/osm_ro/wim/tests/test_actions.py +++ b/osm_ro/wim/tests/test_actions.py @@ -47,7 +47,7 @@ from ...tests.db_helpers import ( disable_foreign_keys, uuid, ) -from ..persistence import WimPersistence +from ..persistence import WimPersistence, preprocess_record from ..wan_link_actions import WanLinkCreate, WanLinkDelete from ..wimconn import WimConnectorError @@ -118,22 +118,118 @@ class TestCreate(TestActionsWithDb): self.assertIn('issue with the local networks', action.error_msg) self.assertIn('SCHEDULED_DELETION', action.error_msg) + def prepare_create__rules(self): + db_state = eg.consistent_set(num_wims=1, num_tenants=1, + num_datacenters=2, + external_ports_config=True) + + instance_nets = eg.instance_nets(num_datacenters=2, num_links=1, + status='ACTIVE') + for i, net in enumerate(instance_nets): + net['vim_info'] = {} + net['vim_info']['provider:physical_network'] = 'provider' + net['vim_info']['encapsulation_type'] = 'vlan' + net['vim_info']['encapsulation_id'] = i + net['sdn_net_id'] = uuid('sdn-net%d' % i) + + instance_action = eg.instance_action(action_id='ACTION-000') + + db_state += [ + {'instance_wim_nets': eg.instance_wim_nets()}, + {'instance_nets': [preprocess_record(r) for r in instance_nets]}, + {'instance_actions': instance_action}] + + action = WanLinkCreate( + eg.wim_actions('CREATE', action_id='ACTION-000')[0]) + # --> ensure it is in the database for updates --> # + action_record = action.as_record() + action_record['extra'] = json.dumps(action_record['extra']) + db_state += [{'vim_wim_actions': action_record}] + + return db_state, action + + @disable_foreign_keys + def test_process__rules(self): + # Given we want 1 WAN link between 2 datacenters + # and the local network in each datacenter is already created + db_state, action = self.prepare_create__rules() + self.populate(db_state) + + instance_action = self.persist.get_by_uuid( + 'instance_actions', action.instance_action_id) + number_done = instance_action['number_done'] + number_failed = instance_action['number_failed'] + + # If the connector works fine + with patch.object(self.connector, 'create_connectivity_service', + lambda *_, **__: (uuid('random-id'), None)): + # When we try to process a CREATE action that refers to the same + # instance_scenario_id and sce_net_id + action.process(self.connector, self.persist, self.ovim) + + # Then the action should be succeeded + db_action = self.persist.query_one('vim_wim_actions', WHERE={ + 'instance_action_id': action.instance_action_id, + 'task_index': action.task_index}) + self.assertEqual(db_action['status'], 'DONE') + + instance_action = self.persist.get_by_uuid( + 'instance_actions', action.instance_action_id) + self.assertEqual(instance_action['number_done'], number_done + 1) + self.assertEqual(instance_action['number_failed'], number_failed) + + @disable_foreign_keys + def test_process__rules_fail(self): + # Given we want 1 WAN link between 2 datacenters + # and the local network in each datacenter is already created + db_state, action = self.prepare_create__rules() + self.populate(db_state) + + instance_action = self.persist.get_by_uuid( + 'instance_actions', action.instance_action_id) + number_done = instance_action['number_done'] + number_failed = instance_action['number_failed'] + + # If the connector raises an error + with patch.object(self.connector, 'create_connectivity_service', + MagicMock(side_effect=WimConnectorError('foobar'))): + # When we try to process a CREATE action that refers to the same + # instance_scenario_id and sce_net_id + action.process(self.connector, self.persist, self.ovim) + + # Then the action should be fail + db_action = self.persist.query_one('vim_wim_actions', WHERE={ + 'instance_action_id': action.instance_action_id, + 'task_index': action.task_index}) + self.assertEqual(db_action['status'], 'FAILED') + + instance_action = self.persist.get_by_uuid( + 'instance_actions', action.instance_action_id) + self.assertEqual(instance_action['number_done'], number_done) + self.assertEqual(instance_action['number_failed'], number_failed + 1) + def prepare_create__sdn(self): - db_state = [{'nfvo_tenants': eg.tenant()}] + eg.wim_set() + db_state = eg.consistent_set(num_wims=1, num_tenants=1, + num_datacenters=2, + external_ports_config=False) + + # Make sure all port_mappings are predictable + switch = 'AA:AA:AA:AA:AA:AA:AA:AA' + port = 1 + port_mappings = next(r['wim_port_mappings'] + for r in db_state if 'wim_port_mappings' in r) + for mapping in port_mappings: + mapping['pop_switch_dpid'] = switch + mapping['pop_switch_port'] = port - instance_nets = eg.instance_nets(num_datacenters=2, num_links=1) - port_mappings = [ - eg.wim_port_mapping(0, 0), - eg.wim_port_mapping(0, 1) - ] instance_action = eg.instance_action(action_id='ACTION-000') + instance_nets = eg.instance_nets(num_datacenters=2, num_links=1, + status='ACTIVE') for i, net in enumerate(instance_nets): - net['status'] = 'ACTIVE' net['sdn_net_id'] = uuid('sdn-net%d' % i) db_state += [{'instance_nets': instance_nets}, {'instance_wim_nets': eg.instance_wim_nets()}, - {'wim_port_mappings': port_mappings}, {'instance_actions': instance_action}] action = WanLinkCreate( @@ -141,15 +237,21 @@ class TestCreate(TestActionsWithDb): # --> ensure it is in the database for updates --> # action_record = action.as_record() action_record['extra'] = json.dumps(action_record['extra']) - self.populate([{'vim_wim_actions': action_record}]) + db_state += [{'vim_wim_actions': action_record}] - return db_state, action + ovim_patch = patch.object( + self.ovim, 'get_ports', MagicMock(return_value=[{ + 'switch_dpid': switch, + 'switch_port': port, + }])) + + return db_state, action, ovim_patch @disable_foreign_keys def test_process__sdn(self): # Given we want 1 WAN link between 2 datacenters # and the local network in each datacenter is already created - db_state, action = self.prepare_create__sdn() + db_state, action, ovim_patch = self.prepare_create__sdn() self.populate(db_state) instance_action = self.persist.get_by_uuid( @@ -161,12 +263,6 @@ class TestCreate(TestActionsWithDb): self.connector, 'create_connectivity_service', lambda *_, **__: (uuid('random-id'), None)) - ovim_patch = patch.object( - self.ovim, 'get_ports', MagicMock(return_value=[{ - 'switch_dpid': 'AA:AA:AA:AA:AA:AA:AA:AA', - 'switch_port': 1, - }])) - # If the connector works fine with connector_patch, ovim_patch: # When we try to process a CREATE action that refers to the same @@ -188,7 +284,7 @@ class TestCreate(TestActionsWithDb): def test_process__sdn_fail(self): # Given we want 1 WAN link between 2 datacenters # and the local network in each datacenter is already created - db_state, action = self.prepare_create__sdn() + db_state, action, ovim_patch = self.prepare_create__sdn() self.populate(db_state) instance_action = self.persist.get_by_uuid( @@ -200,12 +296,6 @@ class TestCreate(TestActionsWithDb): self.connector, 'create_connectivity_service', MagicMock(side_effect=WimConnectorError('foobar'))) - ovim_patch = patch.object( - self.ovim, 'get_ports', MagicMock(return_value=[{ - 'switch_dpid': 'AA:AA:AA:AA:AA:AA:AA:AA', - 'switch_port': 1, - }])) - # If the connector throws an error with connector_patch, ovim_patch: # When we try to process a CREATE action that refers to the same @@ -243,29 +333,32 @@ class TestDelete(TestActionsWithDb): assert action.is_done def prepare_delete(self): - db_state = [{'nfvo_tenants': eg.tenant()}] + eg.wim_set() + db_state = eg.consistent_set(num_wims=1, num_tenants=1, + num_datacenters=2, + external_ports_config=True) - instance_nets = eg.instance_nets(num_datacenters=2, num_links=1) - port_mappings = [ - eg.wim_port_mapping(0, 0), - eg.wim_port_mapping(0, 1) - ] - instance_action = eg.instance_action(action_id='ACTION-000') + instance_nets = eg.instance_nets(num_datacenters=2, num_links=1, + status='ACTIVE') for i, net in enumerate(instance_nets): - net['status'] = 'ACTIVE' + net['vim_info'] = {} + net['vim_info']['provider:physical_network'] = 'provider' + net['vim_info']['encapsulation_type'] = 'vlan' + net['vim_info']['encapsulation_id'] = i net['sdn_net_id'] = uuid('sdn-net%d' % i) - db_state += [{'instance_nets': instance_nets}, - {'instance_wim_nets': eg.instance_wim_nets()}, - {'wim_port_mappings': port_mappings}, - {'instance_actions': instance_action}] + instance_action = eg.instance_action(action_id='ACTION-000') + + db_state += [ + {'instance_wim_nets': eg.instance_wim_nets()}, + {'instance_nets': [preprocess_record(r) for r in instance_nets]}, + {'instance_actions': instance_action}] action = WanLinkDelete( eg.wim_actions('DELETE', action_id='ACTION-000')[0]) # --> ensure it is in the database for updates --> # action_record = action.as_record() action_record['extra'] = json.dumps(action_record['extra']) - self.populate([{'vim_wim_actions': action_record}]) + db_state += [{'vim_wim_actions': action_record}] return db_state, action @@ -331,8 +424,9 @@ class TestDelete(TestActionsWithDb): def test_create_and_delete(self): # Given a CREATE action was well succeeded db_state, delete_action = self.prepare_delete() - delete_action.save(self.persist, task_index=1) self.populate(db_state) + + delete_action.save(self.persist, task_index=1) create_action = self.create_action() connector_patch = patch.multiple( @@ -341,13 +435,7 @@ class TestDelete(TestActionsWithDb): create_connectivity_service=( lambda *_, **__: (uuid('random-id'), None))) - ovim_patch = patch.object( - self.ovim, 'get_ports', MagicMock(return_value=[{ - 'switch_dpid': 'AA:AA:AA:AA:AA:AA:AA:AA', - 'switch_port': 1, - }])) - - with connector_patch, ovim_patch: + with connector_patch: # , ovim_patch: create_action.process(self.connector, self.persist, self.ovim) # When we try to process a CREATE action that refers to the same diff --git a/osm_ro/wim/wan_link_actions.py b/osm_ro/wim/wan_link_actions.py index 61c6dd9f..034e4159 100644 --- a/osm_ro/wim/wan_link_actions.py +++ b/osm_ro/wim/wan_link_actions.py @@ -33,15 +33,19 @@ ## # pylint: disable=E1101,E0203,W0201 import json +from pprint import pformat +from sys import exc_info from time import time +from six import reraise + from ..utils import filter_dict_keys as filter_keys from ..utils import merge_dicts, remove_none_items, safe_get, truncate from .actions import CreateAction, DeleteAction, FindAction from .errors import ( InconsistentState, - MultipleRecordsFound, NoRecordFound, + NoExternalPortFound ) from wimconn import WimConnectorError @@ -157,60 +161,164 @@ class WanLinkCreate(RefreshMixin, CreateAction): Returns: dict: Record representing the wan_port_mapping associated to the given instance_net. The expected fields are: - **wim_id**, **datacenter_id**, **pop_switch_id** (the local + **wim_id**, **datacenter_id**, **pop_switch_dpid** (the local network is expected to be connected at this switch), **pop_switch_port**, **wan_service_endpoint_id**, **wan_service_mapping_info**. """ - wim_account = persistence.get_wim_account_by(uuid=self.wim_account_id) - - # TODO: make more generic to support networks that are not created with - # the SDN assist. This method should have a consistent way of getting - # the endpoint for all different types of networks used in the VIM - # (provider networks, SDN assist, overlay networks, ...) - if instance_net.get('sdn_net_id'): - return self._get_connection_point_info_sdn( - persistence, ovim, instance_net, wim_account['wim_id']) - else: - raise InconsistentState( - 'The field `instance_nets.sdn_net_id` was expected to be ' - 'found in the database for the record %s after the network ' - 'become active, but it is still NULL', instance_net['uuid']) + # First, we need to find a route from the datacenter to the outside + # world. For that, we can use the rules given in the datacenter + # configuration: + datacenter_id = instance_net['datacenter_id'] + datacenter = persistence.get_datacenter_by(datacenter_id) + rules = safe_get(datacenter, 'config.external_connections', {}) or {} + vim_info = instance_net.get('vim_info', {}) or {} + # Alternatively, we can look for it, using the SDN assist + external_port = (self._evaluate_rules(rules, vim_info) or + self._get_port_sdn(ovim, instance_net)) + + if not external_port: + raise NoExternalPortFound(instance_net) + + # Then, we find the WAN switch that is connected to this external port + try: + wim_account = persistence.get_wim_account_by( + uuid=self.wim_account_id) + + criteria = { + 'wim_id': wim_account['wim_id'], + 'pop_switch_dpid': external_port[0], + 'pop_switch_port': external_port[1], + 'datacenter_id': datacenter_id} + + wan_port_mapping = persistence.query_one( + FROM='wim_port_mappings', + WHERE=criteria) + except NoRecordFound: + ex = InconsistentState('No WIM port mapping found:' + 'wim_account: {}\ncriteria:\n{}'.format( + self.wim_account_id, pformat(criteria))) + reraise(ex.__class__, ex, exc_info()[2]) + + # It is important to return encapsulation information if present + mapping = merge_dicts( + wan_port_mapping.get('wan_service_mapping_info'), + filter_keys(vim_info, ('encapsulation_type', 'encapsulation_id')) + ) + + return merge_dicts(wan_port_mapping, wan_service_mapping_info=mapping) - def _get_connection_point_info_sdn(self, persistence, ovim, - instance_net, wim_id): + def _get_port_sdn(self, ovim, instance_net): criteria = {'net_id': instance_net['sdn_net_id']} - local_port_mapping = ovim.get_ports(filter=criteria) + try: + local_port_mapping = ovim.get_ports(filter=criteria) + + if local_port_mapping: + return (local_port_mapping[0]['switch_dpid'], + local_port_mapping[0]['switch_port']) + except: # noqa + self.logger.exception('Problems when calling OpenVIM') + + self.logger.debug('No ports found using criteria:\n%r\n.', criteria) + return None + + def _evaluate_rules(self, rules, vim_info): + """Given a ``vim_info`` dict from a ``instance_net`` record, evaluate + the set of rules provided during the VIM/datacenter registration to + determine an external port used to connect that VIM/datacenter to + other ones where different parts of the NS will be instantiated. + + For example, considering a VIM/datacenter is registered like the + following:: + + vim_record = { + "uuid": ... + ... # Other properties associated with the VIM/datacenter + "config": { + ... # Other configuration + "external_connections": [ + { + "condition": { + "provider:physical_network": "provider_net1", + ... # This method will look up all the keys listed here + # in the instance_nets.vim_info dict and compare the + # values. When all the values match, the associated + # vim_external_port will be selected. + }, + "vim_external_port": {"switch": "switchA", "port": "portB"} + }, + ... # The user can provide as many rules as needed, however + # only the first one to match will be applied. + ] + } + } + + When an ``instance_net`` record is instantiated in that datacenter with + the following information:: + + instance_net = { + "uuid": ... + ... + "vim_info": { + ... + "provider_physical_network": "provider_net1", + } + } - if len(local_port_mapping) > 1: - raise MultipleRecordsFound(criteria, 'ovim.ports') - local_port_mapping = local_port_mapping[0] + Then, ``switchA`` and ``portB`` will be used to stablish the WAN + connection. - criteria = { - 'wim_id': wim_id, - 'pop_switch_dpid': local_port_mapping['switch_dpid'], - 'pop_switch_port': local_port_mapping['switch_port'], - 'datacenter_id': instance_net['datacenter_id']} + Arguments: + rules (list): Set of dicts containing the keys ``condition`` and + ``vim_external_port``. This list should be extracted from + ``vim['config']['external_connections']`` (as stored in the + database). + vim_info (dict): Information given by the VIM Connector, against + which the rules will be evaluated. + + Returns: + tuple: switch id (local datacenter switch) and port or None if + the rule does not match. + """ + rule = next((r for r in rules if self._evaluate_rule(r, vim_info)), {}) + if 'vim_external_port' not in rule: + self.logger.debug('No external port found.\n' + 'rules:\n%r\nvim_info:\n%r\n\n', rules, vim_info) + return None - wan_port_mapping = persistence.query_one( - FROM='wim_port_mappings', - WHERE=criteria) + return (rule['vim_external_port']['switch'], + rule['vim_external_port']['port']) - if local_port_mapping.get('vlan'): - wan_port_mapping['wan_service_mapping_info']['vlan'] = ( - local_port_mapping['vlan']) + @staticmethod + def _evaluate_rule(rule, vim_info): + """Evaluate the conditions from a single rule to ``vim_info`` and + determine if the rule should be applicable or not. - return wan_port_mapping + Please check :obj:`~._evaluate_rules` for more information. + + Arguments: + rule (dict): Data structure containing the keys ``condition`` and + ``vim_external_port``. This should be one of the elements in + ``vim['config']['external_connections']`` (as stored in the + database). + vim_info (dict): Information given by the VIM Connector, against + which the rules will be evaluated. + + Returns: + True or False: If all the conditions are met. + """ + condition = rule.get('condition', {}) or {} + return all(safe_get(vim_info, k) == v for k, v in condition.items()) @staticmethod def _derive_connection_point(wan_info): point = {'service_endpoint_id': wan_info['wan_service_endpoint_id']} # TODO: Cover other scenarios, e.g. VXLAN. details = wan_info.get('wan_service_mapping_info', {}) - if 'vlan' in details: + if details.get('encapsulation_type') == 'vlan': point['service_endpoint_encapsulation_type'] = 'dot1q' point['service_endpoint_encapsulation_info'] = { - 'vlan': details['vlan'] + 'vlan': details['encapsulation_id'] } else: point['service_endpoint_encapsulation_type'] = 'none' @@ -247,7 +355,8 @@ class WanLinkCreate(RefreshMixin, CreateAction): connection_points # TODO: other properties, e.g. bandwidth ) - except (WimConnectorError, InconsistentState) as ex: + except (WimConnectorError, InconsistentState, + NoExternalPortFound) as ex: self.logger.exception(ex) return self.fail( persistence, -- 2.17.1