From 5787434fb4538906ea411268fa95b2665d18b2a7 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 (cherry picked from commit 34225a61f6cf9634379ed49753bb40077a8648fb) --- .../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 496f918503e..e028ad032c2 100644 --- a/neutron/agent/linux/openvswitch_firewall/firewall.py +++ b/neutron/agent/linux/openvswitch_firewall/firewall.py @@ -868,7 +868,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 96cb10a9c09..f094fd49f8f 100644 --- a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py +++ b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py @@ -1089,6 +1089,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)