Fix RequestSpec.instance_group hydration
A mistake was done by hydrating the members field for the InstanceGroup object when creating the RequestSpec object because that field is for getting the instances, not the hosts. In order to provide that information, we need to provide a new version of the InstanceGroup and give a lazy-load for getting it in case we want to have it. Change-Id: I0d892bbe490c9ee727f54077d7398cc2800ce107 Partially-Implements: blueprint request-spec-object
This commit is contained in:
Sylvain Bauza
parent
b3f07e8c1f
commit
c48c1098cb
|
|
@ -1593,6 +1593,10 @@ class InstanceGroupPolicyNotFound(NotFound):
|
|||
msg_fmt = _("Instance group %(group_uuid)s has no policy %(policy)s.")
|
||||
|
||||
|
||||
class InstanceGroupSaveException(NovaException):
|
||||
msg_fmt = _("%(field)s should not be part of the updates.")
|
||||
|
||||
|
||||
class PluginRetriesExceeded(NovaException):
|
||||
msg_fmt = _("Number of retries to plugin (%(num_retries)d) exceeded.")
|
||||
|
||||
|
|
|
|||
|
|
@ -23,6 +23,9 @@ from nova.objects import fields
|
|||
from nova import utils
|
||||
|
||||
|
||||
LAZY_LOAD_FIELDS = ['hosts']
|
||||
|
||||
|
||||
# TODO(berrange): Remove NovaObjectDictCompat
|
||||
@base.NovaObjectRegistry.register
|
||||
class InstanceGroup(base.NovaPersistentObject, base.NovaObject,
|
||||
|
|
@ -37,7 +40,8 @@ class InstanceGroup(base.NovaPersistentObject, base.NovaObject,
|
|||
# Version 1.7: Deprecate metadetails
|
||||
# Version 1.8: Add count_members_by_user()
|
||||
# Version 1.9: Add get_by_instance_uuid()
|
||||
VERSION = '1.9'
|
||||
# Version 1.10: Add hosts field
|
||||
VERSION = '1.10'
|
||||
|
||||
fields = {
|
||||
'id': fields.IntegerField(),
|
||||
|
|
@ -50,6 +54,7 @@ class InstanceGroup(base.NovaPersistentObject, base.NovaObject,
|
|||
|
||||
'policies': fields.ListOfStringsField(nullable=True),
|
||||
'members': fields.ListOfStringsField(nullable=True),
|
||||
'hosts': fields.ListOfStringsField(nullable=True),
|
||||
}
|
||||
|
||||
def obj_make_compatible(self, primitive, target_version):
|
||||
|
|
@ -67,6 +72,8 @@ class InstanceGroup(base.NovaPersistentObject, base.NovaObject,
|
|||
"""
|
||||
# Most of the field names match right now, so be quick
|
||||
for field in instance_group.fields:
|
||||
if field in LAZY_LOAD_FIELDS:
|
||||
continue
|
||||
if field == 'deleted':
|
||||
instance_group.deleted = db_inst['deleted'] == db_inst['id']
|
||||
else:
|
||||
|
|
@ -76,6 +83,15 @@ class InstanceGroup(base.NovaPersistentObject, base.NovaObject,
|
|||
instance_group.obj_reset_changes()
|
||||
return instance_group
|
||||
|
||||
def obj_load_attr(self, attrname):
|
||||
# NOTE(sbauza): Only hosts could be lazy-loaded right now
|
||||
if attrname != 'hosts':
|
||||
raise exception.ObjectActionError(
|
||||
action='obj_load_attr', reason='unable to load %s' % attrname)
|
||||
|
||||
self.hosts = self.get_hosts()
|
||||
self.obj_reset_changes(['hosts'])
|
||||
|
||||
@base.remotable_classmethod
|
||||
def get_by_uuid(cls, context, uuid):
|
||||
db_inst = db.instance_group_get(context, uuid)
|
||||
|
|
@ -112,6 +128,19 @@ class InstanceGroup(base.NovaPersistentObject, base.NovaObject,
|
|||
"""Save updates to this instance group."""
|
||||
|
||||
updates = self.obj_get_changes()
|
||||
|
||||
# NOTE(sbauza): We do NOT save the set of compute nodes that an
|
||||
# instance group is connected to in this method. Instance groups are
|
||||
# implicitly connected to compute nodes when the
|
||||
# InstanceGroup.add_members() method is called, which adds the mapping
|
||||
# table entries.
|
||||
# So, since the only way to have hosts in the updates is to set that
|
||||
# field explicitely, we prefer to raise an Exception so the developer
|
||||
# knows he has to call obj_reset_changes(['hosts']) right after setting
|
||||
# the field.
|
||||
if 'hosts' in updates:
|
||||
raise exception.InstanceGroupSaveException(field='hosts')
|
||||
|
||||
if not updates:
|
||||
return
|
||||
|
||||
|
|
@ -208,7 +237,8 @@ class InstanceGroupList(base.ObjectListBase, base.NovaObject):
|
|||
# Version 1.4: InstanceGroup <= version 1.7
|
||||
# Version 1.5: InstanceGroup <= version 1.8
|
||||
# Version 1.6: InstanceGroup <= version 1.9
|
||||
VERSION = '1.6'
|
||||
# Version 1.7: InstanceGroup <= version 1.10
|
||||
VERSION = '1.7'
|
||||
|
||||
fields = {
|
||||
'objects': fields.ListOfObjectsField('InstanceGroup'),
|
||||
|
|
@ -217,7 +247,7 @@ class InstanceGroupList(base.ObjectListBase, base.NovaObject):
|
|||
obj_relationships = {
|
||||
'objects': [('1.0', '1.3'), ('1.1', '1.4'), ('1.2', '1.5'),
|
||||
('1.3', '1.6'), ('1.4', '1.7'), ('1.5', '1.8'),
|
||||
('1.6', '1.9')],
|
||||
('1.6', '1.9'), ('1.7', '1.10')],
|
||||
}
|
||||
|
||||
@base.remotable_classmethod
|
||||
|
|
|
|||
|
|
@ -24,7 +24,8 @@ class RequestSpec(base.NovaObject):
|
|||
# Version 1.0: Initial version
|
||||
# Version 1.1: ImageMeta version 1.6
|
||||
# Version 1.2: SchedulerRetries version 1.1
|
||||
VERSION = '1.2'
|
||||
# Version 1.3: InstanceGroup version 1.10
|
||||
VERSION = '1.3'
|
||||
|
||||
fields = {
|
||||
'id': fields.IntegerField(),
|
||||
|
|
@ -57,7 +58,7 @@ class RequestSpec(base.NovaObject):
|
|||
'pci_requests': [('1.0', '1.1')],
|
||||
'retry': [('1.0', '1.0'), ('1.2', '1.1')],
|
||||
'limits': [('1.0', '1.0')],
|
||||
'instance_group': [('1.0', '1.9')],
|
||||
'instance_group': [('1.0', '1.9'), ('1.3', '1.10')],
|
||||
}
|
||||
|
||||
@property
|
||||
|
|
@ -140,9 +141,11 @@ class RequestSpec(base.NovaObject):
|
|||
# Old-style group information having ugly dict keys containing sets
|
||||
# NOTE(sbauza): Can be dropped once select_destinations is removed
|
||||
policies = list(filter_properties.get('group_policies'))
|
||||
members = list(filter_properties.get('group_hosts'))
|
||||
hosts = list(filter_properties.get('group_hosts'))
|
||||
self.instance_group = objects.InstanceGroup(policies=policies,
|
||||
members=members)
|
||||
hosts=hosts)
|
||||
# hosts has to be not part of the updates for saving the object
|
||||
self.instance_group.obj_reset_changes(['hosts'])
|
||||
else:
|
||||
# Set the value anyway to avoid any call to obj_attr_is_set for it
|
||||
self.instance_group = None
|
||||
|
|
@ -242,7 +245,7 @@ class RequestSpec(base.NovaObject):
|
|||
# modified by using directly the RequestSpec object, we need to keep
|
||||
# the existing dictionary as a primitive.
|
||||
return {'group_updated': True,
|
||||
'group_hosts': set(self.instance_group.members),
|
||||
'group_hosts': set(self.instance_group.hosts),
|
||||
'group_policies': set(self.instance_group.policies)}
|
||||
|
||||
def to_legacy_request_spec_dict(self):
|
||||
|
|
|
|||
|
|
@ -100,6 +100,23 @@ class _TestInstanceGroupObject(object):
|
|||
'policies': ['policy1'],
|
||||
'server_group_id': _DB_UUID})
|
||||
|
||||
@mock.patch('nova.compute.utils.notify_about_server_group_update')
|
||||
@mock.patch('nova.db.instance_group_update')
|
||||
@mock.patch('nova.db.instance_group_get')
|
||||
def test_save_without_hosts(self, mock_db_get, mock_db_update,
|
||||
mock_notify):
|
||||
mock_db_get.side_effect = [_INST_GROUP_DB, _INST_GROUP_DB]
|
||||
obj = objects.InstanceGroup.get_by_uuid(mock.sentinel.ctx, _DB_UUID)
|
||||
obj.hosts = ['fake-host1']
|
||||
self.assertRaises(exception.InstanceGroupSaveException,
|
||||
obj.save)
|
||||
# make sure that we can save by removing hosts from what is updated
|
||||
obj.obj_reset_changes(['hosts'])
|
||||
obj.save()
|
||||
# since hosts was the only update, there is no actual call
|
||||
self.assertFalse(mock_db_update.called)
|
||||
self.assertFalse(mock_notify.called)
|
||||
|
||||
@mock.patch('nova.compute.utils.notify_about_server_group_update')
|
||||
@mock.patch('nova.db.instance_group_create', return_value=_INST_GROUP_DB)
|
||||
def test_create(self, mock_db_create, mock_notify):
|
||||
|
|
@ -222,6 +239,21 @@ class _TestInstanceGroupObject(object):
|
|||
obj.obj_make_compatible(obj_primitive, '1.6')
|
||||
self.assertEqual({}, obj_primitive['metadetails'])
|
||||
|
||||
@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'),
|
||||
objects.Instance(host='host2')]
|
||||
|
||||
obj = objects.InstanceGroup(mock.sentinel.ctx, members=['uuid1'])
|
||||
self.assertEqual(2, len(obj.hosts))
|
||||
self.assertIn('host1', obj.hosts)
|
||||
self.assertIn('host2', obj.hosts)
|
||||
self.assertNotIn('hosts', obj.obj_what_changed())
|
||||
|
||||
def test_load_anything_else_but_hosts(self):
|
||||
obj = objects.InstanceGroup(mock.sentinel.ctx)
|
||||
self.assertRaises(exception.ObjectActionError, getattr, obj, 'members')
|
||||
|
||||
|
||||
class TestInstanceGroupObject(test_objects._LocalTest,
|
||||
_TestInstanceGroupObject):
|
||||
|
|
|
|||
|
|
@ -1115,8 +1115,8 @@ object_data = {
|
|||
'InstanceExternalEvent': '1.1-6e446ceaae5f475ead255946dd443417',
|
||||
'InstanceFault': '1.2-7ef01f16f1084ad1304a513d6d410a38',
|
||||
'InstanceFaultList': '1.1-f8ec07cbe3b60f5f07a8b7a06311ac0d',
|
||||
'InstanceGroup': '1.9-a413a4ec0ff391e3ef0faa4e3e2a96d0',
|
||||
'InstanceGroupList': '1.6-be18078220513316abd0ae1b2d916873',
|
||||
'InstanceGroup': '1.10-1a0c8c7447dc7ecb9da53849430c4a5f',
|
||||
'InstanceGroupList': '1.7-be18078220513316abd0ae1b2d916873',
|
||||
'InstanceInfoCache': '1.5-cd8b96fefe0fc8d4d337243ba0bf0e1e',
|
||||
'InstanceList': '1.21-6c8ba6147cca3082b1e4643f795068bf',
|
||||
'InstanceMapping': '1.0-47ef26034dfcbea78427565d9177fe50',
|
||||
|
|
@ -1146,7 +1146,7 @@ object_data = {
|
|||
'PciDevicePoolList': '1.1-15ecf022a68ddbb8c2a6739cfc9f8f5e',
|
||||
'Quotas': '1.2-1fe4cd50593aaf5d36a6dc5ab3f98fb3',
|
||||
'QuotasNoOp': '1.2-e041ddeb7dc8188ca71706f78aad41c1',
|
||||
'RequestSpec': '1.2-6922fe208b5d1186bdd825513f677921',
|
||||
'RequestSpec': '1.3-6922fe208b5d1186bdd825513f677921',
|
||||
'S3ImageMapping': '1.0-7dd7366a890d82660ed121de9092276e',
|
||||
'SchedulerLimits': '1.0-249c4bd8e62a9b327b7026b7f19cc641',
|
||||
'SchedulerRetries': '1.1-3c9c8b16143ebbb6ad7030e999d14cc0',
|
||||
|
|
|
|||
|
|
@ -172,7 +172,7 @@ class _TestRequestSpecObject(object):
|
|||
spec._populate_group_info(filt_props)
|
||||
self.assertIsInstance(spec.instance_group, objects.InstanceGroup)
|
||||
self.assertEqual(['affinity'], spec.instance_group.policies)
|
||||
self.assertEqual(['fake1'], spec.instance_group.members)
|
||||
self.assertEqual(['fake1'], spec.instance_group.hosts)
|
||||
|
||||
def test_populate_group_info_missing_values(self):
|
||||
filt_props = {}
|
||||
|
|
@ -316,7 +316,7 @@ class _TestRequestSpecObject(object):
|
|||
vcpu=1.0,
|
||||
disk_gb=10.0,
|
||||
memory_mb=8192.0),
|
||||
instance_group=objects.InstanceGroup(members=['fake1'],
|
||||
instance_group=objects.InstanceGroup(hosts=['fake1'],
|
||||
policies=['affinity']),
|
||||
scheduler_hints={'foo': ['bar']})
|
||||
expected = {'ignore_hosts': ['ignoredhost'],
|
||||
|
|
|
|||
Loading…
Reference in New Issue