Merge "Reset stack status even when lock engine_id is None"

heat/engine/service.py

@@ -1826,30 +1826,48 @@ class EngineService(service.Service):
 
     def reset_stack_status(self):
         cnxt = context.get_admin_context()
-        filters = {'status': parser.Stack.IN_PROGRESS}
+        filters = {
+            'status': parser.Stack.IN_PROGRESS,
+            'convergence': False
+        }
         stacks = stack_object.Stack.get_all(cnxt,
                                             filters=filters,
                                             tenant_safe=False,
                                             show_nested=True) or []
         for s in stacks:
-            lock = stack_lock.StackLock(cnxt, s.id, self.engine_id)
+            stack_id = s.id
-            # If stacklock is released, means stack status may changed.
+            lock = stack_lock.StackLock(cnxt, stack_id, self.engine_id)
             engine_id = lock.get_engine_id()
-            if not engine_id:
-                continue
-            # Try to steal the lock and set status to failed.
             try:
-                lock.acquire(retry=False)
+                with lock.thread_lock(retry=False):
+
+                    # refetch stack and confirm it is still IN_PROGRESS
+                    s = stack_object.Stack.get_by_id(
+                        cnxt,
+                        stack_id,
+                        tenant_safe=False,
+                        eager_load=True)
+                    if s.status != parser.Stack.IN_PROGRESS:
+                        lock.release()
+                        continue
+
+                    stk = parser.Stack.load(cnxt, stack=s,
+                                            use_stored_context=True)
+                    LOG.info(_LI('Engine %(engine)s went down when stack '
+                                 '%(stack_id)s was in action %(action)s'),
+                             {'engine': engine_id, 'action': stk.action,
+                              'stack_id': stk.id})
+
+                    # Set stack and resources status to FAILED in sub thread
+                    self.thread_group_mgr.start_with_acquired_lock(
+                        stk,
+                        lock,
+                        self.set_stack_and_resource_to_failed,
+                        stk
+                    )
             except exception.ActionInProgress:
                 continue
-            stk = parser.Stack.load(cnxt, stack=s,
+            except Exception:
-                                    use_stored_context=True)
+                LOG.exception(_LE('Error while resetting stack: %s')
-            LOG.info(_LI('Engine %(engine)s went down when stack %(stack_id)s'
+                              % stack_id)
-                         ' was in action %(action)s'),
+                continue
-                     {'engine': engine_id, 'action': stk.action,
-                      'stack_id': stk.id})
-
-            # Set stack and resources status to FAILED in sub thread
-            self.thread_group_mgr.start_with_acquired_lock(
-                stk, lock, self.set_stack_and_resource_to_failed, stk
-            )

heat/engine/stack_lock.py

@@ -125,14 +125,17 @@ class StackLock(object):
                                      'stack': self.stack_id})
 
     @contextlib.contextmanager
-    def thread_lock(self):
+    def thread_lock(self, retry=True):
         """Acquire a lock and release it only if there is an exception.
 
         The release method still needs to be scheduled to be run at the
         end of the thread using the Thread.link method.
+
+        :param retry: When True, retry if lock was released while stealing.
+        :type retry: boolean
         """
         try:
-            self.acquire()
+            self.acquire(retry)
             yield
         except exception.ActionInProgress:
             raise

heat/tests/engine/service/test_stack_delete.py

@@ -119,7 +119,7 @@ class StackDeleteTest(common.HeatTestCase):
         mock_load.assert_called_with(self.ctx, stack=st)
         self.assertEqual(2, len(mock_load.mock_calls))
         mock_try.assert_called_once_with()
-        mock_acquire.assert_called_once_with()
+        mock_acquire.assert_called_once_with(True)
         mock_stop.assert_called_once_with(stack.id)
 
     @mock.patch.object(parser.Stack, 'load')
@@ -187,7 +187,7 @@ class StackDeleteTest(common.HeatTestCase):
         mock_alive.assert_called_once_with(self.ctx, OTHER_ENGINE)
         mock_call.assert_called_once_with(self.ctx, OTHER_ENGINE, "stop_stack",
                                           stack_identity=mock.ANY)
-        mock_acquire.assert_called_once_with()
+        mock_acquire.assert_called_once_with(True)
 
     @mock.patch.object(parser.Stack, 'load')
     @mock.patch.object(stack_lock.StackLock, 'try_acquire')
@@ -213,5 +213,5 @@ class StackDeleteTest(common.HeatTestCase):
 
         mock_load.assert_called_with(self.ctx, stack=st)
         mock_try.assert_called_once_with()
-        mock_acquire.assert_called_once_with()
+        mock_acquire.assert_called_once_with(True)
         mock_alive.assert_called_once_with(self.ctx, OTHER_ENGINE)

heat/tests/test_engine_service.py

@@ -1142,6 +1142,7 @@ class StackServiceTest(common.HeatTestCase):
     @mock.patch('heat.engine.service.ThreadGroupManager',
                 return_value=mock.Mock())
     @mock.patch.object(stack_object.Stack, 'get_all')
+    @mock.patch.object(stack_object.Stack, 'get_by_id')
     @mock.patch('heat.engine.stack_lock.StackLock',
                 return_value=mock.Mock())
     @mock.patch.object(parser.Stack, 'load')
@@ -1151,6 +1152,7 @@ class StackServiceTest(common.HeatTestCase):
             mock_admin_context,
             mock_stack_load,
             mock_stacklock,
+            mock_get_by_id,
             mock_get_all,
             mock_thread):
         mock_admin_context.return_value = self.ctx
@@ -1159,7 +1161,19 @@ class StackServiceTest(common.HeatTestCase):
         db_stack.id = 'foo'
         db_stack.status = 'IN_PROGRESS'
         db_stack.status_reason = None
-        mock_get_all.return_value = [db_stack]
+
+        unlocked_stack = mock.MagicMock()
+        unlocked_stack.id = 'bar'
+        unlocked_stack.status = 'IN_PROGRESS'
+        unlocked_stack.status_reason = None
+
+        unlocked_stack_failed = mock.MagicMock()
+        unlocked_stack_failed.id = 'bar'
+        unlocked_stack_failed.status = 'FAILED'
+        unlocked_stack_failed.status_reason = 'because'
+
+        mock_get_all.return_value = [db_stack, unlocked_stack]
+        mock_get_by_id.side_effect = [db_stack, unlocked_stack_failed]
 
         fake_stack = mock.MagicMock()
         fake_stack.action = 'CREATE'
@@ -1168,26 +1182,36 @@ class StackServiceTest(common.HeatTestCase):
 
         mock_stack_load.return_value = fake_stack
 
-        fake_lock = mock.MagicMock()
+        lock1 = mock.MagicMock()
-        fake_lock.get_engine_id.return_value = 'old-engine'
+        lock1.get_engine_id.return_value = 'old-engine'
-        fake_lock.acquire.return_value = None
+        lock1.acquire.return_value = None
-        mock_stacklock.return_value = fake_lock
+        lock2 = mock.MagicMock()
+        lock2.acquire.return_value = None
+        mock_stacklock.side_effect = [lock1, lock2]
 
         self.eng.thread_group_mgr = mock_thread
 
         self.eng.reset_stack_status()
 
         mock_admin_context.assert_called_once_with()
-        filters = {'status': parser.Stack.IN_PROGRESS}
+        filters = {
+            'status': parser.Stack.IN_PROGRESS,
+            'convergence': False
+        }
         mock_get_all.assert_called_once_with(self.ctx,
                                              filters=filters,
                                              tenant_safe=False,
                                              show_nested=True)
+        mock_get_by_id.assert_has_calls([
+            mock.call(self.ctx, 'foo', tenant_safe=False, eager_load=True),
+            mock.call(self.ctx, 'bar', tenant_safe=False, eager_load=True),
+        ])
         mock_stack_load.assert_called_once_with(self.ctx,
                                                 stack=db_stack,
                                                 use_stored_context=True)
+        self.assertTrue(lock2.release.called)
         mock_thread.start_with_acquired_lock.assert_called_once_with(
-            fake_stack, fake_lock,
+            fake_stack, lock1,
             self.eng.set_stack_and_resource_to_failed, fake_stack
         )