From 8ba92778fe14b47ad4ff5b53022e0550a93f37d3 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Wed, 1 Feb 2017 10:35:32 -0500 Subject: [PATCH] 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 | 16 +++++++++-- nova/tests/unit/conductor/test_conductor.py | 30 +++++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index 3d3b3e895e3b..776d3317b7e7 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -898,8 +898,20 @@ class ComputeTaskManager(base.Base): 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 diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py index 2bcdc0ab27cd..c4e159763f3e 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -1542,6 +1542,36 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): 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')