Merge "Recalculate request group - RP mapping during re-schedule"
This commit is contained in:
commit
8e5ef12f9f
|
@ -2125,7 +2125,9 @@ class ComputeManager(manager.Manager):
|
||||||
scheduler_hints)
|
scheduler_hints)
|
||||||
image_meta = objects.ImageMeta.from_dict(image)
|
image_meta = objects.ImageMeta.from_dict(image)
|
||||||
|
|
||||||
if request_spec:
|
if (request_spec
|
||||||
|
and 'requested_resources' in request_spec
|
||||||
|
and request_spec.requested_resources is not None):
|
||||||
request_group_resource_providers_mapping = {
|
request_group_resource_providers_mapping = {
|
||||||
group.requester_id: group.provider_uuids
|
group.requester_id: group.provider_uuids
|
||||||
for group in request_spec.requested_resources
|
for group in request_spec.requested_resources
|
||||||
|
|
|
@ -567,7 +567,7 @@ class ComputeTaskManager(base.Base):
|
||||||
# will the API send the request_spec if using cells v1, so we need
|
# will the API send the request_spec if using cells v1, so we need
|
||||||
# to check and build our own if one is not provided.
|
# to check and build our own if one is not provided.
|
||||||
if request_spec is None:
|
if request_spec is None:
|
||||||
request_spec = scheduler_utils.build_request_spec(
|
legacy_request_spec = scheduler_utils.build_request_spec(
|
||||||
image, instances)
|
image, instances)
|
||||||
else:
|
else:
|
||||||
# TODO(mriedem): This is annoying but to populate the local
|
# TODO(mriedem): This is annoying but to populate the local
|
||||||
|
@ -576,7 +576,10 @@ class ComputeTaskManager(base.Base):
|
||||||
# and we can remove it once the populate_retry and
|
# and we can remove it once the populate_retry and
|
||||||
# populate_filter_properties utility methods are converted to
|
# populate_filter_properties utility methods are converted to
|
||||||
# work on a RequestSpec object rather than filter_properties.
|
# work on a RequestSpec object rather than filter_properties.
|
||||||
request_spec = request_spec.to_legacy_request_spec_dict()
|
# NOTE(gibi): we have to keep a reference to the original
|
||||||
|
# RequestSpec object passed to this function as we lose information
|
||||||
|
# during the below legacy conversion
|
||||||
|
legacy_request_spec = request_spec.to_legacy_request_spec_dict()
|
||||||
|
|
||||||
# 'host_lists' will be None in one of two cases: when running cellsv1,
|
# 'host_lists' will be None in one of two cases: when running cellsv1,
|
||||||
# or during a reschedule from a pre-Queens compute. In all other cases,
|
# or during a reschedule from a pre-Queens compute. In all other cases,
|
||||||
|
@ -593,7 +596,7 @@ class ComputeTaskManager(base.Base):
|
||||||
filter_properties, instances[0].uuid)
|
filter_properties, instances[0].uuid)
|
||||||
instance_uuids = [instance.uuid for instance in instances]
|
instance_uuids = [instance.uuid for instance in instances]
|
||||||
spec_obj = objects.RequestSpec.from_primitives(
|
spec_obj = objects.RequestSpec.from_primitives(
|
||||||
context, request_spec, filter_properties)
|
context, legacy_request_spec, filter_properties)
|
||||||
LOG.debug("Rescheduling: %s", is_reschedule)
|
LOG.debug("Rescheduling: %s", is_reschedule)
|
||||||
if is_reschedule:
|
if is_reschedule:
|
||||||
# Make sure that we have a host, as we may have exhausted all
|
# Make sure that we have a host, as we may have exhausted all
|
||||||
|
@ -629,7 +632,7 @@ class ComputeTaskManager(base.Base):
|
||||||
for instance in instances:
|
for instance in instances:
|
||||||
self._set_vm_state_and_notify(
|
self._set_vm_state_and_notify(
|
||||||
context, instance.uuid, 'build_instances', updates,
|
context, instance.uuid, 'build_instances', updates,
|
||||||
exc, request_spec)
|
exc, legacy_request_spec)
|
||||||
# If num_attempts > 1, we're in a reschedule and probably
|
# If num_attempts > 1, we're in a reschedule and probably
|
||||||
# either hit NoValidHost or MaxRetriesExceeded. Either way,
|
# either hit NoValidHost or MaxRetriesExceeded. Either way,
|
||||||
# the build request should already be gone and we probably
|
# the build request should already be gone and we probably
|
||||||
|
@ -668,6 +671,12 @@ class ComputeTaskManager(base.Base):
|
||||||
elevated, self.report_client, spec_obj,
|
elevated, self.report_client, spec_obj,
|
||||||
instance.uuid, alloc_req,
|
instance.uuid, alloc_req,
|
||||||
host.allocation_request_version)
|
host.allocation_request_version)
|
||||||
|
if request_spec:
|
||||||
|
# NOTE(gibi): redo the request group - resource
|
||||||
|
# provider mapping as the above claim call moves
|
||||||
|
# the allocation of the instance to another host
|
||||||
|
self._fill_provider_mapping(
|
||||||
|
context, instance.uuid, request_spec)
|
||||||
else:
|
else:
|
||||||
# Some deployments use different schedulers that do not
|
# Some deployments use different schedulers that do not
|
||||||
# use Placement, so they will not have an
|
# use Placement, so they will not have an
|
||||||
|
@ -704,11 +713,16 @@ class ComputeTaskManager(base.Base):
|
||||||
# could have come from a compute via reschedule and it would
|
# could have come from a compute via reschedule and it would
|
||||||
# already have some things set, like scheduler_hints.
|
# already have some things set, like scheduler_hints.
|
||||||
local_reqspec = objects.RequestSpec.from_primitives(
|
local_reqspec = objects.RequestSpec.from_primitives(
|
||||||
context, request_spec, local_filter_props)
|
context, legacy_request_spec, local_filter_props)
|
||||||
|
|
||||||
# TODO(gibi): make sure that the request group - provider mapping
|
# NOTE(gibi): at this point the request spec already got converted
|
||||||
# is updated in the request spec during reschedule
|
# to a legacy dict and then back to an object so we lost the non
|
||||||
local_reqspec.requested_resources = []
|
# legacy part of the spec. Re-populate the requested_resource
|
||||||
|
# field based on the original request spec object passed to this
|
||||||
|
# function.
|
||||||
|
if request_spec:
|
||||||
|
local_reqspec.requested_resources = (
|
||||||
|
request_spec.requested_resources)
|
||||||
|
|
||||||
# The block_device_mapping passed from the api doesn't contain
|
# The block_device_mapping passed from the api doesn't contain
|
||||||
# instance specific information
|
# instance specific information
|
||||||
|
|
|
@ -951,6 +951,81 @@ class _BaseTaskTestCase(object):
|
||||||
|
|
||||||
do_test()
|
do_test()
|
||||||
|
|
||||||
|
@mock.patch.object(objects.Instance, 'save', new=mock.MagicMock())
|
||||||
|
@mock.patch.object(query.SchedulerQueryClient, 'select_destinations')
|
||||||
|
@mock.patch.object(conductor_manager.ComputeTaskManager,
|
||||||
|
'_set_vm_state_and_notify', new=mock.MagicMock())
|
||||||
|
def test_build_instances_reschedule_recalculates_provider_mapping(self,
|
||||||
|
mock_select_dests):
|
||||||
|
|
||||||
|
rg1 = objects.RequestGroup(resources={"CUSTOM_FOO": 1})
|
||||||
|
request_spec = objects.RequestSpec(requested_resources=[rg1])
|
||||||
|
|
||||||
|
mock_select_dests.return_value = [[fake_selection1]]
|
||||||
|
instance = fake_instance.fake_instance_obj(self.context)
|
||||||
|
image = {'fake-data': 'should_pass_silently'}
|
||||||
|
|
||||||
|
# build_instances() is a cast, we need to wait for it to complete
|
||||||
|
self.useFixture(cast_as_call.CastAsCall(self))
|
||||||
|
|
||||||
|
@mock.patch('nova.conductor.manager.ComputeTaskManager.'
|
||||||
|
'_fill_provider_mapping')
|
||||||
|
@mock.patch('nova.scheduler.utils.claim_resources')
|
||||||
|
@mock.patch('nova.objects.request_spec.RequestSpec.from_primitives',
|
||||||
|
return_value=request_spec)
|
||||||
|
@mock.patch.object(self.conductor_manager.compute_rpcapi,
|
||||||
|
'build_and_run_instance')
|
||||||
|
@mock.patch.object(self.conductor_manager,
|
||||||
|
'_populate_instance_mapping')
|
||||||
|
@mock.patch.object(self.conductor_manager, '_destroy_build_request')
|
||||||
|
def do_test(mock_destroy_build_req, mock_pop_inst_map,
|
||||||
|
mock_build_and_run, mock_request_spec_from_primitives,
|
||||||
|
mock_claim, mock_rp_mapping):
|
||||||
|
self.conductor.build_instances(
|
||||||
|
context=self.context,
|
||||||
|
instances=[instance],
|
||||||
|
image=image,
|
||||||
|
filter_properties={'retry': {'num_attempts': 1, 'hosts': []}},
|
||||||
|
admin_password='admin_password',
|
||||||
|
injected_files='injected_files',
|
||||||
|
requested_networks=None,
|
||||||
|
security_groups='security_groups',
|
||||||
|
block_device_mapping='block_device_mapping',
|
||||||
|
legacy_bdm=False,
|
||||||
|
host_lists=fake_host_lists1,
|
||||||
|
request_spec=request_spec)
|
||||||
|
|
||||||
|
expected_build_run_host_list = copy.copy(fake_host_lists1[0])
|
||||||
|
if expected_build_run_host_list:
|
||||||
|
expected_build_run_host_list.pop(0)
|
||||||
|
mock_build_and_run.assert_called_once_with(
|
||||||
|
self.context,
|
||||||
|
instance=mock.ANY,
|
||||||
|
host='host1',
|
||||||
|
image=image,
|
||||||
|
request_spec=request_spec,
|
||||||
|
filter_properties={'retry': {'num_attempts': 2,
|
||||||
|
'hosts': [['host1', 'node1']]},
|
||||||
|
'limits': {}},
|
||||||
|
admin_password='admin_password',
|
||||||
|
injected_files='injected_files',
|
||||||
|
requested_networks=None,
|
||||||
|
security_groups='security_groups',
|
||||||
|
block_device_mapping=test.MatchType(
|
||||||
|
objects.BlockDeviceMappingList),
|
||||||
|
node='node1',
|
||||||
|
limits=None,
|
||||||
|
host_list=expected_build_run_host_list)
|
||||||
|
|
||||||
|
mock_rp_mapping.assert_called_once_with(
|
||||||
|
self.context, instance.uuid, mock.ANY)
|
||||||
|
actual_request_spec = mock_rp_mapping.mock_calls[0][1][2]
|
||||||
|
self.assertEqual(
|
||||||
|
rg1.resources,
|
||||||
|
actual_request_spec.requested_resources[0].resources)
|
||||||
|
|
||||||
|
do_test()
|
||||||
|
|
||||||
@mock.patch.object(cinder.API, 'attachment_get')
|
@mock.patch.object(cinder.API, 'attachment_get')
|
||||||
@mock.patch.object(cinder.API, 'attachment_create')
|
@mock.patch.object(cinder.API, 'attachment_create')
|
||||||
@mock.patch.object(block_device_obj.BlockDeviceMapping, 'save')
|
@mock.patch.object(block_device_obj.BlockDeviceMapping, 'save')
|
||||||
|
|
Loading…
Reference in New Issue