diff --git a/senlin/engine/actions/base.py b/senlin/engine/actions/base.py index 82b41f024..8ddf9efc0 100755 --- a/senlin/engine/actions/base.py +++ b/senlin/engine/actions/base.py @@ -496,21 +496,6 @@ class Action(object): def is_resumed(self): return self._check_signal() == self.SIG_RESUME - def _check_result(self, name): - """Check policy status and generate event. - - :param name: Name of policy checked - :return: True if the policy checking can be continued, or False if the - policy checking should be aborted. - """ - reason = self.data['reason'] - if self.data['status'] == policy_mod.CHECK_OK: - return True - - self.data['reason'] = ("Failed policy '%(name)s': %(reason)s" - ) % {'name': name, 'reason': reason} - return False - def policy_check(self, cluster_id, target): """Check all policies attached to cluster and give result. @@ -526,9 +511,10 @@ class Action(object): bindings = cpo.ClusterPolicy.get_all(self.context, cluster_id, sort='priority', filters={'enabled': True}) + # default values - self.data['status'] = policy_mod.CHECK_OK - self.data['reason'] = 'Completed policy checking.' + self.data['status'] = policy_mod.CHECK_NONE + self.data['reason'] = '' for pb in bindings: policy = policy_mod.Policy.load(self.context, pb.policy_id) @@ -545,12 +531,22 @@ class Action(object): else: # target == 'AFTER' method = getattr(policy, 'post_op', None) + # call policy check function + # it will set result in data['status'] if method is not None: method(cluster_id, self) - res = self._check_result(policy.name) - if res is False: + # stop policy check is one of them fails + if self.data['status'] == policy_mod.CHECK_ERROR: + reason = self.data['reason'] + self.data['reason'] = ("Failed policy '%(name)s': %(reason)s" + ) % {'name': policy.name, + 'reason': reason} return + + self.data['status'] = policy_mod.CHECK_OK + self.data['reason'] = 'Completed policy checking.' + return @staticmethod diff --git a/senlin/policies/base.py b/senlin/policies/base.py index 5e95331dd..dd285f11b 100644 --- a/senlin/policies/base.py +++ b/senlin/policies/base.py @@ -25,9 +25,9 @@ from senlin.objects import credential as co from senlin.objects import policy as po CHECK_RESULTS = ( - CHECK_OK, CHECK_ERROR, + CHECK_OK, CHECK_ERROR, CHECK_NONE ) = ( - 'OK', 'ERROR', + 'OK', 'ERROR', 'NONE' ) diff --git a/senlin/tests/unit/engine/actions/test_action_base.py b/senlin/tests/unit/engine/actions/test_action_base.py index 9d6df4f1c..4267e3ea8 100755 --- a/senlin/tests/unit/engine/actions/test_action_base.py +++ b/senlin/tests/unit/engine/actions/test_action_base.py @@ -1024,29 +1024,6 @@ class ActionPolicyCheckTest(base.SenlinTestCase): self.assertEqual(0, mock_pre_op.call_count) self.assertEqual(0, mock_post_op.call_count) - def test_check_result_true(self): - cluster_id = CLUSTER_ID - action = ab.Action(cluster_id, 'OBJECT_ACTION', self.ctx) - action.data['status'] = policy_mod.CHECK_OK - action.data['reason'] = "Completed policy checking." - - res = action._check_result('FAKE_POLICY_NAME') - - self.assertTrue(res) - - def test_check_result_false(self): - cluster_id = CLUSTER_ID - action = ab.Action(cluster_id, 'OBJECT_ACTION', self.ctx) - action.data['status'] = policy_mod.CHECK_ERROR - reason = ("Policy '%s' cooldown is still in progress." % - 'FAKE_POLICY_2') - action.data['reason'] = reason - - res = action._check_result('FAKE_POLICY_NAME') - reason = ("Failed policy '%(name)s': %(reason)s" - ) % {'name': 'FAKE_POLICY_NAME', 'reason': reason} - self.assertFalse(res) - @mock.patch.object(cpo.ClusterPolicy, 'get_all') @mock.patch.object(policy_mod.Policy, 'load') def test_policy_check_pre_op(self, mock_load, mock_load_all): @@ -1112,8 +1089,7 @@ class ActionPolicyCheckTest(base.SenlinTestCase): @mock.patch.object(cpo.ClusterPolicy, 'get_all') @mock.patch.object(policy_mod.Policy, 'load') - @mock.patch.object(ab.Action, '_check_result') - def test_policy_check_abort_in_middle(self, mock_check, mock_load, + def test_policy_check_abort_in_middle(self, mock_load, mock_load_all): cluster_id = CLUSTER_ID # Note: both policies are mocked @@ -1124,6 +1100,13 @@ class ActionPolicyCheckTest(base.SenlinTestCase): TARGET=[('AFTER', 'OBJECT_ACTION')]) policy2.name = 'P2' action = ab.Action(cluster_id, 'OBJECT_ACTION', self.ctx) + action.data = mock.MagicMock() + + # mock action.data to return error for the first policy call + # (i.e. after policy1 post_op method has been called). + # this should stop the policy check and prevent the + # the policy2 post_op method from being called. + action.data.__getitem__.side_effect = [policy_mod.CHECK_ERROR, ''] # Note: policy binding is created but not stored pb1 = self._create_cp_binding(cluster_id, policy1.id) @@ -1131,7 +1114,6 @@ class ActionPolicyCheckTest(base.SenlinTestCase): mock_load_all.return_value = [pb1, pb2] # mock return value for two calls mock_load.side_effect = [policy1, policy2] - mock_check.side_effect = [False, True] res = action.policy_check(cluster_id, 'AFTER') diff --git a/senlin/tests/unit/engine/actions/test_node_action.py b/senlin/tests/unit/engine/actions/test_node_action.py index c841ef047..88406ba2e 100644 --- a/senlin/tests/unit/engine/actions/test_node_action.py +++ b/senlin/tests/unit/engine/actions/test_node_action.py @@ -637,6 +637,40 @@ class NodeActionTest(base.SenlinTestCase): lock.NODE_SCOPE) mock_check.assert_called_once_with('FAKE_CLUSTER', 'BEFORE') + @mock.patch.object(lock, 'cluster_lock_acquire') + @mock.patch.object(lock, 'cluster_lock_release') + @mock.patch.object(base_action.Action, 'policy_check') + def test_execute_policy_check_exception(self, mock_check, mock_release, + mock_acquire, mock_load): + node = mock.Mock() + node.id = 'NID' + node.cluster_id = 'FAKE_CLUSTER' + mock_load.return_value = node + + action = node_action.NodeAction('NODE_ID', 'NODE_FLY', self.ctx, + cause='RPC Request') + action.id = 'ACTION_ID' + action.data = { + 'status': policy_mod.CHECK_NONE, + 'reason': '' + } + mock_acquire.return_value = action.id + + mock_check.side_effect = Exception('error') + + res_code, res_msg = action.execute() + + reason = 'Policy check: ' + self.assertEqual(action.RES_ERROR, res_code) + self.assertEqual(reason, res_msg) + mock_load.assert_called_once_with(action.context, node_id='NODE_ID') + mock_acquire.assert_called_once_with(self.ctx, 'FAKE_CLUSTER', + 'ACTION_ID', None, + lock.NODE_SCOPE, False) + mock_release.assert_called_once_with('FAKE_CLUSTER', 'ACTION_ID', + lock.NODE_SCOPE) + mock_check.assert_called_once_with('FAKE_CLUSTER', 'BEFORE') + @mock.patch.object(lock, 'cluster_lock_acquire') @mock.patch.object(lock, 'cluster_lock_release') @mock.patch.object(lock, 'node_lock_acquire')