Skip to content

Commit 8bf6f7f

Browse files
brianphaleyralonsoh
authored andcommitted
Revert "Delete sg rule which remote is the deleted sg"
This reverts commit 6358495. Reason for revert: This is generating a lot of "SecurityGroupNotFound" errors in neutron-server.log in the tempest-integrated-networking job. Closes-Bug: #2019449 Related-Bug: #2008712 Change-Id: I077fe87435f61bd29d5c1efc979c2adebca26181 (cherry picked from commit ebc0658) (cherry picked from commit d3fb576)
1 parent 8def3b6 commit 8bf6f7f

File tree

3 files changed

+11
-78
lines changed

3 files changed

+11
-78
lines changed

neutron/db/securitygroups_db.py

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -253,12 +253,6 @@ def delete_security_group(self, context, id):
253253
if sg['name'] == 'default' and not context.is_admin:
254254
raise ext_sg.SecurityGroupCannotRemoveDefault()
255255

256-
# Check if there are rules with remote_group_id ponting to
257-
# the security_group to be deleted
258-
rules_ids_as_remote = self._get_security_group_rules_by_remote(
259-
context=context, remote_id=id,
260-
)
261-
262256
self._registry_publish(resources.SECURITY_GROUP,
263257
events.BEFORE_DELETE,
264258
exc_cls=ext_sg.SecurityGroupInUse, id=id,
@@ -287,20 +281,6 @@ def delete_security_group(self, context, id):
287281
context, resource_id=id, states=(sec_group,),
288282
metadata={'security_group_rule_ids': sgr_ids,
289283
'name': sg['name']}))
290-
for rule in rules_ids_as_remote:
291-
registry.publish(
292-
resources.SECURITY_GROUP_RULE,
293-
events.AFTER_DELETE,
294-
self,
295-
payload=events.DBEventPayload(
296-
context,
297-
resource_id=rule['id'],
298-
metadata={'security_group_id': rule['security_group_id'],
299-
'remote_group_id': rule['remote_group_id'],
300-
'rule': rule
301-
}
302-
)
303-
)
304284

305285
@db_api.retry_if_session_inactive()
306286
def update_security_group(self, context, id, security_group):
@@ -387,23 +367,6 @@ def _get_port_security_group_bindings(self, context,
387367
self._make_security_group_binding_dict,
388368
filters=filters, fields=fields)
389369

390-
def _get_security_group_rules_by_remote(self, context, remote_id):
391-
return model_query.get_collection(
392-
context, sg_models.SecurityGroupRule,
393-
self._make_security_group_rule_dict,
394-
filters={'remote_group_id': [remote_id]},
395-
fields=['id',
396-
'remote_group_id',
397-
'security_group_id',
398-
'direction',
399-
'ethertype',
400-
'protocol',
401-
'port_range_min',
402-
'port_range_max',
403-
'normalized_cidr'
404-
]
405-
)
406-
407370
@db_api.retry_if_session_inactive()
408371
def _delete_port_security_group_bindings(self, context, port_id):
409372
with db_api.CONTEXT_WRITER.using(context):

neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,9 @@ def subscribe(self):
261261
registry.subscribe(self._create_security_group,
262262
resources.SECURITY_GROUP,
263263
events.AFTER_CREATE)
264+
registry.subscribe(self._delete_security_group_precommit,
265+
resources.SECURITY_GROUP,
266+
events.PRECOMMIT_DELETE)
264267
registry.subscribe(self._delete_security_group,
265268
resources.SECURITY_GROUP,
266269
events.AFTER_DELETE)
@@ -273,9 +276,6 @@ def subscribe(self):
273276
registry.subscribe(self._process_sg_rule_notification,
274277
resources.SECURITY_GROUP_RULE,
275278
events.BEFORE_DELETE)
276-
registry.subscribe(self._process_sg_rule_after_del_notification,
277-
resources.SECURITY_GROUP_RULE,
278-
events.AFTER_DELETE)
279279

280280
def _clean_hash_ring(self, *args, **kwargs):
281281
admin_context = n_context.get_admin_context()
@@ -392,6 +392,14 @@ def _create_security_group(self, resource, event, trigger, payload):
392392
self._ovn_client.create_security_group(context,
393393
security_group)
394394

395+
def _delete_security_group_precommit(self, resource, event, trigger,
396+
payload):
397+
context = n_context.get_admin_context()
398+
security_group_id = payload.resource_id
399+
for sg_rule in self._plugin.get_security_group_rules(
400+
context, filters={'remote_group_id': [security_group_id]}):
401+
self._ovn_client.delete_security_group_rule(context, sg_rule)
402+
395403
def _delete_security_group(self, resource, event, trigger, payload):
396404
context = payload.context
397405
security_group_id = payload.resource_id
@@ -449,12 +457,6 @@ def _process_sg_rule_notification(
449457
context,
450458
sg_rule)
451459

452-
def _process_sg_rule_after_del_notification(
453-
self, resource, event, trigger, payload):
454-
context = payload.context
455-
sg_rule = payload.metadata['rule']
456-
self._ovn_client.delete_security_group_rule(context, sg_rule)
457-
458460
def _sg_has_rules_with_same_normalized_cidr(self, sg_rule):
459461
compare_keys = [
460462
'ethertype', 'direction', 'protocol',

neutron/tests/unit/db/test_securitygroups_db.py

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -404,38 +404,6 @@ def test_security_group_precommit_and_after_delete_event(self):
404404
self.assertEqual([mock.ANY, mock.ANY],
405405
payload.metadata.get('security_group_rule_ids'))
406406

407-
def test_security_group_rule_after_delete_event_for_remot_group(self):
408-
sg1_dict = self.mixin.create_security_group(self.ctx, FAKE_SECGROUP)
409-
sg2_dict = self.mixin.create_security_group(self.ctx, FAKE_SECGROUP)
410-
411-
fake_rule = copy.deepcopy(FAKE_SECGROUP_RULE)
412-
fake_rule['security_group_rule']['security_group_id'] = sg1_dict['id']
413-
fake_rule['security_group_rule']['remote_group_id'] = sg2_dict['id']
414-
fake_rule['security_group_rule']['remote_ip_prefix'] = None
415-
remote_rule = self.mixin.create_security_group_rule(
416-
self.ctx, fake_rule)
417-
418-
with mock.patch.object(registry, "publish") as mock_publish:
419-
self.mixin.delete_security_group(self.ctx, sg2_dict['id'])
420-
mock_publish.assert_has_calls(
421-
[mock.call('security_group', 'before_delete',
422-
mock.ANY, payload=mock.ANY),
423-
mock.call('security_group', 'precommit_delete',
424-
mock.ANY,
425-
payload=mock.ANY),
426-
mock.call('security_group', 'after_delete',
427-
mock.ANY,
428-
payload=mock.ANY),
429-
mock.call('security_group_rule', 'after_delete',
430-
mock.ANY,
431-
payload=mock.ANY)])
432-
rule_payload = mock_publish.mock_calls[3][2]['payload']
433-
self.assertEqual(remote_rule['id'], rule_payload.resource_id)
434-
self.assertEqual(sg1_dict['id'],
435-
rule_payload.metadata['security_group_id'])
436-
self.assertEqual(sg2_dict['id'],
437-
rule_payload.metadata['remote_group_id'])
438-
439407
def test_security_group_rule_precommit_create_event_fail(self):
440408
registry.subscribe(fake_callback, resources.SECURITY_GROUP_RULE,
441409
events.PRECOMMIT_CREATE)

0 commit comments

Comments
 (0)