Do not trigger agent notification if bindings do not change

The method _process_port_bindings for the ml2 plugin should not
return True when the host binding does not change, otherwise an
unnecessary notification will be sent to the agent

Closes-Bug: #1255680
Related-Bug: #1253896
Partially Implements: neutron-tempest-parallel

Change-Id: I8a40090af347ca430ff6c8e2211fa34bb2bd0f8c

diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py
index 0368930408c62569e30a0a599d84ec415a02307e..82a93fb4bcf9f3d04066fba2ca5f3da6e6c8d05e 100644 (file)
--- a/neutron/plugins/ml2/plugin.py
+++ b/neutron/plugins/ml2/plugin.py
@@ -203,7 +203,6 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
         binding = mech_context._binding
         port = mech_context.current
         self._update_port_dict_binding(port, binding)
-
         host = attrs and attrs.get(portbindings.HOST_ID)
         host_set = attributes.is_attr_set(host)
 
@@ -214,6 +213,11 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
             self.mechanism_manager.unbind_port(mech_context)
             self._update_port_dict_binding(port, binding)
 
+        # Return True only if an agent notification is needed.
+        # This will happen if a new host was specified and that host
+        # differs from the current one. Note that host_set is True
+        # even if the host is an empty string
+        ret_value = host_set and binding.get('host') != host
         if host_set:
             binding.host = host
             port[portbindings.HOST_ID] = host
@@ -222,7 +226,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
             self.mechanism_manager.bind_port(mech_context)
             self._update_port_dict_binding(port, binding)
 
-        return True
+        return ret_value
 
     def _update_port_dict_binding(self, port, binding):
         port[portbindings.HOST_ID] = binding.host

diff --git a/neutron/tests/unit/ml2/test_port_binding.py b/neutron/tests/unit/ml2/test_port_binding.py
index 8b02236136e939bfc0bb8e1bb45b120b5ddfd5f6..c9f784f26c88cacac956a5985b0d1517098982ab 100644 (file)
--- a/neutron/tests/unit/ml2/test_port_binding.py
+++ b/neutron/tests/unit/ml2/test_port_binding.py
@@ -13,6 +13,9 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import mock
+
+from neutron import context
 from neutron.extensions import portbindings
 from neutron import manager
 from neutron.plugins.ml2 import config as config
@@ -76,3 +79,43 @@ class PortBindingTestCase(test_plugin.NeutronDbPluginV2TestCase):
         self._test_port_binding("host-bridge-filter",
                                 portbindings.VIF_TYPE_BRIDGE,
                                 True, True)
+
+    def _test_update_port_binding(self, host, new_host=None):
+        with mock.patch.object(self.plugin,
+                               '_notify_port_updated') as notify_mock:
+            host_arg = {portbindings.HOST_ID: host}
+            update_body = {'name': 'test_update'}
+            if new_host is not None:
+                update_body[portbindings.HOST_ID] = new_host
+            with self.port(name='name', arg_list=(portbindings.HOST_ID,),
+                           **host_arg) as port:
+                neutron_context = context.get_admin_context()
+                updated_port = self._update('ports', port['port']['id'],
+                                            {'port': update_body},
+                                            neutron_context=neutron_context)
+                port_data = updated_port['port']
+                if new_host is not None:
+                    self.assertEqual(port_data['binding:host_id'], new_host)
+                else:
+                    self.assertEqual(port_data['binding:host_id'], host)
+                if new_host is not None and new_host != host:
+                    notify_mock.assert_called_once_with(mock.ANY)
+                else:
+                    self.assertFalse(notify_mock.called)
+
+    def test_update_with_new_host_binding_notifies_agent(self):
+        self._test_update_port_binding('host-ovs-no-filter',
+                                       'host-bridge-no-filter')
+
+    def test_update_with_same_host_binding_does_not_notify(self):
+        self._test_update_port_binding('host-ovs-no-filter',
+                                       'host-ovs-no-filter')
+
+    def test_update_without_binding_does_not_notify(self):
+        self._test_update_port_binding('host-ovs-no-filter')
+
+    def testt_update_from_empty_to_host_binding_notifies_agent(self):
+        self._test_update_port_binding('', 'host-ovs-no-filter')
+
+    def test_update_from_host_to_empty_binding_notifies_agent(self):
+        self._test_update_port_binding('host-ovs-no-filter', '')