From 733eb3c3d3ec3e07cb5e46a1ca17312f5a88419b Mon Sep 17 00:00:00 2001
From: Jon Grimm <jgrimm@linux.vnet.ibm.com>
Date: Wed, 27 Nov 2013 13:10:33 -0600
Subject: [PATCH] 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
---
 neutron/db/allowedaddresspairs_db.py          | 36 +++++++++++++++----
 .../plugins/openvswitch/ovs_neutron_plugin.py | 11 +++---
 .../openvswitch/test_openvswitch_plugin.py    |  6 ++++
 3 files changed, 40 insertions(+), 13 deletions(-)

diff --git a/neutron/db/allowedaddresspairs_db.py b/neutron/db/allowedaddresspairs_db.py
index 8adf47bc67..861139a9ff 100644
--- 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 7f190b9451..723ca7b4f2 100644
--- 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 4fbb4fd4aa..6d76cde9af 100644
--- 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