From fab4fe769bd10c458a218e372dcdddef21e289b4 Mon Sep 17 00:00:00 2001 From: Steve Baker Date: Fri, 3 Jun 2016 09:10:26 +1200 Subject: [PATCH] Load resources from DB for resource list Currently, when a user makes an API call to list resources, they were retrieved from the currently active template associated with the stack. This worked in the legacy engine where no concurrent updates where possible. With convergence and thus concurrent updates, a stack is allowed to have resources of previous traversals and still continue creating new resources. Therefore, relying on the template to list resources won't exactly match the state in the database. For example, on deletes where we update with an empty template, currently, the stack parses the empty template searching for resources. Doing a `resource-list` when the stack is in a state of DELETE_IN_PROGRESS shows an empty list of resources and therefore not matching the state in the database. This change makes iter_resources always call _find_filtered_resources which builds all its resources from a database query. Change-Id: Ibe87a773c38efb6d4865fd3a1dbd079972dd8be4 Closes-Bug: #1523748 Closes-Bug: #1301320 --- heat/engine/resource.py | 7 +- heat/engine/stack.py | 33 +++-- .../engine/service/test_stack_resources.py | 6 +- heat/tests/test_stack.py | 132 ++++++++++-------- 4 files changed, 99 insertions(+), 79 deletions(-) diff --git a/heat/engine/resource.py b/heat/engine/resource.py index e0e8187f9f..11cece0aa0 100644 --- a/heat/engine/resource.py +++ b/heat/engine/resource.py @@ -637,7 +637,12 @@ class Resource(object): Returns a list of names of resources that depend on this resource directly. """ - return [r.name for r in self.stack.dependencies.required_by(self)] + try: + return [r.name for r in self.stack.dependencies.required_by(self)] + except KeyError: + # for convergence, fall back to building from needed_by + return [r.name for r in self.stack.resources.values() + if r.id in self.needed_by] def client(self, name=None, version=None): client_name = name or self.default_client_name diff --git a/heat/engine/stack.py b/heat/engine/stack.py index 354a4c50c2..f0a3fe8db3 100644 --- a/heat/engine/stack.py +++ b/heat/engine/stack.py @@ -314,21 +314,21 @@ class Stack(collections.Mapping): return self._resources def _find_filtered_resources(self, filters): - for rsc in six.itervalues( - resource_objects.Resource.get_all_by_stack( - self.context, self.id, filters)): - yield self.resources[rsc.name] + template_cache = {self.t.id: self.t} + if filters: + resources = resource_objects.Resource.get_all_by_stack( + self.context, self.id, filters) + else: + resources = self._db_resources_get() + for rsc in six.itervalues(resources): + yield self._resource_from_db_resource(rsc, template_cache) def iter_resources(self, nested_depth=0, filters=None): """Iterates over all the resources in a stack. Iterating includes nested stacks up to `nested_depth` levels below. """ - if not filters: - resources = six.itervalues(self.resources) - else: - resources = self._find_filtered_resources(filters) - for res in resources: + for res in self._find_filtered_resources(filters): yield res for res in six.itervalues(self.resources): @@ -350,20 +350,27 @@ class Stack(collections.Mapping): def db_resource_get(self, name): if not self.id: return None + return self._db_resources_get().get(name) + + def _db_resources_get(self): if self._db_resources is None: _db_resources = resource_objects.Resource.get_all_by_stack( self.context, self.id) if not _db_resources: - return None + return {} self._db_resources = _db_resources - return self._db_resources.get(name) + return self._db_resources - def _resource_from_db_resource(self, db_res): + def _resource_from_db_resource(self, db_res, template_cache=None): tid = db_res.current_template_id - if tid == self.t.id: + if tid is None or tid == self.t.id: t = self.t + elif template_cache and tid in template_cache: + t = template_cache[tid] else: t = tmpl.Template.load(self.context, tid) + if template_cache: + template_cache[tid] = t res_defn = t.resource_definitions(self)[db_res.name] return resource.Resource(db_res.name, res_defn, self) diff --git a/heat/tests/engine/service/test_stack_resources.py b/heat/tests/engine/service/test_stack_resources.py index 5bd724b14d..fc9251db87 100644 --- a/heat/tests/engine/service/test_stack_resources.py +++ b/heat/tests/engine/service/test_stack_resources.py @@ -298,11 +298,7 @@ class StackResourcesServiceTest(common.HeatTestCase): mock_load.return_value = stk tools.clean_up_stack(stk) resources = self.eng.list_stack_resources(self.ctx, stack_id) - self.assertEqual(1, len(resources)) - - res = resources[0] - self.assertEqual('DELETE', res['resource_action']) - self.assertEqual('COMPLETE', res['resource_status']) + self.assertEqual(0, len(resources)) @mock.patch.object(service.EngineService, '_get_stack') def test_stack_resources_list_nonexist_stack(self, mock_get): diff --git a/heat/tests/test_stack.py b/heat/tests/test_stack.py index 0fd15a2858..56bdc6c86c 100644 --- a/heat/tests/test_stack.py +++ b/heat/tests/test_stack.py @@ -264,7 +264,37 @@ class StackTest(common.HeatTestCase): self.assertEqual('B', self.stack.resource_get('B').name) self.assertIsNone(self.stack.resource_get('C')) - def test_iter_resources_with_nested(self): + @mock.patch.object(resource_objects.Resource, 'get_all_by_stack') + def test_iter_resources(self, mock_db_call): + tpl = {'HeatTemplateFormatVersion': '2012-12-12', + 'Resources': + {'A': {'Type': 'GenericResourceType'}, + 'B': {'Type': 'GenericResourceType'}}} + self.stack = stack.Stack(self.ctx, 'test_stack', + template.Template(tpl), + status_reason='blarg') + self.stack.store() + + mock_rsc_a = mock.MagicMock(current_template_id=self.stack.t.id) + mock_rsc_a.name = 'A' + mock_rsc_b = mock.MagicMock(current_template_id=self.stack.t.id) + mock_rsc_b.name = 'B' + mock_db_call.return_value = { + 'A': mock_rsc_a, + 'B': mock_rsc_b + } + + all_resources = list(self.stack.iter_resources()) + + # Verify, the db query is called with expected filter + mock_db_call.assert_called_once_with(self.ctx, self.stack.id) + + # And returns the resources + names = sorted([r.name for r in all_resources]) + self.assertEqual(['A', 'B'], names) + + @mock.patch.object(resource_objects.Resource, 'get_all_by_stack') + def test_iter_resources_with_nested(self, mock_db_call): tpl = {'HeatTemplateFormatVersion': '2012-12-12', 'Resources': {'A': {'Type': 'StackResourceType'}, @@ -273,6 +303,17 @@ class StackTest(common.HeatTestCase): template.Template(tpl), status_reason='blarg') + self.stack.store() + + mock_rsc_a = mock.MagicMock(current_template_id=self.stack.t.id) + mock_rsc_a.name = 'A' + mock_rsc_b = mock.MagicMock(current_template_id=self.stack.t.id) + mock_rsc_b.name = 'B' + mock_db_call.return_value = { + 'A': mock_rsc_a, + 'B': mock_rsc_b + } + def get_more(nested_depth=0, filters=None): yield 'X' yield 'Y' @@ -291,28 +332,31 @@ class StackTest(common.HeatTestCase): self.assertEqual(5, len(all_resources)) @mock.patch.object(resource_objects.Resource, 'get_all_by_stack') - @mock.patch('heat.engine.resource.Resource') - def test_iter_resources_with_filters(self, mock_resource, mock_db_call): - mock_rsc = mock.MagicMock() - mock_rsc.name = 'A' - mock_db_call.return_value = {'A': mock_rsc} - mock_resource.return_value = mock_rsc + def test_iter_resources_with_filters(self, mock_db_call): tpl = {'HeatTemplateFormatVersion': '2012-12-12', 'Resources': - {'A': {'Type': 'StackResourceType'}, + {'A': {'Type': 'GenericResourceType'}, 'B': {'Type': 'GenericResourceType'}}} self.stack = stack.Stack(self.ctx, 'test_stack', template.Template(tpl), status_reason='blarg') + self.stack.store() + + mock_rsc = mock.MagicMock() + mock_rsc.name = 'A' + mock_rsc.current_template_id = self.stack.t.id + mock_db_call.return_value = {'A': mock_rsc} all_resources = list(self.stack.iter_resources( filters=dict(name=['A']) )) # Verify, the db query is called with expected filter - mock_db_call.assert_called_once_with(self.ctx, - self.stack.id, - dict(name=['A'])) + mock_db_call.assert_has_calls([ + mock.call(self.ctx, self.stack.id, dict(name=['A'])), + mock.call(self.ctx, self.stack.id), + ]) + # Make sure it returns only one resource. self.assertEqual(1, len(all_resources)) @@ -320,13 +364,7 @@ class StackTest(common.HeatTestCase): self.assertEqual('A', all_resources[0].name) @mock.patch.object(resource_objects.Resource, 'get_all_by_stack') - @mock.patch('heat.engine.resource.Resource') - def test_iter_resources_nested_with_filters(self, mock_resource, - mock_db_call): - mock_rsc = mock.Mock() - mock_rsc.name = 'A' - mock_db_call.return_value = {'A': mock_rsc} - mock_resource.return_value = mock_rsc + def test_iter_resources_nested_with_filters(self, mock_db_call): tpl = {'HeatTemplateFormatVersion': '2012-12-12', 'Resources': {'A': {'Type': 'StackResourceType'}, @@ -335,6 +373,17 @@ class StackTest(common.HeatTestCase): template.Template(tpl), status_reason='blarg') + self.stack.store() + + mock_rsc_a = mock.MagicMock(current_template_id=self.stack.t.id) + mock_rsc_a.name = 'A' + mock_rsc_b = mock.MagicMock(current_template_id=self.stack.t.id) + mock_rsc_b.name = 'B' + mock_db_call.return_value = { + 'A': mock_rsc_a, + 'B': mock_rsc_b + } + def get_more(nested_depth=0, filters=None): if filters: yield 'X' @@ -352,51 +401,14 @@ class StackTest(common.HeatTestCase): )) # Verify, the db query is called with expected filter - mock_db_call.assert_called_once_with(self.ctx, - self.stack.id, - dict(name=['A'])) + mock_db_call.assert_has_calls([ + mock.call(self.ctx, self.stack.id), + mock.call(self.ctx, self.stack.id, dict(name=['A'])), + ]) + # Returns three resources (1 first level + 2 second level) self.assertEqual(3, len(all_resources)) - @mock.patch.object(stack.Stack, 'db_resource_get') - def test_iter_resources_cached(self, mock_drg): - tpl = {'HeatTemplateFormatVersion': '2012-12-12', - 'Resources': - {'A': {'Type': 'StackResourceType'}, - 'B': {'Type': 'GenericResourceType'}}} - - cache_data = {'A': {'reference_id': 'A-id', 'uuid': mock.ANY, - 'id': mock.ANY, 'action': 'CREATE', - 'status': 'COMPLETE'}, - 'B': {'reference_id': 'B-id', 'uuid': mock.ANY, - 'id': mock.ANY, 'action': 'CREATE', - 'status': 'COMPLETE'}} - - self.stack = stack.Stack(self.ctx, 'test_stack', - template.Template(tpl), - status_reason='blarg', - cache_data=cache_data) - - def get_more(nested_depth=0, filters=None): - yield 'X' - yield 'Y' - yield 'Z' - - self.stack['A'].nested = mock.MagicMock() - self.stack['A'].nested.return_value.iter_resources = mock.MagicMock( - side_effect=get_more) - - resource_generator = self.stack.iter_resources() - self.assertIsNot(resource_generator, list) - - first_level_resources = list(resource_generator) - self.assertEqual(2, len(first_level_resources)) - all_resources = list(self.stack.iter_resources(1)) - self.assertEqual(5, len(all_resources)) - - # A cache supplied means we should never query the database. - self.assertFalse(mock_drg.called) - def test_load_parent_resource(self): self.stack = stack.Stack(self.ctx, 'load_parent_resource', self.tmpl, parent_resource='parent')