Merge "Enable health checks after failed operation"

senlin/engine/actions/cluster_action.py

@@ -1161,11 +1161,11 @@ class ClusterAction(base.Action):
         result, reason = method()
 
         # do post-action policy checking
-        if result == self.RES_OK:
-            self.policy_check(self.entity.id, 'AFTER')
-            if self.data['status'] != policy_mod.CHECK_OK:
-                reason = 'Policy check failure: %s' % self.data['reason']
-                return self.RES_ERROR, reason
+        self.inputs['action_result'] = result
+        self.policy_check(self.entity.id, 'AFTER')
+        if self.data['status'] != policy_mod.CHECK_OK:
+            reason = 'Policy check failure: %s' % self.data['reason']
+            return self.RES_ERROR, reason
 
         return result, reason
 

senlin/engine/actions/node_action.py

@@ -278,14 +278,11 @@ class NodeAction(base.Action):
                 reason = 'Failed in locking node'
             else:
                 res, reason = self._execute()
-                if (res == self.RES_OK and saved_cluster_id and
-                        self.cause == consts.CAUSE_RPC):
+                if saved_cluster_id and self.cause == consts.CAUSE_RPC:
                     self.policy_check(saved_cluster_id, 'AFTER')
                     if self.data['status'] != pb.CHECK_OK:
                         res = self.RES_ERROR
                         reason = 'Policy check: ' + self.data['reason']
-                    else:
-                        res = self.RES_OK
         finally:
             senlin_lock.node_lock_release(self.entity.id, self.id)
             if saved_cluster_id and self.cause == consts.CAUSE_RPC:

senlin/engine/health_manager.py

@@ -643,8 +643,12 @@ class HealthManager(service.Service):
             # stop timer
             timer.stop()
 
-            # tell threadgroup to remove timer
-            self.TG.timer_done(timer)
+            try:
+                # tell threadgroup to remove timer
+                self.TG.timer_done(timer)
+            except ValueError:
+                pass
+
             if entry['cluster_id'] in self.health_check_types:
                 self.health_check_types.pop(entry['cluster_id'])
             return

senlin/policies/lb_policy.py

@@ -637,6 +637,11 @@ class LoadBalancingPolicy(base.Policy):
         :returns: Nothing.
         """
 
+        # skip post op if action did not complete successfully
+        action_result = action.inputs.get('action_result', 'OK')
+        if action_result != 'OK':
+            return
+
         # TODO(Yanyanhu): Need special handling for cross-az scenario
         # which is supported by Neutron lbaas.
         candidates = self._get_post_candidates(action)

senlin/tests/unit/engine/actions/test_cluster_action.py

@@ -56,6 +56,26 @@ class ClusterActionTest(base.SenlinTestCase):
             mock.call('FAKE_CLUSTER', 'BEFORE'),
             mock.call('FAKE_CLUSTER', 'AFTER')])
 
+    @mock.patch.object(ab.Action, 'policy_check')
+    def test_execute_failed_action(self, mock_check, mock_load):
+        cluster = mock.Mock()
+        cluster.id = 'FAKE_CLUSTER'
+        mock_load.return_value = cluster
+        action = ca.ClusterAction(cluster.id, 'CLUSTER_FLY', self.ctx)
+        action.do_fly = mock.Mock(return_value=(action.RES_ERROR, 'Good!'))
+        action.data = {
+            'status': pb.CHECK_OK,
+            'reason': 'Policy checking passed'
+        }
+
+        res_code, res_msg = action._execute()
+
+        self.assertEqual(action.RES_ERROR, res_code)
+        self.assertEqual('Good!', res_msg)
+        mock_check.assert_has_calls([
+            mock.call('FAKE_CLUSTER', 'BEFORE'),
+            mock.call('FAKE_CLUSTER', 'AFTER')])
+
     @mock.patch.object(ab.Action, 'policy_check')
     def test_execute_failed_policy_check(self, mock_check, mock_load):
         cluster = mock.Mock()

senlin/tests/unit/engine/actions/test_node_action.py

@@ -805,7 +805,11 @@ class NodeActionTest(base.SenlinTestCase):
         mock_acquire_node.assert_called_once_with(self.ctx, 'NODE_ID',
                                                   'ACTION_ID', None, False)
         mock_release_node.assert_called_once_with('NODE_ID', 'ACTION_ID')
-        mock_check.assert_called_once_with('FAKE_CLUSTER', 'BEFORE')
+        check_calls = [
+            mock.call('FAKE_CLUSTER', 'BEFORE'),
+            mock.call('FAKE_CLUSTER', 'AFTER')
+        ]
+        mock_check.assert_has_calls(check_calls)
 
     @mock.patch.object(lock, 'cluster_lock_acquire')
     @mock.patch.object(lock, 'cluster_lock_release')

senlin/tests/unit/policies/test_lb_policy.py

@@ -808,7 +808,8 @@ class TestLoadBalancingPolicyOperations(base.SenlinTestCase):
         cid = 'CLUSTER_ID'
         cluster = mock.Mock(user='user1', project='project1')
         action = mock.Mock(data={}, context=ctx, action=consts.NODE_CREATE,
-                           node=mock.Mock(id='NODE_ID'))
+                           node=mock.Mock(id='NODE_ID'),
+                           inputs={'action_result': 'OK'})
         action.entity = cluster
         cp = mock.Mock()
         m_load.return_value = cp
@@ -840,7 +841,8 @@ class TestLoadBalancingPolicyOperations(base.SenlinTestCase):
                                'creation': {
                                    'nodes': ['NODE1_ID', 'NODE2_ID']
                                }
-                           })
+                           },
+                           inputs={'action_result': 'OK'})
         action.entity = cluster
         candidates = ['NODE1_ID', 'NODE2_ID']
         m_get.return_value = candidates
@@ -870,7 +872,8 @@ class TestLoadBalancingPolicyOperations(base.SenlinTestCase):
         action = mock.Mock(context='action_context',
                            action=consts.NODE_RECOVER,
                            data={},
-                           outputs={})
+                           outputs={},
+                           inputs={'action_result': 'OK'})
         action.entity = node
         m_recovery.return_value = ['NODE1']
         m_get.return_value = ['NODE1']
@@ -899,7 +902,8 @@ class TestLoadBalancingPolicyOperations(base.SenlinTestCase):
         cluster_id = 'CLUSTER_ID'
         action = mock.Mock(data={'creation': {'nodes': ['NODE1_ID']}},
                            context='action_context',
-                           action=consts.CLUSTER_RESIZE)
+                           action=consts.CLUSTER_RESIZE,
+                           inputs={'action_result': 'OK'})
 
         cp = mock.Mock()
         m_load.return_value = cp