From b1b5460925bb0ce47f9368d916fd1c27636644f7 Mon Sep 17 00:00:00 2001 From: ruijie Date: Tue, 24 Apr 2018 01:50:59 +0800 Subject: [PATCH] separate '_delete_nodes' to different functionalities separate the '_delete_nodes' functionality so that will be easier to manage and improve it later. Change-Id: I7e762ee55d77459ad99186bde0916068ea657029 --- senlin/engine/actions/cluster_action.py | 138 ++++++++++-------- .../tests/unit/engine/actions/test_delete.py | 95 +++++++++++- 2 files changed, 168 insertions(+), 65 deletions(-) diff --git a/senlin/engine/actions/cluster_action.py b/senlin/engine/actions/cluster_action.py index d84e2019a..24e8338d6 100755 --- a/senlin/engine/actions/cluster_action.py +++ b/senlin/engine/actions/cluster_action.py @@ -305,39 +305,23 @@ class ClusterAction(base.Action): ao.Action.update(self.context, action_id, {'status': base.Action.READY}) - def _delete_nodes(self, node_ids): - action_name = consts.NODE_DELETE - - pd = self.data.get('deletion', None) - if pd is not None: - destroy = pd.get('destroy_after_deletion', True) - if destroy is False: - action_name = consts.NODE_LEAVE - - # get lifecycle hook properties if specified - lifecycle_hook = self.data.get('hooks') - lifecycle_hook_timeout = None - if lifecycle_hook: - lifecycle_hook_timeout = lifecycle_hook.get('timeout') - lifecycle_hook_type = lifecycle_hook.get('type', None) - lifecycle_hook_params = lifecycle_hook.get('params') - if lifecycle_hook_type == "zaqar": - lifecycle_hook_target = lifecycle_hook_params.get('queue') - else: - # lifecycle_hook_target = lifecycle_hook_params.get('url') - return self.RES_ERROR, ("Lifecycle hook type '%s' is not " - "implemented") % lifecycle_hook_type - + def _delete_nodes_with_hook(self, action_name, node_ids, lifecycle_hook): + lifecycle_hook_timeout = lifecycle_hook.get('timeout') + lifecycle_hook_type = lifecycle_hook.get('type', None) + lifecycle_hook_params = lifecycle_hook.get('params') + if lifecycle_hook_type == "zaqar": + lifecycle_hook_target = lifecycle_hook_params.get('queue') + else: + # lifecycle_hook_target = lifecycle_hook_params.get('url') + return self.RES_ERROR, ("Lifecycle hook type '%s' is not " + "implemented") % lifecycle_hook_type child = [] for node_id in node_ids: kwargs = { 'name': 'node_delete_%s' % node_id[:8], - 'cause': consts.CAUSE_DERIVED, + 'cause': consts.CAUSE_DERIVED_LCH, } - if lifecycle_hook: - kwargs['cause'] = consts.CAUSE_DERIVED_LCH - action_id = base.Action.create(self.context, node_id, action_name, **kwargs) child.append((action_id, node_id)) @@ -346,28 +330,20 @@ class ClusterAction(base.Action): dobj.Dependency.create(self.context, [aid for aid, nid in child], self.id) for action_id, node_id in child: - # Build dependency and make the new action ready or - # waiting for lifecycle completion if lifecycle hook properties - # are specified in deletion policy - - if not lifecycle_hook: + # wait lifecycle complete if node exists and is active + node = no.Node.get(self.context, node_id) + if not node: + LOG.warning('Node %s is not found. ' + 'Skipping wait for lifecycle completion.', + node_id) + status = base.Action.READY + elif node.status != consts.NS_ACTIVE: + LOG.warning('Node %s is not in ACTIVE status. ' + 'Skipping wait for lifecycle completion.', + node_id) status = base.Action.READY else: - # only go into wait lifecycle complete if node exists and - # is active - node = no.Node.get(self.context, node_id) - if not node: - LOG.warning('Node %s is not found. ' - 'Skipping wait for lifecycle completion.', - node_id) - status = base.Action.READY - elif node.status != consts.NS_ACTIVE: - LOG.warning('Node %s is not in ACTIVE status. ' - 'Skipping wait for lifecycle completion.', - node_id) - status = base.Action.READY - else: - status = base.Action.WAITING_LIFECYCLE_COMPLETION + status = base.Action.WAITING_LIFECYCLE_COMPLETION ao.Action.update(self.context, action_id, {'status': status}) @@ -385,29 +361,71 @@ class ClusterAction(base.Action): action_id, node_id, consts.LIFECYCLE_NODE_TERMINATION) - res = None - if lifecycle_hook: - dispatcher.start_action() - res, reason = self._wait_for_dependents(lifecycle_hook_timeout) + dispatcher.start_action() + res, reason = self._wait_for_dependents(lifecycle_hook_timeout) - if res == self.RES_LIFECYCLE_HOOK_TIMEOUT: - self._handle_lifecycle_timeout(child) + if res == self.RES_LIFECYCLE_HOOK_TIMEOUT: + self._handle_lifecycle_timeout(child) if res is None or res == self.RES_LIFECYCLE_HOOK_TIMEOUT: dispatcher.start_action() res, reason = self._wait_for_dependents() - if res == self.RES_OK: - self.outputs['nodes_removed'] = node_ids - for node_id in node_ids: - self.entity.remove_node(node_id) - else: - reason = 'Failed in deleting nodes.' - return res, reason return self.RES_OK, '' + def _delete_nodes_normally(self, action_name, node_ids): + child = [] + for node_id in node_ids: + kwargs = { + 'name': 'node_delete_%s' % node_id[:8], + 'cause': consts.CAUSE_DERIVED, + } + + action_id = base.Action.create(self.context, node_id, action_name, + **kwargs) + child.append((action_id, node_id)) + + if child: + dobj.Dependency.create(self.context, [aid for aid, nid in child], + self.id) + for action_id, node_id in child: + ao.Action.update(self.context, action_id, + {'status': base.Action.READY}) + + dispatcher.start_action() + res, reason = self._wait_for_dependents() + return res, reason + + return self.RES_OK, '' + + def _delete_nodes(self, node_ids): + action_name = consts.NODE_DELETE + + pd = self.data.get('deletion', None) + if pd is not None: + destroy = pd.get('destroy_after_deletion', True) + if destroy is False: + action_name = consts.NODE_LEAVE + + # get lifecycle hook properties if specified + lifecycle_hook = self.data.get('hooks') + if lifecycle_hook: + res, reason = self._delete_nodes_with_hook(action_name, node_ids, + lifecycle_hook) + else: + res, reason = self._delete_nodes_normally(action_name, node_ids) + + if res == self.RES_OK: + self.outputs['nodes_removed'] = node_ids + for node_id in node_ids: + self.entity.remove_node(node_id) + else: + reason = 'Failed in deleting nodes:%s' % reason + + return res, reason + @profiler.trace('ClusterAction.do_delete', hide_args=False) def do_delete(self): """Handler for the CLUSTER_DELETE action. diff --git a/senlin/tests/unit/engine/actions/test_delete.py b/senlin/tests/unit/engine/actions/test_delete.py index fc9c70f59..39844ce0b 100755 --- a/senlin/tests/unit/engine/actions/test_delete.py +++ b/senlin/tests/unit/engine/actions/test_delete.py @@ -362,8 +362,8 @@ class ClusterDeleteTest(base.SenlinTestCase): # assertions (other assertions are skipped) self.assertEqual(action.RES_ERROR, res_code) - self.assertEqual( - "Lifecycle hook type 'unknown_type' is not implemented", res_msg) + self.assertEqual("Failed in deleting nodes:Lifecycle hook type " + "'unknown_type' is not implemented", res_msg) @mock.patch.object(ao.Action, 'update') @mock.patch.object(ab.Action, 'create') @@ -392,8 +392,8 @@ class ClusterDeleteTest(base.SenlinTestCase): # assertions (other assertions are skipped) self.assertEqual(action.RES_ERROR, res_code) - self.assertEqual( - "Lifecycle hook type \'webhook\' is not implemented", res_msg) + self.assertEqual("Failed in deleting nodes:Lifecycle hook type " + "'webhook' is not implemented", res_msg) @mock.patch.object(ao.Action, 'update') @mock.patch.object(ab.Action, 'create') @@ -418,7 +418,7 @@ class ClusterDeleteTest(base.SenlinTestCase): # assertions (other assertions are skipped) self.assertEqual(action.RES_TIMEOUT, res_code) - self.assertEqual('Failed in deleting nodes.', res_msg) + self.assertEqual('Failed in deleting nodes:Timeout!', res_msg) self.assertEqual({}, action.data) @mock.patch.object(ao.Action, 'delete_by_target') @@ -673,3 +673,88 @@ class ClusterDeleteTest(base.SenlinTestCase): res = action.is_timeout(20) self.assertEqual(False, res) + + @mock.patch.object(ao.Action, 'update') + @mock.patch.object(ab.Action, 'create') + @mock.patch.object(dobj.Dependency, 'create') + @mock.patch.object(dispatcher, 'start_action') + @mock.patch.object(ca.ClusterAction, '_wait_for_dependents') + def test__delete_nodes_normally(self, mock_wait, mock_start, mock_dep, + mock_action, mock_update, mock_load): + # prepare mocks + cluster = mock.Mock(id='CLUSTER_ID', desired_capacity=100) + mock_load.return_value = cluster + + # cluster action is real + action = ca.ClusterAction(cluster.id, 'CLUSTER_ACTION', self.ctx) + action.id = 'CLUSTER_ACTION_ID' + action.inputs = {'destroy_after_deletion': False} + mock_wait.return_value = (action.RES_OK, 'All dependents completed') + mock_action.side_effect = ['NODE_ACTION_1', 'NODE_ACTION_2'] + + # do it + res_code, res_msg = action._delete_nodes_normally('NODE_REMOVE', + ['NODE_1', 'NODE_2']) + + # assertions + self.assertEqual(action.RES_OK, res_code) + self.assertEqual('All dependents completed', res_msg) + self.assertEqual(2, mock_action.call_count) + update_calls = [ + mock.call(action.context, 'NODE_ACTION_1', {'status': 'READY'}), + mock.call(action.context, 'NODE_ACTION_2', {'status': 'READY'}) + ] + mock_update.assert_has_calls(update_calls) + self.assertEqual(1, mock_dep.call_count) + mock_start.assert_called_once_with() + mock_wait.assert_called_once_with() + + @mock.patch.object(ao.Action, 'update') + @mock.patch.object(ab.Action, 'create') + @mock.patch.object(no.Node, 'get') + @mock.patch.object(dobj.Dependency, 'create') + @mock.patch.object(msg.Message, 'post_lifecycle_hook_message') + @mock.patch.object(dispatcher, 'start_action') + @mock.patch.object(ca.ClusterAction, '_wait_for_dependents') + def test__delete_nodes_with_hook(self, mock_wait, mock_start, mock_post, + mock_dep, mock_node_get, mock_action, + mock_update, mock_load): + # prepare mocks + cluster = mock.Mock(id='CLUSTER_ID', desired_capacity=100) + mock_load.return_value = cluster + # cluster action is real + action = ca.ClusterAction(cluster.id, 'CLUSTER_DELETE', self.ctx) + action.id = 'CLUSTER_ACTION_ID' + action.data = { + 'hooks': { + 'timeout': 10, + 'type': 'zaqar', + 'params': { + 'queue': 'myqueue' + } + } + } + mock_wait.return_value = (action.RES_OK, 'All dependents completed') + mock_action.return_value = 'NODE_ACTION_ID' + mock_node_get.return_value = mock.Mock(status=consts.NS_ACTIVE) + # do it + res_code, res_msg = action._delete_nodes_with_hook( + 'NODE_DELETE', ['NODE_ID'], action.data['hooks']) + + # assertions (other assertions are skipped) + self.assertEqual(action.RES_OK, res_code) + self.assertEqual('All dependents completed', res_msg) + self.assertEqual(1, mock_dep.call_count) + mock_action.assert_called_once_with( + action.context, 'NODE_ID', 'NODE_DELETE', + name='node_delete_NODE_ID', cause='Derived Action with ' + 'Lifecycle Hook') + update_calls = [ + mock.call(action.context, 'NODE_ACTION_ID', + {'status': 'WAITING_LIFECYCLE_COMPLETION'}), + ] + mock_update.assert_has_calls(update_calls) + mock_post.assert_called_once_with('NODE_ACTION_ID', 'NODE_ID', + consts.LIFECYCLE_NODE_TERMINATION) + mock_start.assert_called_once_with() + mock_wait.assert_called_once_with(action.data['hooks']['timeout'])