Set error state after failed evacuation

When evacuation fails with NoValidHost, the migration status remains
'accepted' instead of 'error'. This causes problem in case the compute
service starts up again and looks for evacuations with status 'accepted',
as it then removes the local instances for those evacuations even though
the instance was never actually evacuated to another host.

Change-Id: I06d78c744fa75ae5f34c5cfa76bc3c9460767b84
Closes-Bug: #1713783
(cherry picked from commit a8ebf5f1aa)
This commit is contained in:
Előd Illés 2017-08-30 16:54:36 +02:00 committed by Matt Riedemann
parent e87d9f51ab
commit a3f286f43d
3 changed files with 29 additions and 12 deletions

View File

@ -807,6 +807,15 @@ class ComputeTaskManager(base.Base):
with compute_utils.EventReporter(context, 'rebuild_server', with compute_utils.EventReporter(context, 'rebuild_server',
instance.uuid): instance.uuid):
node = limits = None node = limits = None
try:
migration = objects.Migration.get_by_instance_and_status(
context, instance.uuid, 'accepted')
except exception.MigrationNotFoundByStatus:
LOG.debug("No migration record for the rebuild/evacuate "
"request.", instance=instance)
migration = None
# The host variable is passed in two cases: # The host variable is passed in two cases:
# 1. rebuild - the instance.host is passed to rebuild on the # 1. rebuild - the instance.host is passed to rebuild on the
# same host and bypass the scheduler. # same host and bypass the scheduler.
@ -857,6 +866,9 @@ class ComputeTaskManager(base.Base):
host_dict['nodename'], host_dict['nodename'],
host_dict['limits']) host_dict['limits'])
except exception.NoValidHost as ex: except exception.NoValidHost as ex:
if migration:
migration.status = 'error'
migration.save()
request_spec = request_spec.to_legacy_request_spec_dict() request_spec = request_spec.to_legacy_request_spec_dict()
with excutils.save_and_reraise_exception(): with excutils.save_and_reraise_exception():
self._set_vm_state_and_notify(context, instance.uuid, self._set_vm_state_and_notify(context, instance.uuid,
@ -866,6 +878,9 @@ class ComputeTaskManager(base.Base):
LOG.warning("No valid host found for rebuild", LOG.warning("No valid host found for rebuild",
instance=instance) instance=instance)
except exception.UnsupportedPolicyException as ex: except exception.UnsupportedPolicyException as ex:
if migration:
migration.status = 'error'
migration.save()
request_spec = request_spec.to_legacy_request_spec_dict() request_spec = request_spec.to_legacy_request_spec_dict()
with excutils.save_and_reraise_exception(): with excutils.save_and_reraise_exception():
self._set_vm_state_and_notify(context, instance.uuid, self._set_vm_state_and_notify(context, instance.uuid,
@ -875,14 +890,6 @@ class ComputeTaskManager(base.Base):
LOG.warning("Server with unsupported policy " LOG.warning("Server with unsupported policy "
"cannot be rebuilt", instance=instance) "cannot be rebuilt", instance=instance)
try:
migration = objects.Migration.get_by_instance_and_status(
context, instance.uuid, 'accepted')
except exception.MigrationNotFoundByStatus:
LOG.debug("No migration record for the rebuild/evacuate "
"request.", instance=instance)
migration = None
compute_utils.notify_about_instance_usage( compute_utils.notify_about_instance_usage(
self.notifier, context, instance, "rebuild.scheduled") self.notifier, context, instance, "rebuild.scheduled")

View File

@ -118,7 +118,4 @@ class FailedEvacuateStateTests(test.TestCase,
self.assertEqual('evacuation', migrations[0]['migration_type']) self.assertEqual('evacuation', migrations[0]['migration_type'])
self.assertEqual(server['id'], migrations[0]['instance_uuid']) self.assertEqual(server['id'], migrations[0]['instance_uuid'])
self.assertEqual(self.hostname, migrations[0]['source_compute']) self.assertEqual(self.hostname, migrations[0]['source_compute'])
self.assertEqual('accepted', migrations[0]['status']) self.assertEqual('error', migrations[0]['status'])
# NOTE(elod.illes): Migration status should be 'error' and not
# 'accepted'. Needs to be replaced when bug #1713783 is fixed.
# self.assertEqual('error', migrations[0]['status'])

View File

@ -1282,6 +1282,15 @@ class _BaseTaskTestCase(object):
# build_instances() is a cast, we need to wait for it to complete # build_instances() is a cast, we need to wait for it to complete
self.useFixture(cast_as_call.CastAsCall(self)) self.useFixture(cast_as_call.CastAsCall(self))
# Create the migration record (normally created by the compute API).
migration = objects.Migration(self.context,
source_compute=inst_obj.host,
source_node=inst_obj.node,
instance_uuid=inst_obj.uuid,
status='accepted',
migration_type='evacuation')
migration.create()
self.assertRaises(exc.UnsupportedPolicyException, self.assertRaises(exc.UnsupportedPolicyException,
self.conductor.rebuild_instance, self.conductor.rebuild_instance,
self.context, self.context,
@ -1294,6 +1303,10 @@ class _BaseTaskTestCase(object):
self.assertFalse(select_dest_mock.called) self.assertFalse(select_dest_mock.called)
self.assertFalse(rebuild_mock.called) self.assertFalse(rebuild_mock.called)
# Assert the migration status was updated.
migration = objects.Migration.get_by_id(self.context, migration.id)
self.assertEqual('error', migration.status)
def test_rebuild_instance_evacuate_migration_record(self): def test_rebuild_instance_evacuate_migration_record(self):
inst_obj = self._create_fake_instance_obj() inst_obj = self._create_fake_instance_obj()
migration = objects.Migration(context=self.context, migration = objects.Migration(context=self.context,