openstack
/
neutron
Code Issues Proposed changes

[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 <lucasagomes@gmail.com>
This commit is contained in:
Lucas Alvares Gomes 2020-04-24 14:39:20 +01:00
parent 0fab732485
commit ef2260441d
2 changed files with 17 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py
View File

@ -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):

13
neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py
View File

@ -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()