From ef5aa8a818b18e4882b4ab5093c8f7db2ad0f7d1 Mon Sep 17 00:00:00 2001 From: Surya Seetharaman Date: Tue, 14 Aug 2018 14:17:12 +0200 Subject: [PATCH] Minimal construct plumbing for nova show when a cell is down This patch sets the stage for modifying the behavior of nova show which currently gives a 500 when the cell in which the instance lives is down. The new behavior will return a partial construct consisting of uuid, project_id, created_at from instance_mappings table and user_id, flavor, image_ref and availability_zone info from request_specs table. Note that the rest of the keys will be missing. This behavior will be enabled by passing a new enough microversion, handling for which is introduced later in this series. Related to blueprint handling-down-cell Change-Id: Iaea1cb4ed93bb98f451de4f993106d7891ca3682 --- nova/api/openstack/common.py | 6 +- nova/api/openstack/compute/servers.py | 3 +- nova/compute/api.py | 80 ++++++++++-- .../api_sample_tests/test_volumes.py | 3 +- .../compute/admin_only_action_common.py | 18 ++- .../openstack/compute/test_admin_password.py | 3 +- .../compute/test_attach_interfaces.py | 18 ++- .../openstack/compute/test_console_output.py | 3 +- .../openstack/compute/test_deferred_delete.py | 15 ++- .../openstack/compute/test_floating_ips.py | 6 +- .../compute/test_instance_actions.py | 12 +- .../api/openstack/compute/test_lock_server.py | 3 +- .../openstack/compute/test_migrate_server.py | 15 ++- .../api/openstack/compute/test_multinic.py | 3 +- .../openstack/compute/test_server_actions.py | 3 +- .../compute/test_server_diagnostics.py | 3 +- .../compute/test_server_migrations.py | 21 ++-- .../compute/test_server_reset_state.py | 9 +- .../api/openstack/compute/test_serversV21.py | 9 +- .../api/openstack/compute/test_volumes.py | 3 +- nova/tests/unit/api/openstack/fakes.py | 3 +- nova/tests/unit/compute/test_compute_api.py | 115 ++++++++++++++++-- 22 files changed, 276 insertions(+), 78 deletions(-) diff --git a/nova/api/openstack/common.py b/nova/api/openstack/common.py index 7ac2e000db65..d98387f09397 100644 --- a/nova/api/openstack/common.py +++ b/nova/api/openstack/common.py @@ -460,11 +460,13 @@ class ViewBuilder(object): return self._update_link_prefix(orig_url, CONF.api.compute_link_prefix) -def get_instance(compute_api, context, instance_id, expected_attrs=None): +def get_instance(compute_api, context, instance_id, expected_attrs=None, + cell_down_support=False): """Fetch an instance from the compute API, handling error checking.""" try: return compute_api.get(context, instance_id, - expected_attrs=expected_attrs) + expected_attrs=expected_attrs, + cell_down_support=cell_down_support) except exception.InstanceNotFound as e: raise exc.HTTPNotFound(explanation=e.format_message()) diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index 8f3a772598bc..d8b874edd9a3 100644 --- a/nova/api/openstack/compute/servers.py +++ b/nova/api/openstack/compute/servers.py @@ -307,7 +307,8 @@ class ServersController(wsgi.Controller): expected_attrs) instance = common.get_instance(self.compute_api, context, instance_uuid, - expected_attrs=expected_attrs) + expected_attrs=expected_attrs, + cell_down_support=False) return instance @staticmethod diff --git a/nova/compute/api.py b/nova/compute/api.py index e10d098f303a..34c096a21c2f 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -2387,7 +2387,58 @@ class API(base.Base): inst_map = None return inst_map - def _get_instance(self, context, instance_uuid, expected_attrs): + def _get_instance_from_cell(self, context, im, expected_attrs, + cell_down_support): + # NOTE(danms): Even though we're going to scatter/gather to the + # right cell, other code depends on this being force targeted when + # the get call returns. + nova_context.set_target_cell(context, im.cell_mapping) + + uuid = im.instance_uuid + result = nova_context.scatter_gather_single_cell(context, + im.cell_mapping, objects.Instance.get_by_uuid, uuid, + expected_attrs=expected_attrs) + cell_uuid = im.cell_mapping.uuid + if not nova_context.is_cell_failure_sentinel(result[cell_uuid]): + return result[cell_uuid] + elif isinstance(result[cell_uuid], exception.InstanceNotFound): + raise exception.InstanceNotFound(instance_id=uuid) + elif cell_down_support: + if im.queued_for_delete: + # should be treated like deleted instance. + raise exception.InstanceNotFound(instance_id=uuid) + + # instance in down cell, return a minimal construct + LOG.warning("Cell %s is not responding and hence only " + "partial results are available from this " + "cell.", cell_uuid) + try: + rs = objects.RequestSpec.get_by_instance_uuid(context, + uuid) + # For BFV case, we could have rs.image but rs.image.id might + # still not be set. So we check the existence of both image + # and its id. + image_ref = (rs.image.id if rs.image and + 'id' in rs.image else None) + return objects.Instance(context=context, power_state=0, + uuid=uuid, + project_id=im.project_id, + created_at=im.created_at, + user_id=rs.user_id, + flavor=rs.flavor, + image_ref=image_ref, + availability_zone=rs.availability_zone) + except exception.RequestSpecNotFound: + # could be that a deleted instance whose request + # spec has been archived is being queried. + raise exception.InstanceNotFound(instance_id=uuid) + else: + raise exception.NovaException( + _("Cell %s is not responding and hence instance " + "info is not available.") % cell_uuid) + + 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 @@ -2419,9 +2470,8 @@ class API(base.Base): expected_attrs=expected_attrs) inst_map = self._get_instance_map_or_none(context, instance_uuid) if inst_map and (inst_map.cell_mapping is not None): - nova_context.set_target_cell(context, inst_map.cell_mapping) - instance = objects.Instance.get_by_uuid( - context, instance_uuid, expected_attrs=expected_attrs) + instance = self._get_instance_from_cell(context, inst_map, + expected_attrs, cell_down_support) elif inst_map and (inst_map.cell_mapping is None): # This means the instance has not been scheduled and put in # a cell yet. For now it also may mean that the deployer @@ -2436,11 +2486,8 @@ class API(base.Base): inst_map = self._get_instance_map_or_none(context, instance_uuid) if inst_map and (inst_map.cell_mapping is not None): - nova_context.set_target_cell(context, - inst_map.cell_mapping) - instance = objects.Instance.get_by_uuid( - context, instance_uuid, - expected_attrs=expected_attrs) + instance = self._get_instance_from_cell(context, inst_map, + expected_attrs, cell_down_support) else: raise exception.InstanceNotFound(instance_id=instance_uuid) else: @@ -2448,8 +2495,17 @@ class API(base.Base): return instance - def get(self, context, instance_id, expected_attrs=None): - """Get a single instance with the given instance_id.""" + def get(self, context, instance_id, expected_attrs=None, + cell_down_support=False): + """Get a single instance with the given instance_id. + + :param cell_down_support: True if the API (and caller) support + returning a minimal instance + construct if the relevant cell is + down. If False, an error is raised + since the instance cannot be retrieved + due to the cell being down. + """ if not expected_attrs: expected_attrs = [] expected_attrs.extend(['metadata', 'system_metadata', @@ -2461,7 +2517,7 @@ class API(base.Base): instance_uuid=instance_id) instance = self._get_instance(context, instance_id, - expected_attrs) + expected_attrs, cell_down_support=cell_down_support) else: LOG.debug("Failed to fetch instance by id %s", instance_id) raise exception.InstanceNotFound(instance_id=instance_id) diff --git a/nova/tests/functional/api_sample_tests/test_volumes.py b/nova/tests/functional/api_sample_tests/test_volumes.py index 7fa7848e8437..63893c26f6ff 100644 --- a/nova/tests/functional/api_sample_tests/test_volumes.py +++ b/nova/tests/functional/api_sample_tests/test_volumes.py @@ -238,7 +238,8 @@ class VolumeAttachmentsSample(test_servers.ServersSampleBase): def _stub_compute_api_get(self): def fake_compute_api_get(self, context, instance_id, - expected_attrs=None): + expected_attrs=None, + cell_down_support=False): return fake_instance.fake_instance_obj( context, **{'uuid': instance_id}) diff --git a/nova/tests/unit/api/openstack/compute/admin_only_action_common.py b/nova/tests/unit/api/openstack/compute/admin_only_action_common.py index ce7136f7b547..95c23cc64a12 100644 --- a/nova/tests/unit/api/openstack/compute/admin_only_action_common.py +++ b/nova/tests/unit/api/openstack/compute/admin_only_action_common.py @@ -57,7 +57,8 @@ class CommonMixin(object): controller_function, self.req, uuid, body=body_map) self.mock_get.assert_called_once_with(self.context, uuid, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) def _test_action(self, action, body=None, method=None, compute_api_args_map=None): @@ -84,7 +85,8 @@ class CommonMixin(object): mock_method.assert_called_once_with(self.context, instance, *args, **kwargs) self.mock_get.assert_called_once_with(self.context, instance.uuid, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) def _test_not_implemented_state(self, action, method=None): # Reset the mock. @@ -108,7 +110,8 @@ class CommonMixin(object): mock_method.assert_called_once_with(self.context, instance, *args, **kwargs) self.mock_get.assert_called_once_with(self.context, instance.uuid, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) def _test_invalid_state(self, action, method=None, body_map=None, compute_api_args_map=None, @@ -143,7 +146,8 @@ class CommonMixin(object): mock_method.assert_called_once_with(self.context, instance, *args, **kwargs) self.mock_get.assert_called_once_with(self.context, instance.uuid, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) def _test_locked_instance(self, action, method=None, body=None, compute_api_args_map=None): @@ -169,7 +173,8 @@ class CommonMixin(object): mock_method.assert_called_once_with(self.context, instance, *args, **kwargs) self.mock_get.assert_called_once_with(self.context, instance.uuid, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) def _test_instance_not_found_in_compute_api(self, action, method=None, body=None, compute_api_args_map=None): @@ -195,7 +200,8 @@ class CommonMixin(object): mock_method.assert_called_once_with(self.context, instance, *args, **kwargs) self.mock_get.assert_called_once_with(self.context, instance.uuid, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) class CommonTests(CommonMixin, test.NoDBTestCase): diff --git a/nova/tests/unit/api/openstack/compute/test_admin_password.py b/nova/tests/unit/api/openstack/compute/test_admin_password.py index 715c42407df9..0a274b243674 100644 --- a/nova/tests/unit/api/openstack/compute/test_admin_password.py +++ b/nova/tests/unit/api/openstack/compute/test_admin_password.py @@ -23,7 +23,8 @@ from nova.tests.unit.api.openstack import fakes from nova.tests.unit import fake_instance -def fake_get(self, context, id, expected_attrs=None): +def fake_get(self, context, id, expected_attrs=None, + cell_down_support=False): return fake_instance.fake_instance_obj( context, uuid=id, diff --git a/nova/tests/unit/api/openstack/compute/test_attach_interfaces.py b/nova/tests/unit/api/openstack/compute/test_attach_interfaces.py index 3b13cd78e0d0..e730773e0d29 100644 --- a/nova/tests/unit/api/openstack/compute/test_attach_interfaces.py +++ b/nova/tests/unit/api/openstack/compute/test_attach_interfaces.py @@ -357,7 +357,8 @@ class InterfaceAttachTestsV21(test.NoDBTestCase): attach_mock.assert_called_once_with(ctxt, fake_instance, None, None, None, tag=None) get_mock.assert_called_once_with(ctxt, FAKE_UUID1, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) @mock.patch.object(compute_api.API, 'get') @mock.patch.object(compute_api.API, 'attach_interface') @@ -376,7 +377,8 @@ class InterfaceAttachTestsV21(test.NoDBTestCase): attach_mock.assert_called_once_with(ctxt, fake_instance, None, None, None, tag=None) get_mock.assert_called_once_with(ctxt, FAKE_UUID1, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) @mock.patch.object(compute_api.API, 'get') @mock.patch.object(compute_api.API, 'attach_interface') @@ -396,7 +398,8 @@ class InterfaceAttachTestsV21(test.NoDBTestCase): attach_mock.assert_called_once_with(ctxt, fake_instance, None, None, None, tag=None) get_mock.assert_called_once_with(ctxt, FAKE_UUID1, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) @mock.patch.object(compute_api.API, 'get') @mock.patch.object(compute_api.API, 'attach_interface') @@ -412,7 +415,8 @@ class InterfaceAttachTestsV21(test.NoDBTestCase): attach_mock.assert_called_once_with(ctxt, fake_instance, None, None, None, tag=None) get_mock.assert_called_once_with(ctxt, FAKE_UUID1, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) @mock.patch.object(compute_api.API, 'get') @mock.patch.object(compute_api.API, 'attach_interface') @@ -431,7 +435,8 @@ class InterfaceAttachTestsV21(test.NoDBTestCase): attach_mock.assert_called_once_with(ctxt, fake_instance, None, None, None, tag=None) get_mock.assert_called_once_with(ctxt, FAKE_UUID1, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) @mock.patch.object(compute_api.API, 'get') @mock.patch.object(compute_api.API, 'attach_interface') @@ -448,7 +453,8 @@ class InterfaceAttachTestsV21(test.NoDBTestCase): attach_mock.assert_called_once_with(ctxt, fake_instance, None, None, None, tag=None) get_mock.assert_called_once_with(ctxt, FAKE_UUID1, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) def _test_attach_interface_with_invalid_parameter(self, param): self.stub_out('nova.compute.api.API.attach_interface', diff --git a/nova/tests/unit/api/openstack/compute/test_console_output.py b/nova/tests/unit/api/openstack/compute/test_console_output.py index ef0f04af04e6..1953b04ea8ac 100644 --- a/nova/tests/unit/api/openstack/compute/test_console_output.py +++ b/nova/tests/unit/api/openstack/compute/test_console_output.py @@ -48,7 +48,8 @@ def fake_get_console_output_all_characters(self, _ctx, _instance, _tail_len): return string.printable -def fake_get(self, context, instance_uuid, expected_attrs=None): +def fake_get(self, context, instance_uuid, expected_attrs=None, + cell_down_support=False): return fake_instance.fake_instance_obj(context, **{'uuid': instance_uuid}) diff --git a/nova/tests/unit/api/openstack/compute/test_deferred_delete.py b/nova/tests/unit/api/openstack/compute/test_deferred_delete.py index 2d58291cc38c..5e1e401ec139 100644 --- a/nova/tests/unit/api/openstack/compute/test_deferred_delete.py +++ b/nova/tests/unit/api/openstack/compute/test_deferred_delete.py @@ -60,7 +60,8 @@ class DeferredDeleteExtensionTestV21(test.NoDBTestCase): mock_get.assert_called_once_with(self.fake_context, self.fake_uuid, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) mock_force_delete.assert_called_once_with(self.fake_context, instance) @@ -77,7 +78,8 @@ class DeferredDeleteExtensionTestV21(test.NoDBTestCase): mock_get.assert_called_once_with(self.fake_context, self.fake_uuid, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) @mock.patch.object(compute_api.API, 'get') @mock.patch.object(compute_api.API, 'force_delete', @@ -134,7 +136,8 @@ class DeferredDeleteExtensionTestV21(test.NoDBTestCase): mock_get.assert_called_once_with(self.fake_context, self.fake_uuid, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) mock_restore.assert_called_once_with(self.fake_context, instance) @@ -149,7 +152,8 @@ class DeferredDeleteExtensionTestV21(test.NoDBTestCase): mock_get.assert_called_once_with(self.fake_context, self.fake_uuid, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) @mock.patch.object(compute_api.API, 'get') @mock.patch.object(compute_api.API, 'restore') @@ -167,7 +171,8 @@ class DeferredDeleteExtensionTestV21(test.NoDBTestCase): mock_get.assert_called_once_with(self.fake_context, self.fake_uuid, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) mock_restore.assert_called_once_with(self.fake_context, instance) diff --git a/nova/tests/unit/api/openstack/compute/test_floating_ips.py b/nova/tests/unit/api/openstack/compute/test_floating_ips.py index e3e07e7138d4..9071f97a9e56 100644 --- a/nova/tests/unit/api/openstack/compute/test_floating_ips.py +++ b/nova/tests/unit/api/openstack/compute/test_floating_ips.py @@ -61,7 +61,8 @@ def network_api_get_floating_ips_by_project(self, context): 'fixed_ip': None}] -def compute_api_get(self, context, instance_id, expected_attrs=None): +def compute_api_get(self, context, instance_id, expected_attrs=None, + cell_down_support=False): return objects.Instance(uuid=FAKE_UUID, id=instance_id, instance_type_id=1, host='bob') @@ -672,7 +673,8 @@ class FloatingIpTestV21(test.TestCase): def test_floating_ip_associate_invalid_instance(self): - def fake_get(self, context, id, expected_attrs=None): + def fake_get(self, context, id, expected_attrs=None, + cell_down_support=False): raise exception.InstanceNotFound(instance_id=id) self.stubs.Set(compute.api.API, "get", fake_get) diff --git a/nova/tests/unit/api/openstack/compute/test_instance_actions.py b/nova/tests/unit/api/openstack/compute/test_instance_actions.py index 9865b9d03391..aabde1db4a73 100644 --- a/nova/tests/unit/api/openstack/compute/test_instance_actions.py +++ b/nova/tests/unit/api/openstack/compute/test_instance_actions.py @@ -126,7 +126,8 @@ class InstanceActionsTestV21(test.NoDBTestCase): expect_event_hostId = False expect_event_host = False - def fake_get(self, context, instance_uuid, expected_attrs=None): + def fake_get(self, context, instance_uuid, expected_attrs=None, + cell_down_support=False): return objects.Instance(uuid=instance_uuid) def setUp(self): @@ -248,7 +249,8 @@ class InstanceActionsTestV21(test.NoDBTestCase): self.assertRaises(exc.HTTPNotFound, self.controller.index, req, FAKE_UUID) self.mock_get.assert_called_once_with(req.environ['nova.context'], - FAKE_UUID, expected_attrs=None) + FAKE_UUID, expected_attrs=None, + cell_down_support=False) def test_show_instance_not_found(self): self.mock_get.side_effect = exception.InstanceNotFound( @@ -257,13 +259,15 @@ class InstanceActionsTestV21(test.NoDBTestCase): self.assertRaises(exc.HTTPNotFound, self.controller.show, req, FAKE_UUID, 'fake') self.mock_get.assert_called_once_with(req.environ['nova.context'], - FAKE_UUID, expected_attrs=None) + FAKE_UUID, expected_attrs=None, + cell_down_support=False) class InstanceActionsTestV221(InstanceActionsTestV21): wsgi_api_version = "2.21" - def fake_get(self, context, instance_uuid, expected_attrs=None): + def fake_get(self, context, instance_uuid, expected_attrs=None, + cell_down_support=False): self.assertEqual('yes', context.read_deleted) return objects.Instance(uuid=instance_uuid) diff --git a/nova/tests/unit/api/openstack/compute/test_lock_server.py b/nova/tests/unit/api/openstack/compute/test_lock_server.py index ac2c8ef25fda..b7b70de1622c 100644 --- a/nova/tests/unit/api/openstack/compute/test_lock_server.py +++ b/nova/tests/unit/api/openstack/compute/test_lock_server.py @@ -58,7 +58,8 @@ class LockServerTestsV21(admin_only_action_common.CommonTests): self.req, instance.uuid, body) mock_unlock.assert_called_once_with(self.context, instance) self.mock_get.assert_called_once_with(self.context, instance.uuid, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) @mock.patch.object(common, 'get_instance') def test_unlock_override_not_authorized_with_non_admin_user( diff --git a/nova/tests/unit/api/openstack/compute/test_migrate_server.py b/nova/tests/unit/api/openstack/compute/test_migrate_server.py index b063534c5ad1..69f42f821538 100644 --- a/nova/tests/unit/api/openstack/compute/test_migrate_server.py +++ b/nova/tests/unit/api/openstack/compute/test_migrate_server.py @@ -127,7 +127,8 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests): mock_resize.assert_called_once_with( self.context, instance, host_name=self.host_name) self.mock_get.assert_called_once_with(self.context, instance.uuid, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) def test_migrate_too_many_instances(self): exc_info = exception.TooManyInstances(overs='', req='', used=0, @@ -149,7 +150,8 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests): 'hostname', self.force, self.async_) self.mock_get.assert_called_once_with(self.context, instance.uuid, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) def test_migrate_live_enabled(self): param = self._get_params(host='hostname') @@ -226,7 +228,8 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests): self.context, instance, False, self.disk_over_commit, 'hostname', self.force, self.async_) self.mock_get.assert_called_once_with(self.context, instance.uuid, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) def test_migrate_live_compute_service_unavailable(self): self._test_migrate_live_failed_with_exception( @@ -443,7 +446,8 @@ class MigrateServerTestsV234(MigrateServerTestsV230): self.context, instance, None, self.disk_over_commit, 'hostname', self.force, self.async_) self.mock_get.assert_called_once_with(self.context, instance.uuid, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) def test_migrate_live_unexpected_error(self): body = {'os-migrateLive': @@ -461,7 +465,8 @@ class MigrateServerTestsV234(MigrateServerTestsV230): self.context, instance, None, self.disk_over_commit, 'hostname', self.force, self.async_) self.mock_get.assert_called_once_with(self.context, instance.uuid, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) class MigrateServerTestsV256(MigrateServerTestsV234): diff --git a/nova/tests/unit/api/openstack/compute/test_multinic.py b/nova/tests/unit/api/openstack/compute/test_multinic.py index 148d8906ed76..bc98d877c720 100644 --- a/nova/tests/unit/api/openstack/compute/test_multinic.py +++ b/nova/tests/unit/api/openstack/compute/test_multinic.py @@ -41,7 +41,8 @@ def compute_api_remove_fixed_ip(self, context, instance, address): last_remove_fixed_ip = (instance['uuid'], address) -def compute_api_get(self, context, instance_id, expected_attrs=None): +def compute_api_get(self, context, instance_id, expected_attrs=None, + cell_down_support=False): instance = objects.Instance() instance.uuid = instance_id instance.id = 1 diff --git a/nova/tests/unit/api/openstack/compute/test_server_actions.py b/nova/tests/unit/api/openstack/compute/test_server_actions.py index 282fa15a9e47..64905177fb5c 100644 --- a/nova/tests/unit/api/openstack/compute/test_server_actions.py +++ b/nova/tests/unit/api/openstack/compute/test_server_actions.py @@ -136,7 +136,8 @@ class ServerActionsControllerTestV21(test.TestCase): body=body_map.get(action)) mock_get.assert_called_once_with(self.context, uuid, - expected_attrs=['flavor', 'numa_topology']) + expected_attrs=['flavor', 'numa_topology'], + cell_down_support=False) mock_method.assert_called_once_with(self.context, instance, *args, **kwargs) diff --git a/nova/tests/unit/api/openstack/compute/test_server_diagnostics.py b/nova/tests/unit/api/openstack/compute/test_server_diagnostics.py index d39a7d792991..0079ad93aebe 100644 --- a/nova/tests/unit/api/openstack/compute/test_server_diagnostics.py +++ b/nova/tests/unit/api/openstack/compute/test_server_diagnostics.py @@ -29,7 +29,8 @@ from nova.tests.unit.api.openstack import fakes UUID = uuids.abc -def fake_instance_get(self, _context, instance_uuid, expected_attrs=None): +def fake_instance_get(self, _context, instance_uuid, expected_attrs=None, + cell_down_support=False): if instance_uuid != UUID: raise Exception("Invalid UUID") return objects.Instance(uuid=instance_uuid, host='123') diff --git a/nova/tests/unit/api/openstack/compute/test_server_migrations.py b/nova/tests/unit/api/openstack/compute/test_server_migrations.py index d9604a923846..c479c6d41a31 100644 --- a/nova/tests/unit/api/openstack/compute/test_server_migrations.py +++ b/nova/tests/unit/api/openstack/compute/test_server_migrations.py @@ -180,7 +180,8 @@ class ServerMigrationsTestsV223(ServerMigrationsTestsV21): self.assertEqual(migrations_in_progress, response) m_get_instance.assert_called_once_with(self.context, SERVER_UUID, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) @mock.patch('nova.compute.api.API.get') def test_index_invalid_instance(self, m_get_instance): @@ -190,7 +191,8 @@ class ServerMigrationsTestsV223(ServerMigrationsTestsV21): self.req, SERVER_UUID) m_get_instance.assert_called_once_with(self.context, SERVER_UUID, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) @mock.patch('nova.compute.api.API.get_migration_by_id_and_instance') @mock.patch('nova.compute.api.API.get') @@ -202,7 +204,8 @@ class ServerMigrationsTestsV223(ServerMigrationsTestsV21): self.assertEqual(migrations[0], response['migration']) m_get_instance.assert_called_once_with(self.context, SERVER_UUID, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) @mock.patch('nova.compute.api.API.get_migration_by_id_and_instance') @mock.patch('nova.compute.api.API.get') @@ -216,7 +219,8 @@ class ServerMigrationsTestsV223(ServerMigrationsTestsV21): non_progress_mig.id) m_get_instance.assert_called_once_with(self.context, SERVER_UUID, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) @mock.patch('nova.compute.api.API.get_migration_by_id_and_instance') @mock.patch('nova.compute.api.API.get') @@ -231,7 +235,8 @@ class ServerMigrationsTestsV223(ServerMigrationsTestsV21): non_progress_mig.id) m_get_instance.assert_called_once_with(self.context, SERVER_UUID, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) @mock.patch('nova.compute.api.API.get_migration_by_id_and_instance') @mock.patch('nova.compute.api.API.get') @@ -245,7 +250,8 @@ class ServerMigrationsTestsV223(ServerMigrationsTestsV21): migrations_obj[0].id) m_get_instance.assert_called_once_with(self.context, SERVER_UUID, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) @mock.patch('nova.compute.api.API.get') def test_show_migration_invalid_instance(self, m_get_instance): @@ -256,7 +262,8 @@ class ServerMigrationsTestsV223(ServerMigrationsTestsV21): migrations_obj[0].id) m_get_instance.assert_called_once_with(self.context, SERVER_UUID, - expected_attrs=None) + expected_attrs=None, + cell_down_support=False) class ServerMigrationsTestsV224(ServerMigrationsTestsV21): diff --git a/nova/tests/unit/api/openstack/compute/test_server_reset_state.py b/nova/tests/unit/api/openstack/compute/test_server_reset_state.py index 04443cf103a8..c1d598a396b6 100644 --- a/nova/tests/unit/api/openstack/compute/test_server_reset_state.py +++ b/nova/tests/unit/api/openstack/compute/test_server_reset_state.py @@ -80,7 +80,8 @@ class ResetStateTestsV21(test.NoDBTestCase): body={"os-resetState": {"state": "active"}}) self.compute_api.get.assert_called_once_with( - self.context, self.uuid, expected_attrs=None) + self.context, self.uuid, expected_attrs=None, + cell_down_support=False) def test_reset_active(self): expected = dict(vm_state=vm_states.ACTIVE, task_state=None) @@ -101,7 +102,8 @@ class ResetStateTestsV21(test.NoDBTestCase): self.assertEqual(202, status_int) self.compute_api.get.assert_called_once_with( - self.context, self.instance.uuid, expected_attrs=None) + self.context, self.instance.uuid, expected_attrs=None, + cell_down_support=False) self.instance.save.assert_called_once_with(admin_state_reset=True) def test_reset_error(self): @@ -123,5 +125,6 @@ class ResetStateTestsV21(test.NoDBTestCase): self.assertEqual(202, status_int) self.compute_api.get.assert_called_once_with( - self.context, self.instance.uuid, expected_attrs=None) + self.context, self.instance.uuid, expected_attrs=None, + cell_down_support=False) self.instance.save.assert_called_once_with(admin_state_reset=True) diff --git a/nova/tests/unit/api/openstack/compute/test_serversV21.py b/nova/tests/unit/api/openstack/compute/test_serversV21.py index 3dffa8684a16..4546289ab071 100644 --- a/nova/tests/unit/api/openstack/compute/test_serversV21.py +++ b/nova/tests/unit/api/openstack/compute/test_serversV21.py @@ -497,7 +497,7 @@ class ServersControllerTest(ControllerTest): self.mock_get.assert_called_once_with( req.environ['nova.context'], FAKE_UUID, expected_attrs=['flavor', 'info_cache', 'metadata', - 'numa_topology']) + 'numa_topology'], cell_down_support=False) def test_get_server_with_id_image_ref_by_id(self): image_bookmark = "http://localhost/fake/images/10" @@ -514,7 +514,7 @@ class ServersControllerTest(ControllerTest): self.mock_get.assert_called_once_with( req.environ['nova.context'], FAKE_UUID, expected_attrs=['flavor', 'info_cache', 'metadata', - 'numa_topology']) + 'numa_topology'], cell_down_support=False) def _generate_nw_cache_info(self): pub0 = ('172.19.0.1', '172.19.0.2',) @@ -571,7 +571,7 @@ class ServersControllerTest(ControllerTest): self.assertThat(res_dict, matchers.DictMatches(expected)) self.mock_get.assert_called_once_with( req.environ['nova.context'], FAKE_UUID, - expected_attrs=None) + expected_attrs=None, cell_down_support=False) # Make sure we kept the addresses in order self.assertIsInstance(res_dict['addresses'], collections.OrderedDict) labels = [vif['network']['label'] for vif in nw_cache] @@ -591,7 +591,8 @@ class ServersControllerTest(ControllerTest): self.assertRaises(webob.exc.HTTPNotFound, self.ips_controller.index, req, uuids.fake) self.mock_get.assert_called_once_with( - req.environ['nova.context'], uuids.fake, expected_attrs=None) + req.environ['nova.context'], uuids.fake, expected_attrs=None, + cell_down_support=False) def test_show_server_hide_addresses_in_building(self): uuid = FAKE_UUID diff --git a/nova/tests/unit/api/openstack/compute/test_volumes.py b/nova/tests/unit/api/openstack/compute/test_volumes.py index e7102dc4f613..687b49f6db53 100644 --- a/nova/tests/unit/api/openstack/compute/test_volumes.py +++ b/nova/tests/unit/api/openstack/compute/test_volumes.py @@ -59,7 +59,8 @@ FAKE_UUID_C = 'cccccccc-cccc-cccc-cccc-cccccccccccc' IMAGE_UUID = 'c905cedb-7281-47e4-8a62-f26bc5fc4c77' -def fake_get_instance(self, context, instance_id, expected_attrs=None): +def fake_get_instance(self, context, instance_id, expected_attrs=None, + cell_down_support=False): return fake_instance.fake_instance_obj(context, **{'uuid': instance_id}) diff --git a/nova/tests/unit/api/openstack/fakes.py b/nova/tests/unit/api/openstack/fakes.py index a933bb7222c8..99bd9e36fc01 100644 --- a/nova/tests/unit/api/openstack/fakes.py +++ b/nova/tests/unit/api/openstack/fakes.py @@ -364,7 +364,8 @@ def fake_instance_get(**kwargs): def fake_compute_get(**kwargs): - def _return_server_obj(context, uuid, expected_attrs=None): + def _return_server_obj(context, uuid, expected_attrs=None, + cell_down_support=False): return stub_instance_obj(context, **kwargs) return _return_server_obj diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index 5dc735085db3..1eed3f97eb3c 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -26,6 +26,7 @@ from oslo_utils import fixture as utils_fixture from oslo_utils.fixture import uuidsentinel as uuids from oslo_utils import timeutils from oslo_utils import uuidutils +import six from nova.compute import api as compute_api from nova.compute import cells_api as compute_cells_api @@ -5736,6 +5737,101 @@ class _ComputeAPIUnitTestMixIn(object): self.context.project_id, limit=1) + @mock.patch.object(objects.Instance, 'get_by_uuid') + def test_get_instance_from_cell_success(self, mock_get_inst): + cell_mapping = objects.CellMapping(uuid=uuids.cell1, + name='1', id=1) + im = objects.InstanceMapping(instance_uuid=uuids.inst, + cell_mapping=cell_mapping) + mock_get_inst.return_value = objects.Instance(uuid=uuids.inst) + result = self.compute_api._get_instance_from_cell(self.context, + im, [], True) + self.assertEqual(uuids.inst, result.uuid) + mock_get_inst.assert_called_once() + + @mock.patch.object(objects.Instance, 'get_by_uuid') + def test_get_instance_from_cell_failure(self, mock_get_inst): + # Make sure InstanceNotFound is bubbled up and not treated like + # other errors + mock_get_inst.side_effect = exception.InstanceNotFound( + instance_id=uuids.inst) + cell_mapping = objects.CellMapping(uuid=uuids.cell1, + name='1', id=1) + im = objects.InstanceMapping(instance_uuid=uuids.inst, + cell_mapping=cell_mapping) + exp = self.assertRaises(exception.InstanceNotFound, + self.compute_api._get_instance_from_cell, self.context, + im, [], False) + self.assertIn('could not be found', six.text_type(exp)) + + @mock.patch.object(objects.RequestSpec, 'get_by_instance_uuid') + @mock.patch('nova.context.scatter_gather_cells') + def test_get_instance_with_cell_down_support(self, mock_sg, mock_rs): + cell_mapping = objects.CellMapping(uuid=uuids.cell1, + name='1', id=1) + im1 = objects.InstanceMapping(instance_uuid=uuids.inst1, + cell_mapping=cell_mapping, + queued_for_delete=True) + im2 = objects.InstanceMapping(instance_uuid=uuids.inst2, + cell_mapping=cell_mapping, + queued_for_delete=False, + project_id='fake', + created_at=None) + mock_sg.return_value = { + uuids.cell1: context.did_not_respond_sentinel + } + + # No cell down support, error means we return 500 + exp = self.assertRaises(exception.NovaException, + self.compute_api._get_instance_from_cell, self.context, + im1, [], False) + self.assertIn('info is not available', six.text_type(exp)) + + # Have cell down support, error + queued_for_delete = NotFound + exp = self.assertRaises(exception.InstanceNotFound, + self.compute_api._get_instance_from_cell, self.context, + im1, [], True) + self.assertIn('could not be found', six.text_type(exp)) + + # Have cell down support, error + archived reqspec = NotFound + mock_rs.side_effect = exception.RequestSpecNotFound( + instance_uuid=uuids.inst2) + exp = self.assertRaises(exception.InstanceNotFound, + self.compute_api._get_instance_from_cell, self.context, + im2, [], True) + self.assertIn('could not be found', six.text_type(exp)) + + # Have cell down support, error + reqspec + not queued_for_delete + # means we return a minimal instance + req_spec = objects.RequestSpec(instance_uuid=uuids.inst2, + user_id='fake', + flavor=objects.Flavor(name='fake1'), + image=objects.ImageMeta(id=uuids.image, + name='fake1'), + availability_zone='nova') + mock_rs.return_value = req_spec + mock_rs.side_effect = None + result = self.compute_api._get_instance_from_cell(self.context, + im2, [], True) + self.assertIn('user_id', result) + self.assertNotIn('display_name', result) + self.assertEqual(uuids.inst2, result.uuid) + self.assertEqual('nova', result.availability_zone) + self.assertEqual(uuids.image, result.image_ref) + + # Same as above, but boot-from-volume where image is not None but the + # id of the image is not set. + req_spec.image = objects.ImageMeta(name='fake1') + result = self.compute_api._get_instance_from_cell(self.context, + im2, [], True) + self.assertIsNone(result.image_ref) + + # Same as above, but boot-from-volume where image is None + req_spec.image = None + result = self.compute_api._get_instance_from_cell(self.context, + im2, [], True) + self.assertIsNone(result.image_ref) + @mock.patch.object(objects.InstanceMapping, 'get_by_instance_uuid', side_effect=exception.InstanceMappingNotFound(uuid='fake')) @mock.patch.object(objects.BuildRequest, 'get_by_instance_uuid') @@ -5788,13 +5884,11 @@ class _ComputeAPIUnitTestMixIn(object): mock_get_min_service.assert_called_once_with(self.context, 'nova-osapi_compute') - @mock.patch.object(context, 'set_target_cell') @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_buildreq_deleted_inst_in_cell( - self, mock_get_inst, mock_get_build_req, mock_get_inst_map, - mock_target_cell): + self, mock_get_inst, mock_get_build_req, mock_get_inst_map): # This test checks the following scenario: # The instance is not mapped to a cell, so it should be retrieved from # a BuildRequest object. However the BuildRequest does not exist @@ -5804,7 +5898,8 @@ class _ComputeAPIUnitTestMixIn(object): self.useFixture(nova_fixtures.AllServicesCurrent()) build_req_obj = fake_build_request.fake_req_obj(self.context) instance = build_req_obj.instance - inst_map = objects.InstanceMapping(cell_mapping=objects.CellMapping()) + inst_map = objects.InstanceMapping(cell_mapping=objects.CellMapping( + uuid=uuids.cell), instance_uuid=instance.uuid) mock_get_inst_map.side_effect = [ objects.InstanceMapping(cell_mapping=None), inst_map] @@ -5821,8 +5916,7 @@ class _ComputeAPIUnitTestMixIn(object): self.assertEqual(2, mock_get_inst_map.call_count) mock_get_build_req.assert_called_once_with(self.context, instance.uuid) - mock_target_cell.assert_called_once_with(self.context, - inst_map.cell_mapping) + mock_get_inst.assert_called_once_with(self.context, instance.uuid, expected_attrs=[ 'metadata', @@ -5875,19 +5969,19 @@ class _ComputeAPIUnitTestMixIn(object): 'info_cache']) self.assertEqual(instance, inst_from_get) - @mock.patch.object(context, 'set_target_cell') @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_in_cell(self, mock_get_inst, mock_get_build_req, - mock_get_inst_map, mock_target_cell): + mock_get_inst_map): self.useFixture(nova_fixtures.AllServicesCurrent()) # This just checks that the instance is looked up normally and not # synthesized from a BuildRequest object. Verification of pulling the # instance from the proper cell will be added when that capability is. instance = self._create_instance_obj() build_req_obj = fake_build_request.fake_req_obj(self.context) - inst_map = objects.InstanceMapping(cell_mapping=objects.CellMapping()) + inst_map = objects.InstanceMapping(cell_mapping=objects.CellMapping( + uuid=uuids.cell), instance_uuid=instance.uuid) mock_get_inst_map.return_value = inst_map mock_get_build_req.return_value = build_req_obj mock_get_inst.return_value = instance @@ -5897,11 +5991,8 @@ class _ComputeAPIUnitTestMixIn(object): if self.cell_type is None: mock_get_inst_map.assert_called_once_with(self.context, instance.uuid) - mock_target_cell.assert_called_once_with(self.context, - inst_map.cell_mapping) else: self.assertFalse(mock_get_inst_map.called) - self.assertFalse(mock_target_cell.called) self.assertEqual(instance, returned_inst) mock_get_inst.assert_called_once_with(self.context, instance.uuid, expected_attrs=[