diff --git a/senlin/policies/lb_policy.py b/senlin/policies/lb_policy.py index f470f3b8..42fae876 100644 --- a/senlin/policies/lb_policy.py +++ b/senlin/policies/lb_policy.py @@ -398,7 +398,7 @@ class LoadBalancingPolicy(base.Policy): candidates = None if deletion is None: if action.action == consts.NODE_DELETE: - candidates = [action.node.id] + candidates = [action.entity.id] count = 1 elif action.action == consts.CLUSTER_DEL_NODES: # Get candidates from action.input @@ -503,20 +503,40 @@ class LoadBalancingPolicy(base.Policy): def _get_post_candidates(self, action): # This method will parse action data passed from action layer candidates = [] - if action.action == consts.NODE_CREATE: - candidates = [action.node.id] - elif action.action == consts.NODE_RECOVER: - recovery = action.outputs.get('recovery', None) - if recovery is not None and 'action' in recovery: - action_name = recovery['action'] - if action_name.upper() == consts.RECOVER_RECREATE: - candidates = recovery.get('node', []) + if (action.action == consts.NODE_CREATE or + action.action == consts.NODE_RECOVER): + candidates = [action.entity.id] else: creation = action.data.get('creation', None) candidates = creation.get('nodes', []) if creation else [] return candidates + def _process_recovery(self, candidates, policy, driver, action): + # Process node recovery action + node = action.entity + data = node.data + lb_member = data.get('lb_member', None) + recovery = data.pop('recovery', None) + values = {} + + # lb_member is None, need to add to lb pool + if not lb_member: + values['data'] = data + no.Node.update(action.context, values) + return candidates + + # was a member of lb pool, check whether has been recreated + if recovery is not None and recovery == consts.RECOVER_RECREATE: + self._remove_member(candidates, policy, action, driver, + handle_err=False) + data.pop('lb_member', None) + values['data'] = data + no.Node.update(action.context, values) + return candidates + + return None + def pre_op(self, cluster_id, action): """Routine to be called before an action has been executed. @@ -573,8 +593,11 @@ class LoadBalancingPolicy(base.Policy): cp = cluster_policy.ClusterPolicy.load(action.context, cluster_id, self.id) if action.action == consts.NODE_RECOVER: - self._remove_member(candidates, cp, action, lb_driver, - handle_err=False) + candidates = self._process_recovery( + candidates, cp, lb_driver, action) + if not candidates: + return + # Add new nodes to lb pool failed_nodes = self._add_member(candidates, cp, action, lb_driver) if failed_nodes: diff --git a/senlin/tests/unit/policies/test_lb_policy.py b/senlin/tests/unit/policies/test_lb_policy.py index 61430ec2..90c6f12e 100644 --- a/senlin/tests/unit/policies/test_lb_policy.py +++ b/senlin/tests/unit/policies/test_lb_policy.py @@ -264,51 +264,26 @@ class TestLoadBalancingPolicy(base.SenlinTestCase): self.assertEqual((False, 'Failed in adding node into lb pool'), res) self.lb_driver.lb_delete.assert_called_once_with(**lb_data) - def test_get_post_candidates_node_create(self): - action = mock.Mock(action=consts.NODE_CREATE, - node=mock.Mock(id='NODE')) - policy = lb_policy.LoadBalancingPolicy('test-policy', self.spec) - - candidates = policy._get_post_candidates(action) - - self.assertEqual(['NODE'], candidates) - - def test_post_candidates_node_recover(self): - action = mock.Mock(action=consts.NODE_RECOVER, - outputs={ - 'recovery': { - 'action': consts.RECOVER_RECREATE, - 'node': ['NODE1_ID'] - } - }) + def test_post_candidates_node_recover_reboot(self): + node = mock.Mock(id='NODE1_ID') + action = mock.Mock(action=consts.NODE_RECOVER) + action.entity = node policy = lb_policy.LoadBalancingPolicy('test-policy', self.spec) candidates = policy._get_post_candidates(action) self.assertEqual(['NODE1_ID'], candidates) - def test_post_candidates_node_recover_reboot(self): - action = mock.Mock(action=consts.NODE_RECOVER, - outputs={ - 'recovery': { - 'action': consts.RECOVER_REBOOT, - 'node': ['NODE1_ID'] - } - }) - policy = lb_policy.LoadBalancingPolicy('test-policy', self.spec) - - candidates = policy._get_post_candidates(action) - - self.assertEqual([], candidates) - def test_post_candidates_node_recover_empty(self): + node = mock.Mock(id='NODE1_ID') action = mock.Mock(action=consts.NODE_RECOVER, outputs={}) + action.entity = node policy = lb_policy.LoadBalancingPolicy('test-policy', self.spec) candidates = policy._get_post_candidates(action) - self.assertEqual([], candidates) + self.assertEqual(['NODE1_ID'], candidates) def test_post_candidates_cluster_resize(self): action = mock.Mock(action=consts.CLUSTER_RESIZE, @@ -325,7 +300,7 @@ class TestLoadBalancingPolicy(base.SenlinTestCase): def test_get_delete_candidates_for_node_delete(self): action = mock.Mock(action=consts.NODE_DELETE, inputs={}, data={}, - node=mock.Mock(id='NODE_ID')) + entity=mock.Mock(id='NODE_ID')) policy = lb_policy.LoadBalancingPolicy('test-policy', self.spec) res = policy._get_delete_candidates('CLUSTERID', action) @@ -732,22 +707,18 @@ class TestLoadBalancingPolicyOperations(base.SenlinTestCase): self.assertFalse(m_remove.called) @mock.patch.object(lb_policy.LoadBalancingPolicy, '_add_member') - @mock.patch.object(lb_policy.LoadBalancingPolicy, '_remove_member') + @mock.patch.object(lb_policy.LoadBalancingPolicy, '_process_recovery') @mock.patch.object(lb_policy.LoadBalancingPolicy, '_get_post_candidates') - def test_post_op_node_recover(self, m_get, m_remove, m_add, + def test_post_op_node_recover(self, m_get, m_recovery, m_add, m_candidates, m_load): cid = 'CLUSTER_ID' - cluster = mock.Mock(user='user1', project='project1') + node = mock.Mock(user='user1', project='project1', id='NODE1') action = mock.Mock(context='action_context', action=consts.NODE_RECOVER, data={}, - outputs={ - 'recovery': { - 'action': consts.RECOVER_RECREATE, - 'node': ['NODE1'] - } - }) - action.entity = cluster + outputs={}) + action.entity = node + m_recovery.return_value = ['NODE1'] m_get.return_value = ['NODE1'] cp = mock.Mock() m_load.return_value = cp @@ -762,8 +733,8 @@ class TestLoadBalancingPolicyOperations(base.SenlinTestCase): m_get.assert_called_once_with(action) m_load.assert_called_once_with('action_context', cid, policy.id) m_add.assert_called_once_with(['NODE1'], cp, action, self.lb_driver) - m_remove.assert_called_once_with(['NODE1'], cp, action, self.lb_driver, - handle_err=False) + m_recovery.assert_called_once_with(['NODE1'], cp, self.lb_driver, + action) @mock.patch.object(lb_policy.LoadBalancingPolicy, '_add_member') @mock.patch.object(lb_policy.LoadBalancingPolicy, '_remove_member') @@ -989,3 +960,58 @@ class TestLoadBalancingPolicyOperations(base.SenlinTestCase): m_remove.assert_called_once_with( ['NODE1_ID'], mock.ANY, action, self.lb_driver) + + @mock.patch.object(no.Node, 'update') + def test__process_recovery_not_lb_member(self, m_update, m1, m2): + node = mock.Mock(id='NODE', data={}) + action = mock.Mock( + action=consts.NODE_RECOVER, + context='action_context') + action.entity = node + + cp = mock.Mock() + + policy = lb_policy.LoadBalancingPolicy('test-policy', self.spec) + res = policy._process_recovery(['NODE'], cp, self.lb_driver, action) + + self.assertEqual(['NODE'], res) + m_update.assert_called_once_with(action.context, {'data': {}}) + + @mock.patch.object(no.Node, 'update') + @mock.patch.object(lb_policy.LoadBalancingPolicy, '_remove_member') + def test__process_recovery_reboot(self, m_remove, m_update, m1, m2): + node = mock.Mock(id='NODE', data={'lb_member': 'mem_1'}) + action = mock.Mock( + action=consts.NODE_RECOVER, + context='action_context') + action.entity = node + + cp = mock.Mock() + + policy = lb_policy.LoadBalancingPolicy('test-policy', self.spec) + res = policy._process_recovery(['NODE'], cp, self.lb_driver, action) + + self.assertIsNone(res) + + self.assertFalse(m_remove.called) + self.assertFalse(m_update.called) + + @mock.patch.object(no.Node, 'update') + @mock.patch.object(lb_policy.LoadBalancingPolicy, '_remove_member') + def test__process_recovery_recreate(self, m_remove, m_update, m1, m2): + node = mock.Mock(id='NODE', data={'lb_member': 'mem_1', + 'recovery': 'RECREATE'}) + action = mock.Mock( + action=consts.NODE_RECOVER, + context='action_context') + action.entity = node + + cp = mock.Mock() + + policy = lb_policy.LoadBalancingPolicy('test-policy', self.spec) + res = policy._process_recovery(['NODE'], cp, self.lb_driver, action) + + self.assertEqual(['NODE'], res) + m_remove.assert_called_once_with(['NODE'], cp, action, self.lb_driver, + handle_err=False) + m_update.assert_called_once_with(action.context, {'data': {}})