openstack
/
nova
Code Issues Proposed changes

Merge "Add policy to InstanceGroup object"

This commit is contained in:
Zuul 2018-07-12 14:34:04 +00:00 committed by Gerrit Code Review
parent 39b05ee9e3 8fa70e5dfc
commit da16690f4d
8 changed files with 143 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
nova/api/openstack/compute/server_groups.py
View File

@ -166,7 +166,8 @@ class ServerGroupController(wsgi.Controller):
sg.user_id = context.user_id
try:
sg.name = vals.get('name')
sg.policies = vals.get('policies')
policies = vals.get('policies')
sg.policy = policies[0]
sg.create()
except ValueError as e:
raise exc.HTTPBadRequest(explanation=e)

4
nova/db/sqlalchemy/api_models.py
View File

@ -456,10 +456,6 @@ class InstanceGroup(API_BASE):
"group_name": self.name})
return self._policies[0] if self._policies else None
@property
def policies(self):
return [p.policy for p in self._policies]
@property
def members(self):
return [m.instance_uuid for m in self._members]

79
nova/objects/instance_group.py
View File

@ -15,6 +15,7 @@
import copy
from oslo_db import exception as db_exc
from oslo_serialization import jsonutils
from oslo_utils import uuidutils
from oslo_utils import versionutils
from sqlalchemy.orm import contains_eager
@ -110,7 +111,8 @@ class InstanceGroup(base.NovaPersistentObject, base.NovaObject,
# Version 1.8: Add count_members_by_user()
# Version 1.9: Add get_by_instance_uuid()
# Version 1.10: Add hosts field
VERSION = '1.10'
# Version 1.11: Add policy and deprecate policies, add _rules
VERSION = '1.11'
fields = {
'id': fields.IntegerField(),
@ -124,10 +126,39 @@ class InstanceGroup(base.NovaPersistentObject, base.NovaObject,
'policies': fields.ListOfStringsField(nullable=True, read_only=True),
'members': fields.ListOfStringsField(nullable=True),
'hosts': fields.ListOfStringsField(nullable=True),
'policy': fields.StringField(nullable=True),
# NOTE(danms): Use rules not _rules for general access
'_rules': fields.DictOfStringsField(),
}
def __init__(self, *args, **kwargs):
if 'rules' in kwargs:
kwargs['_rules'] = kwargs.pop('rules')
super(InstanceGroup, self).__init__(*args, **kwargs)
@property
def rules(self):
if '_rules' not in self:
return {}
# NOTE(danms): Coerce our rules into a typed dict for convenience
rules = {}
if 'max_server_per_host' in self._rules:
rules['max_server_per_host'] = \
int(self._rules['max_server_per_host'])
return rules
def obj_make_compatible(self, primitive, target_version):
target_version = versionutils.convert_version_to_tuple(target_version)
if target_version < (1, 11):
# NOTE(yikun): Before 1.11, we had a policies property which is
# the list of policy name, even though it was a list, there was
# ever only one entry in the list.
policy = primitive.pop('policy', None)
if policy:
primitive['policies'] = [policy]
else:
primitive['policies'] = []
primitive.pop('rules', None)
if target_version < (1, 7):
# NOTE(danms): Before 1.7, we had an always-empty
# metadetails property
@ -151,8 +182,28 @@ class InstanceGroup(base.NovaPersistentObject, base.NovaObject,
# TODO(mriedem): Remove this when NovaPersistentObject is removed.
ignore = {'deleted': False,
'deleted_at': None}
if field in ignore and not hasattr(db_inst, field):
if '_rules' == field:
db_policy = db_inst['policy']
instance_group._rules = (
jsonutils.loads(db_policy['rules'])
if db_policy and db_policy['rules']
else {})
elif field in ignore and not hasattr(db_inst, field):
instance_group[field] = ignore[field]
elif 'policies' == field:
continue
# NOTE(yikun): The obj.policies is deprecated and marked as
# read_only in version 1.11, and there is no "policies" property
# in InstanceGroup model anymore, so we just skip to set
# "policies" and then load the "policies" when "policy" is set.
elif 'policy' == field:
db_policy = db_inst['policy']
if db_policy:
instance_group.policy = db_policy['policy']
instance_group.policies = [instance_group.policy]
else:
instance_group.policy = None
instance_group.policies = []
else:
instance_group[field] = db_inst[field]
@ -214,7 +265,8 @@ class InstanceGroup(base.NovaPersistentObject, base.NovaObject,
@staticmethod
@db_api.api_context_manager.writer
def _create_in_db(context, values, policies=None, members=None):
def _create_in_db(context, values, policies=None, members=None,
policy=None, rules=None):
try:
group = api_models.InstanceGroup()
group.update(values)
@ -223,13 +275,22 @@ class InstanceGroup(base.NovaPersistentObject, base.NovaObject,
raise exception.InstanceGroupIdExists(group_uuid=values['uuid'])
if policies:
policy = api_models.InstanceGroupPolicy(
db_policy = api_models.InstanceGroupPolicy(
group_id=group['id'], policy=policies[0], rules=None)
group._policies = [policy]
group.save(context.session)
group._policies = [db_policy]
group.rules = None
elif policy:
db_rules = jsonutils.dumps(rules or {})
db_policy = api_models.InstanceGroupPolicy(
group_id=group['id'], policy=policy,
rules=db_rules)
group._policies = [db_policy]
else:
group._policies = []
if group._policies:
group.save(context.session)
if members:
group._members = _instance_group_members_add(context, group,
members)
@ -358,6 +419,8 @@ class InstanceGroup(base.NovaPersistentObject, base.NovaObject,
payload = dict(updates)
updates.pop('id', None)
policies = updates.pop('policies', None)
policy = updates.pop('policy', None)
rules = updates.pop('_rules', None)
members = updates.pop('members', None)
if 'uuid' not in updates:
@ -366,7 +429,9 @@ class InstanceGroup(base.NovaPersistentObject, base.NovaObject,
db_group = self._create_in_db(self._context, updates,
policies=policies,
members=members)
members=members,
policy=policy,
rules=rules)
self._from_db_object(self._context, self, db_group)
payload['server_group_id'] = self.uuid
compute_utils.notify_about_server_group_update(self._context,

2
nova/objects/request_spec.py
View File

@ -210,7 +210,7 @@ class RequestSpec(base.NovaObject):
policies = list(filter_properties.get('group_policies'))
hosts = list(filter_properties.get('group_hosts'))
members = list(filter_properties.get('group_members'))
self.instance_group = objects.InstanceGroup(policies=policies,
self.instance_group = objects.InstanceGroup(policy=policies[0],
hosts=hosts,
members=members)
# hosts has to be not part of the updates for saving the object

18
nova/tests/functional/db/test_instance_group.py
View File

@ -32,7 +32,8 @@ class InstanceGroupObjectTestCase(test.TestCase):
user_id=self.context.user_id,
project_id=self.context.project_id,
name='foogroup',
policies=['foo1'],
policy='anti-affinity',
rules={'max_server_per_host': 1},
members=['memberfoo'])
group.update(values)
group.create()
@ -45,9 +46,11 @@ class InstanceGroupObjectTestCase(test.TestCase):
self.assertIsInstance(db_group.policy, api_models.InstanceGroupPolicy)
self.assertEqual(create_group.policies[0], db_group.policy.policy)
self.assertEqual(create_group.id, db_group.policy.group_id)
self.assertIsNone(db_group.policy.rules)
ovo_fixture.compare_obj(self, create_group, db_group,
allow_missing=('deleted', 'deleted_at'))
ovo_fixture.compare_obj(
self, create_group, db_group,
comparators={'policy': lambda a, b: b == a.policy},
allow_missing=('deleted', 'deleted_at', 'policies', '_rules'))
self.assertEqual({'max_server_per_host': 1}, create_group.rules)
def test_destroy(self):
create_group = self._api_group()
@ -64,8 +67,11 @@ class InstanceGroupObjectTestCase(test.TestCase):
create_group.save()
db_group = create_group._get_from_db_by_uuid(self.context,
create_group.uuid)
ovo_fixture.compare_obj(self, create_group, db_group,
allow_missing=('deleted', 'deleted_at'))
ovo_fixture.compare_obj(
self, create_group, db_group,
comparators={'policy': lambda a, b: b == a.policy},
allow_missing=('deleted', 'deleted_at', 'policies', '_rules'))
self.assertEqual({'max_server_per_host': 1}, create_group.rules)
def test_add_members(self):
create_group = self._api_group()

64
nova/tests/unit/objects/test_instance_group.py
View File

@ -15,6 +15,7 @@
import copy
import mock
from oslo_serialization import jsonutils
from oslo_utils import timeutils
from oslo_versionedobjects import exception as ovo_exc
@ -30,6 +31,10 @@ _TS_NOW = timeutils.utcnow(with_timezone=True)
# in process we lose microsecond resolution.
_TS_NOW = _TS_NOW.replace(microsecond=0)
_DB_UUID = uuids.fake
_INST_GROUP_POLICY_DB = {
'policy': 'policy1',
'rules': jsonutils.dumps({'max_server_per_host': '2'}),
}
_INST_GROUP_DB = {
'id': 1,
'uuid': _DB_UUID,
@ -37,11 +42,18 @@ _INST_GROUP_DB = {
'project_id': 'fake_project',
'name': 'fake_name',
# a group can only have 1 policy associated with it
'policies': ['policy1'],
'policy': _INST_GROUP_POLICY_DB,
'_policies': [_INST_GROUP_POLICY_DB],
'members': ['instance_id1', 'instance_id2'],
'created_at': _TS_NOW,
'updated_at': _TS_NOW
'updated_at': _TS_NOW,
}
_INST_GROUP_OBJ_VALS = dict(
{k: v for k, v in
_INST_GROUP_DB.items()
if k not in ('policy', '_policies')},
policy=_INST_GROUP_POLICY_DB['policy'],
rules=jsonutils.loads(_INST_GROUP_POLICY_DB['rules']))
class _TestInstanceGroupObject(object):
@ -53,11 +65,21 @@ class _TestInstanceGroupObject(object):
_DB_UUID)
mock_api_get.assert_called_once_with(self.context, _DB_UUID)
self.assertEqual(_INST_GROUP_DB['members'], obj.members)
self.assertEqual(_INST_GROUP_DB['policies'], obj.policies)
self.assertEqual([_INST_GROUP_POLICY_DB['policy']], obj.policies)
self.assertEqual(_DB_UUID, obj.uuid)
self.assertEqual(_INST_GROUP_DB['project_id'], obj.project_id)
self.assertEqual(_INST_GROUP_DB['user_id'], obj.user_id)
self.assertEqual(_INST_GROUP_DB['name'], obj.name)
self.assertEqual(_INST_GROUP_POLICY_DB['policy'], obj.policy)
self.assertEqual({'max_server_per_host': 2}, obj.rules)
def test_rules_helper(self):
obj = objects.InstanceGroup()
self.assertEqual({}, obj.rules)
self.assertNotIn('_rules', obj)
obj._rules = {}
self.assertEqual({}, obj.rules)
self.assertIn('_rules', obj)
@mock.patch('nova.objects.InstanceGroup._get_from_db_by_instance',
return_value=_INST_GROUP_DB)
@ -87,9 +109,10 @@ class _TestInstanceGroupObject(object):
changed_group['name'] = 'new_name'
db_group = copy.deepcopy(_INST_GROUP_DB)
mock_db_get.return_value = db_group
obj = objects.InstanceGroup(self.context, **_INST_GROUP_DB)
obj = objects.InstanceGroup(self.context, **_INST_GROUP_OBJ_VALS)
self.assertEqual(obj.name, 'fake_name')
obj.obj_reset_changes()
self.assertEqual(set([]), obj.obj_what_changed())
obj.name = 'new_name'
obj.members = ['instance_id1'] # Remove member 2
obj.save()
@ -105,7 +128,7 @@ class _TestInstanceGroupObject(object):
@mock.patch('nova.objects.InstanceGroup._get_from_db_by_uuid')
def test_save_without_hosts(self, mock_db_get, mock_notify):
mock_db_get.return_value = _INST_GROUP_DB
obj = objects.InstanceGroup(self.context, **_INST_GROUP_DB)
obj = objects.InstanceGroup(self.context, **_INST_GROUP_OBJ_VALS)
obj.obj_reset_changes()
obj.hosts = ['fake-host1']
self.assertRaises(exception.InstanceGroupSaveException,
@ -139,7 +162,7 @@ class _TestInstanceGroupObject(object):
obj.user_id = _INST_GROUP_DB['user_id']
obj.project_id = _INST_GROUP_DB['project_id']
obj.members = _INST_GROUP_DB['members']
obj.policies = _INST_GROUP_DB['policies']
obj.policies = [_INST_GROUP_DB['policy']['policy']]
obj.updated_at = _TS_NOW
obj.created_at = _TS_NOW
obj.create()
@ -153,7 +176,9 @@ class _TestInstanceGroupObject(object):
'updated_at': _TS_NOW,
},
members=_INST_GROUP_DB['members'],
policies=_INST_GROUP_DB['policies'])
policies=[_INST_GROUP_DB['policy']['policy']],
policy=None,
rules=None)
mock_notify.assert_called_once_with(
self.context, "create",
{'uuid': _DB_UUID,
@ -163,7 +188,7 @@ class _TestInstanceGroupObject(object):
'created_at': _TS_NOW,
'updated_at': _TS_NOW,
'members': _INST_GROUP_DB['members'],
'policies': _INST_GROUP_DB['policies'],
'policies': [_INST_GROUP_DB['policy']['policy']],
'server_group_id': _DB_UUID})
def _group_matcher(group):
@ -175,7 +200,7 @@ class _TestInstanceGroupObject(object):
group.created_at == _TS_NOW and
group.updated_at == _TS_NOW and
group.members == _INST_GROUP_DB['members'] and
group.policies == _INST_GROUP_DB['policies'] and
group.policies == [_INST_GROUP_DB['policy']['policy']] and
group.id == 1)
group_matcher = test_utils.CustomMockCallMatcher(_group_matcher)
@ -271,12 +296,31 @@ class _TestInstanceGroupObject(object):
filters=expected_filters)
def test_obj_make_compatible(self):
obj = objects.InstanceGroup(self.context, **_INST_GROUP_DB)
obj = objects.InstanceGroup(self.context, **_INST_GROUP_OBJ_VALS)
obj_primitive = obj.obj_to_primitive()
self.assertNotIn('metadetails', obj_primitive)
obj.obj_make_compatible(obj_primitive, '1.6')
self.assertEqual({}, obj_primitive['metadetails'])
def test_obj_make_compatible_pre_1_11(self):
none_policy_group = copy.deepcopy(_INST_GROUP_DB)
none_policy_group['policy'] = None
dbs = [_INST_GROUP_DB, none_policy_group]
data = lambda x: x['nova_object.data']
for db in dbs:
ig = objects.InstanceGroup()
obj = ig._from_db_object(self.context, ig, db)
# Latest version obj has policy and policies
obj_primitive = obj.obj_to_primitive()
self.assertIn('policy', data(obj_primitive))
self.assertIn('policies', data(obj_primitive))
# Before 1.10, only has polices which is the list of policy name
obj_primitive = obj.obj_to_primitive('1.10')
self.assertNotIn('policy', data(obj_primitive))
self.assertIn('policies', data(obj_primitive))
self.assertEqual([db['policy']['policy']] if db['policy'] else [],
data(obj_primitive)['policies'])
@mock.patch.object(objects.InstanceList, 'get_by_filters')
def test_load_hosts(self, mock_get_by_filt):
mock_get_by_filt.return_value = [objects.Instance(host='host1'),

2
nova/tests/unit/objects/test_objects.py
View File

@ -1103,7 +1103,7 @@ object_data = {
'InstanceExternalEvent': '1.2-23eb6ba79cde5cd06d3445f845ba4589',
'InstanceFault': '1.2-7ef01f16f1084ad1304a513d6d410a38',
'InstanceFaultList': '1.2-6bb72de2872fe49ded5eb937a93f2451',
'InstanceGroup': '1.10-1a0c8c7447dc7ecb9da53849430c4a5f',
'InstanceGroup': '1.11-852ac511d30913ee88f3c3a869a8f30a',
'InstanceGroupList': '1.8-90f8f1a445552bb3bbc9fa1ae7da27d4',
'InstanceInfoCache': '1.5-cd8b96fefe0fc8d4d337243ba0bf0e1e',
'InstanceList': '2.4-d2c5723da8c1d08e07cb00160edfd292',

2
nova/tests/unit/objects/test_request_spec.py
View File

@ -232,7 +232,7 @@ class _TestRequestSpecObject(object):
spec = objects.RequestSpec()
spec._populate_group_info(filt_props)
self.assertIsInstance(spec.instance_group, objects.InstanceGroup)
self.assertEqual(['affinity'], spec.instance_group.policies)
self.assertEqual('affinity', spec.instance_group.policy)
self.assertEqual(['fake1'], spec.instance_group.hosts)
self.assertEqual(['fake-instance1'], spec.instance_group.members)