openstack/neutron
Code Issues Proposed changes

OVS firewall: only remove security group when truly unused

Browse Source 
SG was removed only if both its members and ports were empty.
Change to require neither members nor ports remain before
cleanup, preventing removal of still‐active IPs in SG.

Closes-Bug: #2112648
Change-Id: I57ae71a76c742a7d698d347a72f98e1cff469055
Signed-off-by: Jaroslav Pulchart <jaroslav.pulchart@gooddata.com>
Signed-off-by: Brian Haley <haleyb.dev@gmail.com>
(cherry picked from commit 34225a61f6)
This commit is contained in:
Jaroslav Pulchart
2025-07-08 13:16:45 +02:00
committed by Brian Haley
parent 3751601fbb
commit 5787434fb4
2 changed files with 44 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
neutron/agent/linux/openvswitch_firewall/firewall.py
View File

@@ -868,7 +868,7 @@ class OVSFirewallDriver(firewall.FirewallDriver):
for sg_id in sg_to_delete: for sg_id in sg_to_delete:
sec_group = self.sg_port_map.get_sg(sg_id) sec_group = self.sg_port_map.get_sg(sg_id)
if sec_group.members and sec_group.ports: if sec_group.members or sec_group.ports:
# sec_group is still in use # sec_group is still in use
continue continue

43
neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py
View File

@@ -1089,6 +1089,49 @@ class TestOVSFirewallDriver(base.BaseTestCase):
sg_removed_mock.assert_called_once_with(1) sg_removed_mock.assert_called_once_with(1)
delete_sg_mock.assert_called_once_with(1) delete_sg_mock.assert_called_once_with(1)
def test__cleanup_stale_sg_members_and_ports(self):
self._prepare_security_group()
self.firewall.sg_to_delete = {1}
new_members = {constants.IPv4: [1]}
self.firewall.update_security_group_members(1, new_members)
port_dict = {'device': 'port-id',
'security_groups': [1]}
self.firewall.prepare_port_filter(port_dict)
with mock.patch.object(self.firewall.conj_ip_manager,
'sg_removed') as sg_removed_mock,\
mock.patch.object(self.firewall.sg_port_map,
'delete_sg') as delete_sg_mock:
self.firewall._cleanup_stale_sg()
sg_removed_mock.assert_not_called()
delete_sg_mock.assert_not_called()
def test__cleanup_stale_sg_just_members(self):
self._prepare_security_group()
self.firewall.sg_to_delete = {1}
new_members = {constants.IPv4: [1]}
self.firewall.update_security_group_members(1, new_members)
with mock.patch.object(self.firewall.conj_ip_manager,
'sg_removed') as sg_removed_mock,\
mock.patch.object(self.firewall.sg_port_map,
'delete_sg') as delete_sg_mock:
self.firewall._cleanup_stale_sg()
sg_removed_mock.assert_not_called()
delete_sg_mock.assert_not_called()
def test__cleanup_stale_sg_just_ports(self):
self._prepare_security_group()
self.firewall.sg_to_delete = {1}
port_dict = {'device': 'port-id',
'security_groups': [1]}
self.firewall.prepare_port_filter(port_dict)
with mock.patch.object(self.firewall.conj_ip_manager,
'sg_removed') as sg_removed_mock,\
mock.patch.object(self.firewall.sg_port_map,
'delete_sg') as delete_sg_mock:
self.firewall._cleanup_stale_sg()
sg_removed_mock.assert_not_called()
delete_sg_mock.assert_not_called()
def test_get_ovs_port(self): def test_get_ovs_port(self):
ovs_port = self.firewall.get_ovs_port('port_id') ovs_port = self.firewall.get_ovs_port('port_id')
self.assertEqual(self.fake_ovs_port, ovs_port) self.assertEqual(self.fake_ovs_port, ovs_port)