From d81fcf80323fca1c83aa4a4fd97f39b66315c935 Mon Sep 17 00:00:00 2001 From: Andrew Laski Date: Wed, 28 Sep 2016 09:47:12 -0400 Subject: [PATCH] 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 (cherry picked from commit 9b090aeb7e2d4e4adc0b2a80402cbfb09830bd94) --- nova/conductor/manager.py | 32 +++++++---- nova/tests/unit/conductor/test_conductor.py | 63 +++++++++++++++++++++ 2 files changed, 83 insertions(+), 12 deletions(-) diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index c33d27713730..5f0591b48144 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -513,18 +513,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, diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py index 3f3f870274f3..2860088fd1b3 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -877,6 +877,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