openstack/nova
Code Issues Proposed changes

Merge "Fix wonky reqspec handling in conductor.unshelve_instance"

Browse Source
This commit is contained in:
Zuul 2018-07-24 12:53:40 +00:00 committed by Gerrit Code Review
commit 8137443bc9
2 changed files with 25 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
nova/conductor/manager.py
View File

@ -765,26 +765,17 @@ class ComputeTaskManager(base.Base):
filter_properties = {} filter_properties = {}
request_spec = scheduler_utils.build_request_spec( request_spec = scheduler_utils.build_request_spec(
image, [instance]) image, [instance])
request_spec = objects.RequestSpec.from_primitives(
context, request_spec, filter_properties)
else: else:
# 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
request_spec.reset_forced_destinations() request_spec.reset_forced_destinations()
# TODO(sbauza): Provide directly the RequestSpec object # TODO(sbauza): Provide directly the RequestSpec object
# when populate_filter_properties and populate_retry() # when populate_filter_properties accepts it
# accept it
filter_properties = request_spec.\ filter_properties = request_spec.\
to_legacy_filter_properties_dict() to_legacy_filter_properties_dict()
# FIXME(mriedem): This means we'll lose the is_bfv flag
# set on an existing RequestSpec that had it set.
request_spec = request_spec.\
to_legacy_request_spec_dict()
# TODO(mriedem): we don't even need to call populate_retry
# because unshelve failures aren't rescheduled.
scheduler_utils.populate_retry(filter_properties,
instance.uuid)
request_spec = objects.RequestSpec.from_primitives(
context, request_spec, filter_properties)
# NOTE(cfriesen): Ensure that we restrict the scheduler to # NOTE(cfriesen): Ensure that we restrict the scheduler to
# the cell specified by the instance mapping. # the cell specified by the instance mapping.
instance_mapping = \ instance_mapping = \

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

@ -999,7 +999,6 @@ class _BaseTaskTestCase(object):
fake_spec.instance_group = None fake_spec.instance_group = None
filter_properties = fake_spec.to_legacy_filter_properties_dict() filter_properties = fake_spec.to_legacy_filter_properties_dict()
request_spec = fake_spec.to_legacy_request_spec_dict()
host = {'host': 'host1', 'nodename': 'node1', 'limits': {}} host = {'host': 'host1', 'nodename': 'node1', 'limits': {}}
@ -1010,24 +1009,18 @@ class _BaseTaskTestCase(object):
@mock.patch.object(self.conductor_manager.compute_rpcapi, @mock.patch.object(self.conductor_manager.compute_rpcapi,
'unshelve_instance') 'unshelve_instance')
@mock.patch.object(scheduler_utils, 'populate_filter_properties') @mock.patch.object(scheduler_utils, 'populate_filter_properties')
@mock.patch.object(scheduler_utils, 'populate_retry')
@mock.patch.object(self.conductor_manager, '_schedule_instances') @mock.patch.object(self.conductor_manager, '_schedule_instances')
@mock.patch.object(objects.RequestSpec, 'from_primitives')
@mock.patch.object(objects.RequestSpec, 'to_legacy_request_spec_dict')
@mock.patch.object(objects.RequestSpec, @mock.patch.object(objects.RequestSpec,
'to_legacy_filter_properties_dict') 'to_legacy_filter_properties_dict')
@mock.patch.object(objects.RequestSpec, 'reset_forced_destinations') @mock.patch.object(objects.RequestSpec, 'reset_forced_destinations')
def do_test(reset_forced_destinations, def do_test(reset_forced_destinations,
to_filtprops, to_reqspec, from_primitives, sched_instances, to_filtprops, sched_instances, populate_filter_properties,
populate_retry, populate_filter_properties,
unshelve_instance, get_by_instance_uuid): unshelve_instance, get_by_instance_uuid):
cell_mapping = objects.CellMapping.get_by_uuid(self.context, cell_mapping = objects.CellMapping.get_by_uuid(self.context,
uuids.cell1) uuids.cell1)
get_by_instance_uuid.return_value = objects.InstanceMapping( get_by_instance_uuid.return_value = objects.InstanceMapping(
cell_mapping=cell_mapping) cell_mapping=cell_mapping)
to_filtprops.return_value = filter_properties to_filtprops.return_value = filter_properties
to_reqspec.return_value = request_spec
from_primitives.return_value = fake_spec
sched_instances.return_value = [[fake_selection1]] sched_instances.return_value = [[fake_selection1]]
self.conductor.unshelve_instance(self.context, instance, fake_spec) self.conductor.unshelve_instance(self.context, instance, fake_spec)
# The fake_spec already has a project_id set which doesn't match # The fake_spec already has a project_id set which doesn't match
@ -1035,14 +1028,26 @@ class _BaseTaskTestCase(object):
# overridden using the instance.project_id. # overridden using the instance.project_id.
self.assertNotEqual(fake_spec.project_id, instance.project_id) self.assertNotEqual(fake_spec.project_id, instance.project_id)
reset_forced_destinations.assert_called_once_with() reset_forced_destinations.assert_called_once_with()
from_primitives.assert_called_once_with(self.context, request_spec, # The fake_spec is only going to modified by reference for
filter_properties) # ComputeTaskManager.
self.heal_reqspec_is_bfv_mock.assert_called_once_with( if isinstance(self.conductor,
self.context, fake_spec, test.MatchType(objects.Instance)) conductor_manager.ComputeTaskManager):
sched_instances.assert_called_once_with(self.context, fake_spec, self.heal_reqspec_is_bfv_mock.assert_called_once_with(
self.context, fake_spec, instance)
sched_instances.assert_called_once_with(
self.context, fake_spec, [instance.uuid],
return_alternates=False)
self.assertEqual(cell_mapping,
fake_spec.requested_destination.cell)
else:
# RPC API tests won't have the same request spec or instance
# since they go over the wire.
self.heal_reqspec_is_bfv_mock.assert_called_once_with(
self.context, test.MatchType(objects.RequestSpec),
test.MatchType(objects.Instance))
sched_instances.assert_called_once_with(
self.context, test.MatchType(objects.RequestSpec),
[instance.uuid], return_alternates=False) [instance.uuid], return_alternates=False)
self.assertEqual(cell_mapping,
fake_spec.requested_destination.cell)
# NOTE(sbauza): Since the instance is dehydrated when passing # NOTE(sbauza): Since the instance is dehydrated when passing
# through the RPC API, we can only assert mock.ANY for it # through the RPC API, we can only assert mock.ANY for it
unshelve_instance.assert_called_once_with( unshelve_instance.assert_called_once_with(
@ -1146,11 +1151,7 @@ class _BaseTaskTestCase(object):
self.context, fake_spec, [instance.uuid], return_alternates=False) self.context, fake_spec, [instance.uuid], return_alternates=False)
mock_unshelve.assert_called_once_with( mock_unshelve.assert_called_once_with(
self.context, instance, 'fake_host', image='fake_image', self.context, instance, 'fake_host', image='fake_image',
filter_properties={'limits': {}, filter_properties={'limits': {}}, node='fake_node')
'retry': {'num_attempts': 1,
'hosts': [['fake_host',
'fake_node']]}},
node='fake_node')
def test_unshelve_instance_schedule_and_rebuild_novalid_host(self): def test_unshelve_instance_schedule_and_rebuild_novalid_host(self):
instance = self._create_fake_instance_obj() instance = self._create_fake_instance_obj()
@ -1242,11 +1243,7 @@ class _BaseTaskTestCase(object):
self.context, fake_spec, [instance.uuid], return_alternates=False) self.context, fake_spec, [instance.uuid], return_alternates=False)
mock_unshelve.assert_called_once_with( mock_unshelve.assert_called_once_with(
self.context, instance, 'fake_host', image=None, self.context, instance, 'fake_host', image=None,
filter_properties={'limits': {}, filter_properties={'limits': {}}, node='fake_node')
'retry': {'num_attempts': 1,
'hosts': [['fake_host',
'fake_node']]}},
node='fake_node')
def test_rebuild_instance(self): def test_rebuild_instance(self):
inst_obj = self._create_fake_instance_obj() inst_obj = self._create_fake_instance_obj()