From 717b4be536686d9e56270cf3bc7a433e7c919ae5 Mon Sep 17 00:00:00 2001 From: tengqm Date: Thu, 1 Sep 2016 09:50:43 -0400 Subject: [PATCH] Refactor region placement policy This patch moves the initialization of keystone driver up to the base class. Change-Id: I60558f14a0ca8d613fdd241cf1a3d670fd1dd88c --- senlin/policies/base.py | 14 +++++++ senlin/policies/region_placement.py | 19 +-------- senlin/tests/unit/policies/test_policy.py | 40 ++++++++++++++++--- .../unit/policies/test_region_placement.py | 40 ++++--------------- 4 files changed, 58 insertions(+), 55 deletions(-) diff --git a/senlin/policies/base.py b/senlin/policies/base.py index 318046dcc..ab20142df 100644 --- a/senlin/policies/base.py +++ b/senlin/policies/base.py @@ -112,6 +112,7 @@ class Policy(object): self.singleton = True self._novaclient = None + self._keystoneclient = None @classmethod def _from_object(cls, policy): @@ -248,6 +249,19 @@ class Policy(object): return params + def keystone(self, user, project): + """Construct keystone client based on object. + + :param user: The ID of the requesting user. + :param project: The ID of the requesting project. + :returns: A reference to the keystone client. + """ + if self._keystoneclient is not None: + return self._keystoneclient + params = self._build_conn_params(user, project) + self._keystoneclient = driver.SenlinDriver().identity(params) + return self._keystoneclient + def nova(self, user, project): """Construct nova client based on user and project. diff --git a/senlin/policies/region_placement.py b/senlin/policies/region_placement.py index 49b2c4a7c..3a10c4ea6 100644 --- a/senlin/policies/region_placement.py +++ b/senlin/policies/region_placement.py @@ -25,7 +25,6 @@ from senlin.common import exception as exc from senlin.common.i18n import _, _LE from senlin.common import scaleutils from senlin.common import schema -from senlin.drivers import base as driver_base from senlin.engine import cluster as cm from senlin.objects import cluster as co from senlin.policies import base @@ -89,7 +88,6 @@ class RegionPlacementPolicy(base.Policy): def __init__(self, name, spec, **kwargs): super(RegionPlacementPolicy, self).__init__(name, spec, **kwargs) - self._keystoneclient = None regions = {} for r in self.properties.get(self.REGIONS): regions[r[self.REGION_NAME]] = { @@ -98,26 +96,13 @@ class RegionPlacementPolicy(base.Policy): } self.regions = regions - def _keystone(self, user, project): - """Construct keystone client based on object. - - :param user: The ID of the requesting user. - :param project: The ID of the requesting project. - :returns: A reference to the keystone client. - """ - if self._keystoneclient is not None: - return self._keystoneclient - params = self._build_conn_params(user, project) - self._keystoneclient = driver_base.SenlinDriver().identity(params) - return self._keystoneclient - def validate(self, context, validate_props=False): super(RegionPlacementPolicy, self).validate(context, validate_props) if not validate_props: return True - kc = self._keystone(context.user, context.project) + kc = self.keystone(context.user, context.project) input_regions = sorted(self.regions.keys()) valid_regions = kc.validate_regions(input_regions) invalid_regions = sorted(set(input_regions) - set(valid_regions)) @@ -257,7 +242,7 @@ class RegionPlacementPolicy(base.Policy): count = -count cluster = cm.Cluster.load(action.context, cluster_id) - kc = self._keystone(cluster.user, cluster.project) + kc = self.keystone(cluster.user, cluster.project) regions_good = kc.validate_regions(self.regions.keys()) if len(regions_good) == 0: diff --git a/senlin/tests/unit/policies/test_policy.py b/senlin/tests/unit/policies/test_policy.py index 325817f83..54325b510 100644 --- a/senlin/tests/unit/policies/test_policy.py +++ b/senlin/tests/unit/policies/test_policy.py @@ -351,12 +351,12 @@ class TestPolicyBase(base.SenlinTestCase): res = policy._extract_policy_data(policy_data) self.assertIsNone(res) + @mock.patch.object(pb.Policy, '_build_conn_params') @mock.patch("senlin.drivers.base.SenlinDriver") - def test_nova(self, mock_driver): + def test_nova(self, mock_driver, mock_params): policy = self._create_policy('test-policy') - mock_params = mock.Mock() - mock_build = self.patchobject(policy, '_build_conn_params', - return_value=mock_params) + fake_params = mock.Mock() + mock_params.return_value = fake_params x_driver = mock.Mock() mock_driver.return_value = x_driver @@ -365,8 +365,8 @@ class TestPolicyBase(base.SenlinTestCase): x_nova = x_driver.compute.return_value self.assertEqual(x_nova, result) self.assertEqual(x_nova, policy._novaclient) - mock_build.assert_called_once_with('user1', 'project1') - x_driver.compute.assert_called_once_with(mock_params) + mock_params.assert_called_once_with('user1', 'project1') + x_driver.compute.assert_called_once_with(fake_params) def test_nova_already_initialized(self): policy = self._create_policy('test-policy') @@ -377,6 +377,34 @@ class TestPolicyBase(base.SenlinTestCase): self.assertEqual(x_nova, result) + @mock.patch.object(pb.Policy, '_build_conn_params') + @mock.patch('senlin.drivers.base.SenlinDriver') + def test_keystone(self, mock_sd, mock_params): + policy = self._create_policy('test-policy') + fake_params = mock.Mock() + mock_params.return_value = fake_params + kc = mock.Mock() + driver = mock.Mock() + driver.identity.return_value = kc + mock_sd.return_value = driver + + res = policy.keystone('user1', 'project1') + + self.assertEqual(kc, res) + self.assertEqual(kc, policy._keystoneclient) + mock_params.assert_called_once_with('user1', 'project1') + mock_sd.assert_called_once_with() + driver.identity.assert_called_once_with(fake_params) + + def test_keystone_already_initialized(self): + policy = self._create_policy('test-policy') + x_keystone = mock.Mock() + policy._keystoneclient = x_keystone + + result = policy.keystone('foo', 'bar') + + self.assertEqual(x_keystone, result) + def test_default_need_check(self): action = mock.Mock() action.action = consts.CLUSTER_SCALE_IN diff --git a/senlin/tests/unit/policies/test_region_placement.py b/senlin/tests/unit/policies/test_region_placement.py index 4c43209d9..30aa085a8 100644 --- a/senlin/tests/unit/policies/test_region_placement.py +++ b/senlin/tests/unit/policies/test_region_placement.py @@ -16,7 +16,6 @@ import six from senlin.common import consts from senlin.common import exception as exc from senlin.common import scaleutils as su -from senlin.drivers import base as driver_base from senlin.engine import cluster as cm from senlin.objects import cluster as co from senlin.policies import base as pb @@ -70,25 +69,6 @@ class TestRegionPlacementPolicy(base.SenlinTestCase): } self.assertEqual(expected, policy.regions) - @mock.patch.object(pb.Policy, '_build_conn_params') - @mock.patch.object(driver_base, 'SenlinDriver') - def test__keystone(self, mock_sd, mock_conn): - params = mock.Mock() - mock_conn.return_value = params - kc = mock.Mock() - driver = mock.Mock() - driver.identity.return_value = kc - mock_sd.return_value = driver - policy = rp.RegionPlacementPolicy('p1', self.spec) - - res = policy._keystone('user1', 'project1') - - self.assertEqual(kc, res) - self.assertEqual(kc, policy._keystoneclient) - mock_conn.assert_called_once_with('user1', 'project1') - mock_sd.assert_called_once_with() - driver.identity.assert_called_once_with(params) - @mock.patch.object(pb.Policy, 'validate') def test_validate_okay(self, mock_base_validate): policy = rp.RegionPlacementPolicy('test-policy', self.spec) @@ -305,16 +285,15 @@ class TestRegionPlacementPolicy(base.SenlinTestCase): res = policy._get_count('FOO', action) self.assertEqual(1, res) - @mock.patch.object(rp.RegionPlacementPolicy, '_keystone') @mock.patch.object(cm.Cluster, 'load') - def test_pre_op(self, mock_load, mock_keystone): + def test_pre_op(self, mock_load): # test pre_op method whether returns the correct action.data policy = rp.RegionPlacementPolicy('p1', self.spec) regions = policy.regions kc = mock.Mock() kc.validate_regions.return_value = regions.keys() - mock_keystone.return_value = kc + policy._keystoneclient = kc plan = {'R1': 1, 'R3': 2} self.patchobject(policy, '_create_plan', return_value=plan) @@ -350,16 +329,15 @@ class TestRegionPlacementPolicy(base.SenlinTestCase): policy._create_plan.assert_called_once_with( current_dist, regions, 3, True) - @mock.patch.object(rp.RegionPlacementPolicy, '_keystone') @mock.patch.object(cm.Cluster, 'load') - def test_pre_op_count_from_inputs(self, mock_load, mock_keystone): + def test_pre_op_count_from_inputs(self, mock_load): # test pre_op method whether returns the correct action.data policy = rp.RegionPlacementPolicy('p1', self.spec) regions = policy.regions kc = mock.Mock() kc.validate_regions.return_value = regions.keys() - mock_keystone.return_value = kc + policy._keystoneclient = kc cluster = mock.Mock() current_dist = {'R1': 0, 'R2': 0, 'R3': 0, 'R4': 0} @@ -384,14 +362,13 @@ class TestRegionPlacementPolicy(base.SenlinTestCase): self.assertEqual(1, dist['R1']) self.assertEqual(2, dist['R3']) - @mock.patch.object(rp.RegionPlacementPolicy, '_keystone') @mock.patch.object(cm.Cluster, 'load') - def test_pre_op_no_regions(self, mock_load, mock_keystone): + def test_pre_op_no_regions(self, mock_load): # test pre_op method whether returns the correct action.data policy = rp.RegionPlacementPolicy('p1', self.spec) kc = mock.Mock() kc.validate_regions.return_value = [] - mock_keystone.return_value = kc + policy._keystoneclient = kc action = mock.Mock() action.action = 'CLUSTER_SCALE_OUT' @@ -407,16 +384,15 @@ class TestRegionPlacementPolicy(base.SenlinTestCase): self.assertEqual('ERROR', action.data['status']) self.assertEqual('No region is found usable.', action.data['reason']) - @mock.patch.object(rp.RegionPlacementPolicy, '_keystone') @mock.patch.object(cm.Cluster, 'load') - def test_pre_op_no_feasible_plan(self, mock_load, mock_keystone): + def test_pre_op_no_feasible_plan(self, mock_load): # test pre_op method whether returns the correct action.data policy = rp.RegionPlacementPolicy('p1', self.spec) regions = policy.regions kc = mock.Mock() kc.validate_regions.return_value = regions.keys() - mock_keystone.return_value = kc + policy._keystoneclient = kc self.patchobject(policy, '_create_plan', return_value=None)