From 903e2a8cd1dd9169048d1ad9dd8a566b2ae52395 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Ollivier?= Date: Thu, 21 Aug 2014 20:26:10 +0200 Subject: [PATCH] Add unit tests covering single operations to ODL This commit adds the remaining test cases (create and update operations) to fully cover sync_single_resource. It also defines the filter_* methods as static or class methods and removes their duplicate arguments. Change-Id: I6b6153ea577bd685576690559b1c389490ee525d Closes-Bug: #1325184 --- neutron/plugins/ml2/drivers/mechanism_odl.py | 103 +++++----- neutron/tests/unit/ml2/test_mechanism_odl.py | 192 +++++++++++++++++-- 2 files changed, 231 insertions(+), 64 deletions(-) diff --git a/neutron/plugins/ml2/drivers/mechanism_odl.py b/neutron/plugins/ml2/drivers/mechanism_odl.py index 771839a3151..dfe0c12c950 100644 --- a/neutron/plugins/ml2/drivers/mechanism_odl.py +++ b/neutron/plugins/ml2/drivers/mechanism_odl.py @@ -32,11 +32,8 @@ from neutron.plugins.ml2 import driver_api as api LOG = log.getLogger(__name__) -ODL_NETWORK = 'network' ODL_NETWORKS = 'networks' -ODL_SUBNET = 'subnet' ODL_SUBNETS = 'subnets' -ODL_PORT = 'port' ODL_PORTS = 'ports' odl_opts = [ @@ -99,11 +96,10 @@ class JsessionId(requests.auth.AuthBase): r = requests.get(self.url, auth=(self.username, self.password)) r.raise_for_status() except requests.exceptions.HTTPError as e: - raise OpendaylightAuthError(msg=_("Failed to authenticate with " - "OpenDaylight: %s") % e) + raise OpendaylightAuthError(msg="Failed to authenticate with " + "OpenDaylight: %s" % e) except requests.exceptions.Timeout as e: - raise OpendaylightAuthError(msg=_("Authentication Timed" - " Out: %s") % e) + raise OpendaylightAuthError(msg="Authentication Timed Out: %s" % e) jsessionid = r.cookies.get('JSESSIONID') jsessionidsso = r.cookies.get('JSESSIONIDSSO') @@ -181,24 +177,26 @@ class OpenDaylightMechanismDriver(api.MechanismDriver): else: self.sync_single_resource(operation, object_type, context) - def filter_create_network_attributes(self, network, context, dbcontext): + @staticmethod + def filter_create_network_attributes(network, context): """Filter out network attributes not required for a create.""" try_del(network, ['status', 'subnets']) - def filter_create_subnet_attributes(self, subnet, context, dbcontext): + @staticmethod + def filter_create_subnet_attributes(subnet, context): """Filter out subnet attributes not required for a create.""" pass - def filter_create_port_attributes(self, port, context, dbcontext): + @classmethod + def filter_create_port_attributes(cls, port, context): """Filter out port attributes not required for a create.""" - self.add_security_groups(context, dbcontext, port) + cls.add_security_groups(port, context) # TODO(kmestery): Converting to uppercase due to ODL bug # https://bugs.opendaylight.org/show_bug.cgi?id=477 port['mac_address'] = port['mac_address'].upper() try_del(port, ['status']) - def sync_resources(self, resource_name, collection_name, resources, - context, dbcontext, attr_filter): + def sync_resources(self, collection_name, context): """Sync objects from Neutron over to OpenDaylight. This will handle syncing networks, subnets, and ports from Neutron to @@ -206,6 +204,9 @@ class OpenDaylightMechanismDriver(api.MechanismDriver): valid for create API operations. """ to_be_synced = [] + dbcontext = context._plugin_context + obj_getter = getattr(context._plugin, 'get_%s' % collection_name) + resources = obj_getter(dbcontext) for resource in resources: try: urlpath = collection_name + '/' + resource['id'] @@ -213,11 +214,12 @@ class OpenDaylightMechanismDriver(api.MechanismDriver): except requests.exceptions.HTTPError as e: with excutils.save_and_reraise_exception() as ctx: if e.response.status_code == requests.codes.not_found: - attr_filter(resource, context, dbcontext) + attr_filter = self.create_object_map[collection_name] + attr_filter(resource, context) to_be_synced.append(resource) ctx.reraise = False - - key = resource_name if len(to_be_synced) == 1 else collection_name + key = collection_name[:-1] if len(to_be_synced) == 1 else ( + collection_name) # 400 errors are returned if an object exists, which we ignore. self.sendjson('post', collection_name, {key: to_be_synced}, @@ -232,45 +234,28 @@ class OpenDaylightMechanismDriver(api.MechanismDriver): """ if not self.out_of_sync: return - dbcontext = context._plugin_context - networks = context._plugin.get_networks(dbcontext) - subnets = context._plugin.get_subnets(dbcontext) - ports = context._plugin.get_ports(dbcontext) - - self.sync_resources(ODL_NETWORK, ODL_NETWORKS, networks, - context, dbcontext, - self.filter_create_network_attributes) - self.sync_resources(ODL_SUBNET, ODL_SUBNETS, subnets, - context, dbcontext, - self.filter_create_subnet_attributes) - self.sync_resources(ODL_PORT, ODL_PORTS, ports, - context, dbcontext, - self.filter_create_port_attributes) + for collection_name in [ODL_NETWORKS, ODL_SUBNETS, ODL_PORTS]: + self.sync_resources(collection_name, context) self.out_of_sync = False - def filter_update_network_attributes(self, network, context, dbcontext): + @staticmethod + def filter_update_network_attributes(network, context): """Filter out network attributes for an update operation.""" try_del(network, ['id', 'status', 'subnets', 'tenant_id']) - def filter_update_subnet_attributes(self, subnet, context, dbcontext): + @staticmethod + def filter_update_subnet_attributes(subnet, context): """Filter out subnet attributes for an update operation.""" try_del(subnet, ['id', 'network_id', 'ip_version', 'cidr', 'allocation_pools', 'tenant_id']) - def filter_update_port_attributes(self, port, context, dbcontext): + @classmethod + def filter_update_port_attributes(cls, port, context): """Filter out port attributes for an update operation.""" - self.add_security_groups(context, dbcontext, port) + cls.add_security_groups(port, context) try_del(port, ['network_id', 'id', 'status', 'mac_address', 'tenant_id', 'fixed_ips']) - create_object_map = {ODL_NETWORKS: filter_create_network_attributes, - ODL_SUBNETS: filter_create_subnet_attributes, - ODL_PORTS: filter_create_port_attributes} - - update_object_map = {ODL_NETWORKS: filter_update_network_attributes, - ODL_SUBNETS: filter_update_subnet_attributes, - ODL_PORTS: filter_update_port_attributes} - def sync_single_resource(self, operation, object_type, context): """Sync over a single resource from Neutron to OpenDaylight. @@ -292,7 +277,7 @@ class OpenDaylightMechanismDriver(api.MechanismDriver): method = 'put' attr_filter = self.update_object_map[object_type] resource = context.current.copy() - attr_filter(self, resource, context, context._plugin_context) + attr_filter(resource, context) # 400 errors are returned if an object exists, which we ignore. self.sendjson(method, urlpath, {object_type[:-1]: resource}, [requests.codes.bad_request]) @@ -300,20 +285,21 @@ class OpenDaylightMechanismDriver(api.MechanismDriver): with excutils.save_and_reraise_exception(): self.out_of_sync = True - def add_security_groups(self, context, dbcontext, port): + @staticmethod + def add_security_groups(port, context): """Populate the 'security_groups' field with entire records.""" + dbcontext = context._plugin_context groups = [context._plugin.get_security_group(dbcontext, sg) for sg in port['security_groups']] port['security_groups'] = groups def sendjson(self, method, urlpath, obj, ignorecodes=[]): """Send json to the OpenDaylight controller.""" - headers = {'Content-Type': 'application/json'} data = jsonutils.dumps(obj, indent=2) if obj else None url = '/'.join([self.url, urlpath]) - LOG.debug(_('ODL-----> sending URL (%s) <-----ODL') % url) - LOG.debug(_('ODL-----> sending JSON (%s) <-----ODL') % obj) + LOG.debug("Sending METHOD (%(method)s) URL (%(url)s) JSON (%(obj)s)", + {'method': method, 'url': url, 'obj': obj}) r = requests.request(method, url=url, headers=headers, data=data, auth=self.auth, timeout=self.timeout) @@ -324,8 +310,8 @@ class OpenDaylightMechanismDriver(api.MechanismDriver): r.raise_for_status() def bind_port(self, context): - LOG.debug(_("Attempting to bind port %(port)s on " - "network %(network)s"), + LOG.debug("Attempting to bind port %(port)s on " + "network %(network)s", {'port': context.current['id'], 'network': context.network.current['id']}) for segment in context.network.network_segments: @@ -334,12 +320,12 @@ class OpenDaylightMechanismDriver(api.MechanismDriver): self.vif_type, self.vif_details, status=n_const.PORT_STATUS_ACTIVE) - LOG.debug(_("Bound using segment: %s"), segment) + LOG.debug("Bound using segment: %s", segment) return else: - LOG.debug(_("Refusing to bind port for segment ID %(id)s, " - "segment %(seg)s, phys net %(physnet)s, and " - "network type %(nettype)s"), + LOG.debug("Refusing to bind port for segment ID %(id)s, " + "segment %(seg)s, phys net %(physnet)s, and " + "network type %(nettype)s", {'id': segment[api.ID], 'seg': segment[api.SEGMENTATION_ID], 'physnet': segment[api.PHYSICAL_NETWORK], @@ -354,3 +340,14 @@ class OpenDaylightMechanismDriver(api.MechanismDriver): network_type = segment[api.NETWORK_TYPE] return network_type in [constants.TYPE_LOCAL, constants.TYPE_GRE, constants.TYPE_VXLAN, constants.TYPE_VLAN] + + +OpenDaylightMechanismDriver.create_object_map = { + ODL_NETWORKS: OpenDaylightMechanismDriver.filter_create_network_attributes, + ODL_SUBNETS: OpenDaylightMechanismDriver.filter_create_subnet_attributes, + ODL_PORTS: OpenDaylightMechanismDriver.filter_create_port_attributes} + +OpenDaylightMechanismDriver.update_object_map = { + ODL_NETWORKS: OpenDaylightMechanismDriver.filter_update_network_attributes, + ODL_SUBNETS: OpenDaylightMechanismDriver.filter_update_subnet_attributes, + ODL_PORTS: OpenDaylightMechanismDriver.filter_update_port_attributes} diff --git a/neutron/tests/unit/ml2/test_mechanism_odl.py b/neutron/tests/unit/ml2/test_mechanism_odl.py index e743c345793..ff813778ef0 100644 --- a/neutron/tests/unit/ml2/test_mechanism_odl.py +++ b/neutron/tests/unit/ml2/test_mechanism_odl.py @@ -17,6 +17,7 @@ import mock import requests +from neutron.openstack.common import jsonutils from neutron.plugins.common import constants from neutron.plugins.ml2 import config as config from neutron.plugins.ml2 import driver_api as api @@ -122,11 +123,27 @@ class OpenDaylightMechanismTestPortsV2(test_plugin.TestPortsV2, class AuthMatcher(object): + def __eq__(self, obj): return (obj.username == config.cfg.CONF.ml2_odl.username and obj.password == config.cfg.CONF.ml2_odl.password) +class DataMatcher(object): + + def __init__(self, operation, object_type, context): + self._data = context.current.copy() + self._object_type = object_type + filter_map = getattr(mechanism_odl.OpenDaylightMechanismDriver, + '%s_object_map' % operation) + attr_filter = filter_map["%ss" % object_type] + attr_filter(self._data, context) + + def __eq__(self, s): + data = jsonutils.loads(s) + return self._data == data[self._object_type] + + class OpenDaylightMechanismDriverTestCase(base.BaseTestCase): def setUp(self): @@ -140,18 +157,80 @@ class OpenDaylightMechanismDriverTestCase(base.BaseTestCase): self.mech.initialize() @staticmethod - def _get_mock_delete_resource_context(): - current = {'id': '00000000-1111-2222-3333-444444444444'} + def _get_mock_network_operation_context(): + current = {'status': 'ACTIVE', + 'subnets': [], + 'name': 'net1', + 'provider:physical_network': None, + 'admin_state_up': True, + 'tenant_id': 'test-tenant', + 'provider:network_type': 'local', + 'router:external': False, + 'shared': False, + 'id': 'd897e21a-dfd6-4331-a5dd-7524fa421c3e', + 'provider:segmentation_id': None} context = mock.Mock(current=current) return context + @staticmethod + def _get_mock_subnet_operation_context(): + current = {'ipv6_ra_mode': None, + 'allocation_pools': [{'start': '10.0.0.2', + 'end': '10.0.1.254'}], + 'host_routes': [], + 'ipv6_address_mode': None, + 'cidr': '10.0.0.0/23', + 'id': '72c56c48-e9b8-4dcf-b3a7-0813bb3bd839', + 'name': '', + 'enable_dhcp': True, + 'network_id': 'd897e21a-dfd6-4331-a5dd-7524fa421c3e', + 'tenant_id': 'test-tenant', + 'dns_nameservers': [], + 'gateway_ip': '10.0.0.1', + 'ip_version': 4, + 'shared': False} + context = mock.Mock(current=current) + return context + + @staticmethod + def _get_mock_port_operation_context(): + current = {'status': 'DOWN', + 'binding:host_id': '', + 'allowed_address_pairs': [], + 'device_owner': 'fake_owner', + 'binding:profile': {}, + 'fixed_ips': [], + 'id': '72c56c48-e9b8-4dcf-b3a7-0813bb3bd839', + 'security_groups': ['2f9244b4-9bee-4e81-bc4a-3f3c2045b3d7'], + 'device_id': 'fake_device', + 'name': '', + 'admin_state_up': True, + 'network_id': 'c13bba05-eb07-45ba-ace2-765706b2d701', + 'tenant_id': 'bad_tenant_id', + 'binding:vif_details': {}, + 'binding:vnic_type': 'normal', + 'binding:vif_type': 'unbound', + 'mac_address': '12:34:56:78:21:b6'} + context = mock.Mock(current=current) + context._plugin.get_security_group = mock.Mock(return_value={}) + return context + + @classmethod + def _get_mock_operation_context(cls, object_type): + getter = getattr(cls, '_get_mock_%s_operation_context' % object_type) + return getter() + _status_code_msgs = { + 200: '', + 201: '', 204: '', + 400: '400 Client Error: Bad Request', 401: '401 Client Error: Unauthorized', 403: '403 Client Error: Forbidden', 404: '404 Client Error: Not Found', 409: '409 Client Error: Conflict', - 501: '501 Server Error: Not Implemented' + 501: '501 Server Error: Not Implemented', + 503: '503 Server Error: Service Unavailable', } @classmethod @@ -162,11 +241,9 @@ class OpenDaylightMechanismDriverTestCase(base.BaseTestCase): cls._status_code_msgs[status_code]))) return response - def _test_delete_resource_postcommit(self, object_type, status_code, - exc_class=None): + def _test_single_operation(self, method, context, status_code, + exc_class=None, *args, **kwargs): self.mech.out_of_sync = False - method = getattr(self.mech, 'delete_%s_postcommit' % object_type) - context = self._get_mock_delete_resource_context() request_response = self._get_mock_request_response(status_code) with mock.patch('requests.request', return_value=request_response) as mock_method: @@ -174,12 +251,105 @@ class OpenDaylightMechanismDriverTestCase(base.BaseTestCase): self.assertRaises(exc_class, method, context) else: method(context) + mock_method.assert_called_once_with( + headers={'Content-Type': 'application/json'}, auth=AuthMatcher(), + timeout=config.cfg.CONF.ml2_odl.timeout, *args, **kwargs) + + def _test_create_resource_postcommit(self, object_type, status_code, + exc_class=None): + method = getattr(self.mech, 'create_%s_postcommit' % object_type) + context = self._get_mock_operation_context(object_type) + url = '%s/%ss' % (config.cfg.CONF.ml2_odl.url, object_type) + kwargs = {'url': url, + 'data': DataMatcher('create', object_type, context)} + self._test_single_operation(method, context, status_code, exc_class, + 'post', **kwargs) + + def _test_update_resource_postcommit(self, object_type, status_code, + exc_class=None): + method = getattr(self.mech, 'update_%s_postcommit' % object_type) + context = self._get_mock_operation_context(object_type) url = '%s/%ss/%s' % (config.cfg.CONF.ml2_odl.url, object_type, context.current['id']) - mock_method.assert_called_once_with( - 'delete', url=url, headers={'Content-Type': 'application/json'}, - data=None, auth=AuthMatcher(), - timeout=config.cfg.CONF.ml2_odl.timeout) + kwargs = {'url': url, + 'data': DataMatcher('update', object_type, context)} + self._test_single_operation(method, context, status_code, exc_class, + 'put', **kwargs) + + def _test_delete_resource_postcommit(self, object_type, status_code, + exc_class=None): + method = getattr(self.mech, 'delete_%s_postcommit' % object_type) + context = self._get_mock_operation_context(object_type) + url = '%s/%ss/%s' % (config.cfg.CONF.ml2_odl.url, object_type, + context.current['id']) + kwargs = {'url': url, 'data': None} + self._test_single_operation(method, context, status_code, exc_class, + 'delete', **kwargs) + + def test_create_network_postcommit(self): + for status_code in (requests.codes.created, + requests.codes.bad_request): + self._test_create_resource_postcommit('network', status_code) + self._test_create_resource_postcommit( + 'network', requests.codes.unauthorized, + requests.exceptions.HTTPError) + + def test_create_subnet_postcommit(self): + for status_code in (requests.codes.created, + requests.codes.bad_request): + self._test_create_resource_postcommit('subnet', status_code) + for status_code in (requests.codes.unauthorized, + requests.codes.forbidden, + requests.codes.not_found, + requests.codes.conflict, + requests.codes.not_implemented): + self._test_create_resource_postcommit( + 'subnet', status_code, requests.exceptions.HTTPError) + + def test_create_port_postcommit(self): + for status_code in (requests.codes.created, + requests.codes.bad_request): + self._test_create_resource_postcommit('port', status_code) + for status_code in (requests.codes.unauthorized, + requests.codes.forbidden, + requests.codes.not_found, + requests.codes.conflict, + requests.codes.not_implemented, + requests.codes.service_unavailable): + self._test_create_resource_postcommit( + 'port', status_code, requests.exceptions.HTTPError) + + def test_update_network_postcommit(self): + for status_code in (requests.codes.ok, + requests.codes.bad_request): + self._test_update_resource_postcommit('network', status_code) + for status_code in (requests.codes.forbidden, + requests.codes.not_found): + self._test_update_resource_postcommit( + 'network', status_code, requests.exceptions.HTTPError) + + def test_update_subnet_postcommit(self): + for status_code in (requests.codes.ok, + requests.codes.bad_request): + self._test_update_resource_postcommit('subnet', status_code) + for status_code in (requests.codes.unauthorized, + requests.codes.forbidden, + requests.codes.not_found, + requests.codes.not_implemented): + self._test_update_resource_postcommit( + 'subnet', status_code, requests.exceptions.HTTPError) + + def test_update_port_postcommit(self): + for status_code in (requests.codes.ok, + requests.codes.bad_request): + self._test_update_resource_postcommit('port', status_code) + for status_code in (requests.codes.unauthorized, + requests.codes.forbidden, + requests.codes.not_found, + requests.codes.conflict, + requests.codes.not_implemented): + self._test_update_resource_postcommit( + 'port', status_code, requests.exceptions.HTTPError) def test_delete_network_postcommit(self): self._test_delete_resource_postcommit('network',