Ensure build request exists before creating instance

When creating instances in conductor, the build requests are
coming from the compute API and might be stale by the time
the instance is created, i.e. the build request might have
been deleted from the database before the instance is actually
created in a cell.

This is trivial to recreate; all you need to do is create a
server and then immediately delete it, then try to perform
some kind of action on the server expecting it to be deleted
but the action might not return a 404 for a missing instance.
We're seeing this in Tempest runs where the expected 404 for
the deleted instance is a 409 because the test is trying to
perform an action on a server while it's building, which is
generally not allowed.

This fixes the issue by making a last-second check to make
sure the build request still exists before the instance is
created in a cell.

Change-Id: I6c32d5a4086a227d59ad7b1f6f50e7e532c74c84
Closes-Bug: #1660878

nova/conductor/manager.py

@@ -898,8 +898,20 @@ def schedule_and_build_instances(self, context, build_requests,
 
             cell = host_mapping.cell_mapping
 
-            with obj_target_cell(instance, cell):
-                instance.create()
+            # Before we create the instance, let's make one final check that
+            # the build request is still around and wasn't deleted by the user
+            # already.
+            try:
+                objects.BuildRequest.get_by_instance_uuid(
+                    context, instance.uuid)
+            except exception.BuildRequestNotFound:
+                # the build request is gone so we're done for this instance
+                LOG.debug('While scheduling instance, the build request '
+                          'was already deleted.', instance=instance)
+                continue
+            else:
+                with obj_target_cell(instance, cell):
+                    instance.create()
 
             # send a state update notification for the initial create to
             # show it going from non-existent to BUILDING

nova/tests/unit/conductor/test_conductor.py

@@ -1542,6 +1542,36 @@ def test_schedule_and_build_delete_during_scheduling(self, bury,
         self.assertFalse(bury.called)
         self.assertTrue(br_destroy.called)
 
+    @mock.patch('nova.compute.rpcapi.ComputeAPI.build_and_run_instance')
+    @mock.patch('nova.scheduler.rpcapi.SchedulerAPI.select_destinations')
+    @mock.patch('nova.objects.BuildRequest.get_by_instance_uuid')
+    @mock.patch('nova.objects.BuildRequest.destroy')
+    @mock.patch('nova.conductor.manager.ComputeTaskManager._bury_in_cell0')
+    @mock.patch('nova.objects.Instance.create')
+    def test_schedule_and_build_delete_before_scheduling(self, inst_create,
+                                                         bury, br_destroy,
+                                                         br_get_by_inst,
+                                                         select_destinations,
+                                                         build_and_run):
+        """Tests the case that the build request is deleted before the instance
+        is created, so we do not create the instance.
+        """
+        inst_uuid = self.params['build_requests'][0].instance.uuid
+        br_get_by_inst.side_effect = exc.BuildRequestNotFound(uuid=inst_uuid)
+        self.start_service('compute', host='fake-host')
+        select_destinations.return_value = [{'host': 'fake-host',
+                                             'nodename': 'nodesarestupid',
+                                             'limits': None}]
+        self.conductor.schedule_and_build_instances(**self.params)
+        # we don't create the instance since the build request is gone
+        self.assertFalse(inst_create.called)
+        # we don't build the instance since we didn't create it
+        self.assertFalse(build_and_run.called)
+        # we don't bury the instance in cell0 since it's already deleted
+        self.assertFalse(bury.called)
+        # we don't don't destroy the build request since it's already gone
+        self.assertFalse(br_destroy.called)
+
     @mock.patch('nova.compute.rpcapi.ComputeAPI.build_and_run_instance')
     @mock.patch('nova.scheduler.rpcapi.SchedulerAPI.select_destinations')
     @mock.patch('nova.objects.BuildRequest.destroy')