To reset metadata for resources when mark unhealthy

Some resources do not work if their metadata is in
a wrong state, .e.g the metadata 'scaling_in_progress'
of scaling group/policy might be always True if engine
restarts while scaling.
This patch adds an interface 'handle_metadata_reset' for
resource, then the plugins can override it if needed.
We reset the metadata while marking resource healthy.

Closes-Bug: #1651084
(cherry picked from commit 5b04acb4e25efbea58de998278e390fb64d7c575)

Conflicts:
	heat/tests/engine/service/test_stack_resources.py
Change-Id: Ibd6c18acf6f3f24cf9bf16a524127850968062bc

4 files changed, 63 insertions, 62 deletions

diff --git a/heat/engine/resource.py b/heat/engine/resource.py
index c5f2b08bf..0b7f36c1b 100644
--- a/heat/engine/resource.py
+++ b/heat/engine/resource.py
@@ -431,6 +431,16 @@ class Resource(object):
         db_res.update_metadata(metadata)
         self._rsrc_metadata = metadata
 
+    def handle_metadata_reset(self):
+        """Default implementation; should be overridden by resources.
+
+        Now we override this method to reset the metadata for scale-policy
+        and scale-group resources, because their metadata might hang in a
+        wrong state ('scaling_in_progress' is always True) if engine restarts
+        while scaling.
+        """
+        pass
+
     @classmethod
     def set_needed_by(cls, db_rsrc, needed_by, expected_engine_id=None):
         if db_rsrc:
diff --git a/heat/engine/service.py b/heat/engine/service.py
index 8b98d13dd..e69526faf 100644
--- a/heat/engine/service.py
+++ b/heat/engine/service.py
@@ -1878,6 +1878,7 @@ class EngineService(service.Service):
                 if mark_unhealthy:
                     rsrc.state_set(rsrc.CHECK, rsrc.FAILED, reason=reason)
                 elif rsrc.state == (rsrc.CHECK, rsrc.FAILED):
+                    rsrc.handle_metadata_reset()
                     rsrc.state_set(rsrc.CHECK, rsrc.COMPLETE, reason=reason)
 
         except exception.UpdateInProgress:
diff --git a/heat/scaling/cooldown.py b/heat/scaling/cooldown.py
index 6d1784145..315b543c3 100644
--- a/heat/scaling/cooldown.py
+++ b/heat/scaling/cooldown.py
@@ -69,3 +69,9 @@ class CooldownMixin(object):
             self.metadata_set(metadata)
         except exception.NotFound:
             pass
+
+    def handle_metadata_reset(self):
+        metadata = self.metadata_get()
+        if 'scaling_in_progress' in metadata:
+            metadata['scaling_in_progress'] = False
+            self.metadata_set(metadata)
diff --git a/heat/tests/engine/service/test_stack_resources.py b/heat/tests/engine/service/test_stack_resources.py
index c75295c8e..d9a87ddc3 100644
--- a/heat/tests/engine/service/test_stack_resources.py
+++ b/heat/tests/engine/service/test_stack_resources.py
@@ -20,6 +20,7 @@ from heat.common import identifier
 from heat.engine.clients.os import keystone
 from heat.engine import dependencies
 from heat.engine import resource as res
+from heat.engine.resources.aws.ec2 import instance as ins
 from heat.engine import service
 from heat.engine import stack
 from heat.engine import stack_lock
@@ -536,82 +537,77 @@ class StackResourcesServiceTest(common.HeatTestCase):
         self.assertIsInstance(stack_dependencies, dependencies.Dependencies)
         self.assertEqual(2, len(stack_dependencies.graph()))
 
+    def _test_mark_healthy_asserts(self, action='CHECK', status='FAILED',
+                                   reason='state changed', meta=None):
+        rs = self.eng.describe_stack_resource(
+            self.ctx, self.stack.identifier(),
+            'WebServer', with_attr=None)
+        self.assertIn('resource_action', rs)
+        self.assertIn('resource_status', rs)
+        self.assertIn('resource_status_reason', rs)
+
+        self.assertEqual(action, rs['resource_action'])
+        self.assertEqual(status, rs['resource_status'])
+        self.assertEqual(reason, rs['resource_status_reason'])
+        if meta is not None:
+            self.assertIn('metadata', rs)
+            self.assertEqual(meta, rs['metadata'])
+
     @tools.stack_context('service_mark_healthy_create_complete_test_stk')
     def test_mark_healthy_in_create_complete(self):
         self.eng.resource_mark_unhealthy(self.ctx, self.stack.identifier(),
                                          'WebServer', False,
                                          resource_status_reason='noop')
 
-        r = self.eng.describe_stack_resource(self.ctx, self.stack.identifier(),
-                                             'WebServer', with_attr=None)
-        self.assertIn('resource_action', r)
-        self.assertIn('resource_status', r)
-        self.assertIn('resource_status_reason', r)
-
-        self.assertEqual(r['resource_action'], 'CREATE')
-        self.assertEqual(r['resource_status'], 'COMPLETE')
-        self.assertEqual(r['resource_status_reason'], 'state changed')
+        self._test_mark_healthy_asserts(action='CREATE',
+                                        status='COMPLETE')
 
     @tools.stack_context('service_mark_unhealthy_create_complete_test_stk')
     def test_mark_unhealthy_in_create_complete(self):
+
+        reason = 'Some Reason'
         self.eng.resource_mark_unhealthy(self.ctx, self.stack.identifier(),
                                          'WebServer', True,
-                                         resource_status_reason='Some Reason')
+                                         resource_status_reason=reason)
 
-        r = self.eng.describe_stack_resource(self.ctx, self.stack.identifier(),
-                                             'WebServer', with_attr=None)
-
-        self.assertEqual(r['resource_action'], 'CHECK')
-        self.assertEqual(r['resource_status'], 'FAILED')
-        self.assertEqual(r['resource_status_reason'], 'Some Reason')
+        self._test_mark_healthy_asserts(reason=reason)
 
     @tools.stack_context('service_mark_healthy_check_failed_test_stk')
     def test_mark_healthy_check_failed(self):
+        reason = 'Some Reason'
         self.eng.resource_mark_unhealthy(self.ctx, self.stack.identifier(),
                                          'WebServer', True,
-                                         resource_status_reason='Some Reason')
+                                         resource_status_reason=reason)
+        self._test_mark_healthy_asserts(reason=reason)
 
-        r = self.eng.describe_stack_resource(self.ctx, self.stack.identifier(),
-                                             'WebServer', with_attr=None)
+        meta = {'for_test': True}
+
+        def override_metadata_reset(rsrc):
+            rsrc.metadata_set(meta)
 
-        self.assertEqual(r['resource_action'], 'CHECK')
-        self.assertEqual(r['resource_status'], 'FAILED')
-        self.assertEqual(r['resource_status_reason'], 'Some Reason')
+        ins.Instance.handle_metadata_reset = override_metadata_reset
 
+        reason = 'Good Reason'
         self.eng.resource_mark_unhealthy(self.ctx, self.stack.identifier(),
                                          'WebServer', False,
-                                         resource_status_reason='Good Reason')
-
-        r = self.eng.describe_stack_resource(self.ctx, self.stack.identifier(),
-                                             'WebServer', with_attr=None)
-
-        self.assertEqual(r['resource_action'], 'CHECK')
-        self.assertEqual(r['resource_status'], 'COMPLETE')
-        self.assertEqual(r['resource_status_reason'], 'Good Reason')
+                                         resource_status_reason=reason)
+        self._test_mark_healthy_asserts(status='COMPLETE',
+                                        reason=reason,
+                                        meta=meta)
 
     @tools.stack_context('service_mark_unhealthy_check_failed_test_stack')
     def test_mark_unhealthy_check_failed(self):
+        reason = 'Some Reason'
         self.eng.resource_mark_unhealthy(self.ctx, self.stack.identifier(),
                                          'WebServer', True,
-                                         resource_status_reason='Some Reason')
-
-        r = self.eng.describe_stack_resource(self.ctx, self.stack.identifier(),
-                                             'WebServer', with_attr=None)
-
-        self.assertEqual(r['resource_action'], 'CHECK')
-        self.assertEqual(r['resource_status'], 'FAILED')
-        self.assertEqual(r['resource_status_reason'], 'Some Reason')
+                                         resource_status_reason=reason)
+        self._test_mark_healthy_asserts(reason=reason)
 
+        new_reason = 'New Reason'
         self.eng.resource_mark_unhealthy(self.ctx, self.stack.identifier(),
                                          'WebServer', True,
-                                         resource_status_reason='New Reason')
-
-        r = self.eng.describe_stack_resource(self.ctx, self.stack.identifier(),
-                                             'WebServer', with_attr=None)
-
-        self.assertEqual(r['resource_action'], 'CHECK')
-        self.assertEqual(r['resource_status'], 'FAILED')
-        self.assertEqual(r['resource_status_reason'], 'New Reason')
+                                         resource_status_reason=new_reason)
+        self._test_mark_healthy_asserts(reason=new_reason)
 
     @tools.stack_context('service_mark_unhealthy_invalid_value_test_stk')
     def test_mark_unhealthy_invalid_value(self):
@@ -627,28 +623,16 @@ class StackResourcesServiceTest(common.HeatTestCase):
     def test_mark_unhealthy_none_reason(self):
         self.eng.resource_mark_unhealthy(self.ctx, self.stack.identifier(),
                                          'WebServer', True)
-
-        r = self.eng.describe_stack_resource(self.ctx, self.stack.identifier(),
-                                             'WebServer', with_attr=None)
-
-        self.assertEqual(r['resource_action'], 'CHECK')
-        self.assertEqual(r['resource_status'], 'FAILED')
-        self.assertEqual(r['resource_status_reason'],
-                         'state changed by resource_mark_unhealthy api')
+        default_reason = 'state changed by resource_mark_unhealthy api'
+        self._test_mark_healthy_asserts(reason=default_reason)
 
     @tools.stack_context('service_mark_unhealthy_empty_reason_test_stk')
     def test_mark_unhealthy_empty_reason(self):
         self.eng.resource_mark_unhealthy(self.ctx, self.stack.identifier(),
                                          'WebServer', True,
                                          resource_status_reason="")
-
-        r = self.eng.describe_stack_resource(self.ctx, self.stack.identifier(),
-                                             'WebServer', with_attr=None)
-
-        self.assertEqual(r['resource_action'], 'CHECK')
-        self.assertEqual(r['resource_status'], 'FAILED')
-        self.assertEqual(r['resource_status_reason'],
-                         'state changed by resource_mark_unhealthy api')
+        default_reason = 'state changed by resource_mark_unhealthy api'
+        self._test_mark_healthy_asserts(reason=default_reason)
 
     @tools.stack_context('service_mark_unhealthy_lock_no_converge_test_stk')
     def test_mark_unhealthy_lock_no_convergence(self):