From ebc0658d5566ce527cb1104968d247db10edf3db Mon Sep 17 00:00:00 2001 From: Brian Haley Date: Fri, 19 May 2023 14:45:53 +0000 Subject: [PATCH] Revert "Delete sg rule which remote is the deleted sg" This reverts commit 63584957203ec9f5ba165177978213c3909f81f0. 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 --- neutron/db/securitygroups_db.py | 37 ------------------- .../drivers/ovn/mech_driver/mech_driver.py | 20 +++++----- .../tests/unit/db/test_securitygroups_db.py | 32 ---------------- 3 files changed, 11 insertions(+), 78 deletions(-) diff --git a/neutron/db/securitygroups_db.py b/neutron/db/securitygroups_db.py index 57cf316ee07..f65b1d8fc3e 100644 --- a/neutron/db/securitygroups_db.py +++ b/neutron/db/securitygroups_db.py @@ -251,12 +251,6 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase, if sg['name'] == 'default' and not context.is_admin: raise ext_sg.SecurityGroupCannotRemoveDefault() - # Check if there are rules with remote_group_id ponting to - # the security_group to be deleted - rules_ids_as_remote = self._get_security_group_rules_by_remote( - context=context, remote_id=id, - ) - self._registry_publish(resources.SECURITY_GROUP, events.BEFORE_DELETE, exc_cls=ext_sg.SecurityGroupInUse, id=id, @@ -285,20 +279,6 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase, context, resource_id=id, states=(sec_group,), metadata={'security_group_rule_ids': sgr_ids, 'name': sg['name']})) - for rule in rules_ids_as_remote: - registry.publish( - resources.SECURITY_GROUP_RULE, - events.AFTER_DELETE, - self, - payload=events.DBEventPayload( - context, - resource_id=rule['id'], - metadata={'security_group_id': rule['security_group_id'], - 'remote_group_id': rule['remote_group_id'], - 'rule': rule - } - ) - ) @db_api.retry_if_session_inactive() def update_security_group(self, context, id, security_group): @@ -385,23 +365,6 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase, self._make_security_group_binding_dict, filters=filters, fields=fields) - def _get_security_group_rules_by_remote(self, context, remote_id): - return model_query.get_collection( - context, sg_models.SecurityGroupRule, - self._make_security_group_rule_dict, - filters={'remote_group_id': [remote_id]}, - fields=['id', - 'remote_group_id', - 'security_group_id', - 'direction', - 'ethertype', - 'protocol', - 'port_range_min', - 'port_range_max', - 'normalized_cidr' - ] - ) - @db_api.retry_if_session_inactive() def _delete_port_security_group_bindings(self, context, port_id): with db_api.CONTEXT_WRITER.using(context): diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py index 1ffb4059b51..ef16d06227d 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py @@ -265,6 +265,9 @@ class OVNMechanismDriver(api.MechanismDriver): registry.subscribe(self._create_security_group, resources.SECURITY_GROUP, events.AFTER_CREATE) + registry.subscribe(self._delete_security_group_precommit, + resources.SECURITY_GROUP, + events.PRECOMMIT_DELETE) registry.subscribe(self._delete_security_group, resources.SECURITY_GROUP, events.AFTER_DELETE) @@ -277,9 +280,6 @@ class OVNMechanismDriver(api.MechanismDriver): registry.subscribe(self._process_sg_rule_notification, resources.SECURITY_GROUP_RULE, events.BEFORE_DELETE) - registry.subscribe(self._process_sg_rule_after_del_notification, - resources.SECURITY_GROUP_RULE, - events.AFTER_DELETE) def _clean_hash_ring(self, *args, **kwargs): admin_context = n_context.get_admin_context() @@ -396,6 +396,14 @@ class OVNMechanismDriver(api.MechanismDriver): self._ovn_client.create_security_group(context, security_group) + def _delete_security_group_precommit(self, resource, event, trigger, + payload): + context = n_context.get_admin_context() + security_group_id = payload.resource_id + for sg_rule in self._plugin.get_security_group_rules( + context, filters={'remote_group_id': [security_group_id]}): + self._ovn_client.delete_security_group_rule(context, sg_rule) + def _delete_security_group(self, resource, event, trigger, payload): context = payload.context security_group_id = payload.resource_id @@ -453,12 +461,6 @@ class OVNMechanismDriver(api.MechanismDriver): context, sg_rule) - def _process_sg_rule_after_del_notification( - self, resource, event, trigger, payload): - context = payload.context - sg_rule = payload.metadata['rule'] - self._ovn_client.delete_security_group_rule(context, sg_rule) - def _sg_has_rules_with_same_normalized_cidr(self, sg_rule): compare_keys = [ 'ethertype', 'direction', 'protocol', diff --git a/neutron/tests/unit/db/test_securitygroups_db.py b/neutron/tests/unit/db/test_securitygroups_db.py index 9036a11e9d1..593272027b7 100644 --- a/neutron/tests/unit/db/test_securitygroups_db.py +++ b/neutron/tests/unit/db/test_securitygroups_db.py @@ -404,38 +404,6 @@ class SecurityGroupDbMixinTestCase(testlib_api.SqlTestCase): self.assertEqual([mock.ANY, mock.ANY], payload.metadata.get('security_group_rule_ids')) - def test_security_group_rule_after_delete_event_for_remot_group(self): - sg1_dict = self.mixin.create_security_group(self.ctx, FAKE_SECGROUP) - sg2_dict = self.mixin.create_security_group(self.ctx, FAKE_SECGROUP) - - fake_rule = copy.deepcopy(FAKE_SECGROUP_RULE) - fake_rule['security_group_rule']['security_group_id'] = sg1_dict['id'] - fake_rule['security_group_rule']['remote_group_id'] = sg2_dict['id'] - fake_rule['security_group_rule']['remote_ip_prefix'] = None - remote_rule = self.mixin.create_security_group_rule( - self.ctx, fake_rule) - - with mock.patch.object(registry, "publish") as mock_publish: - self.mixin.delete_security_group(self.ctx, sg2_dict['id']) - mock_publish.assert_has_calls( - [mock.call('security_group', 'before_delete', - mock.ANY, payload=mock.ANY), - mock.call('security_group', 'precommit_delete', - mock.ANY, - payload=mock.ANY), - mock.call('security_group', 'after_delete', - mock.ANY, - payload=mock.ANY), - mock.call('security_group_rule', 'after_delete', - mock.ANY, - payload=mock.ANY)]) - rule_payload = mock_publish.mock_calls[3][2]['payload'] - self.assertEqual(remote_rule['id'], rule_payload.resource_id) - self.assertEqual(sg1_dict['id'], - rule_payload.metadata['security_group_id']) - self.assertEqual(sg2_dict['id'], - rule_payload.metadata['remote_group_id']) - def test_security_group_rule_precommit_create_event_fail(self): registry.subscribe(fake_callback, resources.SECURITY_GROUP_RULE, events.PRECOMMIT_CREATE)