From 347486fb513f34bc054cfce5c85749054d1b3438 Mon Sep 17 00:00:00 2001 From: Bodo Petermann Date: Tue, 16 Aug 2022 14:14:14 +0200 Subject: Fix concurrent port binding activate Fix an issue with concurrent requests to activate a port binding. If there are two activate requests in parallel, one might set the binding on the new host to active and the other request may not find the previously INACTIVE row anymore in _commit_port_binding and initializing the driver_context.PortContext crashed. Closes-Bug: #1986003 Change-Id: I047e33062bc38f36848e0149c6e670cb5828c8e3 (cherry picked from commit 5b4ed5b117f2f418d598af20744f571db581e2ae) --- neutron/plugins/ml2/plugin.py | 8 ++++++++ neutron/tests/unit/plugins/ml2/test_port_binding.py | 18 ++++++++++++++++++ releasenotes/notes/bug-1986003-9bf5ca04f9304336.yaml | 10 ++++++++++ 3 files changed, 36 insertions(+) create mode 100644 releasenotes/notes/bug-1986003-9bf5ca04f9304336.yaml 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 `_ + 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. -- cgit v1.2.1