Merge "Add unit tests covering single operations to ODL"

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 18b12a65a6..05ca33bb7f 100644
--- a/neutron/plugins/ml2/drivers/mechanism_odl.py
+++ b/neutron/plugins/ml2/drivers/mechanism_odl.py
@@ -30,11 +30,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 = [
@@ -97,11 +94,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')
@@ -179,24 +175,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
@@ -204,6 +202,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']
@@ -211,11 +212,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},
@@ -230,45 +232,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.
 
@@ -290,7 +275,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])
@@ -298,20 +283,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)
@@ -322,8 +308,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:
@@ -332,12 +318,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],
@@ -352,3 +338,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 3679f9f1fd..8d87eb5a61 100644
--- a/neutron/tests/unit/ml2/test_mechanism_odl.py
+++ b/neutron/tests/unit/ml2/test_mechanism_odl.py
@@ -16,6 +16,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
@@ -121,11 +122,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):
@@ -139,18 +156,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
@@ -161,11 +240,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:
@@ -173,12 +250,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',