From bf0f5d475458a67a12000ff49a8c5285c3ac9e45 Mon Sep 17 00:00:00 2001 From: Mohammed Naser Date: Wed, 6 Sep 2017 15:19:01 -0400 Subject: [PATCH] Ensure instance mapping is updated in case of quota recheck fails If an instance fails to successfully pass the quota recheck, it will raise a TooManyInstances exception, however, it will not hit the code which saves the instance mapping, leaving an instance with no assigned cell in the mapping table and no BuildRequest as it is removed by _cleanup_build_artifacts. This patch adds a test to make sure that an instance has the correct cell mapping if it fails in the quota recheck phase. In addition, it uses the cell_mapping_cache dictionary to set the correct cell mapping before marking the instance as ERROR. Co-Authored-By: Dan Smith Co-Authored-By: Matt Riedemann Co-Authored-By: melanie witt Closes-Bug: #1715462 Change-Id: I7ecb5feb47a5f358cd51bde87b75a3a6141b5b12 --- nova/conductor/manager.py | 13 +++++++++++-- nova/tests/unit/conductor/test_conductor.py | 4 ++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index a6d94219cc51..1714126f2432 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -1005,7 +1005,8 @@ class ComputeTaskManager(base.Base): with excutils.save_and_reraise_exception(): self._cleanup_build_artifacts(context, exc, instances, build_requests, - request_specs) + request_specs, + cell_mapping_cache) for (build_request, request_spec, host, instance) in six.moves.zip( build_requests, request_specs, hosts, instances): @@ -1073,7 +1074,7 @@ class ComputeTaskManager(base.Base): limits=host['limits']) def _cleanup_build_artifacts(self, context, exc, instances, build_requests, - request_specs): + request_specs, cell_mapping_cache): for (instance, build_request, request_spec) in six.moves.zip( instances, build_requests, request_specs): # Skip placeholders that were buried in cell0 or had their @@ -1085,6 +1086,14 @@ class ComputeTaskManager(base.Base): self._set_vm_state_and_notify(context, instance.uuid, 'build_instances', updates, exc, legacy_spec) + + # TODO(mnaser): The cell mapping should already be populated by + # this point to avoid setting it below here. + inst_mapping = objects.InstanceMapping.get_by_instance_uuid( + context, instance.uuid) + inst_mapping.cell_mapping = cell_mapping_cache[instance.uuid] + inst_mapping.save() + # Be paranoid about artifacts being deleted underneath us. try: build_request.destroy() diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py index 1f1059636330..cd4cfd99a495 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -1803,6 +1803,10 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): self.assertIn('Quota exceeded', instance.fault.message) # Verify we removed the build objects. build_requests = objects.BuildRequestList.get_all(self.ctxt) + # Verify that the instance is mapped to a cell + inst_mapping = objects.InstanceMapping.get_by_instance_uuid( + self.ctxt, instance.uuid) + self.assertIsNotNone(inst_mapping.cell_mapping) self.assertEqual(0, len(build_requests))