Fix ML2 revision_number handling in port updates

The revision number is bumped on the flush that changes
the port or a resource that bumps the port. However, in ML2
we create the dict that is used for after_update events,
mech driver calls, and the API response before all resource
modifications are complete so the dict may not reflect
the correct revision number.

This adjusts the update_port_status to flush changes to the
DB before constructing the dictionary response to give the
event listener a chance to bump the revision.

It also adjusts ML2 to update the 'updated_port' dict with
the latest result from make_port_dict after all of the related
objects have been processed to ensure the result has the latest
info from extensions. The API test for allowed address pairs
was adjusted to stop checking for order on update since no order
is stored in the DB.

The API test for revision numbers and ports was updated to
expect the correct behavior.

Closes-Bug: #1625981
Change-Id: I49d2d79a57d484fd98b8969f511895e607b7f128

neutron/plugins/ml2/plugin.py

@@ -1422,6 +1422,14 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
             need_port_update_notify |= self._update_extra_dhcp_opts_on_port(
                 context, id, port, updated_port)
             levels = db.get_binding_levels(session, id, binding.host)
+            # one of the operations above may have altered the model call
+            # _make_port_dict again to ensure latest state is reflected so mech
+            # drivers, callback handlers, and the API caller see latest state.
+            # We expire here to reflect changed relationships on the obj.
+            # Repeatable read will ensure we still get the state from this
+            # transaction in spite of concurrent updates/deletes.
+            context.session.expire(port_db)
+            updated_port.update(self._make_port_dict(port_db))
             mech_context = driver_context.PortContext(
                 self, context, updated_port, network, binding, levels,
                 original_port=original_port)
@@ -1719,6 +1727,9 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
                 port['device_owner'] != const.DEVICE_OWNER_DVR_INTERFACE):
                 original_port = self._make_port_dict(port)
                 port.status = status
+                # explicit flush before _make_port_dict to ensure extensions
+                # listening for db events can modify the port if necessary
+                context.session.flush()
                 updated_port = self._make_port_dict(port)
                 network = network or self.get_network(
                     context, original_port['network_id'])

neutron/tests/tempest/api/test_allowed_address_pair.py

@@ -83,7 +83,7 @@ class AllowedAddressPairTestJSON(base.BaseNetworkTest):
         body = self.client.update_port(
             port_id, allowed_address_pairs=allowed_address_pairs)
         allowed_address_pair = body['port']['allowed_address_pairs']
-        self.assertEqual(allowed_address_pair, allowed_address_pairs)
+        self.assertItemsEqual(allowed_address_pair, allowed_address_pairs)
 
     @test.idempotent_id('9599b337-272c-47fd-b3cf-509414414ac4')
     def test_update_port_with_address_pair(self):

neutron/tests/tempest/api/test_revisions.py

@@ -90,12 +90,7 @@ class TestRevisions(base.BaseAdminNetworkTest, bsg.BaseSecGroupTest):
         self.client.update_port(
             port['id'], security_groups=[sg['security_group']['id']])
         updated = self.client.show_port(port['id'])
-        self.client.update_port(port['id'], security_groups=[])
-        # TODO(kevinbenton): these extra shows after after the update are
-        # to work around the fact that ML2 creates the result dict before
-        # commit happens if the port is unbound. The update response should
-        # be usable directly once that is fixed.
-        updated2 = self.client.show_port(port['id'])
+        updated2 = self.client.update_port(port['id'], security_groups=[])
         self.assertGreater(updated['port']['revision_number'],
                            port['revision_number'])
         self.assertGreater(updated2['port']['revision_number'],

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

@@ -60,6 +60,7 @@ from neutron.plugins.ml2 import models
 from neutron.plugins.ml2 import plugin as ml2_plugin
 from neutron.services.l3_router import l3_router_plugin
 from neutron.services.qos import qos_consts
+from neutron.services.revisions import revision_plugin
 from neutron.services.segments import db as segments_plugin_db
 from neutron.services.segments import plugin as segments_plugin
 from neutron.tests import base
@@ -1119,6 +1120,30 @@ class TestMl2PortsV2(test_plugin.TestPortsV2, Ml2PluginV2TestCase):
         self.assertTrue(listener.except_raised)
 
 
+class TestMl2PortsV2WithRevisionPlugin(Ml2PluginV2TestCase):
+
+    def setUp(self):
+        super(TestMl2PortsV2WithRevisionPlugin, self).setUp()
+        self.revision_plugin = revision_plugin.RevisionPlugin()
+
+    def test_update_port_status_bumps_revision(self):
+        ctx = context.get_admin_context()
+        plugin = manager.NeutronManager.get_plugin()
+        host_arg = {portbindings.HOST_ID: HOST}
+        with self.port(arg_list=(portbindings.HOST_ID,),
+                       **host_arg) as port:
+            port = plugin.get_port(ctx, port['port']['id'])
+            updated_ports = []
+            receiver = lambda *a, **k: updated_ports.append(k['port'])
+            registry.subscribe(receiver, resources.PORT,
+                               events.AFTER_UPDATE)
+            plugin.update_port_status(
+                ctx, port['id'],
+                constants.PORT_STATUS_ACTIVE, host=HOST)
+            self.assertGreater(updated_ports[0]['revision_number'],
+                               port['revision_number'])
+
+
 class TestMl2PortsV2WithL3(test_plugin.TestPortsV2, Ml2PluginV2TestCase):
     """For testing methods that require the L3 service plugin."""