Merge "Clean up instance mappings, build requests on quota failure"

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()
 

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()