Update preview_update_stack to align with PATCH updates

Currently attempting to do a preview update call with PATCH fails,
because we didn't align the behavior of preview update with the
actual update in the recent additions to fix bug #1224828

So, refactor to ensure both preview_update & update use the same
code, and add a PATCH path to the update API.

Change-Id: I8ce5c0ea4035a7b9563db10ea10433e7f5f99a4f
Closes-Bug: #1501207
(cherry picked from commit 604595a39c9345d214f9f02c2ab8d7d1d768fcbe)

4 files changed, 117 insertions, 64 deletions

diff --git a/heat/api/openstack/v1/__init__.py b/heat/api/openstack/v1/__init__.py
index 6dc6829d4..b4cd061ba 100644
--- a/heat/api/openstack/v1/__init__.py
+++ b/heat/api/openstack/v1/__init__.py
@@ -215,6 +215,12 @@ class API(wsgi.Router):
                         'method': 'PUT'
                     },
                     {
+                        'name': 'preview_stack_update_patch',
+                        'url': '/stacks/{stack_name}/{stack_id}/preview',
+                        'action': 'preview_update_patch',
+                        'method': 'PATCH'
+                    },
+                    {
                         'name': 'stack_delete',
                         'url': '/stacks/{stack_name}/{stack_id}',
                         'action': 'delete',
diff --git a/heat/api/openstack/v1/stacks.py b/heat/api/openstack/v1/stacks.py
index 60d056b29..a41f8ac5f 100644
--- a/heat/api/openstack/v1/stacks.py
+++ b/heat/api/openstack/v1/stacks.py
@@ -503,6 +503,22 @@ class StackController(object):
         return {'resource_changes': changes}
 
     @util.identified_stack
+    def preview_update_patch(self, req, identity, body):
+        """Preview PATCH update for existing stack."""
+        data = InstantiationData(body, patch=True)
+
+        args = self.prepare_args(data)
+        changes = self.rpc_client.preview_update_stack(
+            req.context,
+            identity,
+            data.template(),
+            data.environment(),
+            data.files(),
+            args)
+
+        return {'resource_changes': changes}
+
+    @util.identified_stack
     def delete(self, req, identity):
         """
         Delete the specified stack
diff --git a/heat/engine/service.py b/heat/engine/service.py
index d91b5a7ae..ec4359e7a 100644
--- a/heat/engine/service.py
+++ b/heat/engine/service.py
@@ -728,7 +728,7 @@ class EngineService(service.Service):
 
         return dict(stack.identifier())
 
-    def _prepare_stack_updates(self, cnxt, current_stack, tmpl, params,
+    def _prepare_stack_updates(self, cnxt, current_stack, template, params,
                                files, args):
         """Return the current and updated stack.
 
@@ -737,11 +737,59 @@ class EngineService(service.Service):
 
         :param cnxt: RPC context.
         :param stack: A stack to be updated.
-        :param tmpl: Template object of stack you want to update to.
+        :param template: Template of stack you want to update to.
         :param params: Stack Input Params
         :param files: Files referenced from the template
         :param args: Request parameters/args passed from API
         """
+        # 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 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]
+            clear_params = set(args.get(rpc_api.PARAM_CLEAR_PARAMETERS, []))
+            retained = dict((k, v) for k, v in existing_params.items()
+                            if k not in clear_params)
+            existing_env[env_fmt.PARAMETERS] = retained
+            new_env = environment.Environment(existing_env)
+            new_env.load(params)
+
+            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
+            new_template = template
+
+        tmpl = templatem.Template(new_template, files=new_files, env=new_env)
+
         max_resources = cfg.CONF.max_resources_per_stack
         if max_resources != -1 and len(tmpl[tmpl.RESOURCES]) > max_resources:
             raise exception.RequestLimitExceeded(
@@ -765,7 +813,7 @@ class EngineService(service.Service):
         self._validate_deferred_auth_context(cnxt, updated_stack)
         updated_stack.validate()
 
-        return current_stack, updated_stack
+        return tmpl, current_stack, updated_stack
 
     @context.request_context
     def update_stack(self, cnxt, stack_identity, template, params,
@@ -797,56 +845,8 @@ class EngineService(service.Service):
             msg = _('Updating a stack when it is deleting')
             raise exception.NotSupported(feature=msg)
 
-        # 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 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]
-            clear_params = set(args.get(rpc_api.PARAM_CLEAR_PARAMETERS, []))
-            retained = dict((k, v) for k, v in existing_params.items()
-                            if k not in clear_params)
-            existing_env[env_fmt.PARAMETERS] = retained
-            new_env = environment.Environment(existing_env)
-            new_env.load(params)
-
-            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
-            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)
+        tmpl, current_stack, updated_stack = self._prepare_stack_updates(
+            cnxt, current_stack, template, params, files, args)
 
         if current_stack.convergence:
             current_stack.converge_stack(template=tmpl,
@@ -886,17 +886,8 @@ class EngineService(service.Service):
 
         current_stack = parser.Stack.load(cnxt, stack=db_stack)
 
-        # Now parse the template and any parameters for the updated
-        # stack definition.
-        env = environment.Environment(params)
-        if args.get(rpc_api.PARAM_EXISTING, None):
-            env.patch_previous_parameters(
-                current_stack.env,
-                args.get(rpc_api.PARAM_CLEAR_PARAMETERS, []))
-        tmpl = templatem.Template(template, files=files, env=env)
-
-        current_stack, updated_stack = self._prepare_stack_updates(
-            cnxt, current_stack, tmpl, params, files, args)
+        tmpl, current_stack, updated_stack = self._prepare_stack_updates(
+            cnxt, current_stack, template, params, files, args)
 
         update_task = update.StackUpdate(current_stack, updated_stack, None)
 
diff --git a/heat/tests/api/openstack_v1/test_stacks.py b/heat/tests/api/openstack_v1/test_stacks.py
index a98c55ef3..b8b813cfe 100644
--- a/heat/tests/api/openstack_v1/test_stacks.py
+++ b/heat/tests/api/openstack_v1/test_stacks.py
@@ -1200,6 +1200,46 @@ class StackControllerTest(tools.ControllerTest, common.HeatTestCase):
         self.assertEqual({'resource_changes': resource_changes}, result)
         self.m.VerifyAll()
 
+    def test_preview_update_stack_patch(self, mock_enforce):
+        self._mock_enforce_setup(mock_enforce, 'preview_update_patch', True)
+        identity = identifier.HeatIdentifier(self.tenant, 'wordpress', '6')
+        parameters = {u'InstanceType': u'm1.xlarge'}
+        body = {'template': None,
+                'parameters': parameters,
+                'files': {},
+                'timeout_mins': 30}
+
+        req = self._patch('/stacks/%(stack_name)s/%(stack_id)s/preview' %
+                          identity, json.dumps(body))
+        resource_changes = {'updated': [],
+                            'deleted': [],
+                            'unchanged': [],
+                            'added': [],
+                            'replaced': []}
+
+        self.m.StubOutWithMock(rpc_client.EngineClient, 'call')
+        rpc_client.EngineClient.call(
+            req.context,
+            ('preview_update_stack',
+             {'stack_identity': dict(identity),
+              'template': None,
+              'params': {'parameters': parameters,
+                         'encrypted_param_names': [],
+                         'parameter_defaults': {},
+                         'resource_registry': {}},
+              'files': {},
+              'args': {rpc_api.PARAM_EXISTING: True,
+                       'timeout_mins': 30}}),
+            version='1.15'
+        ).AndReturn(resource_changes)
+        self.m.ReplayAll()
+
+        result = self.controller.preview_update_patch(
+            req, tenant_id=identity.tenant, stack_name=identity.stack_name,
+            stack_id=identity.stack_id, body=body)
+        self.assertEqual({'resource_changes': resource_changes}, result)
+        self.m.VerifyAll()
+
     def test_lookup(self, mock_enforce):
         self._mock_enforce_setup(mock_enforce, 'lookup', True)
         identity = identifier.HeatIdentifier(self.tenant, 'wordpress', '1')