Skip to content

Commit

Permalink
[ovn] Avoid unwanted ACL_NOT_FOUND error when deleting log objects
Browse files Browse the repository at this point in the history
There is the possibility that db_remove raises an error if an ACL was
deleted on parallel for other reasons while deleting a log object. On a
normal situation, this command would remove the no-longer-needed
'log-related' property, but since the ACL is no longer there, it will
raise an error. For this case, it is not problematic to have an ACL
deleted mid-operation, so it should not raise any error.

Related-Bug: #2019887

Signed-off-by: Elvira García <egarciar@redhat.com>
Change-Id: I154393529134b5861e0ef0283257ef964fe057fd
(cherry picked from commit e0a2427)
  • Loading branch information
elvgarrui committed May 22, 2023
1 parent fb51a35 commit 8def3b6
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 1 deletion.
3 changes: 2 additions & 1 deletion neutron/services/logapi/drivers/ovn/driver.py
Expand Up @@ -142,7 +142,8 @@ def _remove_acls_log(self, pgs, ovn_txn, log_name=None):
if hasattr(acl, 'label'):
columns['label'] = 0
ovn_txn.add(self.ovn_nb.db_remove(
"ACL", acl_uuid, 'options', 'log-related'))
"ACL", acl_uuid, 'options', 'log-related',
if_exists=True))
ovn_txn.add(self.ovn_nb.db_set(
"ACL", acl_uuid, *columns.items()))
acl_changes += 1
Expand Down
15 changes: 15 additions & 0 deletions neutron/tests/unit/services/logapi/drivers/ovn/test_driver.py
Expand Up @@ -27,6 +27,7 @@

FAKE_CFG_RATE = 123
FAKE_CFG_BURST = 321
FAKE_LABEL = 1


class TestOVNDriverBase(base.BaseTestCase):
Expand Down Expand Up @@ -113,6 +114,7 @@ def __init__(self, name=None, **acl_dict):
acl_defaults_dict = {
"name": [name] if name else [],
"action": ovn_const.ACL_ACTION_ALLOW_RELATED,
"label": FAKE_LABEL
}
self.__dict__ = {**acl_defaults_dict, **acl_dict}

Expand Down Expand Up @@ -247,6 +249,19 @@ def _mock_lookup(_pg_table, acl_uuid, default):
self.assertEqual(len(pg_dict["acls"]) - 1,
self._nb_ovn.db_set.call_count)

# This test is enforcing the use of if_exists so that we don't get
# unexpected errors while doing parallel operations like erasing log
# objects and security groups
@mock.patch.object(ovn_driver.LOG, 'info')
def test__remove_acls_log_only_if_exists(self, m_info):
pg_dict = self._fake_pg_dict(acls=['acl1', 'acl2', 'acl3'])

def _only_if_exists(_pg_table, acl_uuid, col, val, if_exists):
self.assertTrue(if_exists)

self._nb_ovn.db_remove.side_effect = _only_if_exists
self._log_driver._remove_acls_log([pg_dict], self._nb_ovn.transaction)

@mock.patch.object(ovn_driver.LOG, 'info')
def test__remove_acls_log_with_log_name(self, m_info):
pg_dict = self._fake_pg_dict(acls=['acl1', 'acl2', 'acl3', 'acl4'])
Expand Down

0 comments on commit 8def3b6

Please sign in to comment.