From 1bfc75a71b6f2fb995dfc7fc0365dec5410d7e31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elvira=20Garc=C3=ADa?= Date: Fri, 15 Sep 2023 16:33:43 +0200 Subject: [PATCH] Use safer methods to get security groups on security group logging There is a chance on real environment that a port group doesn't have any correspondent security group (and there are maintenance tasks that will remove them). This patch avoids a DriverError from Neutron in case we are in an environment with a port group that was mistakenly left over due to any reason. Instead, a Warning log will be raised. Related-bug: #2032929 Change-Id: I42208557c8522d6fbc29df8a3c7d0367cace31e4 (cherry picked from commit 67bd591c5b9280f1af7c8a4942dcbb3bd1270cfb) --- neutron/services/logapi/drivers/ovn/driver.py | 28 +++++++++++++------ .../logapi/drivers/ovn/test_driver.py | 22 +++++++++------ 2 files changed, 32 insertions(+), 18 deletions(-) diff --git a/neutron/services/logapi/drivers/ovn/driver.py b/neutron/services/logapi/drivers/ovn/driver.py index 26c5c7c2d46..b220cde619d 100644 --- a/neutron/services/logapi/drivers/ovn/driver.py +++ b/neutron/services/logapi/drivers/ovn/driver.py @@ -199,12 +199,16 @@ class OVNDriver(base.DriverBase): 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 @@ -239,7 +243,8 @@ class OVNDriver(base.DriverBase): 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. @@ -258,11 +263,13 @@ class OVNDriver(base.DriverBase): # No sg, no port, DROP: return DROP pg if log_obj.event == log_const.DROP_EVENT: return [{"name": pg_drop.name, - "acls": [r.uuid for r in pg_drop.acls]}] + "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, - "acls": [r.uuid for r in pg_drop.acls]}) + "external_ids": pg_drop.external_ids, + "acls": [r.uuid for r in pg_drop.acls]}) return pgs except idlutils.RowNotFound: pass @@ -274,6 +281,7 @@ class OVNDriver(base.DriverBase): 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 @@ -286,6 +294,7 @@ class OVNDriver(base.DriverBase): 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 @@ -299,6 +308,7 @@ class OVNDriver(base.DriverBase): 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 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 a75c900836d..8399f244e09 100644 --- a/neutron/tests/unit/services/logapi/drivers/ovn/test_driver.py +++ b/neutron/tests/unit/services/logapi/drivers/ovn/test_driver.py @@ -129,18 +129,16 @@ class TestOVNDriver(TestOVNDriverBase): 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 @@ class TestOVNDriver(TestOVNDriverBase): 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 @@ class TestOVNDriver(TestOVNDriverBase): 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 @@ class TestOVNDriver(TestOVNDriverBase): 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):