Merge "Fix concurrent port binding activate" into stable/yoga

3 files changed, 36 insertions, 0 deletions

diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py
index 203f74fc47..2bd0c599eb 100644
--- a/neutron/plugins/ml2/plugin.py
+++ b/neutron/plugins/ml2/plugin.py
@@ -694,6 +694,14 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
                 # transaction that completed before the deletion.
                 LOG.debug("Port %s has been deleted concurrently", port_id)
                 return orig_context, False, False
+
+            if (new_binding.status == const.INACTIVE and
+                    new_binding.host == cur_binding.host):
+                # The binding is already active on the target host,
+                # probably because of a concurrent activate request.
+                raise exc.PortBindingAlreadyActive(port_id=port_id,
+                                                   host=new_binding.host)
+
             # Since the mechanism driver bind_port() calls must be made
             # outside a DB transaction locking the port state, it is
             # possible (but unlikely) that the port's state could change
diff --git a/neutron/tests/unit/plugins/ml2/test_port_binding.py b/neutron/tests/unit/plugins/ml2/test_port_binding.py
index 1b90b7912a..31886e0bed 100644
--- a/neutron/tests/unit/plugins/ml2/test_port_binding.py
+++ b/neutron/tests/unit/plugins/ml2/test_port_binding.py
@@ -13,6 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from concurrent import futures
 from unittest import mock
 
 from neutron_lib.api.definitions import portbindings
@@ -561,6 +562,23 @@ class ExtendedPortBindingTestCase(test_plugin.NeutronDbPluginV2TestCase):
             retrieved_bindings, const.INACTIVE)
         self._assert_unbound_port_binding(retrieved_inactive_binding)
 
+    def test_activate_port_binding_concurrency(self):
+        port, _ = self._create_port_and_binding()
+        with mock.patch.object(mechanism_test.TestMechanismDriver,
+                               '_check_port_context'):
+            with futures.ThreadPoolExecutor() as executor:
+                f1 = executor.submit(
+                    self._activate_port_binding, port['id'], self.host)
+                f2 = executor.submit(
+                    self._activate_port_binding, port['id'], self.host)
+                result_1 = f1.result()
+                result_2 = f2.result()
+
+        # One request should be successful and the other should receive a
+        # HTTPConflict. The order is arbitrary.
+        self.assertEqual({webob.exc.HTTPConflict.code, webob.exc.HTTPOk.code},
+                         {result_1.status_int, result_2.status_int})
+
     def test_activate_port_binding_for_non_compute_owner(self):
         port, new_binding = self._create_port_and_binding()
         data = {'port': {'device_owner': ''}}
diff --git a/releasenotes/notes/bug-1986003-9bf5ca04f9304336.yaml b/releasenotes/notes/bug-1986003-9bf5ca04f9304336.yaml
new file mode 100644
index 0000000000..6b982035ad
--- /dev/null
+++ b/releasenotes/notes/bug-1986003-9bf5ca04f9304336.yaml
@@ -0,0 +1,10 @@
+---
+fixes:
+  - |
+    `1986003 <https://bugs.launchpad.net/neutron/+bug/1986003>`_
+    Fixed an issue with concurrent requests to activate the same port binding
+    where one of the requests returned a 500 Internal Server Error.
+    With the fix one request will return successfully and the other will
+    return a 409 Conflict (Binding already active).
+    This fixes errors in nova live-migrations where those concurrent requests
+    might be sent. Nova handles the 409/Conflict response gracefully.