openstack/deb-heat
Code Issues Proposed changes

RPC API: Add a template_id parameter to stack_create/update

Browse Source 
This allows us to pre-store a template in the database before
creating/updating a stack over RPC, so that we don't need to send the
contents over RPC. This will help us safely de-duplicate files dicts in
the database, reduce the size of messages, and prevent unnecessary
copies of files (in particular) being loaded into memory.

Change-Id: Id1990d5cbac8b3954ce224af73425d4fd8db17d6
This commit is contained in:
Zane Bitter 2016-05-11 14:56:26 -04:00
parent 32b6878306
commit 9a650a5e2f
9 changed files with 222 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
heat/engine/service.py
View File

@ -294,7 +294,7 @@ class EngineService(service.Service):
by the RPC caller.
"""
RPC_API_VERSION = '1.28'
RPC_API_VERSION = '1.29'
def __init__(self, host, topic):
super(EngineService, self).__init__()
@ -645,7 +645,8 @@ class EngineService(service.Service):
nested_depth=0, user_creds_id=None,
stack_user_project_id=None,
convergence=False,
parent_resource_name=None):
parent_resource_name=None,
template_id=None):
common_params = api.extract_args(args)
# If it is stack-adopt, use parameters from adopt_stack_data
@ -661,10 +662,13 @@ class EngineService(service.Service):
new_params.update(params.get(rpc_api.STACK_PARAMETERS, {}))
params[rpc_api.STACK_PARAMETERS] = new_params
self._merge_environments(environment_files, files, params)
env = environment.Environment(params)
tmpl = templatem.Template(template, files=files, env=env)
if template_id is not None:
tmpl = templatem.Template.load(cnxt, template_id)
env = tmpl.env
else:
self._merge_environments(environment_files, files, params)
env = environment.Environment(params)
tmpl = templatem.Template(template, files=files, env=env)
self._validate_new_stack(cnxt, stack_name, tmpl)
stack = parser.Stack(cnxt, stack_name, tmpl,
@ -745,7 +749,8 @@ class EngineService(service.Service):
def create_stack(self, cnxt, stack_name, template, params, files,
args, environment_files=None,
owner_id=None, nested_depth=0, user_creds_id=None,
stack_user_project_id=None, parent_resource_name=None):
stack_user_project_id=None, parent_resource_name=None,
template_id=None):
"""Create a new stack using the template provided.
Note that at this stage the template has already been fetched from the
@ -768,6 +773,7 @@ class EngineService(service.Service):
:param stack_user_project_id: the parent stack_user_project_id for
nested stacks
:param parent_resource_name: the parent resource name
:param template_id: the ID of a pre-stored template in the DB
"""
LOG.info(_LI('Creating stack %s'), stack_name)
@ -799,7 +805,8 @@ class EngineService(service.Service):
stack = self._parse_template_and_validate_stack(
cnxt, stack_name, template, params, files, environment_files,
args, owner_id, nested_depth, user_creds_id,
stack_user_project_id, convergence, parent_resource_name)
stack_user_project_id, convergence, parent_resource_name,
template_id)
self.resource_enforcer.enforce_stack(stack)
stack_id = stack.store()
@ -820,7 +827,7 @@ class EngineService(service.Service):
return dict(stack.identifier())
def _prepare_stack_updates(self, cnxt, current_stack, template, params,
files, args):
files, args, template_id=None):
"""Return the current and updated stack for a given transition.
Changes *will not* be persisted, this is a helper method for
@ -832,6 +839,7 @@ class EngineService(service.Service):
:param params: Stack Input Params
:param files: Files referenced from the template
:param args: Request parameters/args passed from API
:param template_id: the ID of a pre-stored template in the DB
"""
# Now parse the template and any parameters for the updated
@ -851,6 +859,9 @@ class EngineService(service.Service):
new_files = current_stack.t.files.copy()
new_files.update(files or {})
assert template_id is None, \
"Cannot specify template_id with PARAM_EXISTING"
if template is not None:
new_template = template
elif (current_stack.convergence or
@ -881,8 +892,11 @@ class EngineService(service.Service):
if key not in tmpl.param_schemata():
new_env.params.pop(key)
else:
tmpl = templatem.Template(template, files=files,
env=environment.Environment(params))
if template_id is not None:
tmpl = templatem.Template.load(cnxt, template_id)
else:
tmpl = templatem.Template(template, files=files,
env=environment.Environment(params))
max_resources = cfg.CONF.max_resources_per_stack
if max_resources != -1 and len(tmpl[tmpl.RESOURCES]) > max_resources:
@ -903,7 +917,7 @@ class EngineService(service.Service):
**current_kwargs)
invalid_params = current_stack.parameters.immutable_params_modified(
updated_stack.parameters, params)
updated_stack.parameters, tmpl.env.params)
if invalid_params:
raise exception.ImmutableParameterModified(*invalid_params)
@ -917,7 +931,7 @@ class EngineService(service.Service):
@context.request_context
def update_stack(self, cnxt, stack_identity, template, params,
files, args, environment_files=None):
files, args, environment_files=None, template_id=None):
"""Update an existing stack based on the provided template and params.
Note that at this stage the template has already been fetched from the
@ -932,6 +946,7 @@ class EngineService(service.Service):
:param environment_files: optional ordered list of environment file
names included in the files dict
:type environment_files: list or None
:param template_id: the ID of a pre-stored template in the DB
"""
# Handle server-side environment file resolution
self._merge_environments(environment_files, files, params)
@ -955,7 +970,7 @@ class EngineService(service.Service):
raise exception.NotSupported(feature=msg)
tmpl, current_stack, updated_stack = self._prepare_stack_updates(
cnxt, current_stack, template, params, files, args)
cnxt, current_stack, template, params, files, args, template_id)
if current_stack.convergence:
current_stack.thread_group_mgr = self.thread_group_mgr

3
heat/engine/stack.py
View File

@ -1351,7 +1351,8 @@ class Stack(collections.Mapping):
# Save a copy of the new template. To avoid two DB writes
# we store the ID at the same time as the action/status
prev_tmpl_id = self.prev_raw_template_id
bu_tmpl = copy.deepcopy(newstack.t)
# newstack.t may have been pre-stored, so save with that one
bu_tmpl, newstack.t = newstack.t, copy.deepcopy(newstack.t)
self.prev_raw_template_id = bu_tmpl.store()
self.action = action
self.status = self.IN_PROGRESS

29
heat/rpc/client.py
View File

@ -49,6 +49,7 @@ class EngineClient(object):
1.26 - Add mark_unhealthy
1.27 - Add check_software_deployment
1.28 - Add environment_show call
1.29 - Add template_id to create_stack/update_stack
"""
BASE_RPC_API_VERSION = '1.0'
@ -249,7 +250,8 @@ class EngineClient(object):
def _create_stack(self, ctxt, stack_name, template, params, files,
args, environment_files=None,
owner_id=None, nested_depth=0, user_creds_id=None,
stack_user_project_id=None, parent_resource_name=None):
stack_user_project_id=None, parent_resource_name=None,
template_id=None):
"""Internal interface for engine-to-engine communication via RPC.
Allows some additional options which should not be exposed to users via
@ -260,6 +262,7 @@ class EngineClient(object):
: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
:param template_id: the ID of a pre-stored template in the DB
"""
return self.call(
ctxt, self.make_msg('create_stack', stack_name=stack_name,
@ -270,8 +273,9 @@ class EngineClient(object):
nested_depth=nested_depth,
user_creds_id=user_creds_id,
stack_user_project_id=stack_user_project_id,
parent_resource_name=parent_resource_name),
version='1.23')
parent_resource_name=parent_resource_name,
template_id=template_id),
version='1.29')
def update_stack(self, ctxt, stack_identity, template, params,
files, args, environment_files=None):
@ -290,6 +294,20 @@ class EngineClient(object):
names included in the files dict
:type environment_files: list or None
"""
return self._update_stack(ctxt, stack_identity, template, params,
files, args,
environment_files=environment_files)
def _update_stack(self, ctxt, stack_identity, template, params,
files, args, environment_files=None,
template_id=None):
"""Internal interface for engine-to-engine communication via RPC.
Allows an additional option which should not be exposed to users via
the API:
:param template_id: the ID of a pre-stored template in the DB
"""
return self.call(ctxt,
self.make_msg('update_stack',
stack_identity=stack_identity,
@ -297,8 +315,9 @@ class EngineClient(object):
params=params,
files=files,
environment_files=environment_files,
args=args),
version='1.23')
args=args,
template_id=template_id),
version='1.29')
def preview_update_stack(self, ctxt, stack_identity, template, params,
files, args, environment_files=None):

15
heat/tests/api/cfn/test_api_cfn_v1.py
View File

@ -535,8 +535,9 @@ class CfnStackControllerTest(common.HeatTestCase):
'nested_depth': 0,
'user_creds_id': None,
'parent_resource_name': None,
'stack_user_project_id': None}),
version='1.23'
'stack_user_project_id': None,
'template_id': None}),
version='1.29'
).AndRaise(failure)
def _stub_rpc_create_stack_call_success(self, stack_name, engine_parms,
@ -564,8 +565,9 @@ class CfnStackControllerTest(common.HeatTestCase):
'nested_depth': 0,
'user_creds_id': None,
'parent_resource_name': None,
'stack_user_project_id': None}),
version='1.23'
'stack_user_project_id': None,
'template_id': None}),
version='1.29'
).AndReturn(engine_resp)
self.m.ReplayAll()
@ -874,8 +876,9 @@ class CfnStackControllerTest(common.HeatTestCase):
'params': engine_parms,
'files': {},
'environment_files': None,
'args': engine_args}),
version='1.23'
'args': engine_args,
'template_id': None}),
version='1.29'
).AndReturn(identity)
self.m.ReplayAll()

101
heat/tests/api/openstack_v1/test_stacks.py
View File

@ -97,6 +97,12 @@ blarg: wibble
data = stacks.InstantiationData(body)
self.assertEqual(parsed, data.template())
def test_template_int(self):
template = '42'
body = {'template': template}
data = stacks.InstantiationData(body)
self.assertRaises(webob.exc.HTTPBadRequest, data.template)
def test_template_url(self):
template = {'heat_template_version': '2013-05-23',
'foo': 'bar',
@ -678,8 +684,9 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase):
'nested_depth': 0,
'user_creds_id': None,
'parent_resource_name': None,
'stack_user_project_id': None}),
version='1.23'
'stack_user_project_id': None,
'template_id': None}),
version='1.29'
).AndReturn(dict(identity))
self.m.ReplayAll()
@ -725,8 +732,9 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase):
'nested_depth': 0,
'user_creds_id': None,
'parent_resource_name': None,
'stack_user_project_id': None}),
version='1.23'
'stack_user_project_id': None,
'template_id': None}),
version='1.29'
).AndReturn(dict(identity))
self.m.ReplayAll()
@ -790,8 +798,9 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase):
'nested_depth': 0,
'user_creds_id': None,
'parent_resource_name': None,
'stack_user_project_id': None}),
version='1.23'
'stack_user_project_id': None,
'template_id': None}),
version='1.29'
).AndReturn(dict(identity))
self.m.ReplayAll()
@ -880,8 +889,9 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase):
'nested_depth': 0,
'user_creds_id': None,
'parent_resource_name': None,
'stack_user_project_id': None}),
version='1.23'
'stack_user_project_id': None,
'template_id': None}),
version='1.29'
).AndReturn(dict(identity))
self.m.ReplayAll()
@ -927,8 +937,9 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase):
'nested_depth': 0,
'user_creds_id': None,
'parent_resource_name': None,
'stack_user_project_id': None}),
version='1.23'
'stack_user_project_id': None,
'template_id': None}),
version='1.29'
).AndRaise(tools.to_remote_error(AttributeError()))
rpc_client.EngineClient.call(
req.context,
@ -947,8 +958,9 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase):
'nested_depth': 0,
'user_creds_id': None,
'parent_resource_name': None,
'stack_user_project_id': None}),
version='1.23'
'stack_user_project_id': None,
'template_id': None}),
version='1.29'
).AndRaise(tools.to_remote_error(unknown_parameter))
rpc_client.EngineClient.call(
req.context,
@ -967,8 +979,9 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase):
'nested_depth': 0,
'user_creds_id': None,
'parent_resource_name': None,
'stack_user_project_id': None}),
version='1.23'
'stack_user_project_id': None,
'template_id': None}),
version='1.29'
).AndRaise(tools.to_remote_error(missing_parameter))
self.m.ReplayAll()
resp = tools.request_with_middleware(fault.FaultWrapper,
@ -1027,8 +1040,9 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase):
'nested_depth': 0,
'user_creds_id': None,
'parent_resource_name': None,
'stack_user_project_id': None}),
version='1.23'
'stack_user_project_id': None,
'template_id': None}),
version='1.29'
).AndRaise(tools.to_remote_error(error))
self.m.ReplayAll()
@ -1113,8 +1127,9 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase):
'nested_depth': 0,
'user_creds_id': None,
'parent_resource_name': None,
'stack_user_project_id': None}),
version='1.23'
'stack_user_project_id': None,
'template_id': None}),
version='1.29'
).AndRaise(tools.to_remote_error(error))
self.m.ReplayAll()
@ -1312,8 +1327,9 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase):
u'resource_registry': {}},
'files': {},
'environment_files': None,
'args': {'timeout_mins': 30}}),
version='1.23'
'args': {'timeout_mins': 30},
'template_id': None}),
version='1.29'
).AndRaise(tools.to_remote_error(error))
self.m.ReplayAll()
@ -1790,8 +1806,9 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase):
'resource_registry': {}},
'files': {},
'environment_files': None,
'args': {'timeout_mins': 30}}),
version='1.23'
'args': {'timeout_mins': 30},
'template_id': None}),
version='1.29'
).AndReturn(dict(identity))
self.m.ReplayAll()
@ -1830,8 +1847,9 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase):
'resource_registry': {}},
'files': {},
'environment_files': None,
'args': {'timeout_mins': 30, 'tags': ['tag1', 'tag2']}}),
version='1.23'
'args': {'timeout_mins': 30, 'tags': ['tag1', 'tag2']},
'template_id': None}),
version='1.29'
).AndReturn(dict(identity))
self.m.ReplayAll()
@ -1870,8 +1888,9 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase):
u'resource_registry': {}},
'files': {},
'environment_files': None,
'args': {'timeout_mins': 30}}),
version='1.23'
'args': {'timeout_mins': 30},
'template_id': None}),
version='1.29'
).AndRaise(tools.to_remote_error(error))
self.m.ReplayAll()
@ -1958,8 +1977,9 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase):
'files': {},
'environment_files': None,
'args': {rpc_api.PARAM_EXISTING: True,
'timeout_mins': 30}}),
version='1.23'
'timeout_mins': 30},
'template_id': None}),
version='1.29'
).AndReturn(dict(identity))
self.m.ReplayAll()
@ -1997,8 +2017,9 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase):
'files': {},
'environment_files': None,
'args': {rpc_api.PARAM_EXISTING: True,
'timeout_mins': 30}}),
version='1.23'
'timeout_mins': 30},
'template_id': None}),
version='1.29'
).AndReturn(dict(identity))
self.m.ReplayAll()
@ -2038,8 +2059,9 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase):
'environment_files': None,
'args': {rpc_api.PARAM_EXISTING: True,
'timeout_mins': 30,
'tags': ['tag1', 'tag2']}}),
version='1.23'
'tags': ['tag1', 'tag2']},
'template_id': None}),
version='1.29'
).AndReturn(dict(identity))
self.m.ReplayAll()
@ -2078,8 +2100,9 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase):
'files': {},
'environment_files': None,
'args': {rpc_api.PARAM_EXISTING: True,
'timeout_mins': 30}}),
version='1.23'
'timeout_mins': 30},
'template_id': None}),
version='1.29'
).AndReturn(dict(identity))
self.m.ReplayAll()
@ -2145,8 +2168,9 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase):
'environment_files': None,
'args': {rpc_api.PARAM_EXISTING: True,
'clear_parameters': clear_params,
'timeout_mins': 30}}),
version='1.23'
'timeout_mins': 30},
'template_id': None}),
version='1.29'
).AndReturn(dict(identity))
self.m.ReplayAll()
@ -2189,8 +2213,9 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase):
'environment_files': None,
'args': {rpc_api.PARAM_EXISTING: True,
'clear_parameters': clear_params,
'timeout_mins': 30}}),
version='1.23'
'timeout_mins': 30},
'template_id': None}),
version='1.29'
).AndReturn(dict(identity))
self.m.ReplayAll()

2
heat/tests/engine/service/test_service_engine.py
View File

@ -40,7 +40,7 @@ class ServiceEngineTest(common.HeatTestCase):
def test_make_sure_rpc_version(self):
self.assertEqual(
'1.28',
'1.29',
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 '

29
heat/tests/engine/service/test_stack_create.py
View File

@ -280,6 +280,35 @@ class StackCreateTest(common.HeatTestCase):
self.assertIn(exception.StackResourceLimitExceeded.msg_fmt,
six.text_type(ex.exc_info[1]))
@mock.patch.object(threadgroup, 'ThreadGroup')
@mock.patch.object(stack.Stack, 'validate')
def test_stack_create_nested(self, mock_validate, mock_tg):
stack_name = 'service_create_nested_test_stack'
mock_tg.return_value = tools.DummyThreadGroup()
stk = tools.get_stack(stack_name, self.ctx, with_params=True)
tmpl_id = stk.t.store()
mock_load = self.patchobject(templatem.Template, 'load',
return_value=stk.t)
mock_stack = self.patchobject(stack, 'Stack', return_value=stk)
result = self.man.create_stack(self.ctx, stack_name, None,
None, None, {}, nested_depth=1,
template_id=tmpl_id)
self.assertEqual(stk.identifier(), result)
self.assertIsInstance(result, dict)
self.assertTrue(result['stack_id'])
mock_load.assert_called_once_with(self.ctx, tmpl_id)
mock_stack.assert_called_once_with(self.ctx, stack_name, stk.t,
owner_id=None, nested_depth=1,
user_creds_id=None,
stack_user_project_id=None,
convergence=False,
parent_resource=None)
mock_validate.assert_called_once_with()
def test_stack_validate(self):
stack_name = 'stack_create_test_validate'
stk = tools.get_stack(stack_name, self.ctx)

58
heat/tests/engine/service/test_stack_update.py
View File

@ -129,6 +129,58 @@ class ServiceStackUpdateTest(common.HeatTestCase):
# Verify
mock_merge.assert_called_once_with(environment_files, None, params)
def test_stack_update_nested(self):
stack_name = 'service_update_nested_test_stack'
old_stack = tools.get_stack(stack_name, self.ctx)
sid = old_stack.store()
old_stack.set_stack_user_project_id('1234')
s = stack_object.Stack.get_by_id(self.ctx, sid)
stk = tools.get_stack(stack_name, self.ctx)
tmpl_id = stk.t.store()
# prepare mocks
mock_stack = self.patchobject(stack, 'Stack', return_value=stk)
mock_load = self.patchobject(stack.Stack, 'load',
return_value=old_stack)
mock_tmpl = self.patchobject(templatem.Template, 'load',
return_value=stk.t)
mock_validate = self.patchobject(stk, 'validate', return_value=None)
event_mock = mock.Mock()
self.patchobject(grevent, 'Event', return_value=event_mock)
# do update
api_args = {'timeout_mins': 60}
result = self.man.update_stack(self.ctx, old_stack.identifier(),
None, None, None, api_args,
template_id=tmpl_id)
# assertions
self.assertEqual(old_stack.identifier(), result)
self.assertIsInstance(result, dict)
self.assertTrue(result['stack_id'])
self.assertEqual([event_mock], self.man.thread_group_mgr.events)
mock_tmpl.assert_called_once_with(self.ctx, tmpl_id)
mock_stack.assert_called_once_with(
self.ctx, stk.name, stk.t,
convergence=False,
current_traversal=old_stack.current_traversal,
prev_raw_template_id=None,
current_deps=None,
disable_rollback=True,
nested_depth=0,
owner_id=None,
parent_resource=None,
stack_user_project_id='1234',
strict_validate=True,
tenant_id='test_tenant_id',
timeout_mins=60,
user_creds_id=u'1',
username='test_username')
mock_load.assert_called_once_with(self.ctx, stack=s)
mock_validate.assert_called_once_with()
def test_stack_update_existing_parameters(self):
# Use a template with existing parameters, then update the stack
# with a template containing additional parameters and ensure all
@ -723,7 +775,6 @@ parameters:
self.ctx = utils.dummy_context(password=None)
stack_name = 'test_update_immutable_parameters'
params = {}
old_stack = tools.get_stack(stack_name, self.ctx,
template=template)
sid = old_stack.store()
@ -734,15 +785,12 @@ parameters:
self.patchobject(self.man, '_get_stack', return_value=s)
self.patchobject(stack, 'Stack', return_value=old_stack)
self.patchobject(stack.Stack, 'load', return_value=old_stack)
self.patchobject(templatem, 'Template', return_value=old_stack.t)
self.patchobject(environment, 'Environment',
return_value=old_stack.env)
params = {'param1': 'bar'}
exc = self.assertRaises(dispatcher.ExpectedException,
self.man.update_stack,
self.ctx, old_stack.identifier(),
templatem.Template(template), params,
old_stack.t.t, params,
None, {})
self.assertEqual(exception.ImmutableParameterModified, exc.exc_info[0])
self.assertEqual('The following parameters are immutable and may not '

18
heat/tests/test_rpc_client.py
View File

@ -166,18 +166,24 @@ class EngineRpcAPITestCase(common.HeatTestCase):
call_kwargs['user_creds_id'] = None
call_kwargs['stack_user_project_id'] = None
call_kwargs['parent_resource_name'] = None
call_kwargs['template_id'] = None
expected_message = self.rpcapi.make_msg('create_stack', **call_kwargs)
kwargs['expected_message'] = expected_message
self._test_engine_api('create_stack', 'call', **kwargs)
def test_update_stack(self):
kwargs = dict(stack_identity=self.identity,
template={u'Foo': u'bar'},
params={u'InstanceType': u'm1.xlarge'},
files={},
environment_files=['foo.yaml'],
args=mock.ANY)
call_kwargs = copy.deepcopy(kwargs)
call_kwargs['template_id'] = None
expected_message = self.rpcapi.make_msg('update_stack', **call_kwargs)
self._test_engine_api('update_stack', 'call',
stack_identity=self.identity,
template={u'Foo': u'bar'},
params={u'InstanceType': u'm1.xlarge'},
files={},
environment_files=['foo.yaml'],
args=mock.ANY)
expected_message=expected_message,
**kwargs)
def test_preview_update_stack(self):
self._test_engine_api('preview_update_stack', 'call',