diff --git a/senlin/engine/actions/cluster_action.py b/senlin/engine/actions/cluster_action.py index fad956f8f..bf0b05b6c 100755 --- a/senlin/engine/actions/cluster_action.py +++ b/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 diff --git a/senlin/engine/actions/node_action.py b/senlin/engine/actions/node_action.py index cda587d89..7fb4dd7e4 100755 --- a/senlin/engine/actions/node_action.py +++ b/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: diff --git a/senlin/engine/health_manager.py b/senlin/engine/health_manager.py index 7a2b66c55..4537d2417 100644 --- a/senlin/engine/health_manager.py +++ b/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 diff --git a/senlin/policies/lb_policy.py b/senlin/policies/lb_policy.py index 5fab04dda..afdd0be18 100755 --- a/senlin/policies/lb_policy.py +++ b/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) diff --git a/senlin/tests/unit/engine/actions/test_cluster_action.py b/senlin/tests/unit/engine/actions/test_cluster_action.py index 2795e970c..589f855dd 100755 --- a/senlin/tests/unit/engine/actions/test_cluster_action.py +++ b/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() diff --git a/senlin/tests/unit/engine/actions/test_node_action.py b/senlin/tests/unit/engine/actions/test_node_action.py index 6dce516d1..f3f9f3536 100644 --- a/senlin/tests/unit/engine/actions/test_node_action.py +++ b/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') diff --git a/senlin/tests/unit/policies/test_lb_policy.py b/senlin/tests/unit/policies/test_lb_policy.py index d57d0b750..9c23ce661 100644 --- a/senlin/tests/unit/policies/test_lb_policy.py +++ b/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