openstack
/
nova
Code Issues Proposed changes

Workaround missing RequestSpec.project_id when moving an instance 

The online data migration routine to create request specs for old
instances used an admin context which has an empty project_id,
so when scheduling (moving) one of these, if we try to PUT /allocations
in placement using the FilterScheduler we'll fail because the project_id
is None.

This works around that by putting the instance.project_id on the request
spec before calling the scheduler to pick a node and claim resources
against it.

A later change will need to add some sort of online data migration
routine so that we properly update and persist the fix for these
older records.

Conflicts:
      nova/conductor/manager.py
      nova/conductor/tasks/live_migrate.py
      nova/conductor/tasks/migrate.py
      nova/tests/unit/conductor/test_conductor.py

NOTE(mriedem): The conflicts are due to
ca716ce4dd and
685c16041c not
being in Pike.

Change-Id: I34b1d99a9d0d2aca80f094a79ec1656abaf762dc
Partial-Bug: #1739318
(cherry picked from commit f9a06c4253)
This commit is contained in:
Matt Riedemann 2017-12-19 18:40:31 -05:00
parent c86db10feb
commit 7f8a14dbc8
6 changed files with 26 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
nova/conductor/manager.py
View File

@ -705,6 +705,7 @@ class ComputeTaskManager(base.Base):
objects.Destination(
cell=instance_mapping.cell_mapping))
request_spec.ensure_project_id(instance)
hosts = self._schedule_instances(context, request_spec,
[instance.uuid])
host_state = hosts[0]
@ -867,6 +868,7 @@ class ComputeTaskManager(base.Base):
# is not forced to be the original host
request_spec.reset_forced_destinations()
try:
request_spec.ensure_project_id(instance)
hosts = self._schedule_instances(context, request_spec,
[instance.uuid])
host_dict = hosts.pop(0)

1
nova/conductor/tasks/live_migrate.py
View File

@ -262,6 +262,7 @@ class LiveMigrationTask(base.TaskBase):
request_spec.requested_destination = objects.Destination(
cell=cell_mapping)
request_spec.ensure_project_id(self.instance)
host = None
while host is None:
self._check_not_over_max_retries(attempted_hosts)

1
nova/conductor/tasks/migrate.py
View File

@ -64,6 +64,7 @@ class MigrationTask(base.TaskBase):
self.request_spec.requested_destination = objects.Destination(
cell=instance_mapping.cell_mapping)
self.request_spec.ensure_project_id(self.instance)
hosts = self.scheduler_client.select_destinations(
self.context, self.request_spec, [self.instance.uuid])
host_state = hosts[0]

6
nova/objects/request_spec.py
View File

@ -50,6 +50,8 @@ class RequestSpec(base.NovaObject):
nullable=True),
'pci_requests': fields.ObjectField('InstancePCIRequests',
nullable=True),
# TODO(mriedem): The project_id shouldn't be nullable since the
# scheduler relies on it being set.
'project_id': fields.StringField(nullable=True),
'availability_zone': fields.StringField(nullable=True),
'flavor': fields.ObjectField('Flavor', nullable=False),
@ -432,6 +434,10 @@ class RequestSpec(base.NovaObject):
spec_obj.obj_set_defaults()
return spec_obj
def ensure_project_id(self, instance):
if 'project_id' not in self or self.project_id is None:
self.project_id = instance.project_id
@staticmethod
def _from_db_object(context, spec, db_spec):
spec_obj = spec.obj_from_primitive(jsonutils.loads(db_spec['spec']))

2
nova/tests/unit/conductor/tasks/test_live_migrate.py
View File

@ -304,6 +304,8 @@ class LiveMigrationTaskTestCase(test.NoDBTestCase):
# Make sure the request_spec was updated to include the cell
# mapping.
self.assertIsNotNone(self.fake_spec.requested_destination.cell)
# Make sure the spec was updated to include the project_id.
self.assertEqual(self.fake_spec.project_id, self.instance.project_id)
def test_find_destination_works_with_no_request_spec(self):
task = live_migrate.LiveMigrationTask(

17
nova/tests/unit/conductor/test_conductor.py
View File

@ -952,6 +952,10 @@ class _BaseTaskTestCase(object):
from_primitives.return_value = fake_spec
sched_instances.return_value = [host]
self.conductor.unshelve_instance(self.context, instance, fake_spec)
# The fake_spec already has a project_id set which doesn't match
# the instance.project_id so the spec's project_id won't be
# overridden using the instance.project_id.
self.assertNotEqual(fake_spec.project_id, instance.project_id)
reset_forced_destinations.assert_called_once_with()
from_primitives.assert_called_once_with(self.context, request_spec,
filter_properties)
@ -1226,6 +1230,7 @@ class _BaseTaskTestCase(object):
rebuild_mock.assert_called_once_with(self.context,
instance=inst_obj,
**compute_args)
self.assertEqual(inst_obj.project_id, fake_spec.project_id)
self.assertEqual('compute.instance.rebuild.scheduled',
fake_notifier.NOTIFICATIONS[0].event_type)
@ -2120,7 +2125,8 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase):
flavor=flavor,
availability_zone=None,
pci_requests=None,
numa_topology=None)
numa_topology=None,
project_id=self.context.project_id)
resvs = 'fake-resvs'
image = 'fake-image'
fake_spec = objects.RequestSpec(image=objects.ImageMeta())
@ -2143,6 +2149,7 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase):
True, None)
metadata_mock.assert_called_with({})
sig_mock.assert_called_once_with(self.context, fake_spec)
self.assertEqual(inst_obj.project_id, fake_spec.project_id)
notify_mock.assert_called_once_with(self.context, inst_obj.uuid,
'migrate_server', updates,
exc_info, legacy_request_spec)
@ -2170,7 +2177,8 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase):
flavor=flavor,
numa_topology=None,
pci_requests=None,
availability_zone=None)
availability_zone=None,
project_id=self.context.project_id)
image = 'fake-image'
resvs = 'fake-resvs'
@ -2194,6 +2202,7 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase):
True, None)
metadata_mock.assert_called_with({})
sig_mock.assert_called_once_with(self.context, fake_spec)
self.assertEqual(inst_obj.project_id, fake_spec.project_id)
notify_mock.assert_called_once_with(self.context, inst_obj.uuid,
'migrate_server', updates,
exc_info, legacy_request_spec)
@ -2295,7 +2304,8 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase):
flavor=flavor,
availability_zone=None,
pci_requests=None,
numa_topology=None)
numa_topology=None,
project_id=self.context.project_id)
image = 'fake-image'
resvs = 'fake-resvs'
fake_spec = objects.RequestSpec(image=objects.ImageMeta())
@ -2326,6 +2336,7 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase):
metadata_mock.assert_called_with({})
sig_mock.assert_called_once_with(self.context, fake_spec)
self.assertEqual(inst_obj.project_id, fake_spec.project_id)
select_dest_mock.assert_called_once_with(
self.context, fake_spec, [inst_obj.uuid])
prep_resize_mock.assert_called_once_with(