From 34225a61f6cf9634379ed49753bb40077a8648fb Mon Sep 17 00:00:00 2001 From: Jaroslav Pulchart Date: Tue, 8 Jul 2025 13:16:45 +0200 Subject: [PATCH] OVS firewall: only remove security group when truly unused MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Brian Haley --- .../linux/openvswitch_firewall/firewall.py | 2 +- .../openvswitch_firewall/test_firewall.py | 43 +++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/neutron/agent/linux/openvswitch_firewall/firewall.py b/neutron/agent/linux/openvswitch_firewall/firewall.py index 2f0cfb5e004..6995ca57a38 100644 --- a/neutron/agent/linux/openvswitch_firewall/firewall.py +++ b/neutron/agent/linux/openvswitch_firewall/firewall.py @@ -872,7 +872,7 @@ class OVSFirewallDriver(firewall.FirewallDriver): for sg_id in sg_to_delete: 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 continue diff --git a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py index eca3ee2acf4..8a9d4c09fb2 100644 --- a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py +++ b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py @@ -1090,6 +1090,49 @@ class TestOVSFirewallDriver(base.BaseTestCase): sg_removed_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): ovs_port = self.firewall.get_ovs_port('port_id') self.assertEqual(self.fake_ovs_port, ovs_port)