diff --git a/nova/compute/api.py b/nova/compute/api.py index d6b699ff18..aa44bff5d9 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -915,6 +915,8 @@ class API(base.Base): self.security_group_api.ensure_default(context) LOG.debug("Going to run %s instances...", num_instances) instances = [] + instance_mappings = [] + build_requests = [] try: for i in range(num_instances): # Create a uuid for the instance so we can store the @@ -948,6 +950,7 @@ class API(base.Base): project_id=instance.project_id, block_device_mappings=block_device_mapping) build_request.create() + build_requests.append(build_request) # Create an instance_mapping. The null cell_mapping indicates # that the instance doesn't yet exist in a cell, and lookups # for it need to instead look for the RequestSpec. @@ -959,6 +962,7 @@ class API(base.Base): inst_mapping.project_id = context.project_id inst_mapping.cell_mapping = None inst_mapping.create() + instance_mappings.append(inst_mapping) # TODO(alaski): Cast to conductor here which will call the # scheduler and defer instance creation until the scheduler # has picked a cell/host. Set the instance_mapping to the cell @@ -1003,6 +1007,16 @@ class API(base.Base): instance.destroy() except exception.ObjectActionError: pass + for instance_mapping in instance_mappings: + try: + instance_mapping.destroy() + except exception.InstanceMappingNotFound: + pass + for build_request in build_requests: + try: + build_request.destroy() + except exception.BuildRequestNotFound: + pass finally: quotas.rollback() diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index faaaff3824..f5b4a6c373 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -3550,8 +3550,8 @@ class _ComputeAPIUnitTestMixIn(object): @mock.patch.object(self.compute_api, '_create_block_device_mapping') @mock.patch.object(objects.RequestSpec, 'from_components') @mock.patch.object(objects, 'BuildRequest') - @mock.patch.object(objects.InstanceMapping, 'create') - def do_test(_mock_inst_mapping_create, mock_build_req, + @mock.patch.object(objects, 'InstanceMapping') + def do_test(mock_inst_mapping, mock_build_req, mock_req_spec_from_components, _mock_create_bdm, _mock_ensure_default, mock_inst, mock_check_num_inst_quota): quota_mock = mock.MagicMock() @@ -3567,6 +3567,8 @@ class _ComputeAPIUnitTestMixIn(object): mock_inst.side_effect = inst_mocks build_req_mocks = [mock.MagicMock() for i in range(max_count)] mock_build_req.side_effect = build_req_mocks + inst_map_mocks = [mock.MagicMock() for i in range(max_count)] + mock_inst_mapping.side_effect = inst_map_mocks ctxt = context.RequestContext('fake-user', 'fake-project') flavor = self._create_flavor() @@ -3616,12 +3618,18 @@ class _ComputeAPIUnitTestMixIn(object): shutdown_terminate, instance_group, check_server_group_quota, filter_properties, None) - # First instance is created and destroyed + # First instance, build_req, mapping is created and destroyed self.assertTrue(inst_mocks[0].create.called) self.assertTrue(inst_mocks[0].destroy.called) - # Second instance is not created nor destroyed + self.assertTrue(build_req_mocks[0].create.called) + self.assertTrue(build_req_mocks[0].destroy.called) + self.assertTrue(inst_map_mocks[0].create.called) + self.assertTrue(inst_map_mocks[0].destroy.called) + # Second instance, build_req, mapping is not created nor destroyed self.assertFalse(inst_mocks[1].create.called) self.assertFalse(inst_mocks[1].destroy.called) + self.assertFalse(build_req_mocks[1].destroy.called) + self.assertFalse(inst_map_mocks[1].destroy.called) do_test()