From 941be42a61bca5b4cbfe682a0fa232f16a840d56 Mon Sep 17 00:00:00 2001 From: Nurmatov Mamatisa Date: Tue, 25 May 2021 09:44:28 +0300 Subject: [PATCH] use callback payloads for SECURITY_GROUP_RULE This patch switches over to callback payloads for SECURITY_GROUP_RULE events. Change-Id: Id80dc6790226cc81cb6535dc1bcaba58e991fdcb --- neutron/db/securitygroups_db.py | 75 ++++++++++++------- .../drivers/ovn/mech_driver/mech_driver.py | 18 +++-- neutron/plugins/ml2/ovo_rpc.py | 4 +- .../tests/unit/db/test_securitygroups_db.py | 66 ++++++++++------ .../ovn/mech_driver/test_mech_driver.py | 9 ++- 5 files changed, 109 insertions(+), 63 deletions(-) diff --git a/neutron/db/securitygroups_db.py b/neutron/db/securitygroups_db.py index 1be85e5c1ef..d254f9eac14 100644 --- a/neutron/db/securitygroups_db.py +++ b/neutron/db/securitygroups_db.py @@ -394,17 +394,24 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase, context, rule_dict, validate=False) ret.append(res_rule_dict) for rdict in ret: - registry.notify( - resources.SECURITY_GROUP_RULE, events.AFTER_CREATE, self, - context=context, security_group_rule=rdict) + registry.publish(resources.SECURITY_GROUP_RULE, + events.AFTER_CREATE, + self, + payload=events.DBEventPayload( + context, + resource_id=rdict['id'], + states=(rdict,))) return ret @db_api.retry_if_session_inactive() def create_security_group_rule(self, context, security_group_rule): res = self._create_security_group_rule(context, security_group_rule) - registry.notify( - resources.SECURITY_GROUP_RULE, events.AFTER_CREATE, self, - context=context, security_group_rule=res) + registry.publish(resources.SECURITY_GROUP_RULE, events.AFTER_CREATE, + self, payload=events.DBEventPayload( + context, + resource_id=res['id'], + states=(res,))) + return res def _create_security_group_rule(self, context, security_group_rule, @@ -444,13 +451,14 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase, if port_range_max is not None: args['port_range_max'] = port_range_max - kwargs = { - 'context': context, - 'security_group_rule': args - } - self._registry_notify(resources.SECURITY_GROUP_RULE, - events.BEFORE_CREATE, - exc_cls=ext_sg.SecurityGroupConflict, **kwargs) + self._registry_notify( + resources.SECURITY_GROUP_RULE, + events.BEFORE_CREATE, + exc_cls=ext_sg.SecurityGroupConflict, + payload=events.DBEventPayload( + context, resource_id=args['id'], + states=(args,))) + with db_api.CONTEXT_WRITER.using(context): if validate: self._check_for_duplicate_rules(context, sg_id, @@ -463,11 +471,14 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase, sg_rule = sg_obj.SecurityGroupRule.get_object(context, id=sg_rule.id) res_rule_dict = self._make_security_group_rule_dict(sg_rule.db_obj) - kwargs['security_group_rule'] = res_rule_dict self._registry_notify( resources.SECURITY_GROUP_RULE, events.PRECOMMIT_CREATE, - exc_cls=ext_sg.SecurityGroupConflict, **kwargs) + exc_cls=ext_sg.SecurityGroupConflict, + payload=events.DBEventPayload( + context, resource_id=res_rule_dict['id'], + states=(res_rule_dict,))) + return res_rule_dict def _get_ip_proto_number(self, protocol): @@ -833,26 +844,32 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase, @db_api.retry_if_session_inactive() def delete_security_group_rule(self, context, id): - kwargs = { - 'context': context, - 'security_group_rule_id': id - } self._registry_notify(resources.SECURITY_GROUP_RULE, - events.BEFORE_DELETE, id=id, - exc_cls=ext_sg.SecurityGroupRuleInUse, **kwargs) + events.BEFORE_DELETE, + exc_cls=ext_sg.SecurityGroupRuleInUse, + payload=events.DBEventPayload( + context, resource_id=id,)) with db_api.CONTEXT_WRITER.using(context): sgr = self._get_security_group_rule(context, id) - kwargs['security_group_id'] = sgr['security_group_id'] - self._registry_notify(resources.SECURITY_GROUP_RULE, - events.PRECOMMIT_DELETE, - exc_cls=ext_sg.SecurityGroupRuleInUse, id=id, - **kwargs) + self._registry_notify( + resources.SECURITY_GROUP_RULE, + events.PRECOMMIT_DELETE, + exc_cls=ext_sg.SecurityGroupRuleInUse, + payload=events.DBEventPayload( + context, + resource_id=id, + metadata={'security_group_id': sgr['security_group_id']})) sgr.delete() - registry.notify( - resources.SECURITY_GROUP_RULE, events.AFTER_DELETE, self, - **kwargs) + registry.publish( + resources.SECURITY_GROUP_RULE, + events.AFTER_DELETE, + self, + payload=events.DBEventPayload( + context, + resource_id=id, + metadata={'security_group_id': sgr['security_group_id']})) @staticmethod @resource_extend.extends([port_def.COLLECTION_NAME]) 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 d80dd53b444..e390c4f7b35 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py @@ -379,26 +379,30 @@ class OVNMechanismDriver(api.MechanismDriver): ovn_revision_numbers_db.bump_revision( kwargs['context'], security_group, ovn_const.TYPE_SECURITY_GROUPS) - def _create_sg_rule_precommit(self, resource, event, trigger, **kwargs): - sg_rule = kwargs.get('security_group_rule') - context = kwargs.get('context') + def _create_sg_rule_precommit(self, resource, event, trigger, + payload): + sg_rule = payload.latest_state + context = payload.context ovn_revision_numbers_db.create_initial_revision( context, sg_rule['id'], ovn_const.TYPE_SECURITY_GROUP_RULES, std_attr_id=sg_rule['standard_attr_id']) def _process_sg_rule_notification( - self, resource, event, trigger, **kwargs): + self, resource, event, trigger, payload): + context = payload.context + security_group_rule = payload.latest_state + security_group_rule_id = payload.resource_id if event == events.AFTER_CREATE: self._ovn_client.create_security_group_rule( - kwargs['context'], kwargs.get('security_group_rule')) + context, security_group_rule) elif event == events.BEFORE_DELETE: sg_rule = self._plugin.get_security_group_rule( - kwargs['context'], kwargs.get('security_group_rule_id')) + context, security_group_rule_id) if sg_rule.get('remote_ip_prefix') is not None: if self._sg_has_rules_with_same_normalized_cidr(sg_rule): return self._ovn_client.delete_security_group_rule( - kwargs['context'], + context, sg_rule) def _sg_has_rules_with_same_normalized_cidr(self, sg_rule): diff --git a/neutron/plugins/ml2/ovo_rpc.py b/neutron/plugins/ml2/ovo_rpc.py index c91e5b3468e..e5cb15cd3da 100644 --- a/neutron/plugins/ml2/ovo_rpc.py +++ b/neutron/plugins/ml2/ovo_rpc.py @@ -37,7 +37,9 @@ LOG = logging.getLogger(__name__) class _ObjectChangeHandler(object): - _PAYLOAD_RESOURCES = (resources.NETWORK, resources.ADDRESS_GROUP,) + _PAYLOAD_RESOURCES = (resources.NETWORK, + resources.ADDRESS_GROUP, + resources.SECURITY_GROUP_RULE,) def __init__(self, resource, object_class, resource_push_api): self._resource = resource diff --git a/neutron/tests/unit/db/test_securitygroups_db.py b/neutron/tests/unit/db/test_securitygroups_db.py index 98dc5a60bf9..0271744f9fd 100644 --- a/neutron/tests/unit/db/test_securitygroups_db.py +++ b/neutron/tests/unit/db/test_securitygroups_db.py @@ -124,8 +124,8 @@ class SecurityGroupDbMixinTestCase(testlib_api.SqlTestCase): with mock.patch.object(self.mixin, '_validate_security_group_rule'),\ mock.patch.object(self.mixin, '_check_for_duplicate_rules'),\ - mock.patch.object(registry, "notify") as mock_notify: - mock_notify.side_effect = exceptions.CallbackFailure(Exception()) + mock.patch.object(registry, "publish") as mock_publish: + mock_publish.side_effect = exceptions.CallbackFailure(Exception()) with testtools.ExpectedException( securitygroup.SecurityGroupConflict): self.mixin.create_security_group_rule( @@ -201,8 +201,8 @@ class SecurityGroupDbMixinTestCase(testlib_api.SqlTestCase): context, 'fake', [rule_dict]) def test_delete_security_group_rule_in_use(self): - with mock.patch.object(registry, "notify") as mock_notify: - mock_notify.side_effect = exceptions.CallbackFailure(Exception()) + with mock.patch.object(registry, "publish") as mock_publish: + mock_publish.side_effect = exceptions.CallbackFailure(Exception()) with testtools.ExpectedException( securitygroup.SecurityGroupRuleInUse): self.mixin.delete_security_group_rule(self.ctx, mock.ANY) @@ -415,34 +415,54 @@ class SecurityGroupDbMixinTestCase(testlib_api.SqlTestCase): sg_dict = self.mixin.create_security_group(self.ctx, FAKE_SECGROUP) fake_rule = FAKE_SECGROUP_RULE fake_rule['security_group_rule']['security_group_id'] = sg_dict['id'] - with mock.patch.object(registry, "notify") as mock_notify, \ + with mock.patch.object(registry, "publish") as mock_publish, \ mock.patch.object(self.mixin, '_get_security_group'): - mock_notify.assert_has_calls([mock.call('security_group_rule', - 'precommit_create', mock.ANY, context=mock.ANY, - security_group_rule=self.mixin.create_security_group_rule( - self.ctx, fake_rule))]) + sg_rule = self.mixin.create_security_group_rule(self.ctx, + fake_rule) + mock_publish.assert_has_calls([mock.call( + 'security_group_rule', + 'precommit_create', mock.ANY, payload=mock.ANY)]) + + payload = mock_publish.mock_calls[1][2]['payload'] + self.assertEqual(sg_rule['id'], payload.resource_id) + self.assertEqual(sg_rule, payload.latest_state) def test_sg_rule_before_precommit_and_after_delete_event(self): sg_dict = self.mixin.create_security_group(self.ctx, FAKE_SECGROUP) fake_rule = FAKE_SECGROUP_RULE fake_rule['security_group_rule']['security_group_id'] = sg_dict['id'] - with mock.patch.object(registry, "notify") as mock_notify, \ + with mock.patch.object(registry, "publish") as mock_publish, \ mock.patch.object(self.mixin, '_get_security_group'): sg_rule_dict = self.mixin.create_security_group_rule(self.ctx, - fake_rule) + fake_rule) self.mixin.delete_security_group_rule(self.ctx, - sg_rule_dict['id']) - mock_notify.assert_has_calls([mock.call('security_group_rule', - 'before_delete', mock.ANY, context=mock.ANY, - security_group_rule_id=sg_rule_dict['id'])]) - mock_notify.assert_has_calls([mock.call('security_group_rule', - 'precommit_delete', mock.ANY, context=mock.ANY, - security_group_id=sg_dict['id'], - security_group_rule_id=sg_rule_dict['id'])]) - mock_notify.assert_has_calls([mock.call('security_group_rule', - 'after_delete', mock.ANY, context=mock.ANY, - security_group_rule_id=sg_rule_dict['id'], - security_group_id=sg_dict['id'])]) + sg_rule_dict['id']) + mock_publish.assert_has_calls([mock.call('security_group_rule', + 'before_delete', + mock.ANY, + payload=mock.ANY)]) + mock_publish.assert_has_calls([mock.call('security_group_rule', + 'precommit_delete', + mock.ANY, + payload=mock.ANY)]) + mock_publish.assert_has_calls([mock.call('security_group_rule', + 'after_delete', + mock.ANY, + payload=mock.ANY)]) + + payload = mock_publish.mock_calls[1][2]['payload'] + self.assertEqual(mock.ANY, payload.context) + self.assertEqual(sg_rule_dict, payload.latest_state) + self.assertEqual(sg_rule_dict['id'], payload.resource_id) + + payload = mock_publish.mock_calls[2][2]['payload'] + self.assertEqual(mock.ANY, payload.context) + self.assertEqual(sg_rule_dict, payload.latest_state) + self.assertEqual(sg_rule_dict['id'], payload.resource_id) + + payload = mock_publish.mock_calls[3][2]['payload'] + self.assertEqual(mock.ANY, payload.context) + self.assertEqual(sg_rule_dict['id'], payload.resource_id) def test_get_ip_proto_name_and_num(self): protocols = [constants.PROTO_NAME_UDP, str(constants.PROTO_NUM_TCP), diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index 525fd146f38..bf4b278f0fc 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -258,7 +258,8 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase): rule = {'security_group_id': 'sg_id'} self.mech_driver._process_sg_rule_notification( resources.SECURITY_GROUP_RULE, events.AFTER_CREATE, {}, - security_group_rule=rule, context=self.context) + payload=events.DBEventPayload( + self.context, states=(rule,))) has_same_rules.assert_not_called() ovn_acl_up.assert_called_once_with( mock.ANY, mock.ANY, mock.ANY, @@ -278,7 +279,8 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase): 'remote_ip_prefix': '1.0.0.0/24'} self.mech_driver._process_sg_rule_notification( resources.SECURITY_GROUP_RULE, events.AFTER_CREATE, {}, - security_group_rule=rule, context=self.context) + payload=events.DBEventPayload( + self.context, states=(rule,))) has_same_rules.assert_not_called() ovn_acl_up.assert_called_once_with( mock.ANY, mock.ANY, mock.ANY, @@ -296,7 +298,8 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase): return_value=rule): self.mech_driver._process_sg_rule_notification( resources.SECURITY_GROUP_RULE, events.BEFORE_DELETE, {}, - security_group_rule=rule, context=self.context) + payload=events.DBEventPayload( + self.context, states=(rule,))) ovn_acl_up.assert_called_once_with( mock.ANY, mock.ANY, mock.ANY, 'sg_id', rule, is_add_acl=False, stateless_supported=False)