[OVN] Handle missing acls during log removal
During log delete we fetch all the acls for
all the pgs if there is only one log object
and do clear log of all these acls, but if
one or more acls of any of the pgs is removed
concurrently, db_set fails as acl is not found.
This patch proposes to only do log clear of those
acls which are available and add log message for the
acls which were deleted concurrently.
Also add a unit test for this case where one of
the acl get's missing.
Closes-Bug: #1971569
Change-Id: I58487024c8d0352776307f0185f0812bb00036ae
(cherry picked from commit 1471f531b8
)
This commit is contained in:
parent
751c854dae
commit
9bb9560967
|
@ -152,13 +152,19 @@ class OVNDriver(base.DriverBase):
|
|||
ovn_const.ACL_ACTION_ALLOW}
|
||||
|
||||
def _remove_acls_log(self, pgs, ovn_txn, log_name=None):
|
||||
acl_changes, acl_visits = 0, 0
|
||||
acl_absents, acl_changes, acl_visits = 0, 0, 0
|
||||
for pg in pgs:
|
||||
for acl_uuid in pg["acls"]:
|
||||
acl_visits += 1
|
||||
acl = self.ovn_nb.lookup("ACL", acl_uuid, default=None)
|
||||
# Log message if ACL is not found, as deleted concurrently
|
||||
if acl is None:
|
||||
LOG.debug("ACL %s not found, deleted concurrently",
|
||||
acl_uuid)
|
||||
acl_absents += 1
|
||||
continue
|
||||
# skip acls used by a different network log
|
||||
if log_name:
|
||||
acl = self.ovn_nb.lookup("ACL", acl_uuid)
|
||||
if acl.name and acl.name[0] != log_name:
|
||||
continue
|
||||
ovn_txn.add(self.ovn_nb.db_set(
|
||||
|
@ -169,10 +175,10 @@ class OVNDriver(base.DriverBase):
|
|||
("severity", [])
|
||||
))
|
||||
acl_changes += 1
|
||||
msg = "Cleared %d (out of %d visited) ACLs"
|
||||
msg = "Cleared %d, Not found %d (out of %d visited) ACLs"
|
||||
if log_name:
|
||||
msg += " for network log {}".format(log_name)
|
||||
LOG.info(msg, acl_changes, acl_visits)
|
||||
LOG.info(msg, acl_changes, acl_absents, acl_visits)
|
||||
|
||||
def _set_acls_log(self, pgs, ovn_txn, actions_enabled, log_name):
|
||||
acl_changes, acl_visits = 0, 0
|
||||
|
|
|
@ -270,19 +270,41 @@ class TestOVNDriver(base.BaseTestCase):
|
|||
pg_dict = self._fake_pg_dict(acls=['acl1', 'acl2'])
|
||||
self._log_driver._remove_acls_log([pg_dict], self._nb_ovn.transaction)
|
||||
info_args, _info_kwargs = m_info.call_args_list[0]
|
||||
self.assertIn('Cleared %d (out of %d visited) ACLs', info_args[0])
|
||||
self._nb_ovn.lookup.assert_not_called()
|
||||
self.assertIn('Cleared %d, Not found %d (out of %d visited) ACLs',
|
||||
info_args[0])
|
||||
self._nb_ovn.lookup.assert_has_calls([
|
||||
mock.call('ACL', 'acl1', default=None),
|
||||
mock.call('ACL', 'acl2', default=None)])
|
||||
self.assertEqual(len(pg_dict["acls"]), info_args[1])
|
||||
self.assertEqual(len(pg_dict["acls"]), info_args[2])
|
||||
self.assertEqual(len(pg_dict["acls"]) - 2, info_args[2])
|
||||
self.assertEqual(len(pg_dict["acls"]), info_args[3])
|
||||
self.assertEqual(len(pg_dict["acls"]), self._nb_ovn.db_set.call_count)
|
||||
|
||||
@mock.patch.object(ovn_driver.LOG, 'info')
|
||||
def test__remove_acls_log_missing_acls(self, m_info):
|
||||
pg_dict = self._fake_pg_dict(acls=['acl1', 'acl2', 'acl3'])
|
||||
|
||||
def _mock_lookup(_pg_table, acl_uuid, default):
|
||||
if acl_uuid == 'acl3':
|
||||
return None
|
||||
return self._fake_acl()
|
||||
|
||||
self._nb_ovn.lookup.side_effect = _mock_lookup
|
||||
self._log_driver._remove_acls_log([pg_dict], self._nb_ovn.transaction)
|
||||
info_args, _info_kwargs = m_info.call_args_list[0]
|
||||
self.assertEqual(len(pg_dict["acls"]) - 1, info_args[1])
|
||||
self.assertEqual(len(pg_dict["acls"]) - 2, info_args[2])
|
||||
self.assertEqual(len(pg_dict["acls"]), info_args[3])
|
||||
self.assertEqual(len(pg_dict["acls"]) - 1,
|
||||
self._nb_ovn.db_set.call_count)
|
||||
|
||||
@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'])
|
||||
log_name = 'test_obj_name'
|
||||
used_name = 'test_used_name'
|
||||
|
||||
def _mock_lookup(_pg_table, acl_uuid):
|
||||
def _mock_lookup(_pg_table, acl_uuid, default):
|
||||
if acl_uuid == 'acl2':
|
||||
return self._fake_acl(name=used_name)
|
||||
return self._fake_acl(name=log_name)
|
||||
|
@ -291,10 +313,12 @@ class TestOVNDriver(base.BaseTestCase):
|
|||
self._log_driver._remove_acls_log([pg_dict], self._nb_ovn.transaction,
|
||||
log_name)
|
||||
info_args, _info_kwargs = m_info.call_args_list[0]
|
||||
self.assertIn('Cleared %d (out of %d visited) ACLs', info_args[0])
|
||||
self.assertIn('Cleared %d, Not found %d (out of %d visited) ACLs',
|
||||
info_args[0])
|
||||
self.assertIn('for network log {}'.format(log_name), info_args[0])
|
||||
self.assertEqual(len(pg_dict["acls"]) - 1, info_args[1])
|
||||
self.assertEqual(len(pg_dict["acls"]), info_args[2])
|
||||
self.assertEqual(len(pg_dict["acls"]) - 4, info_args[2])
|
||||
self.assertEqual(len(pg_dict["acls"]), info_args[3])
|
||||
self.assertEqual(len(pg_dict["acls"]) - 1,
|
||||
self._nb_ovn.db_set.call_count)
|
||||
|
||||
|
|
Loading…
Reference in New Issue