From 87ce73141dddb840ffc523f253fdf87aef11d1c7 Mon Sep 17 00:00:00 2001 From: Oleg Bondarev Date: Tue, 1 Dec 2015 11:47:05 +0300 Subject: [PATCH] Notify about port create/update unconditionally 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 | 18 +--- neutron/tests/unit/plugins/ml2/test_plugin.py | 82 +++---------------- 2 files changed, 15 insertions(+), 85 deletions(-) diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index cd0dcde06..d35a93500 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/neutron/plugins/ml2/plugin.py @@ -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, } diff --git a/neutron/tests/unit/plugins/ml2/test_plugin.py b/neutron/tests/unit/plugins/ml2/test_plugin.py index b4e906000..c5e4f83e1 100644 --- a/neutron/tests/unit/plugins/ml2/test_plugin.py +++ b/neutron/tests/unit/plugins/ml2/test_plugin.py @@ -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 -- 2.45.2