openstack
/
neutron
Code Issues Proposed changes

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 67bd591c5b)
This commit is contained in:
Elvira García 2023-09-15 16:33:43 +02:00 committed by Elvira García Ruiz
parent 05aed84c2a
commit f1638bb6d1
2 changed files with 29 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
neutron/services/logapi/drivers/ovn/driver.py
View File

@ -157,11 +157,15 @@ class OVNDriver(base.DriverBase):
acl_changes, acl_visits = 0, 0 acl_changes, acl_visits = 0, 0
for pg in pgs: for pg in pgs:
meter_name = self.meter_name meter_name = self.meter_name
if ovn_const.OVN_DROP_PORT_GROUP_NAME not in pg["name"]: if pg["name"] != ovn_const.OVN_DROP_PORT_GROUP_NAME:
stateful = (sg_obj.SecurityGroup.get_sg_by_id(context, sg = sg_obj.SecurityGroup.get_sg_by_id(context,
pg["name"].replace('pg_', '', 1).replace('_', '-')) pg["external_ids"][ovn_const.OVN_SG_EXT_ID_KEY])
.stateful) if not sg:
if not stateful: 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") meter_name = meter_name + ("_stateless")
for acl_uuid in pg["acls"]: for acl_uuid in pg["acls"]:
acl_visits += 1 acl_visits += 1
@ -196,7 +200,8 @@ class OVNDriver(base.DriverBase):
def _pgs_all(self): def _pgs_all(self):
return self.ovn_nb.db_list( 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): def _pgs_from_log_obj(self, context, log_obj):
"""Map Neutron log_obj into affected port groups in OVN. """Map Neutron log_obj into affected port groups in OVN.
@ -215,10 +220,12 @@ class OVNDriver(base.DriverBase):
# No sg, no port, DROP: return DROP pg # No sg, no port, DROP: return DROP pg
if log_obj.event == log_const.DROP_EVENT: if log_obj.event == log_const.DROP_EVENT:
return [{"name": pg_drop.name, return [{"name": pg_drop.name,
"external_ids": pg_drop.external_ids,
"acls": [r.uuid for r in pg_drop.acls]}] "acls": [r.uuid for r in pg_drop.acls]}]
# No sg, no port, ACCEPT: return all except DROP pg # No sg, no port, ACCEPT: return all except DROP pg
pgs = self._pgs_all() pgs = self._pgs_all()
pgs.remove({"name": pg_drop.name, pgs.remove({"name": pg_drop.name,
"external_ids": pg_drop.external_ids,
"acls": [r.uuid for r in pg_drop.acls]}) "acls": [r.uuid for r in pg_drop.acls]})
return pgs return pgs
except idlutils.RowNotFound: except idlutils.RowNotFound:
@ -231,6 +238,7 @@ class OVNDriver(base.DriverBase):
pg = self.ovn_nb.lookup("Port_Group", pg = self.ovn_nb.lookup("Port_Group",
ovn_const.OVN_DROP_PORT_GROUP_NAME) ovn_const.OVN_DROP_PORT_GROUP_NAME)
pgs.append({"name": pg.name, pgs.append({"name": pg.name,
"external_ids": pg.external_ids,
"acls": [r.uuid for r in pg.acls]}) "acls": [r.uuid for r in pg.acls]})
except idlutils.RowNotFound: except idlutils.RowNotFound:
pass pass
@ -243,6 +251,7 @@ class OVNDriver(base.DriverBase):
utils.ovn_port_group_name( utils.ovn_port_group_name(
log_obj.resource_id)) log_obj.resource_id))
pgs.append({"name": pg.name, pgs.append({"name": pg.name,
"external_ids": pg.external_ids,
"acls": [r.uuid for r in pg.acls]}) "acls": [r.uuid for r in pg.acls]})
except idlutils.RowNotFound: except idlutils.RowNotFound:
pass pass
@ -256,6 +265,7 @@ class OVNDriver(base.DriverBase):
pg = self.ovn_nb.lookup("Port_Group", pg = self.ovn_nb.lookup("Port_Group",
utils.ovn_port_group_name(sg_id)) utils.ovn_port_group_name(sg_id))
pgs.append({"name": pg.name, pgs.append({"name": pg.name,
"external_ids": pg.external_ids,
"acls": [r.uuid for r in pg.acls]}) "acls": [r.uuid for r in pg.acls]})
except idlutils.RowNotFound: except idlutils.RowNotFound:
pass pass

22
neutron/tests/unit/services/logapi/drivers/ovn/test_driver.py
View File

@ -129,18 +129,16 @@ class TestOVNDriver(TestOVNDriverBase):
self.__dict__ = {**acl_defaults_dict, **acl_dict} self.__dict__ = {**acl_defaults_dict, **acl_dict}
def _fake_pg_dict(self, **kwargs): def _fake_pg_dict(self, **kwargs):
uuid = uuidutils.generate_uuid()
pg_defaults_dict = { 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": [] "acls": []
} }
return {**pg_defaults_dict, **kwargs} return {**pg_defaults_dict, **kwargs}
def _fake_pg(self, **kwargs): def _fake_pg(self, **kwargs):
pg_defaults_dict = { pg_dict = self._fake_pg_dict(**kwargs)
"name": ovn_utils.ovn_port_group_name(uuidutils.generate_uuid()),
"acls": []
}
pg_dict = {**pg_defaults_dict, **kwargs}
return mock.Mock(**pg_dict) return mock.Mock(**pg_dict)
def _fake_log_obj(self, **kwargs): 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) pgs = self._log_driver._pgs_from_log_obj(self.context, log_obj)
mock_pgs_all.assert_not_called() mock_pgs_all.assert_not_called()
self.assertEqual(2, self._nb_ovn.lookup.call_count) 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): def test__pgs_from_log_obj_pg(self):
with mock.patch.object(self._log_driver, '_pgs_all', with mock.patch.object(self._log_driver, '_pgs_all',
@ -204,7 +204,9 @@ class TestOVNDriver(TestOVNDriverBase):
mock_pgs_all.assert_not_called() mock_pgs_all.assert_not_called()
self._nb_ovn.lookup.assert_called_once_with( self._nb_ovn.lookup.assert_called_once_with(
"Port_Group", ovn_utils.ovn_port_group_name('resource_id')) "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): def test__pgs_from_log_obj_port(self):
with mock.patch.object(self._log_driver, '_pgs_all', 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._nb_ovn.lookup.assert_called_once_with("Port_Group", pg_name)
self.fake_get_sgs_attached_to_port.assert_called_once_with( self.fake_get_sgs_attached_to_port.assert_called_once_with(
self.context, 'target_id') 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') @mock.patch.object(ovn_driver.LOG, 'info')
def test__remove_acls_log(self, m_info): def test__remove_acls_log(self, m_info):