From ef2260441d56036b42fc71b0f03b70a4a02fd954 Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Fri, 24 Apr 2020 14:39:20 +0100 Subject: [PATCH] [OVN] Do not delete port's revision on RowNotFound The delete_port() method from OVNClient has a potential problem of leaving stale ports when RowNotFound is raised from the process to delete the port from the OVN database. Since the exception is not granular enough, the RowNotFound could be raised from other objects that are part of the same transaction (such as ACLs, DNS entries, etc...) resulting in the revision for the port being deleted even tho the port is still in the database. Instead of giving a pass on the RowNotFound exception, this patch is logging the error and re-raising it without deleting the revision. Change-Id: I25b93b7c080403fc38365b638e4e03298b447d0f Partial-Bug: #1874733 Signed-off-by: Lucas Alvares Gomes --- .../ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py | 6 ++++-- .../ml2/drivers/ovn/mech_driver/test_mech_driver.py | 13 +++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py index 660b12194a8..16dbead2820 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py @@ -717,8 +717,10 @@ class OVNClient(object): def delete_port(self, context, port_id, port_object=None): try: self._delete_port(port_id, port_object=port_object) - except idlutils.RowNotFound: - pass + except Exception as e: + with excutils.save_and_reraise_exception(): + LOG.error('Failed to delete port %(port)s. Error: ' + '%(error)s', {'port': port_id, 'error': e}) db_rev.delete_revision(context, port_id, ovn_const.TYPE_PORTS) def _create_or_update_floatingip(self, floatingip, txn=None): diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index 65f9dd8dc48..317590caae5 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -780,6 +780,19 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase): self.assertEqual( 1, self.nb_ovn.update_address_set.call_count) + @mock.patch.object(ovn_revision_numbers_db, 'delete_revision') + @mock.patch.object(ovn_client.OVNClient, '_delete_port') + def test_delete_port_exception_delete_revision(self, mock_del_port, + mock_del_rev): + mock_del_port.side_effect = Exception('BoOoOoOoOmmmmm!!!') + with self.network(set_context=True, tenant_id='test') as net: + with self.subnet(network=net) as subnet: + with self.port(subnet=subnet, + set_context=True, tenant_id='test') as port: + self._delete('ports', port['port']['id']) + # Assert that delete_revision wasn't invoked + mock_del_rev.assert_not_called() + def _test_set_port_status_up(self, is_compute_port=False): port_device_owner = 'compute:nova' if is_compute_port else '' self.mech_driver._plugin.nova_notifier = mock.Mock()