Ignore BuildRequest during an instance reschedule
When booting an instance there is logic in the conductor to check if a delete has been issued. This is done by looking for a BuildRequest object and discontinuing the build if it's not found. However the conductor then deletes the BuildRequest so a reschedule attempt will not find the BuildRequest object. This incorrectly stops the reschedule. The filter_properties dict is updated with the number of scheduling attempts for each reschedule so by looking at the value found there we know if a reschedule is being attempted. If that's the case then bypass the logic that checks for, and deletes, the BuildRequest object. Change-Id: Ibf28d1d8f54703b465ccc497281419356cd0136e Closes-Bug: 1628530
This commit is contained in:
Andrew Laski
parent
ebb3b93fb2
commit
9b090aeb7e
|
|
@ -525,18 +525,26 @@ class ComputeTaskManager(base.Base):
|
|||
bdms = objects.BlockDeviceMappingList.get_by_instance_uuid(
|
||||
context, instance.uuid)
|
||||
|
||||
inst_mapping = self._populate_instance_mapping(context, instance,
|
||||
host)
|
||||
try:
|
||||
self._destroy_build_request(context, instance)
|
||||
except exception.BuildRequestNotFound:
|
||||
# This indicates an instance delete has been requested in the
|
||||
# API. Stop the build, cleanup the instance_mapping and
|
||||
# potentially the block_device_mappings
|
||||
# TODO(alaski): Handle block_device_mapping cleanup
|
||||
if inst_mapping:
|
||||
inst_mapping.destroy()
|
||||
return
|
||||
# This is populated in scheduler_utils.populate_retry
|
||||
num_attempts = local_filter_props.get('retry',
|
||||
{}).get('num_attempts', 1)
|
||||
if num_attempts <= 1:
|
||||
# If this is a reschedule the instance is already mapped to
|
||||
# this cell and the BuildRequest is already deleted so ignore
|
||||
# the logic below.
|
||||
inst_mapping = self._populate_instance_mapping(context,
|
||||
instance,
|
||||
host)
|
||||
try:
|
||||
self._destroy_build_request(context, instance)
|
||||
except exception.BuildRequestNotFound:
|
||||
# This indicates an instance delete has been requested in
|
||||
# the API. Stop the build, cleanup the instance_mapping and
|
||||
# potentially the block_device_mappings
|
||||
# TODO(alaski): Handle block_device_mapping cleanup
|
||||
if inst_mapping:
|
||||
inst_mapping.destroy()
|
||||
return
|
||||
|
||||
self.compute_rpcapi.build_and_run_instance(context,
|
||||
instance=instance, host=host['host'], image=image,
|
||||
|
|
|
|||
|
|
@ -927,6 +927,69 @@ class _BaseTaskTestCase(object):
|
|||
mock_service_version.assert_called_once_with(self.context,
|
||||
'nova-osapi_compute')
|
||||
|
||||
@mock.patch.object(objects.Instance, 'refresh', new=mock.MagicMock())
|
||||
@mock.patch.object(scheduler_client.SchedulerClient,
|
||||
'select_destinations')
|
||||
@mock.patch.object(conductor_manager.ComputeTaskManager,
|
||||
'_set_vm_state_and_notify', new=mock.MagicMock())
|
||||
def test_build_instances_reschedule_ignores_build_request(self,
|
||||
mock_select_dests):
|
||||
# This test calls build_instances as if it was a reschedule. This means
|
||||
# that the exc.BuildRequestNotFound() exception raised by
|
||||
# conductor_manager._destroy_build_request() should not cause the
|
||||
# build to stop.
|
||||
|
||||
mock_select_dests.return_value = [
|
||||
{'host': 'host1', 'nodename': 'node1', 'limits': []}]
|
||||
|
||||
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.stubs))
|
||||
|
||||
@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',
|
||||
side_effect=exc.BuildRequestNotFound(uuid='fake'))
|
||||
def do_test(mock_destroy_build_req, mock_pop_inst_map,
|
||||
mock_build_and_run):
|
||||
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)
|
||||
|
||||
mock_build_and_run.assert_called_once_with(
|
||||
self.context,
|
||||
instance=mock.ANY,
|
||||
host='host1',
|
||||
image=image,
|
||||
request_spec=mock.ANY,
|
||||
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=[])
|
||||
mock_pop_inst_map.assert_not_called()
|
||||
mock_destroy_build_req.assert_not_called()
|
||||
|
||||
do_test()
|
||||
|
||||
def test_unshelve_instance_on_host(self):
|
||||
instance = self._create_fake_instance_obj()
|
||||
instance.vm_state = vm_states.SHELVED
|
||||
|
|
|
|||
Loading…
Reference in New Issue