Merge "Do not persist RequestSpec.ignore_hosts" into stable/ocata
This commit is contained in:
commit
ab3dd99fa5
|
@ -733,8 +733,7 @@ class ComputeTaskManager(base.Base):
|
||||||
elif recreate:
|
elif recreate:
|
||||||
# NOTE(sbauza): Augment the RequestSpec object by excluding
|
# NOTE(sbauza): Augment the RequestSpec object by excluding
|
||||||
# the source host for avoiding the scheduler to pick it
|
# the source host for avoiding the scheduler to pick it
|
||||||
request_spec.ignore_hosts = request_spec.ignore_hosts or []
|
request_spec.ignore_hosts = [instance.host]
|
||||||
request_spec.ignore_hosts.append(instance.host)
|
|
||||||
# NOTE(sbauza): Force_hosts/nodes needs to be reset
|
# NOTE(sbauza): Force_hosts/nodes needs to be reset
|
||||||
# if we want to make sure that the next destination
|
# if we want to make sure that the next destination
|
||||||
# is not forced to be the original host
|
# is not forced to be the original host
|
||||||
|
|
|
@ -434,7 +434,13 @@ class RequestSpec(base.NovaObject):
|
||||||
# though they should match.
|
# though they should match.
|
||||||
if key in ['id', 'instance_uuid']:
|
if key in ['id', 'instance_uuid']:
|
||||||
setattr(spec, key, db_spec[key])
|
setattr(spec, key, db_spec[key])
|
||||||
else:
|
elif key == 'ignore_hosts':
|
||||||
|
# NOTE(mriedem): Do not override the 'ignore_hosts'
|
||||||
|
# field which is not persisted. It is not a lazy-loadable
|
||||||
|
# field. If it is not set, set None.
|
||||||
|
if not spec.obj_attr_is_set(key):
|
||||||
|
setattr(spec, key, None)
|
||||||
|
elif key in spec_obj:
|
||||||
setattr(spec, key, getattr(spec_obj, key))
|
setattr(spec, key, getattr(spec_obj, key))
|
||||||
spec._context = context
|
spec._context = context
|
||||||
|
|
||||||
|
@ -494,9 +500,11 @@ class RequestSpec(base.NovaObject):
|
||||||
if 'instance_group' in spec and spec.instance_group:
|
if 'instance_group' in spec and spec.instance_group:
|
||||||
spec.instance_group.members = None
|
spec.instance_group.members = None
|
||||||
spec.instance_group.hosts = None
|
spec.instance_group.hosts = None
|
||||||
# NOTE(mriedem): Don't persist retries since those are per-request
|
# NOTE(mriedem): Don't persist retries or ignored hosts since
|
||||||
if 'retry' in spec and spec.retry:
|
# those are per-request
|
||||||
spec.retry = None
|
for excluded in ('retry', 'ignore_hosts'):
|
||||||
|
if excluded in spec and getattr(spec, excluded):
|
||||||
|
setattr(spec, excluded, None)
|
||||||
|
|
||||||
db_updates = {'spec': jsonutils.dumps(spec.obj_to_primitive())}
|
db_updates = {'spec': jsonutils.dumps(spec.obj_to_primitive())}
|
||||||
if 'instance_uuid' in updates:
|
if 'instance_uuid' in updates:
|
||||||
|
|
|
@ -69,21 +69,17 @@ class ResizeEvacuateTestCase(integrated_helpers._IntegratedTestBase,
|
||||||
# Disable the host on which the server is now running (host2).
|
# Disable the host on which the server is now running (host2).
|
||||||
host2.stop()
|
host2.stop()
|
||||||
self.api.force_down_service('host2', 'nova-compute', forced_down=True)
|
self.api.force_down_service('host2', 'nova-compute', forced_down=True)
|
||||||
|
|
||||||
# Now try to evacuate the server back to the original source compute.
|
# Now try to evacuate the server back to the original source compute.
|
||||||
# FIXME(mriedem): This is bug 1669054 where the evacuate fails with
|
|
||||||
# NoValidHost because the RequestSpec.ignore_hosts field has the
|
|
||||||
# original source host in it which is the only other available host to
|
|
||||||
# which we can evacuate the server.
|
|
||||||
req = {'evacuate': {'onSharedStorage': False}}
|
req = {'evacuate': {'onSharedStorage': False}}
|
||||||
self.api.post_server_action(server['id'], req,
|
self.api.post_server_action(server['id'], req)
|
||||||
check_response_status=[500])
|
server = self._wait_for_state_change(self.api, server, 'ACTIVE')
|
||||||
# There should be fault recorded with the server.
|
# The evacuate flow in the compute manager is annoying in that it
|
||||||
server = self._wait_for_state_change(self.api, server, 'ERROR')
|
# sets the instance status to ACTIVE before updating the host, so we
|
||||||
self.assertIn('fault', server)
|
# have to wait for the migration record to be 'done' to avoid a race.
|
||||||
self.assertIn('No valid host was found', server['fault']['message'])
|
self._wait_for_migration_status(server, 'done')
|
||||||
# Assert the RequestSpec.ignore_hosts is still populated.
|
self.assertEqual(self.compute.host, server['OS-EXT-SRV-ATTR:host'])
|
||||||
|
|
||||||
|
# Assert the RequestSpec.ignore_hosts field is not populated.
|
||||||
reqspec = objects.RequestSpec.get_by_instance_uuid(
|
reqspec = objects.RequestSpec.get_by_instance_uuid(
|
||||||
context.get_admin_context(), server['id'])
|
context.get_admin_context(), server['id'])
|
||||||
self.assertIsNotNone(reqspec.ignore_hosts)
|
self.assertIsNone(reqspec.ignore_hosts)
|
||||||
self.assertIn(self.compute.host, reqspec.ignore_hosts)
|
|
||||||
|
|
|
@ -1374,7 +1374,7 @@ class _BaseTaskTestCase(object):
|
||||||
instance=inst_obj,
|
instance=inst_obj,
|
||||||
**compute_args)
|
**compute_args)
|
||||||
|
|
||||||
def test_rebuild_instance_with_request_spec(self):
|
def test_evacuate_instance_with_request_spec(self):
|
||||||
inst_obj = self._create_fake_instance_obj()
|
inst_obj = self._create_fake_instance_obj()
|
||||||
inst_obj.host = 'noselect'
|
inst_obj.host = 'noselect'
|
||||||
expected_host = 'thebesthost'
|
expected_host = 'thebesthost'
|
||||||
|
@ -1382,11 +1382,11 @@ class _BaseTaskTestCase(object):
|
||||||
expected_limits = 'fake-limits'
|
expected_limits = 'fake-limits'
|
||||||
request_spec = {}
|
request_spec = {}
|
||||||
filter_properties = {'ignore_hosts': [(inst_obj.host)]}
|
filter_properties = {'ignore_hosts': [(inst_obj.host)]}
|
||||||
fake_spec = objects.RequestSpec(ignore_hosts=[])
|
fake_spec = objects.RequestSpec(ignore_hosts=[uuids.ignored_host])
|
||||||
augmented_spec = objects.RequestSpec(ignore_hosts=[inst_obj.host])
|
augmented_spec = objects.RequestSpec(ignore_hosts=[inst_obj.host])
|
||||||
rebuild_args, compute_args = self._prepare_rebuild_args(
|
rebuild_args, compute_args = self._prepare_rebuild_args(
|
||||||
{'host': None, 'node': expected_node, 'limits': expected_limits,
|
{'host': None, 'node': expected_node, 'limits': expected_limits,
|
||||||
'request_spec': fake_spec})
|
'request_spec': fake_spec, 'recreate': True})
|
||||||
with test.nested(
|
with test.nested(
|
||||||
mock.patch.object(self.conductor_manager.compute_rpcapi,
|
mock.patch.object(self.conductor_manager.compute_rpcapi,
|
||||||
'rebuild_instance'),
|
'rebuild_instance'),
|
||||||
|
@ -1409,14 +1409,13 @@ class _BaseTaskTestCase(object):
|
||||||
self.conductor_manager.rebuild_instance(context=self.context,
|
self.conductor_manager.rebuild_instance(context=self.context,
|
||||||
instance=inst_obj,
|
instance=inst_obj,
|
||||||
**rebuild_args)
|
**rebuild_args)
|
||||||
if rebuild_args['recreate']:
|
reset_fd.assert_called_once_with()
|
||||||
reset_fd.assert_called_once_with()
|
|
||||||
else:
|
|
||||||
reset_fd.assert_not_called()
|
|
||||||
to_reqspec.assert_called_once_with()
|
to_reqspec.assert_called_once_with()
|
||||||
to_filtprops.assert_called_once_with()
|
to_filtprops.assert_called_once_with()
|
||||||
fp_mock.assert_called_once_with(self.context, request_spec,
|
fp_mock.assert_called_once_with(self.context, request_spec,
|
||||||
filter_properties)
|
filter_properties)
|
||||||
|
# The RequestSpec.ignore_hosts field should be overwritten.
|
||||||
|
self.assertEqual([inst_obj.host], fake_spec.ignore_hosts)
|
||||||
select_dest_mock.assert_called_once_with(self.context,
|
select_dest_mock.assert_called_once_with(self.context,
|
||||||
augmented_spec)
|
augmented_spec)
|
||||||
compute_args['host'] = expected_host
|
compute_args['host'] = expected_host
|
||||||
|
|
|
@ -504,7 +504,8 @@ class _TestRequestSpecObject(object):
|
||||||
fake_spec['instance_uuid'])
|
fake_spec['instance_uuid'])
|
||||||
|
|
||||||
self.assertEqual(1, req_obj.num_instances)
|
self.assertEqual(1, req_obj.num_instances)
|
||||||
self.assertEqual(['host2', 'host4'], req_obj.ignore_hosts)
|
# ignore_hosts is not persisted
|
||||||
|
self.assertIsNone(req_obj.ignore_hosts)
|
||||||
self.assertEqual('fake', req_obj.project_id)
|
self.assertEqual('fake', req_obj.project_id)
|
||||||
self.assertEqual({'hint': ['over-there']}, req_obj.scheduler_hints)
|
self.assertEqual({'hint': ['over-there']}, req_obj.scheduler_hints)
|
||||||
self.assertEqual(['host1', 'host3'], req_obj.force_hosts)
|
self.assertEqual(['host1', 'host3'], req_obj.force_hosts)
|
||||||
|
@ -527,7 +528,7 @@ class _TestRequestSpecObject(object):
|
||||||
jsonutils.loads(changes['spec']))
|
jsonutils.loads(changes['spec']))
|
||||||
|
|
||||||
# primitive fields
|
# primitive fields
|
||||||
for field in ['instance_uuid', 'num_instances', 'ignore_hosts',
|
for field in ['instance_uuid', 'num_instances',
|
||||||
'project_id', 'scheduler_hints', 'force_hosts',
|
'project_id', 'scheduler_hints', 'force_hosts',
|
||||||
'availability_zone', 'force_nodes']:
|
'availability_zone', 'force_nodes']:
|
||||||
self.assertEqual(getattr(req_obj, field),
|
self.assertEqual(getattr(req_obj, field),
|
||||||
|
@ -543,6 +544,7 @@ class _TestRequestSpecObject(object):
|
||||||
self.assertIsNone(serialized_obj.instance_group.members)
|
self.assertIsNone(serialized_obj.instance_group.members)
|
||||||
self.assertIsNone(serialized_obj.instance_group.hosts)
|
self.assertIsNone(serialized_obj.instance_group.hosts)
|
||||||
self.assertIsNone(serialized_obj.retry)
|
self.assertIsNone(serialized_obj.retry)
|
||||||
|
self.assertIsNone(serialized_obj.ignore_hosts)
|
||||||
|
|
||||||
def test_create(self):
|
def test_create(self):
|
||||||
req_obj = fake_request_spec.fake_spec_obj(remove_id=True)
|
req_obj = fake_request_spec.fake_spec_obj(remove_id=True)
|
||||||
|
@ -563,6 +565,39 @@ class _TestRequestSpecObject(object):
|
||||||
|
|
||||||
self.assertRaises(exception.ObjectActionError, req_obj.create)
|
self.assertRaises(exception.ObjectActionError, req_obj.create)
|
||||||
|
|
||||||
|
def test_save_does_not_persist_requested_fields(self):
|
||||||
|
req_obj = fake_request_spec.fake_spec_obj(remove_id=True)
|
||||||
|
req_obj.create()
|
||||||
|
# change something to make sure _save_in_db is called
|
||||||
|
expected_destination = request_spec.Destination(host='sample-host')
|
||||||
|
req_obj.requested_destination = expected_destination
|
||||||
|
expected_retry = objects.SchedulerRetries(
|
||||||
|
num_attempts=2,
|
||||||
|
hosts=objects.ComputeNodeList(objects=[
|
||||||
|
objects.ComputeNode(host='host1', hypervisor_hostname='node1'),
|
||||||
|
objects.ComputeNode(host='host2', hypervisor_hostname='node2'),
|
||||||
|
]))
|
||||||
|
req_obj.retry = expected_retry
|
||||||
|
req_obj.ignore_hosts = [uuids.ignored_host]
|
||||||
|
|
||||||
|
orig_save_in_db = request_spec.RequestSpec._save_in_db
|
||||||
|
with mock.patch.object(request_spec.RequestSpec, '_save_in_db') \
|
||||||
|
as mock_save_in_db:
|
||||||
|
mock_save_in_db.side_effect = orig_save_in_db
|
||||||
|
req_obj.save()
|
||||||
|
mock_save_in_db.assert_called_once()
|
||||||
|
updates = mock_save_in_db.mock_calls[0][1][2]
|
||||||
|
# assert that the following fields are not stored in the db
|
||||||
|
# 1. ignore_hosts
|
||||||
|
data = jsonutils.loads(updates['spec'])['nova_object.data']
|
||||||
|
self.assertIsNone(data['ignore_hosts'])
|
||||||
|
self.assertIsNotNone(data['instance_uuid'])
|
||||||
|
|
||||||
|
# also we expect that the following fields are not reset after save
|
||||||
|
# 1. ignore_hosts
|
||||||
|
self.assertIsNotNone(req_obj.ignore_hosts)
|
||||||
|
self.assertEqual([uuids.ignored_host], req_obj.ignore_hosts)
|
||||||
|
|
||||||
def test_save(self):
|
def test_save(self):
|
||||||
req_obj = fake_request_spec.fake_spec_obj()
|
req_obj = fake_request_spec.fake_spec_obj()
|
||||||
|
|
||||||
|
|
|
@ -529,7 +529,11 @@ class FakeDriver(driver.ComputeDriver):
|
||||||
return
|
return
|
||||||
|
|
||||||
def confirm_migration(self, context, migration, instance, network_info):
|
def confirm_migration(self, context, migration, instance, network_info):
|
||||||
return
|
# Confirm migration cleans up the guest from the source host so just
|
||||||
|
# destroy the guest to remove it from the list of tracked instances
|
||||||
|
# unless it is a same-host resize.
|
||||||
|
if migration.source_compute != migration.dest_compute:
|
||||||
|
self.destroy(context, instance, network_info)
|
||||||
|
|
||||||
def pre_live_migration(self, context, instance, block_device_info,
|
def pre_live_migration(self, context, instance, block_device_info,
|
||||||
network_info, disk_info, migrate_data):
|
network_info, disk_info, migrate_data):
|
||||||
|
|
Loading…
Reference in New Issue