Merge "Use safer methods to get security groups on security group logging" into stable/2023.1

Author: Zuul

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

@@ -157,12 +157,16 @@ def _set_acls_log(self, pgs, context, ovn_txn, actions_enabled, log_name):
         acl_changes, acl_visits = 0, 0
         for pg in pgs:
             meter_name = self.meter_name
-            if ovn_const.OVN_DROP_PORT_GROUP_NAME not in pg["name"]:
-                stateful = (sg_obj.SecurityGroup
-                            .get_sg_by_id(context, pg["name"]
-                                          .replace('pg_', '', 1)
-                                          .replace('_', '-')).stateful)
-                if not stateful:
+            if pg["name"] != ovn_const.OVN_DROP_PORT_GROUP_NAME:
+                sg = sg_obj.SecurityGroup.get_sg_by_id(
+                    context,
+                    pg["external_ids"][ovn_const.OVN_SG_EXT_ID_KEY])
+                if not sg:
+                    LOG.warning("Port Group %s is missing a corresponding "
+                                "security group, skipping its network log "
+                                "setting...", pg["name"])
+                    continue
+                if not sg.stateful:
                     meter_name = meter_name + ("_stateless")
             for acl_uuid in pg["acls"]:
                 acl_visits += 1
@@ -197,7 +201,8 @@ def _update_log_objs(self, context, ovn_txn, log_objs):
 
     def _pgs_all(self):
         return self.ovn_nb.db_list(
-            "Port_Group", columns=["name", "acls"]).execute(check_error=True)
+            "Port_Group",
+            columns=["name", "external_ids", "acls"]).execute(check_error=True)
 
     def _pgs_from_log_obj(self, context, log_obj):
         """Map Neutron log_obj into affected port groups in OVN.
@@ -216,10 +221,12 @@ def _pgs_from_log_obj(self, context, log_obj):
                 # No sg, no port, DROP: return DROP pg
                 if log_obj.event == log_const.DROP_EVENT:
                     return [{"name": pg_drop.name,
+                             "external_ids": pg_drop.external_ids,
                              "acls": [r.uuid for r in pg_drop.acls]}]
                 # No sg, no port, ACCEPT: return all except DROP pg
                 pgs = self._pgs_all()
                 pgs.remove({"name": pg_drop.name,
+                            "external_ids": pg_drop.external_ids,
                             "acls": [r.uuid for r in pg_drop.acls]})
                 return pgs
             except idlutils.RowNotFound:
@@ -232,6 +239,7 @@ def _pgs_from_log_obj(self, context, log_obj):
                 pg = self.ovn_nb.lookup("Port_Group",
                                         ovn_const.OVN_DROP_PORT_GROUP_NAME)
                 pgs.append({"name": pg.name,
+                            "external_ids": pg.external_ids,
                             "acls": [r.uuid for r in pg.acls]})
             except idlutils.RowNotFound:
                 pass
@@ -244,6 +252,7 @@ def _pgs_from_log_obj(self, context, log_obj):
                                         utils.ovn_port_group_name(
                                             log_obj.resource_id))
                 pgs.append({"name": pg.name,
+                            "external_ids": pg.external_ids,
                             "acls": [r.uuid for r in pg.acls]})
             except idlutils.RowNotFound:
                 pass
@@ -257,6 +266,7 @@ def _pgs_from_log_obj(self, context, log_obj):
                     pg = self.ovn_nb.lookup("Port_Group",
                                             utils.ovn_port_group_name(sg_id))
                     pgs.append({"name": pg.name,
+                                "external_ids": pg.external_ids,
                                 "acls": [r.uuid for r in pg.acls]})
                 except idlutils.RowNotFound:
                     pass

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

@@ -129,18 +129,16 @@ def __init__(self, name=None, **acl_dict):
             self.__dict__ = {**acl_defaults_dict, **acl_dict}
 
     def _fake_pg_dict(self, **kwargs):
+        uuid = uuidutils.generate_uuid()
         pg_defaults_dict = {
-            "name": ovn_utils.ovn_port_group_name(uuidutils.generate_uuid()),
+            "name": ovn_utils.ovn_port_group_name(uuid),
+            "external_ids": {ovn_const.OVN_SG_EXT_ID_KEY: uuid},
             "acls": []
         }
         return {**pg_defaults_dict, **kwargs}
 
     def _fake_pg(self, **kwargs):
-        pg_defaults_dict = {
-            "name": ovn_utils.ovn_port_group_name(uuidutils.generate_uuid()),
-            "acls": []
-        }
-        pg_dict = {**pg_defaults_dict, **kwargs}
+        pg_dict = self._fake_pg_dict(**kwargs)
         return mock.Mock(**pg_dict)
 
     def _fake_log_obj(self, **kwargs):
@@ -190,7 +188,9 @@ def _mock_lookup(_pg_table, pg_name):
             pgs = self._log_driver._pgs_from_log_obj(self.context, log_obj)
             mock_pgs_all.assert_not_called()
             self.assertEqual(2, self._nb_ovn.lookup.call_count)
-            self.assertEqual([{'acls': [], 'name': pg.name}], pgs)
+            self.assertEqual([{'acls': [],
+                               'external_ids': pg.external_ids,
+                               'name': pg.name}], pgs)
 
     def test__pgs_from_log_obj_pg(self):
         with mock.patch.object(self._log_driver, '_pgs_all',
@@ -204,7 +204,9 @@ def test__pgs_from_log_obj_pg(self):
             mock_pgs_all.assert_not_called()
             self._nb_ovn.lookup.assert_called_once_with(
                 "Port_Group", ovn_utils.ovn_port_group_name('resource_id'))
-            self.assertEqual([{'acls': [], 'name': pg.name}], pgs)
+            self.assertEqual([{'acls': [],
+                               'external_ids': pg.external_ids,
+                               'name': pg.name}], pgs)
 
     def test__pgs_from_log_obj_port(self):
         with mock.patch.object(self._log_driver, '_pgs_all',
@@ -221,7 +223,9 @@ def test__pgs_from_log_obj_port(self):
             self._nb_ovn.lookup.assert_called_once_with("Port_Group", pg_name)
             self.fake_get_sgs_attached_to_port.assert_called_once_with(
                 self.context, 'target_id')
-            self.assertEqual([{'acls': [], 'name': pg.name}], pgs)
+            self.assertEqual([{'acls': [],
+                               'external_ids': pg.external_ids,
+                               'name': pg.name}], pgs)
 
     @mock.patch.object(ovn_driver.LOG, 'info')
     def test__remove_acls_log(self, m_info):