diff --git a/nova/compute/api.py b/nova/compute/api.py index e52d5994b705..a43183d007f7 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -2439,33 +2439,9 @@ class API(base.Base): def _get_instance(self, context, instance_uuid, expected_attrs, cell_down_support=False): - # Before service version 15 the BuildRequest is not cleaned up during - # a delete request so there is no reason to look it up here as we can't - # trust that it's not referencing a deleted instance. Also even if - # there is an instance mapping we don't need to honor it for older - # service versions. - service_version = objects.Service.get_minimum_version( - context, 'nova-osapi_compute') - # If we're on cellsv1, we also need to consult the top-level - # merged replica instead of the cell directly, so fall through - # here in that case as well. - if service_version < 15 or CONF.cells.enable: - # If not using cells v1, we need to log a warning about the API - # service version being less than 15 (that check was added in - # newton), which indicates there is some lingering data during the - # transition to cells v2 which could cause an InstanceNotFound - # here. The warning message is a sort of breadcrumb. - # This can all go away once we drop cells v1 and assert that all - # deployments have upgraded from a base cells v2 setup with - # mappings. - if not CONF.cells.enable: - LOG.warning('The nova-osapi_compute service version is from ' - 'before Ocata and may cause problems looking up ' - 'instances in a cells v2 setup. Check your ' - 'nova-api service configuration and cell ' - 'mappings. You may need to remove stale ' - 'nova-osapi_compute service records from the cell ' - 'database.') + # If we're on cellsv1, we need to consult the top-level + # merged replica instead of the cell directly. + if CONF.cells.enable: return objects.Instance.get_by_uuid(context, instance_uuid, expected_attrs=expected_attrs) inst_map = self._get_instance_map_or_none(context, instance_uuid) @@ -2491,6 +2467,13 @@ class API(base.Base): else: raise exception.InstanceNotFound(instance_id=instance_uuid) else: + # If we got here, we don't have an instance mapping, but we aren't + # sure why. The instance mapping might be missing because the + # upgrade is incomplete (map_instances wasn't run). Or because the + # instance was deleted and the DB was archived at which point the + # mapping is deleted. The former case is bad, but because of the + # latter case we can't really log any kind of warning/error here + # since it might be normal. raise exception.InstanceNotFound(instance_id=instance_uuid) return instance diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 24f301fe67ff..633e2efc21a2 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -284,9 +284,10 @@ class BaseTestCase(test.TestCase): return fake_instance.fake_instance_obj(None, **updates) def _create_fake_instance_obj(self, params=None, type_name='m1.tiny', - services=False, context=None): + services=False, ctxt=None): + ctxt = ctxt or self.context flavor = flavors.get_flavor_by_name(type_name) - inst = objects.Instance(context=context or self.context) + inst = objects.Instance(context=ctxt) inst.vm_state = vm_states.ACTIVE inst.task_state = None inst.power_state = power_state.RUNNING @@ -321,7 +322,18 @@ class BaseTestCase(test.TestCase): if services: _create_service_entries(self.context.elevated(), [['fake_zone', [inst.host]]]) - inst.create() + cell1 = self.cell_mappings[test.CELL1_NAME] + with context.target_cell(ctxt, cell1) as cctxt: + inst._context = cctxt + inst.create() + + # Create an instance mapping in cell1 so the API can get the instance. + inst_map = objects.InstanceMapping( + ctxt, + instance_uuid=inst.uuid, + project_id=inst.project_id, + cell_mapping=cell1) + inst_map.create() return inst @@ -6270,7 +6282,7 @@ class ComputeTestCase(BaseTestCase, c = context.get_admin_context() params = {'info_cache': objects.InstanceInfoCache( network_info=network_model.NetworkInfo([]))} - instance = self._create_fake_instance_obj(params=params, context=c) + instance = self._create_fake_instance_obj(params=params, ctxt=c) instance.host = self.compute.host dest = 'desthost' @@ -6326,7 +6338,7 @@ class ComputeTestCase(BaseTestCase, c = context.get_admin_context() params = {'info_cache': objects.InstanceInfoCache( network_info=network_model.NetworkInfo([]))} - instance = self._create_fake_instance_obj(params=params, context=c) + instance = self._create_fake_instance_obj(params=params, ctxt=c) instance.host = self.compute.host dest = 'desthost' @@ -6430,7 +6442,7 @@ class ComputeTestCase(BaseTestCase, 'host': srchost, 'state_description': 'migrating', 'state': power_state.PAUSED}, - context=c) + ctxt=c) instance.update({'task_state': task_states.MIGRATING, 'power_state': power_state.PAUSED}) @@ -6510,7 +6522,7 @@ class ComputeTestCase(BaseTestCase, 'node': srcnode, 'state_description': 'migrating', 'state': power_state.PAUSED}, - context=c) + ctxt=c) instance.update({'task_state': task_states.MIGRATING, 'power_state': power_state.PAUSED}) @@ -6561,7 +6573,7 @@ class ComputeTestCase(BaseTestCase, 'host': self.compute.host, 'state_description': 'migrating', 'state': power_state.PAUSED}, - context=c) + ctxt=c) instance.update({'task_state': task_states.MIGRATING, 'power_state': power_state.PAUSED}) instance.save() diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index 1eed3f97eb3c..dca76b475fa9 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -5856,12 +5856,11 @@ class _ComputeAPIUnitTestMixIn(object): 'security_groups', 'info_cache']) - @mock.patch.object(objects.Service, 'get_minimum_version', return_value=15) @mock.patch.object(objects.InstanceMapping, 'get_by_instance_uuid') @mock.patch.object(objects.BuildRequest, 'get_by_instance_uuid') @mock.patch.object(objects.Instance, 'get_by_uuid') def test_get_instance_not_in_cell(self, mock_get_inst, mock_get_build_req, - mock_get_inst_map, mock_get_min_service): + mock_get_inst_map): build_req_obj = fake_build_request.fake_req_obj(self.context) mock_get_inst_map.return_value = objects.InstanceMapping( cell_mapping=None) @@ -5881,8 +5880,6 @@ class _ComputeAPIUnitTestMixIn(object): expected_attrs=['metadata', 'system_metadata', 'security_groups', 'info_cache']) self.assertEqual(instance, inst_from_build_req) - mock_get_min_service.assert_called_once_with(self.context, - 'nova-osapi_compute') @mock.patch.object(objects.InstanceMapping, 'get_by_instance_uuid') @mock.patch.object(objects.BuildRequest, 'get_by_instance_uuid')