[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 1471f53)

Author: karelyatin

neutron/services/logapi/drivers/ovn/driver.py

@@ -154,13 +154,19 @@ def _acl_actions_enabled(log_obj):
                 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(
@@ -171,10 +177,10 @@ def _remove_acls_log(self, pgs, ovn_txn, log_name=None):
                     ("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

neutron/tests/unit/services/logapi/drivers/ovn/test_driver.py

@@ -270,19 +270,41 @@ def test__remove_acls_log(self, m_info):
         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 @@ def _mock_lookup(_pg_table, acl_uuid):
         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)