From bb0be93cf17b7488bbdfabeae9f5da27e6265191 Mon Sep 17 00:00:00 2001 From: Jacob Estelle Date: Thu, 2 May 2019 11:16:10 -0700 Subject: [PATCH] Move node replacement candidates to dict in inputs Node replace iterates through all inputs treating key/value pairs as UUIDs to nodes. However, when the cluster has policy it injects a key called last_op into self.inputs in Action.policy_check, treating this as a pair of UUIDs causes node replace to fail. Therefore we move the nodes into inputs['candidates'] and only iterate through those. Change-Id: I14c6c591e3f6cec2c0600a5d9726c7720256d647 --- senlin/engine/actions/cluster_action.py | 7 ++++- senlin/engine/service.py | 2 +- .../unit/engine/actions/test_replace_nodes.py | 26 +++++++++++++------ .../unit/engine/service/test_clusters.py | 2 +- 4 files changed, 26 insertions(+), 11 deletions(-) diff --git a/senlin/engine/actions/cluster_action.py b/senlin/engine/actions/cluster_action.py index 4db827c3e..3a6d42ceb 100755 --- a/senlin/engine/actions/cluster_action.py +++ b/senlin/engine/actions/cluster_action.py @@ -684,7 +684,12 @@ class ClusterAction(base.Action): :returns: A tuple containing the result and the corresponding reason. """ - node_dict = self.inputs + node_dict = self.inputs.get('candidates') + if not node_dict: + return ( + self.RES_ERROR, + 'Candidates must be a non-empty dict.' + ' Instead got {}'.format(node_dict)) errors = [] original_nodes = [] diff --git a/senlin/engine/service.py b/senlin/engine/service.py index cb9b361f9..68103b13c 100755 --- a/senlin/engine/service.py +++ b/senlin/engine/service.py @@ -1179,7 +1179,7 @@ class EngineService(service.Service): 'name': 'cluster_replace_nodes_%s' % db_cluster.id[:8], 'cause': consts.CAUSE_RPC, 'status': action_mod.Action.READY, - 'inputs': nodes, + 'inputs': {'candidates': nodes}, } action_id = action_mod.Action.create(ctx, db_cluster.id, consts.CLUSTER_REPLACE_NODES, diff --git a/senlin/tests/unit/engine/actions/test_replace_nodes.py b/senlin/tests/unit/engine/actions/test_replace_nodes.py index a12fb71bb..4e185d943 100644 --- a/senlin/tests/unit/engine/actions/test_replace_nodes.py +++ b/senlin/tests/unit/engine/actions/test_replace_nodes.py @@ -45,7 +45,7 @@ class ClusterReplaceNodesTest(base.SenlinTestCase): action = ca.ClusterAction(cluster.id, 'CLUSTER_ACTION', self.ctx) action.id = 'CLUSTER_ACTION_ID' - action.inputs = {'O_NODE_1': 'R_NODE_1'} + action.inputs = {'candidates': {'O_NODE_1': 'R_NODE_1'}, 'foo': 'bar'} action.outputs = {} origin_node = mock.Mock(id='O_NODE_1', cluster_id='CLUSTER_ID', @@ -106,7 +106,7 @@ class ClusterReplaceNodesTest(base.SenlinTestCase): def test_do_replace_nodes_original_not_found(self, mock_get_node, mock_load): action = ca.ClusterAction('ID', 'CLUSTER_ACTION', self.ctx) - action.inputs = {'ORIGIN_NODE': 'REPLACE_NODE'} + action.inputs = {'candidates': {'ORIGIN_NODE': 'REPLACE_NODE'}} origin_node = None replace_node = mock.Mock(id='REPLACE_NODE', cluster_id='', ACTIVE='ACTIVE', status='ACTIVE') @@ -119,11 +119,21 @@ class ClusterReplaceNodesTest(base.SenlinTestCase): self.assertEqual('Original node ORIGIN_NODE not found.', res_msg) + def test_do_replace_nodes_empty_candidates(self, mock_load): + action = ca.ClusterAction('ID', 'CLUSTER_ACTION', self.ctx) + action.inputs = {'candidates': {}} + res_code, res_msg = action.do_replace_nodes() + + self.assertEqual(action.RES_ERROR, res_code) + self.assertEqual( + 'Candidates must be a non-empty dict. Instead got {}', + res_msg) + @mock.patch.object(no.Node, 'get') def test_do_replace_nodes_replacement_not_found(self, mock_get_node, mock_load): action = ca.ClusterAction('ID', 'CLUSTER_ACTION', self.ctx) - action.inputs = {'ORIGIN_NODE': 'REPLACE_NODE'} + action.inputs = {'candidates': {'ORIGIN_NODE': 'REPLACE_NODE'}} origin_node = mock.Mock(id='ORIGIN_NODE', cluster_id='CLUSTER_ID', ACTIVE='ACTIVE', status='ACTIVE') replace_node = None @@ -143,7 +153,7 @@ class ClusterReplaceNodesTest(base.SenlinTestCase): mock_load.return_value = cluster action = ca.ClusterAction(cluster.id, 'CLUSTER_ACTION', self.ctx) - action.inputs = {'ORIGIN_NODE': 'REPLACE_NODE'} + action.inputs = {'candidates': {'ORIGIN_NODE': 'REPLACE_NODE'}} origin_node = mock.Mock(id='ORIGIN_NODE', cluster_id='') mock_get_node.return_value = origin_node @@ -162,7 +172,7 @@ class ClusterReplaceNodesTest(base.SenlinTestCase): mock_load.return_value = cluster action = ca.ClusterAction(cluster.id, 'CLUSTER_ACTION', self.ctx) - action.inputs = {'ORIGIN_NODE': 'REPLACE_NODE'} + action.inputs = {'candidates': {'ORIGIN_NODE': 'REPLACE_NODE'}} replace_node = mock.Mock(id='REPLACE_NODE', cluster_id='FAKE_CLUSTER') @@ -184,7 +194,7 @@ class ClusterReplaceNodesTest(base.SenlinTestCase): action = ca.ClusterAction(cluster.id, 'CLUSTER_ACTION', self.ctx) action.id = 'CLUSTER_ACTION_ID' - action.inputs = {'ORIGIN_NODE': 'REPLACE_NODE'} + action.inputs = {'candidates': {'ORIGIN_NODE': 'REPLACE_NODE'}} action.outputs = {} origin_node = mock.Mock(id='ORIGIN_NODE', cluster_id='CLUSTER_ID', @@ -208,7 +218,7 @@ class ClusterReplaceNodesTest(base.SenlinTestCase): action = ca.ClusterAction(cluster.id, 'CLUSTER_ACTION', self.ctx) action.id = 'CLUSTER_ACTION_ID' - action.inputs = {'ORIGIN_NODE': 'REPLACE_NODE'} + action.inputs = {'candidates': {'ORIGIN_NODE': 'REPLACE_NODE'}} action.outputs = {} origin_node = mock.Mock(id='ORIGIN_NODE', cluster_id='CLUSTER_ID', @@ -238,7 +248,7 @@ class ClusterReplaceNodesTest(base.SenlinTestCase): action = ca.ClusterAction(cluster.id, 'CLUSTER_ACTION', self.ctx) action.id = 'CLUSTER_ACTION_ID' - action.inputs = {'O_NODE_1': 'R_NODE_1'} + action.inputs = {'candidates': {'O_NODE_1': 'R_NODE_1'}} action.outputs = {} origin_node = mock.Mock(id='O_NODE_1', cluster_id='CLUSTER_ID', diff --git a/senlin/tests/unit/engine/service/test_clusters.py b/senlin/tests/unit/engine/service/test_clusters.py index 05062f69f..c259090b0 100755 --- a/senlin/tests/unit/engine/service/test_clusters.py +++ b/senlin/tests/unit/engine/service/test_clusters.py @@ -1891,7 +1891,7 @@ class ClusterTest(base.SenlinTestCase): name='cluster_replace_nodes_CID', cause=consts.CAUSE_RPC, status=am.Action.READY, - inputs={'ORIGINAL': 'REPLACE'}) + inputs={'candidates': {'ORIGINAL': 'REPLACE'}}) notify.assert_called_once_with() @mock.patch.object(service.EngineService, '_validate_replace_nodes')