Clean up instance mappings, build requests on quota failure

During the boot process there is a point where failures, most often
quota failures, cause the newly created instance to be deleted. In this
situation the BuildRequest and InstanceMapping object that were just
created should be deleted as well. If they are not then it's possible
that an instance list will return the BuildRequest.instance as a regular
instance though it should be deleted.

Change-Id: Ic2dd3bb7db3ce563a358bed03adaa37ff12c30fd
Partially-implements: bp add-buildrequest-ob

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