Skip to content

Commit 78b7cf8

Browse files
elajkatkatonalala
authored andcommitted
Delete sg rule which remote is the deleted sg
Based on bug #2008712 if we have a security-group which is the remote group of a 2nd security-group, the backend never deletes the rule of the 2nd group which remote_group_id is the original security-group. By AFTER_DELETE event for each rule that has the security_group_id as remote_group_id, we can make the mech drivers do their work and delete these rules in the backend. One version of this fix was merged: https://review.opendev.org/q/I207ecf7954b06507e03cb16b502ceb6e2807e0e7 and reverted due to #2019449: https://review.opendev.org/q/I077fe87435f61bd29d5c1efc979c2adebca26181 This patch is based on https://review.opendev.org/c/openstack/neutron/+/876716/1 Closes-Bug: #2008712 Related-Bug: #2019449 Change-Id: I9e8ddfa26c5402fefd573b0e2ea5f3a57983ca35 (cherry picked from commit 67a0b07)
1 parent 73ba302 commit 78b7cf8

File tree

2 files changed

+56
-1
lines changed

2 files changed

+56
-1
lines changed

neutron/api/rpc/handlers/securitygroups_rpc.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,13 @@ def _clear_child_sg_rules(self, rtype, event, trigger, payload):
303303
for rule in self.rcache.get_resources('SecurityGroupRule', filters):
304304
self.rcache.record_resource_delete(context, 'SecurityGroupRule',
305305
rule.id)
306+
# If there's a rule which remote is the deleted sg, remove that also.
307+
rules = self.rcache.match_resources_with_func(
308+
'SecurityGroupRule',
309+
lambda sg_rule: sg_rule.remote_group_id == existing.id)
310+
for rule in rules:
311+
self.rcache.record_resource_delete(context, 'SecurityGroupRule',
312+
rule.id)
306313

307314
def _handle_sg_rule_delete(self, rtype, event, trigger, payload):
308315
existing = payload.states[0]

neutron/tests/unit/api/rpc/handlers/test_securitygroups_rpc.py

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
from unittest import mock
1616

1717
import netaddr
18+
from neutron_lib.callbacks import events
1819
from neutron_lib import context
1920
from oslo_utils import uuidutils
2021

@@ -120,13 +121,14 @@ def _make_address_group_ovo(self, *args, **kwargs):
120121
return_value=False)
121122
def _make_security_group_ovo(self, *args, **kwargs):
122123
attrs = {'id': uuidutils.generate_uuid(), 'revision_number': 1}
124+
r_group = kwargs.get('remote_group_id') or attrs['id']
123125
sg_rule = securitygroup.SecurityGroupRule(
124126
id=uuidutils.generate_uuid(),
125127
security_group_id=attrs['id'],
126128
direction='ingress',
127129
ethertype='IPv4', protocol='tcp',
128130
port_range_min=400,
129-
remote_group_id=attrs['id'],
131+
remote_group_id=r_group,
130132
revision_number=1,
131133
remote_address_group_id=kwargs.get('remote_address_group_id',
132134
None),
@@ -198,6 +200,52 @@ def test_sg_member_update_events(self):
198200
self.sg_agent.security_groups_member_updated.assert_called_with(
199201
{s1.id})
200202

203+
def test_sg_delete_events_with_remote(self):
204+
s1 = self._make_security_group_ovo(remote_group_id='')
205+
s2 = self._make_security_group_ovo(remote_group_id=s1.id)
206+
rules = self.rcache.get_resources(
207+
'SecurityGroupRule',
208+
filters={'security_group_id': (s1.id, s2.id)})
209+
self.assertEqual(2, len(rules))
210+
self.assertEqual(s1.id, rules[0].remote_group_id)
211+
212+
self.shim._clear_child_sg_rules(
213+
'SecurityGroup', 'after_delete', '',
214+
events.DBEventPayload(
215+
context=self.ctx,
216+
states=[s1]
217+
)
218+
)
219+
rules = self.rcache.get_resources(
220+
'SecurityGroupRule',
221+
filters={'security_group_id': (s1.id, s2.id)})
222+
self.assertEqual(0, len(rules))
223+
224+
def test_sg_delete_events_without_remote(self):
225+
s1 = self._make_security_group_ovo()
226+
s2 = self._make_security_group_ovo()
227+
rules = self.rcache.get_resources(
228+
'SecurityGroupRule',
229+
filters={'security_group_id': (s1.id, s2.id)})
230+
self.assertEqual(2, len(rules))
231+
self.assertEqual(s1.id, rules[0].remote_group_id)
232+
233+
self.shim._clear_child_sg_rules(
234+
'SecurityGroup', 'after_delete', '',
235+
events.DBEventPayload(
236+
context=self.ctx,
237+
states=[s1]
238+
)
239+
)
240+
s1_rules = self.rcache.get_resources(
241+
'SecurityGroupRule',
242+
filters={'security_group_id': (s1.id, )})
243+
self.assertEqual(0, len(s1_rules))
244+
s2_rules = self.rcache.get_resources(
245+
'SecurityGroupRule',
246+
filters={'security_group_id': (s2.id, )})
247+
self.assertEqual(1, len(s2_rules))
248+
201249
def test_get_secgroup_ids_for_address_group(self):
202250
ag = self._make_address_group_ovo()
203251
sg1 = self._make_security_group_ovo(remote_address_group_id=ag.id)

0 commit comments

Comments
 (0)