Openvswitch update_port should return updated port info

Found when I enabled test_extension_allowedaddress_pairs, where
test_create_port_removed_allowed_address_pairs would fail due to the
returned port still containing the original addresspair.  The cause is
ovs simply not updating the port info being returned.

This patch additionally enables test_extension_allowedaddress_pairs for
openvswitch.

Moved checks and updating into method similar to what we do for
extradhcpopts and security_groups.

Additionally, this required fixing is_address_pairs_attribute_updated() as
it was passing (non-hashable) dicts to utils.compare_elements.

Change-Id: Ic871fea68fb9fcc862b1fd5ae5fe7aec540e4a30
Partial-Bug: #1255150

diff --git a/neutron/db/allowedaddresspairs_db.py b/neutron/db/allowedaddresspairs_db.py
index 8adf47bc676c1b712f9e05db72ca7fe60a7a2ec6..861139a9fff4f6630e83d625dccf262e4962f9a4 100644 (file)
--- a/neutron/db/allowedaddresspairs_db.py
+++ b/neutron/db/allowedaddresspairs_db.py
@@ -17,7 +17,6 @@ import sqlalchemy as sa
 from sqlalchemy import orm
 
 from neutron.api.v2 import attributes as attr
-from neutron.common import utils
 from neutron.db import db_base_plugin_v2
 from neutron.db import model_base
 from neutron.db import models_v2
@@ -126,13 +125,36 @@ class AllowedAddressPairsMixin(object):
     def is_address_pairs_attribute_updated(self, port, update_attrs):
         """Check if the address pairs attribute is being updated.
 
-        This method returns a flag which indicates whether there is an update
-        and therefore a port update notification should be sent to agents or
-        third party controllers.
+        Returns True if there is an update. This can be used to decide
+        if a port update notification should be sent to agents or third
+        party controllers.
         """
+
         new_pairs = update_attrs.get(addr_pair.ADDRESS_PAIRS)
-        if new_pairs and not utils.compare_elements(
-            port.get(addr_pair.ADDRESS_PAIRS), new_pairs):
-            return True
+        if new_pairs is None:
+            return False
+        old_pairs = port.get(addr_pair.ADDRESS_PAIRS)
+
         # Missing or unchanged address pairs in attributes mean no update
+        return new_pairs != old_pairs
+
+    def update_address_pairs_on_port(self, context, port_id, port,
+                                     original_port, updated_port):
+        """Update allowed address pairs on port.
+
+        Returns True if an update notification is required. Notification
+        is not done here because other changes on the port may need
+        notification. This method is expected to be called within
+        a transaction.
+        """
+        new_pairs = port['port'].get(addr_pair.ADDRESS_PAIRS)
+
+        if self.is_address_pairs_attribute_updated(original_port,
+                                                   port['port']):
+            updated_port[addr_pair.ADDRESS_PAIRS] = new_pairs
+            self._delete_allowed_address_pairs(context, port_id)
+            self._process_create_allowed_address_pairs(
+                context, updated_port, new_pairs)
+            return True
+
         return False

diff --git a/neutron/plugins/openvswitch/ovs_neutron_plugin.py b/neutron/plugins/openvswitch/ovs_neutron_plugin.py
index 7f190b9451e9d8bfb3048ff516fd8977109a83c3..723ca7b4f2a459032de9cf4830d5236d00f348f4 100644 (file)
--- a/neutron/plugins/openvswitch/ovs_neutron_plugin.py
+++ b/neutron/plugins/openvswitch/ovs_neutron_plugin.py
@@ -587,12 +587,11 @@ class OVSNeutronPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
                 context, id)
             updated_port = super(OVSNeutronPluginV2, self).update_port(
                 context, id, port)
-            if self.is_address_pairs_attribute_updated(original_port, port):
-                self._delete_allowed_address_pairs(context, id)
-                self._process_create_allowed_address_pairs(
-                    context, updated_port,
-                    port['port'][addr_pair.ADDRESS_PAIRS])
-                need_port_update_notify = True
+            if addr_pair.ADDRESS_PAIRS in port['port']:
+                need_port_update_notify |= (
+                    self.update_address_pairs_on_port(context, id, port,
+                                                      original_port,
+                                                      updated_port))
             elif changed_fixed_ips:
                 self._check_fixed_ips_and_address_pairs_no_overlap(
                     context, updated_port)

diff --git a/neutron/tests/unit/openvswitch/test_openvswitch_plugin.py b/neutron/tests/unit/openvswitch/test_openvswitch_plugin.py
index 4fbb4fd4aa23f3f5d71f81873c07891e76028650..6d76cde9af79e8f93dd98b3e3d1ac59a41616cf5 100644 (file)
--- a/neutron/tests/unit/openvswitch/test_openvswitch_plugin.py
+++ b/neutron/tests/unit/openvswitch/test_openvswitch_plugin.py
@@ -16,6 +16,7 @@
 from neutron.extensions import portbindings
 from neutron.tests.unit import _test_extension_portbindings as test_bindings
 from neutron.tests.unit import test_db_plugin as test_plugin
+from neutron.tests.unit import test_extension_allowedaddresspairs as test_pair
 from neutron.tests.unit import test_security_groups_rpc as test_sg_rpc
 
 
@@ -73,3 +74,8 @@ class TestOpenvswitchPortBindingHost(
     OpenvswitchPluginV2TestCase,
     test_bindings.PortBindingsHostTestCaseMixin):
     pass
+
+
+class TestOpenvswitchAllowedAddressPairs(OpenvswitchPluginV2TestCase,
+                                         test_pair.TestAllowedAddressPairs):
+    pass