From 87bdfd12a8984f54d27f287c547b3822c9fe3d91 Mon Sep 17 00:00:00 2001 From: Bence Romsics Date: Wed, 22 Jan 2025 13:03:49 +0100 Subject: [PATCH] Do not assume the existence of a trunk bridge since os-vif may have deleted it Since https://review.opendev.org/c/openstack/neutron/+/837780 and https://review.opendev.org/c/openstack/os-vif/+/841499 it is possible that nova/os-vif already deleted the trunk bridge before we get to handle_trunk_remove() -> unwire_subports_for_trunk() -> _update_trunk_metadata(). In the past we assumed that nobody else deleted the trunk bridge before us and because of that when os-vif started deleting the trunk bridge we started producing error messages like: "Failed to store metadata for trunk ...: Cannot find Bridge with name=tbr-..." and "Error executing command (ListPortsCommand): ovsdbapp.backend.ovs_idl.idlutils.RowNotFound: Cannot find Bridge with name=tbr-...: With this patch we no longer assume that the trunk bridge exists in _update_trunk_metadata(). Change-Id: I2b9202ddf653c0755c2aacb6b88ce8c983bb29dc Closes-Bug: #2095000 Related-Bug: #1869244 Related-Bug: #1997025 --- .../trunk/drivers/openvswitch/agent/ovsdb_handler.py | 9 +++++++-- .../drivers/openvswitch/agent/test_ovsdb_handler.py | 12 ++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/neutron/services/trunk/drivers/openvswitch/agent/ovsdb_handler.py b/neutron/services/trunk/drivers/openvswitch/agent/ovsdb_handler.py index 0de71045153..73dfcf86288 100644 --- a/neutron/services/trunk/drivers/openvswitch/agent/ovsdb_handler.py +++ b/neutron/services/trunk/drivers/openvswitch/agent/ovsdb_handler.py @@ -450,8 +450,13 @@ class OVSDBHandler: :param subport_ids: subports affecting the metadata. :param wire: if True subport_ids are added, otherwise removed. """ - trunk_bridge = trunk_bridge or ovs_lib.OVSBridge( - utils.gen_trunk_br_name(trunk_id)) + bridge_name = utils.gen_trunk_br_name(trunk_id) + trunk_bridge = trunk_bridge or ovs_lib.OVSBridge(bridge_name) + if not trunk_bridge.bridge_exists(bridge_name): + LOG.info( + "Could not update trunk metadata in a non-existent bridge " + "(likely already deleted by nova/os-vif): %s", bridge_name) + return port = port or self._get_parent_port(trunk_bridge) _port_id, _trunk_id, old_subports = self._get_trunk_metadata(port) if wire: diff --git a/neutron/tests/unit/services/trunk/drivers/openvswitch/agent/test_ovsdb_handler.py b/neutron/tests/unit/services/trunk/drivers/openvswitch/agent/test_ovsdb_handler.py index cbd9b690131..9e6420b8efa 100644 --- a/neutron/tests/unit/services/trunk/drivers/openvswitch/agent/test_ovsdb_handler.py +++ b/neutron/tests/unit/services/trunk/drivers/openvswitch/agent/test_ovsdb_handler.py @@ -336,3 +336,15 @@ class TestOVSDBHandler(base.BaseTestCase): expected_subport_ids = '["foo_subport_1"]' self._test__update_trunk_metadata_wire_flag( mock_br, False, external_ids, subport_ids, expected_subport_ids) + + @mock.patch('neutron.agent.common.ovs_lib.OVSBridge') + def test__update_trunk_metadata_no_tbr_no_raise(self, br): + mock_br = br.return_value + mock_br.configure_mock(**{'bridge_exists.return_value': False}) + try: + self.ovsdb_handler._update_trunk_metadata( + mock_br, None, self.trunk_id, ['foo_subport_1']) + except RuntimeError: + self.fail( + "_update_trunk_metadata() should not raise RuntimeError " + "when the trunk bridge does not exist")