From 5fac17ae960d36eeb7a642725a37f82e8ca95ec1 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Fri, 26 May 2017 08:27:07 -0400 Subject: [PATCH] Use targeted context when burying instances in cell0 After Iccdf6b80f5fc8adcc8a89ce6ece3f37b6cbcaee2 we need to use the yielded context which is targeted to the cell when we do DB operations, which in this case is creating the instance in the cell0 database and then updating it's status. There is another place in here where this was missed, which is when we're trying to delete a build request which was already deleted. Closes-Bug: #1693438 Change-Id: I142f97d691fa55e9824714c9c224f998ad72337e --- nova/conductor/manager.py | 18 +++++++------ nova/tests/unit/conductor/test_conductor.py | 29 +++++++++++++++------ 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index 1f79e39ae56c..478df18974fb 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -883,12 +883,16 @@ class ComputeTaskManager(base.Base): updates = {'vm_state': vm_states.ERROR, 'task_state': None} legacy_spec = request_spec.to_legacy_request_spec_dict() for instance in instances_by_uuid.values(): - with obj_target_cell(instance, cell0): + with obj_target_cell(instance, cell0) as cctxt: instance.create() + # Use the context targeted to cell0 here since the instance is + # now in cell0. self._set_vm_state_and_notify( - context, instance.uuid, 'build_instances', updates, + cctxt, instance.uuid, 'build_instances', updates, exc, legacy_spec) try: + # We don't need the cell0-targeted context here because the + # instance mapping is in the API DB. inst_mapping = \ objects.InstanceMapping.get_by_instance_uuid( context, instance.uuid) @@ -988,7 +992,7 @@ class ComputeTaskManager(base.Base): inst_mapping.save() if not self._delete_build_request( - context, build_request, instance, cell, instance_bdms): + build_request, instance, cell, instance_bdms): # The build request was deleted before/during scheduling so # the instance is gone and we don't have anything to build for # this one. @@ -1013,15 +1017,13 @@ class ComputeTaskManager(base.Base): host=host['host'], node=host['nodename'], limits=host['limits']) - def _delete_build_request(self, context, build_request, instance, cell, + def _delete_build_request(self, build_request, instance, cell, instance_bdms): """Delete a build request after creating the instance in the cell. This method handles cleaning up the instance in case the build request is already deleted by the time we try to delete it. - :param context: the context of the request being handled - :type context: nova.context.RequestContext' :param build_request: the build request to delete :type build_request: nova.objects.BuildRequest :param instance: the instance created from the build_request @@ -1039,9 +1041,9 @@ class ComputeTaskManager(base.Base): # This indicates an instance deletion request has been # processed, and the build should halt here. Clean up the # bdm and instance record. - with obj_target_cell(instance, cell): + with obj_target_cell(instance, cell) as cctxt: with compute_utils.notify_about_instance_delete( - self.notifier, context, instance): + self.notifier, cctxt, instance): try: instance.destroy() except exception.InstanceNotFound: diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py index 2490c35b8c38..23622a5d41c7 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -1571,7 +1571,9 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): @mock.patch('nova.scheduler.rpcapi.SchedulerAPI.select_destinations') @mock.patch('nova.objects.BuildRequest.destroy') @mock.patch('nova.conductor.manager.ComputeTaskManager._bury_in_cell0') - def test_schedule_and_build_delete_during_scheduling(self, bury, + @mock.patch('nova.conductor.manager.obj_target_cell') + def test_schedule_and_build_delete_during_scheduling(self, target_cell, + bury, br_destroy, select_destinations, build_and_run, @@ -1582,6 +1584,9 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): select_destinations.return_value = [{'host': 'fake-host', 'nodename': 'nodesarestupid', 'limits': None}] + targeted_context = mock.MagicMock( + autospec='nova.context.RequestContext') + target_cell.return_value.__enter__.return_value = targeted_context self.conductor.schedule_and_build_instances(**self.params) self.assertFalse(build_and_run.called) self.assertFalse(bury.called) @@ -1589,7 +1594,7 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): test_utils.assert_instance_delete_notification_by_uuid( notify, self.params['build_requests'][0].instance_uuid, - self.conductor.notifier, self.ctxt) + self.conductor.notifier, targeted_context) @mock.patch('nova.objects.Instance.destroy') @mock.patch('nova.compute.utils.notify_about_instance_usage') @@ -1597,9 +1602,10 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): @mock.patch('nova.scheduler.rpcapi.SchedulerAPI.select_destinations') @mock.patch('nova.objects.BuildRequest.destroy') @mock.patch('nova.conductor.manager.ComputeTaskManager._bury_in_cell0') + @mock.patch('nova.conductor.manager.obj_target_cell') def test_schedule_and_build_delete_during_scheduling_host_changed( - self, bury, br_destroy, select_destinations, build_and_run, - notify, instance_destroy): + self, target_cell, bury, br_destroy, select_destinations, + build_and_run, notify, instance_destroy): br_destroy.side_effect = exc.BuildRequestNotFound(uuid='foo') instance_destroy.side_effect = [ @@ -1612,6 +1618,9 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): select_destinations.return_value = [{'host': 'fake-host', 'nodename': 'nodesarestupid', 'limits': None}] + targeted_context = mock.MagicMock( + autospec='nova.context.RequestContext') + target_cell.return_value.__enter__.return_value = targeted_context self.conductor.schedule_and_build_instances(**self.params) self.assertFalse(build_and_run.called) self.assertFalse(bury.called) @@ -1619,7 +1628,7 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): self.assertEqual(2, instance_destroy.call_count) test_utils.assert_instance_delete_notification_by_uuid( notify, self.params['build_requests'][0].instance_uuid, - self.conductor.notifier, self.ctxt) + self.conductor.notifier, targeted_context) @mock.patch('nova.objects.Instance.destroy') @mock.patch('nova.compute.utils.notify_about_instance_usage') @@ -1627,9 +1636,10 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): @mock.patch('nova.scheduler.rpcapi.SchedulerAPI.select_destinations') @mock.patch('nova.objects.BuildRequest.destroy') @mock.patch('nova.conductor.manager.ComputeTaskManager._bury_in_cell0') + @mock.patch('nova.conductor.manager.obj_target_cell') def test_schedule_and_build_delete_during_scheduling_instance_not_found( - self, bury, br_destroy, select_destinations, build_and_run, - notify, instance_destroy): + self, target_cell, bury, br_destroy, select_destinations, + build_and_run, notify, instance_destroy): br_destroy.side_effect = exc.BuildRequestNotFound(uuid='foo') instance_destroy.side_effect = [ @@ -1641,6 +1651,9 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): select_destinations.return_value = [{'host': 'fake-host', 'nodename': 'nodesarestupid', 'limits': None}] + targeted_context = mock.MagicMock( + autospec='nova.context.RequestContext') + target_cell.return_value.__enter__.return_value = targeted_context self.conductor.schedule_and_build_instances(**self.params) self.assertFalse(build_and_run.called) self.assertFalse(bury.called) @@ -1648,7 +1661,7 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): self.assertEqual(1, instance_destroy.call_count) test_utils.assert_instance_delete_notification_by_uuid( notify, self.params['build_requests'][0].instance_uuid, - self.conductor.notifier, self.ctxt) + self.conductor.notifier, targeted_context) @mock.patch('nova.compute.rpcapi.ComputeAPI.build_and_run_instance') @mock.patch('nova.scheduler.rpcapi.SchedulerAPI.select_destinations')