From edf86aeac2f7b68243b0eccc3c49fa1a579e664b Mon Sep 17 00:00:00 2001 From: Angus Salkeld Date: Tue, 7 Apr 2015 09:25:50 +1000 Subject: [PATCH] Persist parent_resource_name and make sure it's available We are persisting for a number of reasons: - so we don't have to pass this through ever rpc call - the API exposes parent_resource (currently always None as it is not persisted) Closes-bug: #1438978 Change-Id: Id2db36c0234a085ec4f0ce2ab114ec483ea29d81 --- .../versions/062_parent_resource.py | 45 +++++++++++++++++++ heat/db/sqlalchemy/models.py | 1 + heat/engine/resources/stack_resource.py | 4 +- heat/engine/service.py | 12 +++-- heat/engine/stack.py | 24 +++++----- heat/objects/stack.py | 1 + heat/rpc/client.py | 8 ++-- heat/tests/db/test_migrations.py | 3 ++ heat/tests/test_api_cfn_v1.py | 6 ++- heat/tests/test_api_openstack_v1.py | 24 ++++++---- heat/tests/test_engine_service.py | 17 ++++--- heat/tests/test_rpc_client.py | 1 + heat/tests/test_stack.py | 30 ++++++++----- heat/tests/test_stack_resource.py | 5 --- .../functional/test_template_resource.py | 6 ++- 15 files changed, 134 insertions(+), 53 deletions(-) create mode 100644 heat/db/sqlalchemy/migrate_repo/versions/062_parent_resource.py diff --git a/heat/db/sqlalchemy/migrate_repo/versions/062_parent_resource.py b/heat/db/sqlalchemy/migrate_repo/versions/062_parent_resource.py new file mode 100644 index 000000000..bc812efd3 --- /dev/null +++ b/heat/db/sqlalchemy/migrate_repo/versions/062_parent_resource.py @@ -0,0 +1,45 @@ +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import sqlalchemy + +from heat.db.sqlalchemy import utils as migrate_utils + + +def upgrade(migrate_engine): + meta = sqlalchemy.MetaData(bind=migrate_engine) + + stack = sqlalchemy.Table('stack', meta, autoload=True) + parent_resource_name = sqlalchemy.Column('parent_resource_name', + sqlalchemy.String(255)) + parent_resource_name.create(stack) + + +def downgrade(migrate_engine): + meta = sqlalchemy.MetaData(bind=migrate_engine) + + stack = sqlalchemy.Table('stack', meta, autoload=True) + if migrate_engine.name == 'sqlite': + _downgrade_062_sqlite(migrate_engine, meta, stack) + else: + stack.c.parent_resource_name.drop() + + +def _downgrade_062_sqlite(migrate_engine, metadata, table): + new_table = migrate_utils.clone_table( + table.name + '__tmp__', table, metadata, + ignorecols=['parent_resource_name']) + migrate_utils.migrate_data(migrate_engine, + table, + new_table, + ['parent_resource_name']) diff --git a/heat/db/sqlalchemy/models.py b/heat/db/sqlalchemy/models.py index 3de90832e..d70a6b05b 100644 --- a/heat/db/sqlalchemy/models.py +++ b/heat/db/sqlalchemy/models.py @@ -169,6 +169,7 @@ class Stack(BASE, HeatBase, SoftDelete, StateAware): sqlalchemy.Integer, sqlalchemy.ForeignKey('user_creds.id')) owner_id = sqlalchemy.Column(sqlalchemy.String(36)) + parent_resource_name = sqlalchemy.Column(sqlalchemy.String(255)) timeout = sqlalchemy.Column(sqlalchemy.Integer) disable_rollback = sqlalchemy.Column(sqlalchemy.Boolean, nullable=False) stack_user_project_id = sqlalchemy.Column(sqlalchemy.String(64)) diff --git a/heat/engine/resources/stack_resource.py b/heat/engine/resources/stack_resource.py index 92724573b..964fce792 100644 --- a/heat/engine/resources/stack_resource.py +++ b/heat/engine/resources/stack_resource.py @@ -97,7 +97,6 @@ class StackResource(resource.Resource): if self._nested is None and self.resource_id is not None: self._nested = parser.Stack.load(self.context, self.resource_id, - parent_resource=self.name, show_deleted=show_deleted, force_reload=force_reload) @@ -259,7 +258,8 @@ class StackResource(resource.Resource): owner_id=self.stack.id, user_creds_id=self.stack.user_creds_id, stack_user_project_id=stack_user_project_id, - nested_depth=new_nested_depth) + nested_depth=new_nested_depth, + parent_resource_name=self.name) except Exception as ex: self.raise_local_exception(ex) diff --git a/heat/engine/service.py b/heat/engine/service.py index dd352bcdf..97dab8810 100644 --- a/heat/engine/service.py +++ b/heat/engine/service.py @@ -266,7 +266,7 @@ class EngineService(service.Service): by the RPC caller. """ - RPC_API_VERSION = '1.6' + RPC_API_VERSION = '1.7' def __init__(self, host, topic, manager=None): super(EngineService, self).__init__() @@ -549,7 +549,8 @@ class EngineService(service.Service): params, files, args, owner_id=None, nested_depth=0, user_creds_id=None, stack_user_project_id=None, - convergence=False): + convergence=False, + parent_resource_name=None): # If it is stack-adopt, use parameters from adopt_stack_data common_params = api.extract_args(args) if (rpc_api.PARAM_ADOPT_STACK_DATA in common_params and @@ -574,6 +575,7 @@ class EngineService(service.Service): user_creds_id=user_creds_id, stack_user_project_id=stack_user_project_id, convergence=convergence, + parent_resource=parent_resource_name, **common_params) self._validate_deferred_auth_context(cnxt, stack) @@ -615,7 +617,7 @@ class EngineService(service.Service): @context.request_context def create_stack(self, cnxt, stack_name, template, params, files, args, owner_id=None, nested_depth=0, user_creds_id=None, - stack_user_project_id=None): + stack_user_project_id=None, parent_resource_name=None): """ The create_stack method creates a new stack using the template provided. @@ -635,6 +637,7 @@ class EngineService(service.Service): :param user_creds_id: the parent user_creds record for nested stacks :param stack_user_project_id: the parent stack_user_project_id for nested stacks + :param parent_resource_name: the parent resource name """ LOG.info(_LI('Creating stack %s'), stack_name) @@ -667,7 +670,8 @@ class EngineService(service.Service): stack = self._parse_template_and_validate_stack( cnxt, stack_name, template, params, files, args, owner_id, - nested_depth, user_creds_id, stack_user_project_id, convergence) + nested_depth, user_creds_id, stack_user_project_id, convergence, + parent_resource_name) stack.store() diff --git a/heat/engine/stack.py b/heat/engine/stack.py index 496846a19..dece3867a 100755 --- a/heat/engine/stack.py +++ b/heat/engine/stack.py @@ -331,7 +331,7 @@ class Stack(collections.Mapping): return deps @classmethod - def load(cls, context, stack_id=None, stack=None, parent_resource=None, + def load(cls, context, stack_id=None, stack=None, show_deleted=True, use_stored_context=False, force_reload=False): '''Retrieve a Stack from the database.''' if stack is None: @@ -347,7 +347,7 @@ class Stack(collections.Mapping): if force_reload: stack.refresh() - return cls._from_db(context, stack, parent_resource=parent_resource, + return cls._from_db(context, stack, use_stored_context=use_stored_context) @classmethod @@ -370,7 +370,7 @@ class Stack(collections.Mapping): yield cls._from_db(context, stack, resolve_data=resolve_data) @classmethod - def _from_db(cls, context, stack, parent_resource=None, resolve_data=True, + def _from_db(cls, context, stack, resolve_data=True, use_stored_context=False): template = tmpl.Template.load( context, stack.raw_template_id, stack.raw_template) @@ -384,7 +384,7 @@ class Stack(collections.Mapping): timeout_mins=stack.timeout, resolve_data=resolve_data, disable_rollback=stack.disable_rollback, - parent_resource=parent_resource, + parent_resource=stack.parent_resource_name, owner_id=stack.owner_id, stack_user_project_id=stack.stack_user_project_id, created_time=stack.created_at, @@ -412,8 +412,6 @@ class Stack(collections.Mapping): stack = { 'owner_id': self.owner_id, 'username': self.username, - 'tenant_id': self.tenant_id, - 'timeout_mins': self.timeout_mins, 'disable_rollback': self.disable_rollback, 'stack_user_project_id': self.stack_user_project_id, 'user_creds_id': self.user_creds_id, @@ -426,8 +424,15 @@ class Stack(collections.Mapping): 'action': self.action, 'status': self.status, 'status_reason': self.status_reason}) - if not only_db: + + if only_db: + stack['parent_resource_name'] = self.parent_resource_name + stack['tenant'] = self.tenant_id + stack['timeout'] = self.timeout_mins + else: stack['parent_resource'] = self.parent_resource_name + stack['tenant_id'] = self.tenant_id + stack['timeout_mins'] = self.timeout_mins stack['strict_validate'] = self.strict_validate return stack @@ -446,11 +451,6 @@ class Stack(collections.Mapping): s['raw_template_id'] = self.t.store(self.context) else: s['raw_template_id'] = self.t.id - # name inconsistencies - s['tenant'] = s['tenant_id'] - del s['tenant_id'] - s['timeout'] = s['timeout_mins'] - del s['timeout_mins'] if self.id: stack_object.Stack.update_by_id(self.context, self.id, s) diff --git a/heat/objects/stack.py b/heat/objects/stack.py index 58b4d9a3a..451cfce3d 100755 --- a/heat/objects/stack.py +++ b/heat/objects/stack.py @@ -58,6 +58,7 @@ class Stack( 'prev_raw_template_id': fields.IntegerField(), 'prev_raw_template': fields.ObjectField('RawTemplate'), 'tags': fields.ObjectField('StackTagList'), + 'parent_resource_name': fields.StringField(nullable=True), } @staticmethod diff --git a/heat/rpc/client.py b/heat/rpc/client.py index 5a56e8051..57feb4040 100644 --- a/heat/rpc/client.py +++ b/heat/rpc/client.py @@ -185,7 +185,7 @@ class EngineClient(object): def _create_stack(self, ctxt, stack_name, template, params, files, args, owner_id=None, nested_depth=0, user_creds_id=None, - stack_user_project_id=None): + stack_user_project_id=None, parent_resource_name=None): """ Internal create_stack interface for engine-to-engine communication via RPC. Allows some additional options which should not be exposed to @@ -194,6 +194,7 @@ class EngineClient(object): :param nested_depth: nested depth for nested stacks :param user_creds_id: user_creds record for nested stack :param stack_user_project_id: stack user project for nested stack + :param parent_resource_name: the parent resource name """ return self.call( ctxt, self.make_msg('create_stack', stack_name=stack_name, @@ -202,8 +203,9 @@ class EngineClient(object): owner_id=owner_id, nested_depth=nested_depth, user_creds_id=user_creds_id, - stack_user_project_id=stack_user_project_id), - version='1.2') + stack_user_project_id=stack_user_project_id, + parent_resource_name=parent_resource_name), + version='1.7') def update_stack(self, ctxt, stack_identity, template, params, files, args): diff --git a/heat/tests/db/test_migrations.py b/heat/tests/db/test_migrations.py index 69a4f1251..ba9da817f 100644 --- a/heat/tests/db/test_migrations.py +++ b/heat/tests/db/test_migrations.py @@ -606,6 +606,9 @@ class HeatMigrationsCheckers(test_migrations.WalkVersionsMixin, self.assertColumnType(engine, tab_name, 'status_reason', sqlalchemy.Text) + def _check_062(self, engine, data): + self.assertColumnExists(engine, 'stack', 'parent_resource_name') + class TestHeatMigrationsMySQL(HeatMigrationsCheckers, test_base.MySQLOpportunisticTestCase): diff --git a/heat/tests/test_api_cfn_v1.py b/heat/tests/test_api_cfn_v1.py index cdf40a698..e30531861 100644 --- a/heat/tests/test_api_cfn_v1.py +++ b/heat/tests/test_api_cfn_v1.py @@ -506,8 +506,9 @@ class CfnStackControllerTest(common.HeatTestCase): 'owner_id': None, 'nested_depth': 0, 'user_creds_id': None, + 'parent_resource_name': None, 'stack_user_project_id': None}), - version='1.2' + version='1.7' ).AndRaise(failure) def _stub_rpc_create_stack_call_success(self, stack_name, engine_parms, @@ -533,8 +534,9 @@ class CfnStackControllerTest(common.HeatTestCase): 'owner_id': None, 'nested_depth': 0, 'user_creds_id': None, + 'parent_resource_name': None, 'stack_user_project_id': None}), - version='1.2' + version='1.7' ).AndReturn(engine_resp) self.m.ReplayAll() diff --git a/heat/tests/test_api_openstack_v1.py b/heat/tests/test_api_openstack_v1.py index 308517982..3fe024063 100644 --- a/heat/tests/test_api_openstack_v1.py +++ b/heat/tests/test_api_openstack_v1.py @@ -741,8 +741,9 @@ class StackControllerTest(ControllerTest, common.HeatTestCase): 'owner_id': None, 'nested_depth': 0, 'user_creds_id': None, + 'parent_resource_name': None, 'stack_user_project_id': None}), - version='1.2' + version='1.7' ).AndReturn(dict(identity)) self.m.ReplayAll() @@ -803,8 +804,9 @@ class StackControllerTest(ControllerTest, common.HeatTestCase): 'owner_id': None, 'nested_depth': 0, 'user_creds_id': None, + 'parent_resource_name': None, 'stack_user_project_id': None}), - version='1.2' + version='1.7' ).AndReturn(dict(identity)) self.m.ReplayAll() @@ -889,8 +891,9 @@ class StackControllerTest(ControllerTest, common.HeatTestCase): 'owner_id': None, 'nested_depth': 0, 'user_creds_id': None, + 'parent_resource_name': None, 'stack_user_project_id': None}), - version='1.2' + version='1.7' ).AndReturn(dict(identity)) self.m.ReplayAll() @@ -932,8 +935,9 @@ class StackControllerTest(ControllerTest, common.HeatTestCase): 'owner_id': None, 'nested_depth': 0, 'user_creds_id': None, + 'parent_resource_name': None, 'stack_user_project_id': None}), - version='1.2' + version='1.7' ).AndRaise(to_remote_error(AttributeError())) rpc_client.EngineClient.call( req.context, @@ -948,8 +952,9 @@ class StackControllerTest(ControllerTest, common.HeatTestCase): 'owner_id': None, 'nested_depth': 0, 'user_creds_id': None, + 'parent_resource_name': None, 'stack_user_project_id': None}), - version='1.2' + version='1.7' ).AndRaise(to_remote_error(unknown_parameter)) rpc_client.EngineClient.call( req.context, @@ -964,8 +969,9 @@ class StackControllerTest(ControllerTest, common.HeatTestCase): 'owner_id': None, 'nested_depth': 0, 'user_creds_id': None, + 'parent_resource_name': None, 'stack_user_project_id': None}), - version='1.2' + version='1.7' ).AndRaise(to_remote_error(missing_parameter)) self.m.ReplayAll() resp = request_with_middleware(fault.FaultWrapper, @@ -1017,8 +1023,9 @@ class StackControllerTest(ControllerTest, common.HeatTestCase): 'owner_id': None, 'nested_depth': 0, 'user_creds_id': None, + 'parent_resource_name': None, 'stack_user_project_id': None}), - version='1.2' + version='1.7' ).AndRaise(to_remote_error(error)) self.m.ReplayAll() @@ -1097,8 +1104,9 @@ class StackControllerTest(ControllerTest, common.HeatTestCase): 'owner_id': None, 'nested_depth': 0, 'user_creds_id': None, + 'parent_resource_name': None, 'stack_user_project_id': None}), - version='1.2' + version='1.7' ).AndRaise(to_remote_error(error)) self.m.ReplayAll() diff --git a/heat/tests/test_engine_service.py b/heat/tests/test_engine_service.py index c5b3288cb..a19e12452 100644 --- a/heat/tests/test_engine_service.py +++ b/heat/tests/test_engine_service.py @@ -473,7 +473,8 @@ class StackServiceCreateUpdateDeleteTest(common.HeatTestCase): stack.t, owner_id=None, nested_depth=0, user_creds_id=None, stack_user_project_id=None, - convergence=False).AndReturn(stack) + convergence=False, + parent_resource=None).AndReturn(stack) self.m.StubOutWithMock(stack, 'validate') stack.validate().AndReturn(None) @@ -528,7 +529,8 @@ class StackServiceCreateUpdateDeleteTest(common.HeatTestCase): nested_depth=0, user_creds_id=None, stack_user_project_id=None, - convergence=False).AndReturn(stack) + convergence=False, + parent_resource=None).AndReturn(stack) self.m.StubOutWithMock(stack, 'validate') stack.validate().AndRaise(exception.StackValidationFailed( @@ -697,7 +699,8 @@ class StackServiceCreateUpdateDeleteTest(common.HeatTestCase): stack.t, owner_id=None, nested_depth=0, user_creds_id=None, stack_user_project_id=None, - convergence=False).AndReturn(stack) + convergence=False, + parent_resource=None).AndReturn(stack) templatem.Template(template, files=None, env=stack.env).AndReturn(stack.t) @@ -706,7 +709,8 @@ class StackServiceCreateUpdateDeleteTest(common.HeatTestCase): stack.t, owner_id=None, nested_depth=0, user_creds_id=None, stack_user_project_id=None, - convergence=False).AndReturn(stack) + convergence=False, + parent_resource=None).AndReturn(stack) self.m.ReplayAll() @@ -755,7 +759,8 @@ class StackServiceCreateUpdateDeleteTest(common.HeatTestCase): nested_depth=0, user_creds_id=None, stack_user_project_id=None, - convergence=False).AndReturn(stack) + convergence=False, + parent_resource=None).AndReturn(stack) self.m.ReplayAll() @@ -1731,7 +1736,7 @@ class StackServiceTest(common.HeatTestCase): def test_make_sure_rpc_version(self): self.assertEqual( - '1.6', + '1.7', service.EngineService.RPC_API_VERSION, ('RPC version is changed, please update this test to new version ' 'and make sure additional test cases are added for RPC APIs ' diff --git a/heat/tests/test_rpc_client.py b/heat/tests/test_rpc_client.py index 20250261d..5ed750f2e 100644 --- a/heat/tests/test_rpc_client.py +++ b/heat/tests/test_rpc_client.py @@ -154,6 +154,7 @@ class EngineRpcAPITestCase(common.HeatTestCase): call_kwargs['nested_depth'] = 0 call_kwargs['user_creds_id'] = None call_kwargs['stack_user_project_id'] = None + call_kwargs['parent_resource_name'] = None expected_message = self.rpcapi.make_msg('create_stack', **call_kwargs) kwargs['expected_message'] = expected_message self._test_engine_api('create_stack', 'call', **kwargs) diff --git a/heat/tests/test_stack.py b/heat/tests/test_stack.py index 310acce9e..7c2e6152e 100644 --- a/heat/tests/test_stack.py +++ b/heat/tests/test_stack.py @@ -266,7 +266,8 @@ class StackTest(common.HeatTestCase): self.assertEqual('test value', stk.root_stack) def test_load_parent_resource(self): - self.stack = stack.Stack(self.ctx, 'load_parent_resource', self.tmpl) + self.stack = stack.Stack(self.ctx, 'load_parent_resource', self.tmpl, + parent_resource='parent') self.stack.store() stk = stack_object.Stack.get_by_id(self.ctx, self.stack.id) @@ -295,8 +296,7 @@ class StackTest(common.HeatTestCase): tags=mox.IgnoreArg()) self.m.ReplayAll() - stack.Stack.load(self.ctx, stack_id=self.stack.id, - parent_resource='parent') + stack.Stack.load(self.ctx, stack_id=self.stack.id) self.m.VerifyAll() @@ -1812,13 +1812,11 @@ class StackKwargsForCloningTest(common.HeatTestCase): not_included=['action', 'status', 'status_reason'])), ('only_db', dict(keep_status=False, only_db=True, not_included=['action', 'status', 'status_reason', - 'parent_resource', 'strict_validate'])), ('keep_status', dict(keep_status=True, only_db=False, not_included=[])), ('status_db', dict(keep_status=True, only_db=True, - not_included=['parent_resource', - 'strict_validate'])), + not_included=['strict_validate'])), ] def test_kwargs(self): @@ -1832,6 +1830,13 @@ class StackKwargsForCloningTest(common.HeatTestCase): username='jo', nested_depth=3, strict_validate=True, convergence=False, current_traversal=45) + db_map = {'parent_resource': 'parent_resource_name', + 'tenant_id': 'tenant', 'timeout_mins': 'timeout'} + test_db_data = {} + for key in test_data: + dbkey = db_map.get(key, key) + test_db_data[dbkey] = test_data[key] + self.stack = stack.Stack(ctx, utils.random_name(), tmpl, **test_data) res = self.stack.get_kwargs_for_cloning(keep_status=self.keep_status, @@ -1841,8 +1846,13 @@ class StackKwargsForCloningTest(common.HeatTestCase): for key in test_data: if key not in self.not_included: - self.assertEqual(test_data[key], res[key]) + dbkey = db_map.get(key, key) + if self.only_db: + self.assertEqual(test_data[key], res[dbkey]) + else: + self.assertEqual(test_data[key], res[key]) - # just make sure that the kwargs are valid - # (no exception should be raised) - stack.Stack(ctx, utils.random_name(), tmpl, **res) + if not self.only_db: + # just make sure that the kwargs are valid + # (no exception should be raised) + stack.Stack(ctx, utils.random_name(), tmpl, **res) diff --git a/heat/tests/test_stack_resource.py b/heat/tests/test_stack_resource.py index 055ce0d32..f408db719 100644 --- a/heat/tests/test_stack_resource.py +++ b/heat/tests/test_stack_resource.py @@ -430,7 +430,6 @@ class StackResourceTest(common.HeatTestCase): self.m.StubOutWithMock(parser.Stack, 'load') parser.Stack.load(self.parent_resource.context, self.parent_resource.resource_id, - parent_resource=self.parent_resource.name, show_deleted=False, force_reload=False).AndReturn('s') self.m.ReplayAll() @@ -443,7 +442,6 @@ class StackResourceTest(common.HeatTestCase): self.m.StubOutWithMock(parser.Stack, 'load') parser.Stack.load(self.parent_resource.context, self.parent_resource.resource_id, - parent_resource=self.parent_resource.name, show_deleted=False, force_reload=True).AndReturn('ok') self.m.ReplayAll() @@ -457,7 +455,6 @@ class StackResourceTest(common.HeatTestCase): self.m.StubOutWithMock(parser.Stack, 'load') parser.Stack.load(self.parent_resource.context, self.parent_resource.resource_id, - parent_resource=self.parent_resource.name, show_deleted=False, force_reload=False).AndReturn(None) self.m.ReplayAll() @@ -475,7 +472,6 @@ class StackResourceTest(common.HeatTestCase): self.m.StubOutWithMock(parser.Stack, 'load') parser.Stack.load(self.parent_resource.context, self.parent_resource.resource_id, - parent_resource=self.parent_resource.name, show_deleted=False, force_reload=True).AndReturn('s') self.m.ReplayAll() @@ -489,7 +485,6 @@ class StackResourceTest(common.HeatTestCase): self.m.StubOutWithMock(parser.Stack, 'load') parser.Stack.load(self.parent_resource.context, self.parent_resource.resource_id, - parent_resource=self.parent_resource.name, show_deleted=False, force_reload=True).AndReturn(None) self.m.ReplayAll() diff --git a/heat_integrationtests/functional/test_template_resource.py b/heat_integrationtests/functional/test_template_resource.py index 58936797b..392bdddd1 100644 --- a/heat_integrationtests/functional/test_template_resource.py +++ b/heat_integrationtests/functional/test_template_resource.py @@ -79,7 +79,11 @@ resource_registry: template=main_templ, files={'nested.yaml': nested_templ}, environment=env_templ) - self.assert_resource_is_a_stack(stack_identifier, 'secret1') + nested_ident = self.assert_resource_is_a_stack(stack_identifier, + 'secret1') + # prove that resource.parent_resource is populated. + sec2 = self.client.resources.get(nested_ident, 'secret2') + self.assertEqual('secret1', sec2.parent_resource) def test_no_infinite_recursion(self): """Prove that we can override a python resource.