From 18aa65e1d20b0715240d75098739e599f831bd26 Mon Sep 17 00:00:00 2001 From: RUIJIE YUAN Date: Fri, 14 Jul 2017 15:49:21 +0800 Subject: [PATCH] revise lb policy to handle node_recover this patch revises lb policy to handle node_recover action: 1. node is not member of lb pool, will add it to lb pool 2. node is already member of lb pool, but the recovery action is not 'RECREATE', do nothing 3. node is already member of lb pool, and the recovery action is 'RECREATE', need to remove it from lb pool and then add it to it since the ip of the resource has already been changed. Change-Id: I69fe9ca8e646682333b65f8c90dc1cf245153622 --- senlin/policies/lb_policy.py | 45 +++++-- senlin/tests/unit/policies/test_lb_policy.py | 116 ++++++++++++------- 2 files changed, 105 insertions(+), 56 deletions(-) diff --git a/senlin/policies/lb_policy.py b/senlin/policies/lb_policy.py index f470f3b87..42fae876f 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 61430ec29..90c6f12e1 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': {}})