From c1ed203ccb816ac0a3a0e015d2790ed3aee04564 Mon Sep 17 00:00:00 2001 From: =?utf8?q?C=C3=A9dric=20Ollivier?= Date: Thu, 29 May 2014 12:01:28 +0200 Subject: [PATCH] Add delete operations for the ODL MechanismDriver This commit adds delete operations (networks, subnets and ports) for the ODL MechanismDriver. It also modifies sync_single_resource to reduce db operations. Change-Id: I03ca04c83ac2ef9c879fbd87e74bae495daea16d Closes-Bug: #1324450 Partial-Bug: #1325184 --- neutron/plugins/ml2/drivers/mechanism_odl.py | 66 +++++--------- neutron/tests/unit/ml2/test_mechanism_odl.py | 93 ++++++++++++++++++++ 2 files changed, 117 insertions(+), 42 deletions(-) diff --git a/neutron/plugins/ml2/drivers/mechanism_odl.py b/neutron/plugins/ml2/drivers/mechanism_odl.py index 416e870d1..771839a31 100644 --- a/neutron/plugins/ml2/drivers/mechanism_odl.py +++ b/neutron/plugins/ml2/drivers/mechanism_odl.py @@ -39,10 +39,6 @@ ODL_SUBNETS = 'subnets' ODL_PORT = 'port' ODL_PORTS = 'ports' -not_found_exception_map = {ODL_NETWORKS: n_exc.NetworkNotFound, - ODL_SUBNETS: n_exc.SubnetNotFound, - ODL_PORTS: n_exc.PortNotFound} - odl_opts = [ cfg.StrOpt('url', help=_("HTTP URL of OpenDaylight REST interface.")), @@ -183,7 +179,7 @@ class OpenDaylightMechanismDriver(api.MechanismDriver): if self.out_of_sync: self.sync_full(context) else: - self.sync_object(operation, object_type, context) + self.sync_single_resource(operation, object_type, context) def filter_create_network_attributes(self, network, context, dbcontext): """Filter out network attributes not required for a create.""" @@ -216,7 +212,7 @@ class OpenDaylightMechanismDriver(api.MechanismDriver): self.sendjson('get', urlpath, None) except requests.exceptions.HTTPError as e: with excutils.save_and_reraise_exception() as ctx: - if e.response.status_code == 404: + if e.response.status_code == requests.codes.not_found: attr_filter(resource, context, dbcontext) to_be_synced.append(resource) ctx.reraise = False @@ -224,14 +220,15 @@ class OpenDaylightMechanismDriver(api.MechanismDriver): key = resource_name 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}, [400]) + self.sendjson('post', collection_name, {key: to_be_synced}, + [requests.codes.bad_request]) @utils.synchronized('odl-sync-full') def sync_full(self, context): """Resync the entire database to ODL. Transition to the in-sync state on success. - Note: we only allow a single thead in here at a time. + Note: we only allow a single thread in here at a time. """ if not self.out_of_sync: return @@ -274,49 +271,34 @@ class OpenDaylightMechanismDriver(api.MechanismDriver): ODL_SUBNETS: filter_update_subnet_attributes, ODL_PORTS: filter_update_port_attributes} - def sync_single_resource(self, operation, object_type, obj_id, - context, attr_filter_create, attr_filter_update): + def sync_single_resource(self, operation, object_type, context): """Sync over a single resource from Neutron to OpenDaylight. Handle syncing a single operation over to OpenDaylight, and correctly filter attributes out which are not required for the requisite operation (create or update) being handled. """ - dbcontext = context._plugin_context - if operation == 'create': - urlpath = object_type - method = 'post' - else: - urlpath = object_type + '/' + obj_id - method = 'put' - try: - obj_getter = getattr(context._plugin, 'get_%s' % object_type[:-1]) - resource = obj_getter(dbcontext, obj_id) - except not_found_exception_map[object_type]: - LOG.debug(_('%(object_type)s not found (%(obj_id)s)'), - {'object_type': object_type.capitalize(), - 'obj_id': obj_id}) - else: - if operation == 'create': - attr_filter_create(self, resource, context, dbcontext) - elif operation == 'update': - attr_filter_update(self, resource, context, dbcontext) - try: + obj_id = context.current['id'] + if operation == 'delete': + self.sendjson('delete', object_type + '/' + obj_id, None) + else: + if operation == 'create': + urlpath = object_type + method = 'post' + attr_filter = self.create_object_map[object_type] + elif operation == 'update': + urlpath = object_type + '/' + obj_id + method = 'put' + attr_filter = self.update_object_map[object_type] + resource = context.current.copy() + attr_filter(self, resource, context, context._plugin_context) # 400 errors are returned if an object exists, which we ignore. self.sendjson(method, urlpath, {object_type[:-1]: resource}, - [400]) - except Exception: - with excutils.save_and_reraise_exception(): - self.out_of_sync = True - - def sync_object(self, operation, object_type, context): - """Synchronize the single modified record to ODL.""" - obj_id = context.current['id'] - - self.sync_single_resource(operation, object_type, obj_id, context, - self.create_object_map[object_type], - self.update_object_map[object_type]) + [requests.codes.bad_request]) + except Exception: + with excutils.save_and_reraise_exception(): + self.out_of_sync = True def add_security_groups(self, context, dbcontext, port): """Populate the 'security_groups' field with entire records.""" diff --git a/neutron/tests/unit/ml2/test_mechanism_odl.py b/neutron/tests/unit/ml2/test_mechanism_odl.py index 472387c20..22ebfbe7a 100644 --- a/neutron/tests/unit/ml2/test_mechanism_odl.py +++ b/neutron/tests/unit/ml2/test_mechanism_odl.py @@ -14,6 +14,9 @@ # under the License. # @author: Kyle Mestery, Cisco Systems, Inc. +import mock +import requests + from neutron.plugins.common import constants from neutron.plugins.ml2 import config as config from neutron.plugins.ml2 import driver_api as api @@ -115,3 +118,93 @@ class OpenDaylightMechanismTestSubnetsV2(test_plugin.TestSubnetsV2, class OpenDaylightMechanismTestPortsV2(test_plugin.TestPortsV2, OpenDaylightTestCase): pass + + +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 OpenDaylightMechanismDriverTestCase(base.BaseTestCase): + + def setUp(self): + super(OpenDaylightMechanismDriverTestCase, self).setUp() + config.cfg.CONF.set_override('mechanism_drivers', + ['logger', 'opendaylight'], 'ml2') + config.cfg.CONF.set_override('url', 'http://127.0.0.1:9999', 'ml2_odl') + config.cfg.CONF.set_override('username', 'someuser', 'ml2_odl') + config.cfg.CONF.set_override('password', 'somepass', 'ml2_odl') + self.mech = mechanism_odl.OpenDaylightMechanismDriver() + self.mech.initialize() + + @staticmethod + def _get_mock_delete_resource_context(): + current = {'id': '00000000-1111-2222-3333-444444444444'} + context = mock.Mock(current=current) + return context + + _status_code_msgs = { + 204: '', + 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' + } + + @classmethod + def _get_mock_request_response(cls, status_code): + response = mock.Mock(status_code=status_code) + response.raise_for_status = mock.Mock() if status_code < 400 else ( + mock.Mock(side_effect=requests.exceptions.HTTPError( + cls._status_code_msgs[status_code]))) + return response + + def _test_delete_resource_postcommit(self, object_type, status_code, + exc_class=None): + 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: + if exc_class is not None: + self.assertRaises(exc_class, method, context) + else: + method(context) + 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) + + def test_delete_network_postcommit(self): + self._test_delete_resource_postcommit('network', + requests.codes.no_content) + for status_code in (requests.codes.unauthorized, + requests.codes.not_found, + requests.codes.conflict): + self._test_delete_resource_postcommit( + 'network', status_code, requests.exceptions.HTTPError) + + def test_delete_subnet_postcommit(self): + self._test_delete_resource_postcommit('subnet', + requests.codes.no_content) + for status_code in (requests.codes.unauthorized, + requests.codes.not_found, + requests.codes.conflict, + requests.codes.not_implemented): + self._test_delete_resource_postcommit( + 'subnet', status_code, requests.exceptions.HTTPError) + + def test_delete_port_postcommit(self): + self._test_delete_resource_postcommit('port', + requests.codes.no_content) + for status_code in (requests.codes.unauthorized, + requests.codes.forbidden, + requests.codes.not_found, + requests.codes.not_implemented): + self._test_delete_resource_postcommit( + 'port', status_code, requests.exceptions.HTTPError) -- 2.45.2