]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Notify about port create/update unconditionally
authorOleg Bondarev <obondarev@mirantis.com>
Tue, 1 Dec 2015 08:47:05 +0000 (11:47 +0300)
committerSwaminathan Vasudevan <swaminathan.vasudevan@hp.com>
Wed, 2 Dec 2015 19:51:50 +0000 (11:51 -0800)
The notification about port create/update should be done no matter
if host (or any other field) was changed or not - it should be up to
handler to decide how to handle it.

This also fixes the bug when there was actually no notification to l3
dvr agent on compute node on new port creation:
_get_host_port_if_changed() always returned None - this is a regression
from commit 2ee08c3464c53abaf9bc5493132ad7958611e3b8

Closes-Bug: #1521524
Change-Id: I5bb416d2aaab632526f06a5620c0a4ea96340d21

neutron/plugins/ml2/plugin.py
neutron/tests/unit/plugins/ml2/test_plugin.py

index cd0dcde06571af53e9ad15214c76bc442561afa2..d35a9350093451ffb057decd1bf263b32267100a 100644 (file)
@@ -211,12 +211,6 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
                 if self.type_manager.network_matches_filters(network, filters)
                 ]
 
-    def _get_host_port_if_changed(self, mech_context, attrs):
-        binding = mech_context._binding
-        if attrs and portbindings.HOST_ID in attrs:
-            if binding.host != attrs.get(portbindings.HOST_ID):
-                return mech_context.current
-
     def _check_mac_update_allowed(self, orig_port, port, binding):
         unplugged_types = (portbindings.VIF_TYPE_BINDING_FAILED,
                            portbindings.VIF_TYPE_UNBOUND)
@@ -1054,11 +1048,9 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
         return result, mech_context
 
     def create_port(self, context, port):
-        attrs = port[attributes.PORT]
         result, mech_context = self._create_port_db(context, port)
-        new_host_port = self._get_host_port_if_changed(mech_context, attrs)
         # notify any plugin that is interested in port create events
-        kwargs = {'context': context, 'port': new_host_port}
+        kwargs = {'context': context, 'port': result}
         registry.notify(resources.PORT, events.AFTER_CREATE, self, **kwargs)
 
         try:
@@ -1094,9 +1086,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
         for obj in objects:
             attrs = obj['attributes']
             if attrs and attrs.get(portbindings.HOST_ID):
-                new_host_port = self._get_host_port_if_changed(
-                    obj['mech_context'], attrs)
-                kwargs = {'context': context, 'port': new_host_port}
+                kwargs = {'context': context, 'port': obj['result']}
                 registry.notify(
                     resources.PORT, events.AFTER_CREATE, self, **kwargs)
 
@@ -1200,8 +1190,6 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
             mech_context = driver_context.PortContext(
                 self, context, updated_port, network, binding, levels,
                 original_port=original_port)
-            new_host_port = self._get_host_port_if_changed(
-                mech_context, attrs)
             need_port_update_notify |= self._process_port_binding(
                 mech_context, attrs)
             # For DVR router interface ports we need to retrieve the
@@ -1233,7 +1221,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
         # Notifications must be sent after the above transaction is complete
         kwargs = {
             'context': context,
-            'port': new_host_port,
+            'port': updated_port,
             'mac_address_updated': mac_address_updated,
             'original_port': original_port,
         }
index b4e906000cf91be4b8e878fa34de23791ac4f809..c5e4f83e100712066983812d771e4ffb736644c2 100644 (file)
@@ -1658,14 +1658,12 @@ class TestMl2PluginCreateUpdateDeletePort(base.BaseTestCase):
         exit = transaction.__exit__.call_count
         self.assertEqual(enter, exit)
 
-    def _create_plugin_for_create_update_port(self, new_host_port):
+    def _create_plugin_for_create_update_port(self):
         plugin = ml2_plugin.Ml2Plugin()
         plugin.extension_manager = mock.Mock()
         plugin.type_manager = mock.Mock()
         plugin.mechanism_manager = mock.Mock()
         plugin.notifier = mock.Mock()
-        plugin._get_host_port_if_changed = mock.Mock(
-            return_value=new_host_port)
         plugin._check_mac_update_allowed = mock.Mock(return_value=True)
         plugin._extend_availability_zone = mock.Mock()
 
@@ -1674,76 +1672,19 @@ class TestMl2PluginCreateUpdateDeletePort(base.BaseTestCase):
 
         return plugin
 
-    def _test__get_host_port_if_changed(
-        self, mech_context, attrs=None, expected_retval=None):
-        with mock.patch.object(ml2_plugin.Ml2Plugin,
-                               '__init__',
-                               return_value=None):
-            plugin = ml2_plugin.Ml2Plugin()
-            test_return = plugin._get_host_port_if_changed(mech_context, attrs)
-            self.assertEqual(expected_retval, test_return)
-
-    def test__get_host_port_if_changed_no_attrs(self):
-        mech_context = mock.Mock()
-        mech_context._binding.host = 'Host-1'
-        self._test__get_host_port_if_changed(
-            mech_context, attrs=None, expected_retval=None)
-
-    def test__get_host_port_if_changed_no_binding_change(self):
-        mech_context = mock.Mock()
-        mech_context._binding.host = 'Host-1'
-        mech_context.current = {
-            'id': 'fake-id',
-            'mac_address': '2a:2b:2c:2d:2e:2f'
-        }
-        attrs = {'mac_address': '0a:0b:0c:0d:0e:0f'}
-        self._test__get_host_port_if_changed(
-            mech_context, attrs=attrs, expected_retval=None)
-
-        attrs = {
-            portbindings.HOST_ID: 'Host-1',
-            'mac_address': '0a:0b:0c:0d:0e:0f',
-        }
-        self._test__get_host_port_if_changed(
-            mech_context, attrs=attrs, expected_retval=None)
-
-    def test__get_host_port_if_changed_with_binding_removed(self):
-        expected_return = {
-            'id': 'fake-id',
-            portbindings.HOST_ID: None,
-            'mac_address': '2a:2b:2c:2d:2e:2f'
-        }
-        mech_context = mock.Mock()
-        mech_context._binding.host = 'Host-1'
-        mech_context.current = expected_return
-        attrs = {portbindings.HOST_ID: None}
-        self._test__get_host_port_if_changed(
-            mech_context, attrs=attrs, expected_retval=expected_return)
-
-    def test__get_host_port_if_changed_with_binding_added(self):
-        expected_return = {
-            'id': 'fake-id',
-            portbindings.HOST_ID: 'host-1',
-            'mac_address': '2a:2b:2c:2d:2e:2f'
-        }
-        mech_context = mock.Mock()
-        mech_context.current = expected_return
-        attrs = {portbindings.HOST_ID: 'host-1'}
-        self._test__get_host_port_if_changed(
-            mech_context, attrs=attrs, expected_retval=expected_return)
-
     def test_create_port_rpc_outside_transaction(self):
         with mock.patch.object(ml2_plugin.Ml2Plugin, '__init__') as init,\
                 mock.patch.object(base_plugin.NeutronDbPluginV2,
-                                  'create_port'):
+                                  'create_port') as db_create_port:
             init.return_value = None
 
-            new_host_port = mock.Mock()
-            plugin = self._create_plugin_for_create_update_port(new_host_port)
+            new_port = mock.MagicMock()
+            db_create_port.return_value = new_port
+            plugin = self._create_plugin_for_create_update_port()
 
             plugin.create_port(self.context, mock.MagicMock())
 
-            kwargs = {'context': self.context, 'port': new_host_port}
+            kwargs = {'context': self.context, 'port': new_port}
             self.notify.assert_called_once_with('port', 'after_create',
                 plugin, **kwargs)
 
@@ -1772,17 +1713,18 @@ class TestMl2PluginCreateUpdateDeletePort(base.BaseTestCase):
                 mock.patch.object(ml2_db, 'get_locked_port_and_binding',
                                   return_value=(original_port_db, binding)),\
                 mock.patch.object(base_plugin.NeutronDbPluginV2,
-                                  'update_port'):
+                                  'update_port') as db_update_port:
             init.return_value = None
-            new_host_port = mock.Mock()
-            plugin = self._create_plugin_for_create_update_port(new_host_port)
+            updated_port = mock.MagicMock()
+            db_update_port.return_value = updated_port
+            plugin = self._create_plugin_for_create_update_port()
             original_port = plugin._make_port_dict(original_port_db)
 
             plugin.update_port(self.context, port_id, mock.MagicMock())
 
             kwargs = {
                 'context': self.context,
-                'port': new_host_port,
+                'port': updated_port,
                 'mac_address_updated': True,
                 'original_port': original_port,
             }
@@ -1803,7 +1745,7 @@ class TestMl2PluginCreateUpdateDeletePort(base.BaseTestCase):
                 mock.patch.object(manager.NeutronManager,
                                   'get_service_plugins',
                                   return_value={'L3_ROUTER_NAT': l3plugin}):
-            plugin = self._create_plugin_for_create_update_port(mock.Mock())
+            plugin = self._create_plugin_for_create_update_port()
             # Set backend manually here since __init__ was mocked
             plugin.set_ipam_backend()
             # deleting the port will call registry.notify, which will