Check for provisioning blocks before updating port up

There is a race we need to check for where a port is created and
then updated with binding information via another API shortly
afterward that causes the bug referenced in this message.

* The port is created and a DHCP provisioning block is added.
* The DHCP agent finishes setting up the reservation and emits a
  message to clear the block.
* The _port_provisioned callback in ML2 is triggered.
* A port update comes in that binds the port and adds new blocks.
* The _port_provisioned callback now does a get_port and sees
  that the port is bound so it assumes everything is done and
  the port can be marked ACTIVE.
* The port is now ACTIVE before the L2 agent has had a chance to
  do its wiring and the VM boots.
* The L2 agent requests the details and the port flaps back to
  BUILD.
* The L2 agent finishes and clears the provisioning block a second
  time so the port goes back to ACTIVE.

This will randomly cause failures because the VM is booting before the
L2 agent is done and tempest may see the port in the BUILD state after
the agent grabs port details.

The fix is relatively simple. Just check for any new provisioning blocks
added *after* doing a get_port in the _port_provisioned callback to make
sure we don't update to ACTIVE if there are newly added blocks.

Closes-Bug: #1600396
Change-Id: I14f41a5fda0707e8bba064c5cd952553686c30cd

neutron/db/provisioning_blocks.py

@@ -152,6 +152,22 @@ def provisioning_complete(context, object_id, object_type, entity):
                         context=context, object_id=object_id)
 
 
+def is_object_blocked(context, object_id, object_type):
+    """Return boolean indicating if object has a provisioning block.
+
+    :param context: neutron api request context
+    :param object_id: ID of object that has been provisioned
+    :param object_type: callback resource type of the object
+    """
+    standard_attr_id = _get_standard_attr_id(context, object_id,
+                                             object_type)
+    if not standard_attr_id:
+        # object doesn't exist so it has no blocks
+        return False
+    return bool(context.session.query(ProvisioningBlock).filter_by(
+                standard_attr_id=standard_attr_id).count())
+
+
 def _get_standard_attr_id(context, object_id, object_type):
     model = _RESOURCE_TO_MODEL_MAP.get(object_type)
     if not model:

neutron/plugins/ml2/plugin.py

@@ -215,6 +215,16 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
             LOG.debug("Port %s cannot update to ACTIVE because it "
                       "is not bound.", port_id)
             return
+        else:
+            # port is bound, but we have to check for new provisioning blocks
+            # one last time to detect the case where we were triggered by an
+            # unbound port and the port became bound with new provisioning
+            # blocks before 'get_port' was called above
+            if provisioning_blocks.is_object_blocked(context, port_id,
+                                                     resources.PORT):
+                LOG.debug("Port %s had new provisioning blocks added so it "
+                          "will not transition to active.", port_id)
+                return
         self.update_port_status(context, port_id, const.PORT_STATUS_ACTIVE)
 
     @property

neutron/tests/unit/db/test_provisioning_blocks.py

@@ -94,6 +94,18 @@ class TestStatusBarriers(testlib_api.SqlTestCase):
                                  resources.PORT, 'entity2')
         self.assertFalse(self.provisioned.called)
 
+    def test_is_object_blocked(self):
+        pb.add_provisioning_component(self.ctx, self.port.id, resources.PORT,
+                                      'e1')
+        self.assertTrue(pb.is_object_blocked(self.ctx, self.port.id,
+                                             resources.PORT))
+        self.assertFalse(pb.is_object_blocked(self.ctx, 'xyz',
+                                              resources.PORT))
+        pb.provisioning_complete(self.ctx, self.port.id,
+                                 resources.PORT, 'e1')
+        self.assertFalse(pb.is_object_blocked(self.ctx, self.port.id,
+                                              resources.PORT))
+
     def test_remove_provisioning_component(self):
         pb.add_provisioning_component(self.ctx, self.port.id, resources.PORT,
                                       'e1')

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

@@ -512,6 +512,17 @@ class TestMl2DbOperationBoundsTenant(TestMl2DbOperationBounds):
 
 class TestMl2PortsV2(test_plugin.TestPortsV2, Ml2PluginV2TestCase):
 
+    def test__port_provisioned_with_blocks(self):
+        plugin = manager.NeutronManager.get_plugin()
+        ups = mock.patch.object(plugin, 'update_port_status').start()
+        with self.port() as port:
+            mock.patch('neutron.plugins.ml2.plugin.db.get_port').start()
+            provisioning_blocks.add_provisioning_component(
+                self.context, port['port']['id'], 'port', 'DHCP')
+            plugin._port_provisioned('port', 'evt', 'trigger',
+                                     self.context, port['port']['id'])
+        self.assertFalse(ups.called)
+
     def test__port_provisioned_no_binding(self):
         plugin = manager.NeutronManager.get_plugin()
         with self.network() as net: