Skip to content

Commit

Permalink
Add user_id to RequestSpec
Browse files Browse the repository at this point in the history
The user_id field was not implemented in RequestSpec like project_id was.
Some people have out of tree filters which use the user_id field.

This change makes the user_id field available.

Closes-bug: #1768107
Change-Id: I3e174ae76931f8279540e92328c7c36a7bcaabc0
  • Loading branch information
mgagne committed May 1, 2018
1 parent 19dd4eb commit 6e49019
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 13 deletions.
4 changes: 2 additions & 2 deletions nova/conductor/manager.py
Expand Up @@ -794,7 +794,7 @@ def safe_image_show(ctx, image_id):
objects.Destination(
cell=instance_mapping.cell_mapping))

request_spec.ensure_project_id(instance)
request_spec.ensure_project_and_user_id(instance)
host_lists = self._schedule_instances(context,
request_spec, [instance.uuid],
return_alternates=False)
Expand Down Expand Up @@ -942,7 +942,7 @@ def rebuild_instance(self, context, instance, orig_image_ref, image_ref,
# is not forced to be the original host
request_spec.reset_forced_destinations()
try:
request_spec.ensure_project_id(instance)
request_spec.ensure_project_and_user_id(instance)
host_lists = self._schedule_instances(context,
request_spec, [instance.uuid],
return_alternates=False)
Expand Down
2 changes: 1 addition & 1 deletion nova/conductor/tasks/live_migrate.py
Expand Up @@ -301,7 +301,7 @@ def _get_request_spec_for_select_destinations(self, attempted_hosts=None):
request_spec.requested_destination = objects.Destination(
cell=cell_mapping)

request_spec.ensure_project_id(self.instance)
request_spec.ensure_project_and_user_id(self.instance)

return request_spec

Expand Down
2 changes: 1 addition & 1 deletion nova/conductor/tasks/migrate.py
Expand Up @@ -219,7 +219,7 @@ def _execute(self):
# instance record.
migration = self._preallocate_migration()

self.request_spec.ensure_project_id(self.instance)
self.request_spec.ensure_project_and_user_id(self.instance)
# On an initial call to migrate, 'self.host_list' will be None, so we
# have to call the scheduler to get a list of acceptable hosts to
# migrate to. That list will consist of a selected host, along with
Expand Down
25 changes: 19 additions & 6 deletions nova/objects/request_spec.py
Expand Up @@ -41,7 +41,8 @@ class RequestSpec(base.NovaObject):
# Version 1.6: Added requested_destination
# Version 1.7: Added destroy()
# Version 1.8: Added security_groups
VERSION = '1.8'
# Version 1.9: Added user_id
VERSION = '1.9'

fields = {
'id': fields.IntegerField(),
Expand All @@ -53,6 +54,7 @@ class RequestSpec(base.NovaObject):
# TODO(mriedem): The project_id shouldn't be nullable since the
# scheduler relies on it being set.
'project_id': fields.StringField(nullable=True),
'user_id': fields.StringField(nullable=True),
'availability_zone': fields.StringField(nullable=True),
'flavor': fields.ObjectField('Flavor', nullable=False),
'num_instances': fields.IntegerField(default=1),
Expand Down Expand Up @@ -82,6 +84,9 @@ class RequestSpec(base.NovaObject):
def obj_make_compatible(self, primitive, target_version):
super(RequestSpec, self).obj_make_compatible(primitive, target_version)
target_version = versionutils.convert_version_to_tuple(target_version)
if target_version < (1, 9):
if 'user_id' in primitive:
del primitive['user_id']
if target_version < (1, 8):
if 'security_groups' in primitive:
del primitive['security_groups']
Expand Down Expand Up @@ -153,7 +158,7 @@ def _from_instance(self, instance):
return

instance_fields = ['numa_topology', 'pci_requests', 'uuid',
'project_id', 'availability_zone']
'project_id', 'user_id', 'availability_zone']
for field in instance_fields:
if field == 'uuid':
setattr(self, 'instance_uuid', getter(instance, field))
Expand Down Expand Up @@ -307,7 +312,8 @@ def _to_legacy_instance(self):
# fields, we can only return a dict.
instance = {}
instance_fields = ['numa_topology', 'pci_requests',
'project_id', 'availability_zone', 'instance_uuid']
'project_id', 'user_id', 'availability_zone',
'instance_uuid']
for field in instance_fields:
if not self.obj_attr_is_set(field):
continue
Expand Down Expand Up @@ -388,7 +394,8 @@ def to_legacy_filter_properties_dict(self):
@classmethod
def from_components(cls, context, instance_uuid, image, flavor,
numa_topology, pci_requests, filter_properties, instance_group,
availability_zone, security_groups=None, project_id=None):
availability_zone, security_groups=None, project_id=None,
user_id=None):
"""Returns a new RequestSpec object hydrated by various components.
This helper is useful in creating the RequestSpec from the various
Expand All @@ -409,6 +416,8 @@ def from_components(cls, context, instance_uuid, image, flavor,
set security_groups on the resulting object.
:param project_id: The project_id for the requestspec (should match
the instance project_id).
:param user_id: The user_id for the requestspec (should match
the instance user_id).
"""
spec_obj = cls(context)
spec_obj.num_instances = 1
Expand All @@ -417,6 +426,7 @@ def from_components(cls, context, instance_uuid, image, flavor,
if spec_obj.instance_group is None and filter_properties:
spec_obj._populate_group_info(filter_properties)
spec_obj.project_id = project_id or context.project_id
spec_obj.user_id = user_id or context.user_id
spec_obj._image_meta_from_image(image)
spec_obj._from_flavor(flavor)
spec_obj._from_instance_pci_requests(pci_requests)
Expand All @@ -438,9 +448,11 @@ def from_components(cls, context, instance_uuid, image, flavor,
spec_obj.obj_set_defaults()
return spec_obj

def ensure_project_id(self, instance):
def ensure_project_and_user_id(self, instance):
if 'project_id' not in self or self.project_id is None:
self.project_id = instance.project_id
if 'user_id' not in self or self.user_id is None:
self.user_id = instance.user_id

@staticmethod
def _from_db_object(context, spec, db_spec):
Expand Down Expand Up @@ -628,7 +640,8 @@ def _create_minimal_request_spec(context, instance):
instance.flavor, instance.numa_topology,
instance.pci_requests,
{}, None, instance.availability_zone,
project_id=instance.project_id
project_id=instance.project_id,
user_id=instance.user_id
)
scheduler_utils.setup_instance_group(context, request_spec)
request_spec.create()
Expand Down
1 change: 1 addition & 0 deletions nova/tests/unit/fake_request_spec.py
Expand Up @@ -80,6 +80,7 @@ def fake_spec_obj(remove_id=False):
req_obj.limits = objects.SchedulerLimits()
req_obj.instance_group = objects.InstanceGroup(uuid=uuids.instgroup)
req_obj.project_id = 'fake'
req_obj.user_id = 'fake-user'
req_obj.num_instances = 1
req_obj.availability_zone = None
req_obj.ignore_hosts = ['host2', 'host4']
Expand Down
2 changes: 1 addition & 1 deletion nova/tests/unit/objects/test_objects.py
Expand Up @@ -1141,7 +1141,7 @@ def obj_name(cls):
'PowerVMLiveMigrateData': '1.2-b62cd242c5205a853545b1085b072340',
'Quotas': '1.3-40fcefe522111dddd3e5e6155702cf4e',
'QuotasNoOp': '1.3-347a039fc7cfee7b225b68b5181e0733',
'RequestSpec': '1.8-35033ecef47a880f9a5e46e2269e2b97',
'RequestSpec': '1.9-e506ccb22cd7807a1207c22a3f179387',
'S3ImageMapping': '1.0-7dd7366a890d82660ed121de9092276e',
'SchedulerLimits': '1.0-249c4bd8e62a9b327b7026b7f19cc641',
'SchedulerRetries': '1.1-3c9c8b16143ebbb6ad7030e999d14cc0',
Expand Down
21 changes: 19 additions & 2 deletions nova/tests/unit/objects/test_request_spec.py
Expand Up @@ -78,12 +78,13 @@ def test_from_instance_as_object(self):
instance.numa_topology = None
instance.pci_requests = None
instance.project_id = fakes.FAKE_PROJECT_ID
instance.user_id = fakes.FAKE_USER_ID
instance.availability_zone = 'nova'

spec = objects.RequestSpec()
spec._from_instance(instance)
instance_fields = ['numa_topology', 'pci_requests', 'uuid',
'project_id', 'availability_zone']
'project_id', 'user_id', 'availability_zone']
for field in instance_fields:
if field == 'uuid':
self.assertEqual(getattr(instance, field),
Expand All @@ -97,12 +98,13 @@ def test_from_instance_as_dict(self):
numa_topology=None,
pci_requests=None,
project_id=fakes.FAKE_PROJECT_ID,
user_id=fakes.FAKE_USER_ID,
availability_zone='nova')

spec = objects.RequestSpec()
spec._from_instance(instance)
instance_fields = ['numa_topology', 'pci_requests', 'uuid',
'project_id', 'availability_zone']
'project_id', 'user_id', 'availability_zone']
for field in instance_fields:
if field == 'uuid':
self.assertEqual(instance.get(field),
Expand All @@ -124,6 +126,7 @@ def test_from_instance_with_pci_requests(self, pci_from_spec):
vcpus=1,
numa_topology=None,
project_id=fakes.FAKE_PROJECT_ID,
user_id=fakes.FAKE_USER_ID,
availability_zone='nova',
pci_requests={
'instance_uuid': 'fakeid',
Expand All @@ -142,6 +145,7 @@ def test_from_instance_with_numa_stuff(self):
memory_mb=10,
vcpus=1,
project_id=fakes.FAKE_PROJECT_ID,
user_id=fakes.FAKE_USER_ID,
availability_zone='nova',
pci_requests=None,
numa_topology={'cells': [{'id': 1, 'cpuset': ['1'], 'memory': 8192,
Expand Down Expand Up @@ -180,6 +184,7 @@ def test_to_legacy_instance(self):
spec.numa_topology = None
spec.pci_requests = None
spec.project_id = fakes.FAKE_PROJECT_ID
spec.user_id = fakes.FAKE_USER_ID
spec.availability_zone = 'nova'

instance = spec._to_legacy_instance()
Expand All @@ -190,6 +195,7 @@ def test_to_legacy_instance(self):
'numa_topology': None,
'pci_requests': None,
'project_id': fakes.FAKE_PROJECT_ID,
'user_id': fakes.FAKE_USER_ID,
'availability_zone': 'nova'}, instance)

def test_to_legacy_instance_with_unset_values(self):
Expand Down Expand Up @@ -281,6 +287,7 @@ def test_from_primitives(self, mock_limits):
numa_topology=None,
pci_requests=None,
project_id=1,
user_id=2,
availability_zone='nova')}
filt_props = {}

Expand Down Expand Up @@ -624,6 +631,16 @@ def test_compat_security_groups(self):
version_manifest=versions)
self.assertNotIn('security_groups', primitive)

def test_compat_user_id(self):
req_obj = objects.RequestSpec(project_id=fakes.FAKE_PROJECT_ID,
user_id=fakes.FAKE_USER_ID)
versions = ovo_base.obj_tree_get_versions('RequestSpec')
primitive = req_obj.obj_to_primitive(target_version='1.8',
version_manifest=versions)
primitive = primitive['nova_object.data']
self.assertNotIn('user_id', primitive)
self.assertIn('project_id', primitive)

def test_default_requested_destination(self):
req_obj = objects.RequestSpec()
self.assertIsNone(req_obj.requested_destination)
Expand Down

0 comments on commit 6e49019

Please sign in to comment.