Don't persist RequestSpec.requested_destination

The RequestSpec.requested_destination, similar to the
retry field, is per-request/operation, and persisting
it can caues issues when subsequent move requests.

For example, if you cold migrate a server to a specific
host and then live migrate that server without specifying
a host, the requested target host from the cold migrate
is sent to the scheduler for the live migration, but since
that is where the instance is already running, it's
rejected with NoValidHost.

This is a similar issue to the need to call
RequestSpec.reset_forced_destinations() in all move operations
in conductor. However, rather than try to whack this mole in
every place the request spec is sent to the scheduler, like
reset_forced_destinations() is used, we simply don't need to
persist the requested_destination field since it's just a
vehicle to tell the scheduler which host we want.

The related functional regression test is updated to show
the bug is now fixed.

Change-Id: I2a78f0754c63381c57e7e1c610d0938b6df0f537
Closes-Bug: #1797580
This commit is contained in:
Matt Riedemann 2018-10-12 12:03:41 -04:00
parent bfc8d1052b
commit ce3af5e33a
3 changed files with 12 additions and 8 deletions

View File

@ -558,9 +558,11 @@ class RequestSpec(base.NovaObject):
if 'instance_group' in spec and spec.instance_group:
spec.instance_group.members = None
spec.instance_group.hosts = None
# NOTE(mriedem): Don't persist retries since those are per-request
if 'retry' in spec and spec.retry:
spec.retry = None
# NOTE(mriedem): Don't persist retries or requested_destination
# since those are per-request
for excluded in ('retry', 'requested_destination'):
if excluded in spec and getattr(spec, excluded):
setattr(spec, excluded, None)
# NOTE(stephenfin): Don't persist network metadata since we have
# no need for it after scheduling
if 'network_metadata' in spec and spec.network_metadata:

View File

@ -92,8 +92,6 @@ class ColdMigrateTargetHostThenLiveMigrateTest(
'os-migrateLive': {'host': None, 'block_migration': 'auto'}}
self.admin_api.post_server_action(server['id'], live_migrate_req)
server = self._wait_for_state_change(self.admin_api, server, 'ACTIVE')
# FIXME(mriedem): Until bug 1797580 is resolved the migration will
# fail during scheduling.
migration = self._wait_for_migration_status(server, ['error'])
self.assertEqual('live-migration', migration['migration_type'])
# The live migration should have been successful and the server is now
# back on the original host.
self.assertEqual(original_host, server['OS-EXT-SRV-ATTR:host'])

View File

@ -594,6 +594,7 @@ class _TestRequestSpecObject(object):
self.assertIsNone(serialized_obj.instance_group.members)
self.assertIsNone(serialized_obj.instance_group.hosts)
self.assertIsNone(serialized_obj.retry)
self.assertIsNone(serialized_obj.requested_destination)
def test_create(self):
req_obj = fake_request_spec.fake_spec_obj(remove_id=True)
@ -616,6 +617,9 @@ class _TestRequestSpecObject(object):
def test_save(self):
req_obj = fake_request_spec.fake_spec_obj()
# Make sure the requested_destination is not persisted since it is
# only valid per request/operation.
req_obj.requested_destination = objects.Destination(host='fake')
def _test_save_args(self2, context, instance_uuid, changes):
self._check_update_primitive(req_obj, changes)