diff --git a/nova/compute/api.py b/nova/compute/api.py index 0b10ec6d0642..5caa6eb351ad 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -976,14 +976,6 @@ class API(base.Base): self._create_block_device_mapping( instance_type, instance.uuid, block_device_mapping) - # The BuildRequest needs to be stored until the instance is in - # an instance table. At that point it will never be used again - # and should be deleted. As instance creation is pushed back, - # as noted above, this will move with it. - # TODO(alaski): Sync API updates to the build_request to the - # instance before it is destroyed. Right now only locked_by can - # be updated before this is destroyed. - build_request.destroy() if instance_group: if check_server_group_quota: diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index 014267e7f863..5c9197013966 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -369,6 +369,24 @@ class ComputeTaskManager(base.Base): self.compute_rpcapi, self.scheduler_client) + def _destroy_build_request(self, context, instance): + try: + build_request = objects.BuildRequest.get_by_instance_uuid(context, + instance.uuid) + except exception.BuildRequestNotFound: + LOG.debug('BuildRequest not found for instance %(uuid)s, likely ' + 'due to an older nova-api service running.', + {'uuid': instance.uuid}) + return + + # The BuildRequest needs to be stored until the instance is mapped to + # an instance table. At that point it will never be used again and + # should be deleted. + # TODO(alaski): Sync API updates to the build_request to the + # instance before it is destroyed. Right now only locked_by can + # be updated before this is destroyed. + build_request.destroy() + def _populate_instance_mapping(self, context, instance, host): try: inst_mapping = objects.InstanceMapping.get_by_instance_uuid( @@ -452,6 +470,7 @@ class ComputeTaskManager(base.Base): context, instance.uuid) self._populate_instance_mapping(context, instance, host) + self._destroy_build_request(context, instance) self.compute_rpcapi.build_and_run_instance(context, instance=instance, host=host['host'], image=image, diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index b80e759ac7bd..525ce917ebd3 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -3166,7 +3166,7 @@ class _ComputeAPIUnitTestMixIn(object): do_test() - def test_provision_instances_creates_destroys_build_request(self): + def test_provision_instances_creates_build_request(self): @mock.patch.object(self.compute_api, '_check_num_instances_quota') @mock.patch.object(objects, 'Instance') @mock.patch.object(self.compute_api.security_group_api, @@ -3258,7 +3258,6 @@ class _ComputeAPIUnitTestMixIn(object): mock_build_req.assert_has_calls(build_req_calls) for build_req_mock in build_req_mocks: build_req_mock.create.assert_called_once_with() - build_req_mock.destroy.assert_called_once_with() do_test() diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py index a32a5133608e..6b72e37fcbee 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -759,6 +759,93 @@ class _BaseTaskTestCase(object): mock_get_by_host.assert_has_calls([mock.call(self.context, 'host1'), mock.call(self.context, 'host2')]) + @mock.patch.object(objects.Instance, 'refresh', new=mock.MagicMock()) + @mock.patch.object(objects.BuildRequest, 'get_by_instance_uuid') + @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_destroy_build_request(self, mock_select_dests, + mock_build_req_get): + + mock_select_dests.return_value = [ + {'host': 'host1', 'nodename': 'node1', 'limits': []}, + {'host': 'host2', 'nodename': 'node2', 'limits': []}] + + num_instances = 2 + instances = [fake_instance.fake_instance_obj(self.context) + for i in range(num_instances)] + build_req_mocks = [mock.Mock() for i in range(num_instances)] + mock_build_req_get.side_effect = build_req_mocks + 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', new=mock.MagicMock()) + @mock.patch.object(self.conductor_manager, + '_populate_instance_mapping', new=mock.MagicMock()) + def do_test(): + self.conductor.build_instances( + context=self.context, + instances=instances, + image=image, + filter_properties={}, + admin_password='admin_password', + injected_files='injected_files', + requested_networks=None, + security_groups='security_groups', + block_device_mapping='block_device_mapping', + legacy_bdm=False) + + do_test() + + for build_req in build_req_mocks: + build_req.destroy.assert_called_once_with() + + @mock.patch.object(objects.Instance, 'refresh', new=mock.MagicMock()) + @mock.patch.object(objects.BuildRequest, 'get_by_instance_uuid', + side_effect=exc.BuildRequestNotFound(uuid='fake')) + @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_build_request_not_found(self, mock_select_dests, + mock_build_req_get): + + mock_select_dests.return_value = [ + {'host': 'host1', 'nodename': 'node1', 'limits': []}, + {'host': 'host2', 'nodename': 'node2', 'limits': []}] + + num_instances = 2 + instances = [fake_instance.fake_instance_obj(self.context) + for i in range(num_instances)] + 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', new=mock.MagicMock()) + def do_test(mock_build_and_run): + self.conductor.build_instances( + context=self.context, + instances=instances, + image=image, + filter_properties={}, + admin_password='admin_password', + injected_files='injected_files', + requested_networks=None, + security_groups='security_groups', + block_device_mapping='block_device_mapping', + legacy_bdm=False) + self.assertTrue(mock_build_and_run.called) + + do_test() + def test_unshelve_instance_on_host(self): instance = self._create_fake_instance_obj() instance.vm_state = vm_states.SHELVED