From 8862b2de236cccf5d0378f485acf90eab5c8c77a Mon Sep 17 00:00:00 2001 From: Mohammed Naser Date: Wed, 14 Feb 2018 18:39:38 -0500 Subject: [PATCH] Store block device mappings in cell0 If an instance fails to get scheduled, it gets buried in cell0 but none of it's block device mappings are stored. At the API layer, Nova reserves and creates attachments for new instances when it gets a create request so these attachments are orphaned if the block device mappings are not registered in the database somewhere. This patch makes sure that if an instance is being buried in cell0, all of it's block device mappings are recorded as well so they can be later removed when the instance is deleted. Change-Id: I64074923fb741fbf5459f66b8ab1a23c16f3303f Related-Bug: #1404867 (cherry picked from commit ad9e2a568fcef4f69b185952352db9f651c37ad4) --- nova/conductor/manager.py | 21 +++++++++++++---- nova/tests/unit/conductor/test_conductor.py | 26 ++++++++++++++++++++- 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index f0327422fd73..c0f99fb60756 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -1034,7 +1034,8 @@ class ComputeTaskManager(base.Base): return tags def _bury_in_cell0(self, context, request_spec, exc, - build_requests=None, instances=None): + build_requests=None, instances=None, + block_device_mapping=None): """Ensure all provided build_requests and instances end up in cell0. Cell0 is the fake cell we schedule dead instances to when we can't @@ -1070,6 +1071,14 @@ class ComputeTaskManager(base.Base): for instance in instances_by_uuid.values(): with obj_target_cell(instance, cell0) as cctxt: instance.create() + + # NOTE(mnaser): In order to properly clean-up volumes after + # being buried in cell0, we need to store BDMs. + if block_device_mapping: + self._create_block_device_mapping( + cell0, instance.flavor, instance.uuid, + block_device_mapping) + # Use the context targeted to cell0 here since the instance is # now in cell0. self._set_vm_state_and_notify( @@ -1108,7 +1117,8 @@ class ComputeTaskManager(base.Base): except Exception as exc: LOG.exception('Failed to schedule instances') self._bury_in_cell0(context, request_specs[0], exc, - build_requests=build_requests) + build_requests=build_requests, + block_device_mapping=block_device_mapping) return host_mapping_cache = {} @@ -1131,9 +1141,10 @@ class ComputeTaskManager(base.Base): LOG.error('No host-to-cell mapping found for selected ' 'host %(host)s. Setup is incomplete.', {'host': host.service_host}) - self._bury_in_cell0(context, request_spec, exc, - build_requests=[build_request], - instances=[instance]) + self._bury_in_cell0( + context, request_spec, exc, + build_requests=[build_request], instances=[instance], + block_device_mapping=block_device_mapping) # This is a placeholder in case the quota recheck fails. instances.append(None) continue diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py index 4a69918b3e7c..4662e60c9ffe 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -1859,12 +1859,15 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): select_dest, build_and_run): def _fake_bury(ctxt, request_spec, exc, - build_requests=None, instances=None): + build_requests=None, instances=None, + block_device_mapping=None): self.assertIn('not mapped to any cell', str(exc)) self.assertEqual(1, len(build_requests)) self.assertEqual(1, len(instances)) self.assertEqual(build_requests[0].instance_uuid, instances[0].uuid) + self.assertEqual(self.params['block_device_mapping'], + block_device_mapping) bury.side_effect = _fake_bury select_dest.return_value = [[fake_selection1]] @@ -2005,6 +2008,27 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): self.assertEqual(expected, inst_states) + @mock.patch.object(objects.CellMapping, 'get_by_uuid') + @mock.patch.object(conductor_manager.ComputeTaskManager, + '_create_block_device_mapping') + def test_bury_in_cell0_with_block_device_mapping(self, mock_create_bdm, + mock_get_cell): + mock_get_cell.return_value = self.cell_mappings['cell0'] + + inst_br = fake_build_request.fake_req_obj(self.ctxt) + del inst_br.instance.id + inst_br.create() + inst = inst_br.get_new_instance(self.ctxt) + + self.conductor._bury_in_cell0( + self.ctxt, self.params['request_specs'][0], Exception('Foo'), + build_requests=[inst_br], instances=[inst], + block_device_mapping=self.params['block_device_mapping']) + + mock_create_bdm.assert_called_once_with( + self.cell_mappings['cell0'], inst.flavor, inst.uuid, + self.params['block_device_mapping']) + def test_reset(self): with mock.patch('nova.compute.rpcapi.ComputeAPI') as mock_rpc: old_rpcapi = self.conductor_manager.compute_rpcapi