From a5cb850841ddea87f5e3702298295cc2a090f2fc Mon Sep 17 00:00:00 2001 From: Steven Hardy Date: Sat, 25 Jul 2015 12:32:00 +0100 Subject: [PATCH] PATCH update reuse existing template This is a replacement for the approach outlined in https://review.openstack.org/#/c/154620, we reuse the previous template, even if the stack is in a failed state, by making use of the prev_raw_template now stored at the start of each update. Closes-Bug: 1224828 Change-Id: I0294745dea0d25b91dbcc50a8bd55b3e1ea87d81 --- heat/api/openstack/v1/stacks.py | 10 ++- heat/engine/service.py | 34 +++++++++- heat/tests/api/openstack_v1/test_stacks.py | 35 +++++++++++ .../tests/engine/service/test_stack_update.py | 62 +++++++++++++++++++ 4 files changed, 136 insertions(+), 5 deletions(-) diff --git a/heat/api/openstack/v1/stacks.py b/heat/api/openstack/v1/stacks.py index 2ff26327d1..023b621609 100644 --- a/heat/api/openstack/v1/stacks.py +++ b/heat/api/openstack/v1/stacks.py @@ -66,6 +66,7 @@ class InstantiationData(object): to distinguish. """ self.data = data + self.patch = patch if patch: self.data[rpc_api.PARAM_EXISTING] = True @@ -92,6 +93,7 @@ class InstantiationData(object): Get template file contents, either inline, from stack adopt data or from a URL, in JSON or YAML format. """ + template_data = None if rpc_api.PARAM_ADOPT_STACK_DATA in self.data: adopt_data = self.data[rpc_api.PARAM_ADOPT_STACK_DATA] try: @@ -112,8 +114,12 @@ class InstantiationData(object): except IOError as ex: err_reason = _('Could not retrieve template: %s') % ex raise exc.HTTPBadRequest(err_reason) - else: - raise exc.HTTPBadRequest(_("No template specified")) + + if template_data is None: + if self.patch: + return None + else: + raise exc.HTTPBadRequest(_("No template specified")) with self.parse_error_check('Template'): return template_format.parse(template_data) diff --git a/heat/engine/service.py b/heat/engine/service.py index 54d58a6762..675cbfa9f8 100644 --- a/heat/engine/service.py +++ b/heat/engine/service.py @@ -791,7 +791,8 @@ class EngineService(service.Service): # Now parse the template and any parameters for the updated # stack definition. If PARAM_EXISTING is specified, we merge - # any environment provided into the existing one. + # any environment provided into the existing one and attempt + # to use the existing stack template, if one is not provided. if args.get(rpc_api.PARAM_EXISTING, None): existing_env = current_stack.env.user_env_as_dict() existing_params = existing_env[env_fmt.PARAMETERS] @@ -804,15 +805,42 @@ class EngineService(service.Service): new_files = current_stack.t.files.copy() new_files.update(files or {}) + + if template is not None: + new_template = template + elif (current_stack.convergence or + current_stack.status == current_stack.COMPLETE): + # If convergence is enabled, or the stack is complete, we can + # just use the current template... + new_template = current_stack.t.t + else: + # ..but if it's FAILED without convergence things may be in an + # inconsistent state, so we try to fall back on a stored copy + # of the previous template + if current_stack.prev_raw_template_id is not None: + # Use the stored previous template + prev_t = templatem.Template.load( + cnxt, current_stack.prev_raw_template_id) + new_template = prev_t.t + else: + # Nothing we can do, the failed update happened before + # we started storing prev_raw_template_id + LOG.error("PATCH update to FAILED stack only possible if " + "convergence enabled or previous template " + "stored") + msg = _('PATCH update to non-COMPLETE stack') + raise exception.NotSupported(feature=msg) else: new_env = environment.Environment(params) new_files = files - tmpl = templatem.Template(template, files=new_files, env=new_env) + new_template = template + + tmpl = templatem.Template(new_template, files=new_files, env=new_env) current_stack, updated_stack = self._prepare_stack_updates( cnxt, current_stack, tmpl, params, files, args) - if current_stack.get_kwargs_for_cloning()['convergence']: + if current_stack.convergence: current_stack.converge_stack(template=tmpl, new_stack=updated_stack) else: diff --git a/heat/tests/api/openstack_v1/test_stacks.py b/heat/tests/api/openstack_v1/test_stacks.py index e1ecf4eeed..72e1e76fa3 100644 --- a/heat/tests/api/openstack_v1/test_stacks.py +++ b/heat/tests/api/openstack_v1/test_stacks.py @@ -1661,6 +1661,41 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase): self.assertEqual(403, resp.status_int) self.assertIn('403 Forbidden', six.text_type(resp)) + def test_update_with_existing_template(self, mock_enforce): + self._mock_enforce_setup(mock_enforce, 'update_patch', True) + identity = identifier.HeatIdentifier(self.tenant, 'wordpress', '6') + body = {'template': None, + 'parameters': {}, + 'files': {}, + 'timeout_mins': 30} + + req = self._patch('/stacks/%(stack_name)s/%(stack_id)s' % identity, + json.dumps(body)) + + self.m.StubOutWithMock(rpc_client.EngineClient, 'call') + rpc_client.EngineClient.call( + req.context, + ('update_stack', + {'stack_identity': dict(identity), + 'template': None, + 'params': {'parameters': {}, + 'encrypted_param_names': [], + 'parameter_defaults': {}, + 'resource_registry': {}}, + 'files': {}, + 'args': {rpc_api.PARAM_EXISTING: True, + 'timeout_mins': 30}}) + ).AndReturn(dict(identity)) + self.m.ReplayAll() + + self.assertRaises(webob.exc.HTTPAccepted, + self.controller.update_patch, + req, tenant_id=identity.tenant, + stack_name=identity.stack_name, + stack_id=identity.stack_id, + body=body) + self.m.VerifyAll() + def test_update_with_existing_parameters(self, mock_enforce): self._mock_enforce_setup(mock_enforce, 'update_patch', True) identity = identifier.HeatIdentifier(self.tenant, 'wordpress', '6') diff --git a/heat/tests/engine/service/test_stack_update.py b/heat/tests/engine/service/test_stack_update.py index 56260a6dc2..913c480176 100644 --- a/heat/tests/engine/service/test_stack_update.py +++ b/heat/tests/engine/service/test_stack_update.py @@ -601,6 +601,68 @@ class ServiceStackUpdateTest(common.HeatTestCase): user_creds_id=u'1', username='test_username') mock_load.assert_called_once_with(self.ctx, stack=s) + def test_stack_update_existing_template(self): + '''Update a stack using the same template.''' + stack_name = 'service_update_test_stack_existing_template' + api_args = {rpc_api.PARAM_TIMEOUT: 60, + rpc_api.PARAM_EXISTING: True} + t = template_format.parse(tools.wp_template) + # Don't actually run the update as the mocking breaks it, instead + # we just ensure the expected template is passed in to the updated + # template, and that the update task is scheduled. + self.man.thread_group_mgr = tools.DummyThreadGroupMgrLogStart() + + params = {} + stack = utils.parse_stack(t, stack_name=stack_name, + params=params) + stack.set_stack_user_project_id('1234') + self.assertEqual(stack.t.t, + t) + stack.action = stack.CREATE + stack.status = stack.COMPLETE + + with mock.patch('heat.engine.stack.Stack') as mock_stack: + mock_stack.load.return_value = stack + mock_stack.validate.return_value = None + result = self.man.update_stack(self.ctx, stack.identifier(), + None, + params, + None, api_args) + tmpl = mock_stack.call_args[0][2] + self.assertEqual(t, + tmpl.t) + self.assertEqual(stack.identifier(), result) + self.assertEqual(1, len(self.man.thread_group_mgr.started)) + + def test_stack_update_existing_failed(self): + '''Update a stack using the same template doesn't work when FAILED.''' + stack_name = 'service_update_test_stack_existing_template' + api_args = {rpc_api.PARAM_TIMEOUT: 60, + rpc_api.PARAM_EXISTING: True} + t = template_format.parse(tools.wp_template) + # Don't actually run the update as the mocking breaks it, instead + # we just ensure the expected template is passed in to the updated + # template, and that the update task is scheduled. + self.man.thread_group_mgr = tools.DummyThreadGroupMgrLogStart() + + params = {} + stack = utils.parse_stack(t, stack_name=stack_name, + params=params) + stack.set_stack_user_project_id('1234') + self.assertEqual(stack.t.t, + t) + stack.action = stack.UPDATE + stack.status = stack.FAILED + + ex = self.assertRaises(dispatcher.ExpectedException, + self.man.update_stack, + self.ctx, stack.identifier(), + None, params, None, api_args) + + self.assertEqual(exception.NotSupported, ex.exc_info[0]) + self.assertIn("PATCH update to non-COMPLETE stack", + six.text_type(ex.exc_info[1])) + class ServiceStackUpdatePreviewTest(common.HeatTestCase):