openstack
/
senlin
Code Issues Proposed changes

Refactor region placement policy 

This patch moves the initialization of keystone driver up to the base
class.

Change-Id: I60558f14a0ca8d613fdd241cf1a3d670fd1dd88c
This commit is contained in:
tengqm 2016-09-01 09:50:43 -04:00
parent 48494c14a3
commit 717b4be536
4 changed files with 58 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
senlin/policies/base.py
View File

@ -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.

19
senlin/policies/region_placement.py
View File

@ -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:

40
senlin/tests/unit/policies/test_policy.py
View File

@ -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

40
senlin/tests/unit/policies/test_region_placement.py
View File

@ -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)