From 89abce3efe0645cc08e840b4cc4d455889ee8866 Mon Sep 17 00:00:00 2001 From: Reedip Date: Wed, 22 Mar 2017 07:24:33 +0000 Subject: [PATCH] Pass the complete info in sg/rules db into PRECOMMIT_XXX callback PRECOMMIT_XXX events callback need completed sg info, like the sg id and its related rules for registered driver. Change-Id: I6f49f25eb2ad16221357024f45a6bb6175d5cd55 Co-Authored-By: Rui Wang Co-Authored-By: Manjeet Singh Bhatia Co-Authored-By: Yalei Wang Closes-bug: #1546910 --- neutron/db/securitygroups_db.py | 45 ++++++++-------- .../tests/unit/db/test_securitygroups_db.py | 54 +++++++++++++++---- 2 files changed, 67 insertions(+), 32 deletions(-) diff --git a/neutron/db/securitygroups_db.py b/neutron/db/securitygroups_db.py index ba4b3bb308a..e4aea0655b4 100644 --- a/neutron/db/securitygroups_db.py +++ b/neutron/db/securitygroups_db.py @@ -116,16 +116,15 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase): sg.rules.append(egress_rule) sg.obj_reset_changes(['rules']) + # fetch sg from db to load the sg rules with sg model. + sg = sg_obj.SecurityGroup.get_object(context, id=sg.id) + secgroup_dict = self._make_security_group_dict(sg) + kwargs['security_group'] = secgroup_dict self._registry_notify(resources.SECURITY_GROUP, events.PRECOMMIT_CREATE, exc_cls=ext_sg.SecurityGroupConflict, **kwargs) - # fetch sg from db to load the sg rules with sg model. - sg = sg_obj.SecurityGroup.get_object(context, id=sg.id) - secgroup_dict = self._make_security_group_dict(sg) - - kwargs['security_group'] = secgroup_dict registry.notify(resources.SECURITY_GROUP, events.AFTER_CREATE, self, **kwargs) return secgroup_dict @@ -220,6 +219,7 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase): ports = self._get_port_security_group_bindings(context, filters) sg = self._get_security_group(context, id) kwargs['security_group_rule_ids'] = [r['id'] for r in sg.rules] + kwargs['security_group'] = self._make_security_group_dict(sg) self._registry_notify(resources.SECURITY_GROUP, events.PRECOMMIT_DELETE, exc_cls=ext_sg.SecurityGroupInUse, id=id, @@ -246,6 +246,8 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase): sg = self._get_security_group(context, id) if sg.name == 'default' and 'name' in s: raise ext_sg.SecurityGroupCannotUpdateDefault() + kwargs['security_group'] = self._make_security_group_dict(sg) + self._registry_notify( resources.SECURITY_GROUP, events.PRECOMMIT_UPDATE, @@ -344,14 +346,6 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase): if validate: self._validate_security_group_rule(context, security_group_rule) rule_dict = security_group_rule['security_group_rule'] - kwargs = { - 'context': context, - 'security_group_rule': rule_dict - } - self._registry_notify(resources.SECURITY_GROUP_RULE, - events.BEFORE_CREATE, - exc_cls=ext_sg.SecurityGroupConflict, **kwargs) - remote_ip_prefix = rule_dict.get('remote_ip_prefix') if remote_ip_prefix: remote_ip_prefix = utils.AuthenticIPNetwork(remote_ip_prefix) @@ -381,19 +375,30 @@ 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) with db_api.context_manager.writer.using(context): if validate: self._check_for_duplicate_rules_in_db(context, security_group_rule) sg_rule = sg_obj.SecurityGroupRule(context, **args) sg_rule.create() + + # fetch sg_rule from db to load the sg rules with sg model + # otherwise a DetachedInstanceError can occur for model extensions + 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) - # fetch sg_rule from db to load the sg rules with sg model otherwise - # a DetachedInstanceError can occur for model extensions - sg_rule = sg_obj.SecurityGroupRule.get_object(context, id=sg_rule.id) - return self._make_security_group_rule_dict(sg_rule.db_obj) + return res_rule_dict def _get_ip_proto_number(self, protocol): if protocol is None: @@ -678,16 +683,14 @@ class SecurityGroupDbMixin(ext_sg.SecurityGroupPluginBase): exc_cls=ext_sg.SecurityGroupRuleInUse, **kwargs) with db_api.context_manager.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) - - sgr = self._get_security_group_rule(context, id) sgr.delete() - kwargs['security_group_id'] = sgr['security_group_id'] - registry.notify( resources.SECURITY_GROUP_RULE, events.AFTER_DELETE, self, **kwargs) diff --git a/neutron/tests/unit/db/test_securitygroups_db.py b/neutron/tests/unit/db/test_securitygroups_db.py index 2a81b499e86..61ddd6b0297 100644 --- a/neutron/tests/unit/db/test_securitygroups_db.py +++ b/neutron/tests/unit/db/test_securitygroups_db.py @@ -254,28 +254,60 @@ class SecurityGroupDbMixinTestCase(testlib_api.SqlTestCase): self.assertTrue(mock_rollback.called) def test_security_group_precommit_create_event(self): + DEFAULT_SECGROUP = { + 'tenant_id': FAKE_SECGROUP['security_group']['tenant_id'], + 'name': 'default', + 'description': 'Default security group', + } + DEFAULT_SECGROUP_DICT = { + 'id': mock.ANY, + 'tenant_id': FAKE_SECGROUP['security_group']['tenant_id'], + 'project_id': FAKE_SECGROUP['security_group']['tenant_id'], + 'name': 'default', + 'description': 'Default security group', + 'security_group_rules': mock.ANY, + } with mock.patch.object(registry, "notify") as mock_notify: - self.mixin.create_security_group(self.ctx, FAKE_SECGROUP) - mock_notify.assert_has_calls([mock.call('security_group', - 'precommit_create', mock.ANY, context=mock.ANY, - is_default=mock.ANY, security_group=mock.ANY)]) + sg_dict = self.mixin.create_security_group(self.ctx, FAKE_SECGROUP) + mock_notify.assert_has_calls([ + mock.call('security_group', 'before_create', mock.ANY, + context=mock.ANY, is_default=False, + security_group=FAKE_SECGROUP['security_group']), + + mock.call('security_group', 'before_create', mock.ANY, + context=mock.ANY, is_default=True, + security_group=DEFAULT_SECGROUP), + mock.call('security_group', 'precommit_create', mock.ANY, + context=mock.ANY, is_default=True, + security_group=DEFAULT_SECGROUP_DICT), + mock.call('security_group', 'after_create', mock.ANY, + context=mock.ANY, is_default=True, + security_group=DEFAULT_SECGROUP_DICT), + + mock.call('security_group', 'precommit_create', mock.ANY, + context=mock.ANY, is_default=False, + security_group=sg_dict), + mock.call('security_group', 'after_create', mock.ANY, + context=mock.ANY, is_default=False, + security_group=sg_dict)]) def test_security_group_precommit_update_event(self): sg_dict = self.mixin.create_security_group(self.ctx, FAKE_SECGROUP) with mock.patch.object(registry, "notify") as mock_notify: - self.mixin.update_security_group(self.ctx, sg_dict['id'], - FAKE_SECGROUP) mock_notify.assert_has_calls([mock.call('security_group', 'precommit_update', mock.ANY, context=mock.ANY, - security_group=mock.ANY, security_group_id=sg_dict['id'])]) + security_group=self.mixin.update_security_group( + self.ctx, sg_dict['id'], FAKE_SECGROUP), + security_group_id=sg_dict['id'])]) def test_security_group_precommit_and_after_delete_event(self): sg_dict = self.mixin.create_security_group(self.ctx, FAKE_SECGROUP) with mock.patch.object(registry, "notify") as mock_notify: self.mixin.delete_security_group(self.ctx, sg_dict['id']) + sg_dict['security_group_rules'] = mock.ANY mock_notify.assert_has_calls( [mock.call('security_group', 'precommit_delete', - mock.ANY, context=mock.ANY, security_group=mock.ANY, + mock.ANY, context=mock.ANY, security_group=sg_dict, security_group_id=sg_dict['id'], security_group_rule_ids=[mock.ANY, mock.ANY]), mock.call('security_group', 'after_delete', @@ -319,11 +351,10 @@ class SecurityGroupDbMixinTestCase(testlib_api.SqlTestCase): fake_rule['security_group_rule']['security_group_id'] = sg_dict['id'] with mock.patch.object(registry, "notify") as mock_notify, \ mock.patch.object(self.mixin, '_get_security_group'): - self.mixin.create_security_group_rule(self.ctx, - fake_rule) mock_notify.assert_has_calls([mock.call('security_group_rule', 'precommit_create', mock.ANY, context=mock.ANY, - security_group_rule=mock.ANY)]) + security_group_rule=self.mixin.create_security_group_rule( + self.ctx, fake_rule))]) def test_sg_rule_before_precommit_and_after_delete_event(self): sg_dict = self.mixin.create_security_group(self.ctx, FAKE_SECGROUP) @@ -340,6 +371,7 @@ class SecurityGroupDbMixinTestCase(testlib_api.SqlTestCase): 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,