From 8def3b694dd800da09e44f332c0ea11e802dbde4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elvira=20Garc=C3=ADa?= Date: Fri, 19 May 2023 20:30:47 +0200 Subject: [PATCH] [ovn] Avoid unwanted ACL_NOT_FOUND error when deleting log objects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Change-Id: I154393529134b5861e0ef0283257ef964fe057fd (cherry picked from commit e0a2427a2f701adae14cd8196e3c2f064416d044) --- neutron/services/logapi/drivers/ovn/driver.py | 3 ++- .../services/logapi/drivers/ovn/test_driver.py | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/neutron/services/logapi/drivers/ovn/driver.py b/neutron/services/logapi/drivers/ovn/driver.py index c58f83fbc5b..5882b11300f 100644 --- a/neutron/services/logapi/drivers/ovn/driver.py +++ b/neutron/services/logapi/drivers/ovn/driver.py @@ -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 diff --git a/neutron/tests/unit/services/logapi/drivers/ovn/test_driver.py b/neutron/tests/unit/services/logapi/drivers/ovn/test_driver.py index 4f4cd7a7ba1..db8e2c17731 100644 --- a/neutron/tests/unit/services/logapi/drivers/ovn/test_driver.py +++ b/neutron/tests/unit/services/logapi/drivers/ovn/test_driver.py @@ -27,6 +27,7 @@ FAKE_CFG_RATE = 123 FAKE_CFG_BURST = 321 +FAKE_LABEL = 1 class TestOVNDriverBase(base.BaseTestCase): @@ -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} @@ -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'])