Optimize nested stack status check

Currently, StackResource loads the whole stack when checking for status
(in check_*_complete method), but only care about the state of the
stack. This is a fairly expensive operation, as it retrieves the
template and reparses everything. This simplifies it with a new API that
simply query the stack status from the database.

Closes-Bug: #1549213
Change-Id: I18df89a2b9959241ddbec2593a53c5e2aa6a4717

heat/db/api.py

@@ -134,6 +134,10 @@ def stack_get(context, stack_id, show_deleted=False, tenant_safe=True,
                           eager_load=eager_load)
 
 
+def stack_get_status(context, stack_id):
+    return IMPL.stack_get_status(context, stack_id)
+
+
 def stack_get_by_name_and_owner_id(context, stack_name, owner_id):
     return IMPL.stack_get_by_name_and_owner_id(context, stack_name,
                                                owner_id=owner_id)

heat/db/sqlalchemy/api.py

@@ -366,6 +366,18 @@ def stack_get(context, stack_id, show_deleted=False, tenant_safe=True,
     return result
 
 
+def stack_get_status(context, stack_id):
+    query = model_query(context, models.Stack)
+    query = query.options(
+        orm.load_only("action", "status", "status_reason", "updated_at"))
+    result = query.filter_by(id=stack_id).first()
+    if result is None:
+        raise exception.NotFound(_('Stack with id %s not found') % stack_id)
+
+    return (result.action, result.status, result.status_reason,
+            result.updated_at)
+
+
 def stack_get_all_by_owner_id(context, owner_id):
     results = soft_delete_aware_query(
         context, models.Stack).filter_by(owner_id=owner_id).all()

heat/engine/resources/stack_resource.py

@@ -30,6 +30,8 @@ from heat.engine import resource
 from heat.engine import scheduler
 from heat.engine import stack as parser
 from heat.engine import template
+from heat.objects import stack as stack_object
+from heat.objects import stack_lock
 from heat.rpc import api as rpc_api
 
 LOG = logging.getLogger(__name__)
@@ -123,24 +125,15 @@ class StackResource(resource.Resource):
 
         return False
 
-    def nested(self, force_reload=False, show_deleted=False):
+    def nested(self):
         """Return a Stack object representing the nested (child) stack.
 
         If we catch NotFound exception when loading, return None.
-
-        :param force_reload: Forces reloading from the DB instead of returning
-                             the locally cached Stack object
-        :param show_deleted: Returns the stack even if it's been deleted
         """
-        if force_reload:
-            self._nested = None
-
         if self._nested is None and self.resource_id is not None:
             try:
                 self._nested = parser.Stack.load(self.context,
-                                                 self.resource_id,
+                                                 self.resource_id)
-                                                 show_deleted=show_deleted,
-                                                 force_reload=force_reload)
             except exception.NotFound:
                 return None
 
@@ -337,19 +330,23 @@ class StackResource(resource.Resource):
         raise exception.ResourceFailure(message, self, action=self.action)
 
     def check_create_complete(self, cookie=None):
-        return self._check_status_complete(resource.Resource.CREATE)
+        return self._check_status_complete(self.CREATE)
 
-    def _check_status_complete(self, action, show_deleted=False,
+    def _check_status_complete(self, expected_action, cookie=None):
-                               cookie=None):
+
-        nested = self.nested(force_reload=True, show_deleted=show_deleted)
+        try:
-        if nested is None:
+            data = stack_object.Stack.get_status(self.context,
-            if action == resource.Resource.DELETE:
+                                                 self.resource_id)
+        except exception.NotFound:
+            if expected_action == self.DELETE:
                 return True
             # It's possible the engine handling the create hasn't persisted
             # the stack to the DB when we first start polling for state
             return False
 
-        if nested.action != action:
+        action, status, status_reason, updated_time = data
+
+        if action != expected_action:
             return False
 
         # Has the action really started?
@@ -368,25 +365,29 @@ class StackResource(resource.Resource):
         if cookie is not None:
             prev_state = cookie['previous']['state']
             prev_updated_at = cookie['previous']['updated_at']
-            if (prev_updated_at == nested.updated_time and
+            if (prev_updated_at == updated_time and
-                    prev_state == nested.state):
+                    prev_state == (action, status)):
                 return False
 
-        if nested.status == resource.Resource.IN_PROGRESS:
+        if status == self.IN_PROGRESS:
             return False
-        elif nested.status == resource.Resource.COMPLETE:
+        elif status == self.COMPLETE:
-            return True
+            ret = stack_lock.StackLock.get_engine_id(self.resource_id) is None
-        elif nested.status == resource.Resource.FAILED:
+            if ret:
-            raise exception.ResourceFailure(nested.status_reason, self,
+                # Reset nested, to indicate we changed status
+                self._nested = None
+            return ret
+        elif status == self.FAILED:
+            raise exception.ResourceFailure(status_reason, self,
                                             action=action)
         else:
             raise exception.ResourceUnknownStatus(
-                resource_status=nested.status,
+                resource_status=status,
-                status_reason=nested.status_reason,
+                status_reason=status_reason,
                 result=_('Stack unknown status'))
 
     def check_adopt_complete(self, cookie=None):
-        return self._check_status_complete(resource.Resource.ADOPT)
+        return self._check_status_complete(self.ADOPT)
 
     def update_with_template(self, child_template, user_params=None,
                              timeout_mins=None):
@@ -430,7 +431,7 @@ class StackResource(resource.Resource):
         return cookie
 
     def check_update_complete(self, cookie=None):
-        return self._check_status_complete(resource.Resource.UPDATE,
+        return self._check_status_complete(self.UPDATE,
                                            cookie=cookie)
 
     def delete_nested(self):
@@ -450,8 +451,7 @@ class StackResource(resource.Resource):
         return self.delete_nested()
 
     def check_delete_complete(self, cookie=None):
-        return self._check_status_complete(resource.Resource.DELETE,
+        return self._check_status_complete(self.DELETE)
-                                           show_deleted=True)
 
     def handle_suspend(self):
         stack = self.nested()
@@ -465,7 +465,7 @@ class StackResource(resource.Resource):
         self.rpc_client().stack_suspend(self.context, dict(stack_identity))
 
     def check_suspend_complete(self, cookie=None):
-        return self._check_status_complete(resource.Resource.SUSPEND)
+        return self._check_status_complete(self.SUSPEND)
 
     def handle_resume(self):
         stack = self.nested()
@@ -479,7 +479,7 @@ class StackResource(resource.Resource):
         self.rpc_client().stack_resume(self.context, dict(stack_identity))
 
     def check_resume_complete(self, cookie=None):
-        return self._check_status_complete(resource.Resource.RESUME)
+        return self._check_status_complete(self.RESUME)
 
     def handle_check(self):
         stack = self.nested()
@@ -494,7 +494,7 @@ class StackResource(resource.Resource):
         self.rpc_client().stack_check(self.context, dict(stack_identity))
 
     def check_check_complete(self, cookie=None):
-        return self._check_status_complete(resource.Resource.CHECK)
+        return self._check_status_complete(self.CHECK)
 
     def prepare_abandon(self):
         self.abandon_in_progress = True

heat/objects/stack.py

@@ -205,3 +205,8 @@ class Stack(
     @classmethod
     def encrypt_hidden_parameters(cls, tmpl):
         raw_template.RawTemplate.encrypt_hidden_parameters(tmpl)
+
+    @classmethod
+    def get_status(cls, context, stack_id):
+        """Return action and status for the given stack."""
+        return db_api.stack_get_status(context, stack_id)

heat/tests/db/test_sqlalchemy_api.py

@@ -424,6 +424,22 @@ class SqlAlchemyTest(common.HeatTestCase):
         st = db_api.stack_get(self.ctx, UUID1, show_deleted=True)
         self.assertEqual(UUID1, st.id)
 
+    def test_stack_get_status(self):
+        stack = self._setup_test_stack('stack', UUID1)[1]
+
+        st = db_api.stack_get_status(self.ctx, UUID1)
+        self.assertEqual(('CREATE', 'IN_PROGRESS', '', None), st)
+
+        stack.delete()
+        st = db_api.stack_get_status(self.ctx, UUID1)
+        self.assertEqual(
+            ('DELETE', 'COMPLETE',
+             'Stack DELETE completed successfully', None),
+            st)
+
+        self.assertRaises(exception.NotFound,
+                          db_api.stack_get_status, self.ctx, UUID2)
+
     def test_stack_get_show_deleted_context(self):
         stack = self._setup_test_stack('stack', UUID1)[1]
 

heat/tests/test_stack_resource.py

@@ -25,6 +25,8 @@ from heat.common import template_format
 from heat.engine.resources import stack_resource
 from heat.engine import stack as parser
 from heat.engine import template as templatem
+from heat.objects import stack as stack_object
+from heat.objects import stack_lock
 from heat.tests import common
 from heat.tests import generic_resource as generic_rsrc
 from heat.tests import utils
@@ -452,34 +454,17 @@ class StackResourceTest(StackResourceBaseTest):
         self.parent_resource.resource_id = 319
         self.m.StubOutWithMock(parser.Stack, 'load')
         parser.Stack.load(self.parent_resource.context,
-                          self.parent_resource.resource_id,
+                          self.parent_resource.resource_id).AndReturn('s')
-                          show_deleted=False,
-                          force_reload=False).AndReturn('s')
         self.m.ReplayAll()
         self.parent_resource.nested()
         self.m.VerifyAll()
 
-    def test_load_nested_force_reload(self):
-        self.parent_resource._nested = 'write-over-me'
-        self.parent_resource.resource_id = 319
-        self.m.StubOutWithMock(parser.Stack, 'load')
-        parser.Stack.load(self.parent_resource.context,
-                          self.parent_resource.resource_id,
-                          show_deleted=False,
-                          force_reload=True).AndReturn('ok')
-        self.m.ReplayAll()
-        self.parent_resource.nested(force_reload=True)
-        self.assertEqual('ok', self.parent_resource._nested)
-        self.m.VerifyAll()
-
     def test_load_nested_non_exist(self):
         self.parent_resource._nested = None
         self.parent_resource.resource_id = '90-8'
         self.m.StubOutWithMock(parser.Stack, 'load')
         parser.Stack.load(self.parent_resource.context,
-                          self.parent_resource.resource_id,
+                          self.parent_resource.resource_id).AndRaise(
-                          show_deleted=False,
-                          force_reload=False).AndRaise(
             exception.NotFound)
         self.m.ReplayAll()
 
@@ -490,32 +475,6 @@ class StackResourceTest(StackResourceBaseTest):
         self.parent_resource._nested = 'gotthis'
         self.assertEqual('gotthis', self.parent_resource.nested())
 
-    def test_load_nested_force_reload_ok(self):
-        self.parent_resource._nested = mock.MagicMock()
-        self.parent_resource.resource_id = '90-8'
-        self.m.StubOutWithMock(parser.Stack, 'load')
-        parser.Stack.load(self.parent_resource.context,
-                          self.parent_resource.resource_id,
-                          show_deleted=False,
-                          force_reload=True).AndReturn('s')
-        self.m.ReplayAll()
-        st = self.parent_resource.nested(force_reload=True)
-        self.assertEqual('s', st)
-        self.m.VerifyAll()
-
-    def test_load_nested_force_reload_none(self):
-        self.parent_resource._nested = mock.MagicMock()
-        self.parent_resource.resource_id = '90-8'
-        self.m.StubOutWithMock(parser.Stack, 'load')
-        parser.Stack.load(self.parent_resource.context,
-                          self.parent_resource.resource_id,
-                          show_deleted=False,
-                          force_reload=True).AndRaise(
-            exception.NotFound)
-        self.m.ReplayAll()
-        self.assertIsNone(self.parent_resource.nested(force_reload=True))
-        self.m.VerifyAll()
-
     def test_delete_nested_none_nested_stack(self):
         self.parent_resource._nested = None
         self.assertIsNone(self.parent_resource.delete_nested())
@@ -713,22 +672,18 @@ class StackResourceAttrTest(StackResourceBaseTest):
 
 class StackResourceCheckCompleteTest(StackResourceBaseTest):
     scenarios = [
-        ('create', dict(action='create', show_deleted=False)),
+        ('create', dict(action='create')),
-        ('update', dict(action='update', show_deleted=False)),
+        ('update', dict(action='update')),
-        ('suspend', dict(action='suspend', show_deleted=False)),
+        ('suspend', dict(action='suspend')),
-        ('resume', dict(action='resume', show_deleted=False)),
+        ('resume', dict(action='resume')),
-        ('delete', dict(action='delete', show_deleted=True)),
+        ('delete', dict(action='delete')),
     ]
 
     def setUp(self):
         super(StackResourceCheckCompleteTest, self).setUp()
-        self.nested = mock.MagicMock()
+        self.status = [self.action.upper(), None, None, None]
-        self.nested.name = 'nested-stack'
+        self.mock_status = self.patchobject(stack_object.Stack, 'get_status')
-        self.parent_resource.nested = mock.MagicMock(return_value=self.nested)
+        self.mock_status.return_value = self.status
-        self.parent_resource._nested = self.nested
-        setattr(self.nested, self.action.upper(), self.action.upper())
-        self.nested.action = self.action.upper()
-        self.nested.COMPLETE = 'COMPLETE'
 
     def test_state_ok(self):
         """Test case when check_create_complete should return True.
@@ -736,12 +691,17 @@ class StackResourceCheckCompleteTest(StackResourceBaseTest):
         check_create_complete should return True create task is
         done and the nested stack is in (<action>,COMPLETE) state.
         """
-        self.nested.status = 'COMPLETE'
+        self.mock_lock = self.patchobject(stack_lock.StackLock,
+                                          'get_engine_id')
+        self.mock_lock.return_value = None
+        self.status[1] = 'COMPLETE'
         complete = getattr(self.parent_resource,
                            'check_%s_complete' % self.action)
         self.assertIs(True, complete(None))
-        self.parent_resource.nested.assert_called_once_with(
+        self.mock_status.assert_called_once_with(
-            show_deleted=self.show_deleted, force_reload=True)
+            self.parent_resource.context, self.parent_resource.resource_id)
+        self.mock_lock.assert_called_once_with(
+            self.parent_resource.resource_id)
 
     def test_state_err(self):
         """Test case when check_create_complete should raise error.
@@ -749,20 +709,20 @@ class StackResourceCheckCompleteTest(StackResourceBaseTest):
         check_create_complete should raise error when create task is
         done but the nested stack is not in (<action>,COMPLETE) state
         """
-        self.nested.status = 'FAILED'
+        self.status[1] = 'FAILED'
         reason = ('Resource %s failed: ValueError: '
                   'resources.%s: broken on purpose' % (
                       self.action.upper(),
                       'child_res'))
         exp_path = 'resources.test.resources.child_res'
         exp = 'ValueError: %s: broken on purpose' % exp_path
-        self.nested.status_reason = reason
+        self.status[2] = reason
         complete = getattr(self.parent_resource,
                            'check_%s_complete' % self.action)
         exc = self.assertRaises(exception.ResourceFailure, complete, None)
         self.assertEqual(exp, six.text_type(exc))
-        self.parent_resource.nested.assert_called_once_with(
+        self.mock_status.assert_called_once_with(
-            show_deleted=self.show_deleted, force_reload=True)
+            self.parent_resource.context, self.parent_resource.resource_id)
 
     def test_state_unknown(self):
         """Test case when check_create_complete should raise error.
@@ -770,30 +730,29 @@ class StackResourceCheckCompleteTest(StackResourceBaseTest):
         check_create_complete should raise error when create task is
         done but the nested stack is not in (<action>,COMPLETE) state
         """
-        self.nested.status = 'WTF'
+        self.status[1] = 'WTF'
-        self.nested.status_reason = 'broken on purpose'
+        self.status[2] = 'broken on purpose'
         complete = getattr(self.parent_resource,
                            'check_%s_complete' % self.action)
         self.assertRaises(exception.ResourceUnknownStatus, complete, None)
-        self.parent_resource.nested.assert_called_once_with(
+        self.mock_status.assert_called_once_with(
-            show_deleted=self.show_deleted, force_reload=True)
+            self.parent_resource.context, self.parent_resource.resource_id)
 
     def test_in_progress(self):
-        self.nested.status = 'IN_PROGRESS'
+        self.status[1] = 'IN_PROGRESS'
         complete = getattr(self.parent_resource,
                            'check_%s_complete' % self.action)
         self.assertFalse(complete(None))
-        self.parent_resource.nested.assert_called_once_with(
+        self.mock_status.assert_called_once_with(
-            show_deleted=self.show_deleted, force_reload=True)
+            self.parent_resource.context, self.parent_resource.resource_id)
 
     def test_update_not_started(self):
         if self.action != 'update':
             # only valid for updates at the moment.
             return
 
-        self.nested.status = 'COMPLETE'
+        self.status[1] = 'COMPLETE'
-        self.nested.state = ('UPDATE', 'COMPLETE')
+        self.status[3] = 'test'
-        self.nested.updated_time = 'test'
         cookie = {'previous': {'state': ('UPDATE', 'COMPLETE'),
                                'updated_at': 'test'}}
 
@@ -801,16 +760,16 @@ class StackResourceCheckCompleteTest(StackResourceBaseTest):
                            'check_%s_complete' % self.action)
 
         self.assertFalse(complete(cookie=cookie))
-        self.parent_resource.nested.assert_called_once_with(
+        self.mock_status.assert_called_once_with(
-            show_deleted=self.show_deleted, force_reload=True)
+            self.parent_resource.context, self.parent_resource.resource_id)
 
     def test_wrong_action(self):
-        self.nested.action = 'COMPLETE'
+        self.status[0] = 'COMPLETE'
         complete = getattr(self.parent_resource,
                            'check_%s_complete' % self.action)
         self.assertFalse(complete(None))
-        self.parent_resource.nested.assert_called_once_with(
+        self.mock_status.assert_called_once_with(
-            show_deleted=self.show_deleted, force_reload=True)
+            self.parent_resource.context, self.parent_resource.resource_id)
 
 
 class WithTemplateTest(StackResourceBaseTest):