Allow in-place updates for all compatible types

We previously only allowed a resource to update in-place when the 'type'
field in the old and new templates matched. In an age of environment type
mappings, this is incorrect. Instead, allow a resource to be updated
in-place when the old and new resource plugins are of the same class, since
this is actually what determines whether handle_update is capable of
correctly updating the resource in-place.

There is a special-case for TemplateResource as we currently subclass it
for each template type, but all TemplateResource subclasses are in fact
capable of converting between each other via a stack update.

Change-Id: Iba4abf5efd9737ca6a17d8c91d5c54ab6d3f12e0
Co-Authored-By: Zane Bitter <zbitter@redhat.com>
Closes-Bug: #1508115

heat/engine/resource.py

@@ -391,6 +391,19 @@ class Resource(object):
         # common resources have not nested, StackResource overrides it
         return False
 
+    def resource_class(self):
+        """Return the resource class.
+
+        This is used to compare old and new resources when updating, to ensure
+        that in-place updates are possible. This method shold return the
+        highest common class in the hierarchy whose subclasses are all capable
+        of converting to each other's types via handle_update().
+
+        This mechanism may disappear again in future, so third-party resource
+        types should not rely on it.
+        """
+        return type(self)
+
     def has_hook(self, hook):
         # Clear the cache to make sure the data is up to date:
         self._data = None

heat/engine/resources/template_resource.py

@@ -65,6 +65,11 @@ class TemplateResource(stack_resource.StackResource):
         if self.validation_exception is None:
             self._generate_schema(self.t)
 
+    def resource_class(self):
+        # All TemplateResource subclasses can be converted to each other with
+        # a stack update, so allow them to cross-update in place.
+        return TemplateResource
+
     def _get_resource_info(self, rsrc_defn):
         tri = self.stack.env.get_resource_info(
             rsrc_defn.resource_type,

heat/engine/update.py

@@ -135,10 +135,10 @@ class StackUpdate(object):
     @scheduler.wrappertask
     def _process_new_resource_update(self, new_res):
         res_name = new_res.name
-        res_type = new_res.type()
+        res_class = new_res.resource_class()
 
         if (res_name in self.existing_stack and
-                res_type == self.existing_stack[res_name].type()):
+                self.existing_stack[res_name].resource_class() is res_class):
             existing_res = self.existing_stack[res_name]
             try:
                 yield self._update_in_place(existing_res,

heat_integrationtests/functional/test_create_update.py

@@ -331,6 +331,105 @@ resources:
         self.assertEqual(nested_resources,
                          self.list_resources(nested_identifier))
 
+    def test_stack_update_alias_type(self):
+        env = {'resource_registry':
+               {'My::TestResource': 'OS::Heat::RandomString',
+                'My::TestResource2': 'OS::Heat::RandomString'}}
+        stack_identifier = self.stack_create(
+            template=self.provider_template,
+            environment=env
+        )
+        p_res = self.client.resources.get(stack_identifier, 'test1')
+        self.assertEqual('My::TestResource', p_res.resource_type)
+
+        initial_resources = {'test1': 'My::TestResource'}
+        self.assertEqual(initial_resources,
+                         self.list_resources(stack_identifier))
+        res = self.client.resources.get(stack_identifier, 'test1')
+        # Modify the type of the resource alias to My::TestResource2
+        tmpl_update = copy.deepcopy(self.provider_template)
+        tmpl_update['resources']['test1']['type'] = 'My::TestResource2'
+        self.update_stack(stack_identifier, tmpl_update, environment=env)
+        res_a = self.client.resources.get(stack_identifier, 'test1')
+        self.assertEqual(res.physical_resource_id, res_a.physical_resource_id)
+        self.assertEqual(res.attributes['value'], res_a.attributes['value'])
+
+    def test_stack_update_alias_changes(self):
+        env = {'resource_registry':
+               {'My::TestResource': 'OS::Heat::RandomString'}}
+        stack_identifier = self.stack_create(
+            template=self.provider_template,
+            environment=env
+        )
+        p_res = self.client.resources.get(stack_identifier, 'test1')
+        self.assertEqual('My::TestResource', p_res.resource_type)
+
+        initial_resources = {'test1': 'My::TestResource'}
+        self.assertEqual(initial_resources,
+                         self.list_resources(stack_identifier))
+        res = self.client.resources.get(stack_identifier, 'test1')
+        # Modify the resource alias to point to a different type
+        env = {'resource_registry':
+               {'My::TestResource': 'OS::Heat::TestResource'}}
+        self.update_stack(stack_identifier, template=self.provider_template,
+                          environment=env)
+        res_a = self.client.resources.get(stack_identifier, 'test1')
+        self.assertNotEqual(res.physical_resource_id,
+                            res_a.physical_resource_id)
+
+    def test_stack_update_provider_type(self):
+        template = _change_rsrc_properties(
+            test_template_one_resource, ['test1'],
+            {'value': 'test_provider_template'})
+        files = {'provider.template': json.dumps(template)}
+        env = {'resource_registry':
+               {'My::TestResource': 'provider.template',
+                'My::TestResource2': 'provider.template'}}
+        stack_identifier = self.stack_create(
+            template=self.provider_template,
+            files=files,
+            environment=env
+        )
+        p_res = self.client.resources.get(stack_identifier, 'test1')
+        self.assertEqual('My::TestResource', p_res.resource_type)
+
+        initial_resources = {'test1': 'My::TestResource'}
+        self.assertEqual(initial_resources,
+                         self.list_resources(stack_identifier))
+
+        # Prove the resource is backed by a nested stack, save the ID
+        nested_identifier = self.assert_resource_is_a_stack(stack_identifier,
+                                                            'test1')
+        nested_id = nested_identifier.split('/')[-1]
+
+        # Then check the expected resources are in the nested stack
+        nested_resources = {'test1': 'OS::Heat::TestResource'}
+        self.assertEqual(nested_resources,
+                         self.list_resources(nested_identifier))
+        n_res = self.client.resources.get(nested_identifier, 'test1')
+
+        # Modify the type of the provider resource to My::TestResource2
+        tmpl_update = copy.deepcopy(self.provider_template)
+        tmpl_update['resources']['test1']['type'] = 'My::TestResource2'
+        self.update_stack(stack_identifier, tmpl_update,
+                          environment=env, files=files)
+        p_res = self.client.resources.get(stack_identifier, 'test1')
+        self.assertEqual('My::TestResource2', p_res.resource_type)
+
+        # Parent resources should be unchanged and the nested stack
+        # should have been updated in-place without replacement
+        self.assertEqual({u'test1': u'My::TestResource2'},
+                         self.list_resources(stack_identifier))
+        rsrc = self.client.resources.get(stack_identifier, 'test1')
+        self.assertEqual(rsrc.physical_resource_id, nested_id)
+
+        # Then check the expected resources are in the nested stack
+        self.assertEqual(nested_resources,
+                         self.list_resources(nested_identifier))
+        n_res2 = self.client.resources.get(nested_identifier, 'test1')
+        self.assertEqual(n_res.physical_resource_id,
+                         n_res2.physical_resource_id)
+
     def test_stack_update_provider_group(self):
         """Test two-level nested update."""