Fix non-destructive upgrade for deprecated res types in convergence

When a user updates from a deprecated resource type to an
equivalent-but-differently-named one (e.g. from
OS::Heat::SoftwareDeployments to OS::Heat::SoftwareDeploymentGroup), Heat
is supposed to change the type without replacing the resource as it would
normally do when a resource type changes. This was broken in convergence,
because since 45073226752c58d640ea5a59b7e532c022a4939b the new Resource
object we create during the check_resource operation (using the new type's
plugin) is no longer automatically initialised with data from the database
as resources in the legacy path are.

Move the substitution checking to the Resource.load() method, so that it
now returns an instance of the new plugin where allowed. In the actual
update_convergence() method then we need only check that the resource class
is the one we'd expect from the new template, and replace the resource if
not.

We do have a test that is designed to check that this is working, but in it
we didn't compare the physical IDs of the resource that is potentially
getting replaced, but rather the physical IDs of some other resource that
can't possibly get modified (surprise! it doesn't change).

Conflicts:
	heat/engine/resource.py

This patch is modified with respect to the one on master, since the
branch does not contain 960f626c2455b77e654aea1d79597fadb91dc894. Thus,
resource_owning_stack.defn is substituted for initial_stk_defn and
curr_stack.defn for latest_stk_defn.

It is also modified with respect to the one on stable/pike, since the
branch does not contain 764b8fb25132ac60c7760f134b78d600269ff516 or
a7376f7494b310e9367ebe5dcb43b432a4053023. Thus, resource definitions are
obtained from the Template rather than the StackDefinition.

Change-Id: I75778abc303525a71d0a918f7192f00a43c21284
Closes-Bug: #1729439
(cherry picked from commit 93b4551d9a1ff9194b30a079f02db691129c69b0)

2 files changed, 33 insertions, 18 deletions

diff --git a/heat/engine/resource.py b/heat/engine/resource.py
index 0d173090a..cd4909bdd 100644
--- a/heat/engine/resource.py
+++ b/heat/engine/resource.py
@@ -337,11 +337,28 @@ class Resource(object):
             resource_owning_stack = stack_mod.Stack.load(context,
                                                          stack=db_stack)
 
-        # Load only the resource in question; don't load all resources
-        # by invoking stack.resources. Maintain light-weight stack.
         res_defn = resource_owning_stack.t.resource_definitions(
             resource_owning_stack)[db_res.name]
-        resource = cls(db_res.name, res_defn, resource_owning_stack)
+        res_type = resource_owning_stack.env.registry.get_class_to_instantiate(
+            res_defn.resource_type, resource_name=db_res.name)
+
+        # If the resource type has changed and the new one is a valid
+        # substitution, use that as the class to instantiate.
+        if is_update and (curr_stack is not resource_owning_stack):
+            new_res_defns = curr_stack.t.resource_definitions(
+                resource_owning_stack)
+            if db_res.name in new_res_defns:
+                new_res_defn = new_res_defns[db_res.name]
+                new_registry = curr_stack.env.registry
+                new_res_type = new_registry.get_class_to_instantiate(
+                    new_res_defn.resource_type, resource_name=db_res.name)
+
+                if res_type.check_is_substituted(new_res_type):
+                    res_type = new_res_type
+
+        # Load only the resource in question; don't load all resources
+        # by invoking stack.resources. Maintain light-weight stack.
+        resource = res_type(db_res.name, res_defn, resource_owning_stack)
         resource._load_data(db_res)
 
         # assign current stack to the resource for updates
@@ -1164,6 +1181,9 @@ class Resource(object):
             )
 
         with self.lock(engine_id):
+            # Check that the resource type matches. If the type has changed by
+            # a legitimate substitution, the load()ed resource will already be
+            # of the new type.
             registry = new_stack.env.registry
             new_res_def = new_stack.t.resource_definitions(
                 new_stack)[self.name]
@@ -1171,8 +1191,7 @@ class Resource(object):
                 new_res_def.resource_type, resource_name=self.name)
             restricted_actions = registry.get_rsrc_restricted_actions(
                 self.name)
-            is_substituted = self.check_is_substituted(new_res_type)
-            if type(self) is not new_res_type and not is_substituted:
+            if type(self) is not new_res_type:
                 self._check_for_convergence_replace(restricted_actions)
 
             action_rollback = self.stack.action == self.stack.ROLLBACK
@@ -1186,15 +1205,7 @@ class Resource(object):
                                    six.text_type(failure))
                     raise failure
 
-            # Use new resource as update method if existing resource
-            # need to be substituted.
-            if is_substituted:
-                substitute = new_res_type(self.name, self.t, self.stack)
-                self.stack.resources[self.name] = substitute
-                updater = substitute.update
-            else:
-                updater = self.update
-            runner = scheduler.TaskRunner(updater, new_res_def)
+            runner = scheduler.TaskRunner(self.update, new_res_def)
             try:
                 runner(timeout=timeout, progress_callback=progress_callback)
                 update_tmpl_id_and_requires()
diff --git a/heat_integrationtests/functional/test_replace_deprecated.py b/heat_integrationtests/functional/test_replace_deprecated.py
index 5e7fdc67e..bbf2e66cc 100644
--- a/heat_integrationtests/functional/test_replace_deprecated.py
+++ b/heat_integrationtests/functional/test_replace_deprecated.py
@@ -69,24 +69,28 @@ properties:
             parameters=parms,
             template=deployments_template,
             enable_cleanup=self.enable_cleanup)
+
         expected_resources = {'config': 'OS::Heat::SoftwareConfig',
                               'dep': 'OS::Heat::SoftwareDeployments',
                               'server': 'OS::Nova::Server'}
-        resource = self.client.resources.get(stack_identifier, 'server')
         self.assertEqual(expected_resources,
                          self.list_resources(stack_identifier))
+
+        resource = self.client.resources.get(stack_identifier, 'dep')
         initial_phy_id = resource.physical_resource_id
+
         resources = deployments_template['resources']
         resources['dep'] = yaml.safe_load(self.deployment_group_snippet)
         self.update_stack(
             stack_identifier,
             deployments_template,
             parameters=parms)
-        resource = self.client.resources.get(stack_identifier, 'server')
-        self.assertEqual(initial_phy_id,
-                         resource.physical_resource_id)
+
         expected_new_resources = {'config': 'OS::Heat::SoftwareConfig',
                                   'dep': 'OS::Heat::SoftwareDeploymentGroup',
                                   'server': 'OS::Nova::Server'}
         self.assertEqual(expected_new_resources,
                          self.list_resources(stack_identifier))
+
+        resource = self.client.resources.get(stack_identifier, 'dep')
+        self.assertEqual(initial_phy_id, resource.physical_resource_id)