diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index 0040f552ec..148cb509c7 100644 --- a/nova/api/openstack/compute/servers.py +++ b/nova/api/openstack/compute/servers.py @@ -270,12 +270,15 @@ class ServersController(wsgi.Controller): if is_detail: instance_list._context = context instance_list.fill_faults() - response = self._view_builder.detail(req, instance_list) + response = self._view_builder.detail( + req, instance_list, cell_down_support=cell_down_support) else: - response = self._view_builder.index(req, instance_list) + response = self._view_builder.index( + req, instance_list, cell_down_support=cell_down_support) return response - def _get_server(self, context, req, instance_uuid, is_detail=False): + def _get_server(self, context, req, instance_uuid, is_detail=False, + cell_down_support=False): """Utility function for looking up an instance by uuid. :param context: request context for auth @@ -283,6 +286,10 @@ class ServersController(wsgi.Controller): :param instance_uuid: UUID of the server instance to get :param is_detail: True if you plan on showing the details of the instance in the response, False otherwise. + :param cell_down_support: True if the API (and caller) support + returning a minimal instance + construct if the relevant cell is + down. """ expected_attrs = ['flavor', 'numa_topology'] if is_detail: @@ -295,7 +302,7 @@ class ServersController(wsgi.Controller): instance = common.get_instance(self.compute_api, context, instance_uuid, expected_attrs=expected_attrs, - cell_down_support=False) + cell_down_support=cell_down_support) return instance @staticmethod @@ -389,8 +396,12 @@ class ServersController(wsgi.Controller): """Returns server details by server id.""" context = req.environ['nova.context'] context.can(server_policies.SERVERS % 'show') - instance = self._get_server(context, req, id, is_detail=True) - return self._view_builder.show(req, instance) + # TODO(tssurya): enable cell_down_support after bumping the + # microversion. + instance = self._get_server( + context, req, id, is_detail=True, cell_down_support=False) + return self._view_builder.show( + req, instance, cell_down_support=False) @wsgi.response(202) @wsgi.expected_errors((400, 403, 409)) diff --git a/nova/api/openstack/compute/views/servers.py b/nova/api/openstack/compute/views/servers.py index 1eab3d61d1..78813c44c6 100644 --- a/nova/api/openstack/compute/views/servers.py +++ b/nova/api/openstack/compute/views/servers.py @@ -93,8 +93,23 @@ class ViewBuilder(common.ViewBuilder): def basic(self, request, instance, show_extra_specs=False, show_extended_attr=None, show_host_status=None, - show_sec_grp=None, bdms=None): + show_sec_grp=None, bdms=None, cell_down_support=False): """Generic, non-detailed view of an instance.""" + if cell_down_support and 'display_name' not in instance: + # NOTE(tssurya): If the microversion is >= 2.69, this boolean will + # be true in which case we check if there are instances from down + # cells (by checking if their objects have missing keys like + # `display_name`) and return partial constructs based on the + # information available from the nova_api database. + return { + "server": { + "id": instance.uuid, + "status": "UNKNOWN", + "links": self._get_links(request, + instance.uuid, + self._collection_name), + }, + } return { "server": { "id": instance["uuid"], @@ -125,16 +140,51 @@ class ViewBuilder(common.ViewBuilder): # results. return sorted(list(set(self._show_expected_attrs + expected_attrs))) + def _show_from_down_cell(self, request, instance, show_extra_specs): + """Function that constructs the partial response for the instance.""" + ret = { + "server": { + "id": instance.uuid, + "status": "UNKNOWN", + "tenant_id": instance.project_id, + "created": utils.isotime(instance.created_at), + }, + } + if 'flavor' in instance: + # If the key 'flavor' is present for an instance from a down cell + # it means that the request is ``GET /servers/{server_id}`` and + # thus we include the information from the request_spec of the + # instance like its flavor, image, avz, and user_id in addition to + # the basic information from its instance_mapping. + # If 'flavor' key is not present for an instance from a down cell + # down cell it means the request is ``GET /servers/detail`` and we + # do not expose the flavor in the response when listing servers + # with details for performance reasons of fetching it from the + # request specs table for the whole list of instances. + ret["server"]["image"] = self._get_image(request, instance) + ret["server"]["flavor"] = self._get_flavor(request, instance, + show_extra_specs) + # in case availability zone was not requested by the user during + # boot time, return UNKNOWN. + avz = instance.availability_zone or "UNKNOWN" + ret["server"]["OS-EXT-AZ:availability_zone"] = avz + ret["server"]["OS-EXT-STS:power_state"] = instance.power_state + # in case its an old request spec which doesn't have the user_id + # data migrated, return UNKNOWN. + ret["server"]["user_id"] = instance.user_id or "UNKNOWN" + else: + # GET /servers/detail includes links for GET /servers/{server_id}. + ret['server']["links"] = self._get_links( + request, instance.uuid, self._collection_name) + return ret + def show(self, request, instance, extend_address=True, show_extra_specs=None, show_AZ=True, show_config_drive=True, show_extended_attr=None, show_host_status=None, show_keypair=True, show_srv_usg=True, show_sec_grp=True, show_extended_status=True, show_extended_volumes=True, - bdms=None): + bdms=None, cell_down_support=False): """Detailed view of a single instance.""" - ip_v4 = instance.get('access_ip_v4') - ip_v6 = instance.get('access_ip_v6') - if show_extra_specs is None: # detail will pre-calculate this for us. If we're doing show, # then figure it out here. @@ -144,6 +194,17 @@ class ViewBuilder(common.ViewBuilder): show_extra_specs = context.can( fes_policies.POLICY_ROOT % 'index', fatal=False) + if cell_down_support and 'display_name' not in instance: + # NOTE(tssurya): If the microversion is >= 2.69, this boolean will + # be true in which case we check if there are instances from down + # cells (by checking if their objects have missing keys like + # `display_name`) and return partial constructs based on the + # information available from the nova_api database. + return self._show_from_down_cell( + request, instance, show_extra_specs) + ip_v4 = instance.get('access_ip_v4') + ip_v6 = instance.get('access_ip_v6') + server = { "server": { "id": instance["uuid"], @@ -280,13 +341,13 @@ class ViewBuilder(common.ViewBuilder): return server - def index(self, request, instances): + def index(self, request, instances, cell_down_support=False): """Show a list of servers without many details.""" coll_name = self._collection_name return self._list_view(self.basic, request, instances, coll_name, - False) + False, cell_down_support=cell_down_support) - def detail(self, request, instances): + def detail(self, request, instances, cell_down_support=False): """Detailed view of a list of instance.""" coll_name = self._collection_name + '/detail' context = request.environ['nova.context'] @@ -318,7 +379,8 @@ class ViewBuilder(common.ViewBuilder): show_extended_attr=show_extended_attr, show_host_status=show_host_status, show_sec_grp=False, - bdms=bdms) + bdms=bdms, + cell_down_support=cell_down_support) self._add_security_grps(request, list(servers_dict["servers"]), instances) @@ -326,7 +388,7 @@ class ViewBuilder(common.ViewBuilder): def _list_view(self, func, request, servers, coll_name, show_extra_specs, show_extended_attr=None, show_host_status=None, - show_sec_grp=False, bdms=None): + show_sec_grp=False, bdms=None, cell_down_support=False): """Provide a view for a list of servers. :param func: Function used to format the server data @@ -341,13 +403,18 @@ class ViewBuilder(common.ViewBuilder): :param show_sec_grp: If the security group should be included in the response dict. :param bdms: Instances bdms info from multiple cells. + :param cell_down_support: True if the API (and caller) support + returning a minimal instance + construct if the relevant cell is + down. :returns: Server data in dictionary format """ server_list = [func(request, server, show_extra_specs=show_extra_specs, show_extended_attr=show_extended_attr, show_host_status=show_host_status, - show_sec_grp=show_sec_grp, bdms=bdms)["server"] + show_sec_grp=show_sec_grp, bdms=bdms, + cell_down_support=cell_down_support)["server"] for server in servers] servers_links = self._get_collection_links(request, servers, diff --git a/nova/tests/unit/api/openstack/compute/test_serversV21.py b/nova/tests/unit/api/openstack/compute/test_serversV21.py index 5acfadb9bd..b5d10a0ac0 100644 --- a/nova/tests/unit/api/openstack/compute/test_serversV21.py +++ b/nova/tests/unit/api/openstack/compute/test_serversV21.py @@ -7339,6 +7339,307 @@ class ServersViewBuilderTest(test.TestCase): self.assertThat(output, matchers.DictMatches(expected_server)) +class ServersViewBuilderTestV269(ServersViewBuilderTest): + """Server ViewBuilder test for microversion 2.69 + + The intent here is simply to verify that when showing server details + after microversion 2.69 the response could have missing keys for those + servers from the down cells. + """ + wsgi_api_version = '2.69' + + def setUp(self): + super(ServersViewBuilderTestV269, self).setUp() + self.view_builder = views.servers.ViewBuilder() + self.ctxt = context.RequestContext('fake', 'fake') + + def fake_is_supported(req, min_version="2.1", max_version="2.69"): + return (fakes.api_version.APIVersionRequest(max_version) >= + req.api_version_request >= + fakes.api_version.APIVersionRequest(min_version)) + self.stub_out('nova.api.openstack.api_version_request.is_supported', + fake_is_supported) + + def req(self, url, use_admin_context=False): + return fakes.HTTPRequest.blank(url, + use_admin_context=use_admin_context, + version=self.wsgi_api_version) + + def test_get_server_list_detail_with_down_cells(self): + # Fake out 1 partially constructued instance and one full instance. + self.instances = [ + self.instance, + objects.Instance( + context=self.ctxt, + uuid=uuids.fake1, + project_id='fake', + created_at=datetime.datetime(1955, 11, 5) + ) + ] + + req = self.req('/fake/servers/detail') + output = self.view_builder.detail(req, self.instances, True) + + self.assertEqual(2, len(output['servers'])) + image_bookmark = "http://localhost/fake/images/5" + expected = { + "servers": [{ + "id": self.uuid, + "user_id": "fake_user", + "tenant_id": "fake_project", + "updated": "2010-11-11T11:00:00Z", + "created": "2010-10-10T12:00:00Z", + "progress": 0, + "name": "test_server", + "status": "ACTIVE", + "hostId": '', + "image": { + "id": "5", + "links": [ + { + "rel": "bookmark", + "href": image_bookmark, + }, + ], + }, + "flavor": { + 'disk': 1, + 'ephemeral': 1, + 'vcpus': 1, + 'ram': 256, + 'original_name': 'flavor1', + 'extra_specs': {}, + 'swap': 0 + }, + "addresses": { + 'test1': [ + {'version': 4, 'addr': '192.168.1.100', + 'OS-EXT-IPS:type': 'fixed', + 'OS-EXT-IPS-MAC:mac_addr': 'aa:aa:aa:aa:aa:aa'}, + {'version': 6, 'addr': '2001:db8:0:1::1', + 'OS-EXT-IPS:type': 'fixed', + 'OS-EXT-IPS-MAC:mac_addr': 'aa:aa:aa:aa:aa:aa'}, + {'version': 4, 'addr': '192.168.2.100', + 'OS-EXT-IPS:type': 'fixed', + 'OS-EXT-IPS-MAC:mac_addr': 'bb:bb:bb:bb:bb:bb'} + ], + 'test2': [ + {'version': 4, 'addr': '192.168.3.100', + 'OS-EXT-IPS:type': 'fixed', + 'OS-EXT-IPS-MAC:mac_addr': 'cc:cc:cc:cc:cc:cc'}, + ] + }, + "metadata": {}, + "tags": [], + "links": [ + { + "rel": "self", + "href": self.self_link, + }, + { + "rel": "bookmark", + "href": self.bookmark_link, + }, + ], + "OS-DCF:diskConfig": "MANUAL", + "OS-EXT-SRV-ATTR:root_device_name": None, + "accessIPv4": '', + "accessIPv6": '', + "host_status": '', + "OS-EXT-SRV-ATTR:user_data": None, + "trusted_image_certificates": None, + "OS-EXT-AZ:availability_zone": "nova", + "OS-EXT-SRV-ATTR:kernel_id": '', + "OS-EXT-SRV-ATTR:reservation_id": '', + "config_drive": None, + "OS-EXT-SRV-ATTR:host": None, + "OS-EXT-SRV-ATTR:hypervisor_hostname": None, + "OS-EXT-SRV-ATTR:hostname": 'test_server', + "OS-EXT-SRV-ATTR:instance_name": "instance-00000001", + "key_name": '', + "locked": False, + "description": None, + "OS-SRV-USG:launched_at": None, + "OS-SRV-USG:terminated_at": None, + "security_groups": [{'name': 'fake-0-0'}, + {'name': 'fake-0-1'}], + "OS-EXT-STS:task_state": None, + "OS-EXT-STS:vm_state": vm_states.ACTIVE, + "OS-EXT-STS:power_state": 1, + "OS-EXT-SRV-ATTR:launch_index": 0, + "OS-EXT-SRV-ATTR:ramdisk_id": '', + "os-extended-volumes:volumes_attached": [ + {'id': 'some_volume_1', 'delete_on_termination': True}, + {'id': 'some_volume_2', 'delete_on_termination': False}, + ] + }, + { + 'created': '1955-11-05T00:00:00Z', + 'id': uuids.fake1, + 'tenant_id': 'fake', + "status": "UNKNOWN", + "links": [ + { + "rel": "self", + "href": "http://localhost/v2/fake/servers/%s" % + uuids.fake1, + }, + { + "rel": "bookmark", + "href": "http://localhost/fake/servers/%s" % + uuids.fake1, + }, + ], + }] + } + self.assertThat(output, matchers.DictMatches(expected)) + + def test_get_server_list_with_down_cells(self): + # Fake out 1 partially constructued instance and one full instance. + self.instances = [ + self.instance, + objects.Instance( + context=self.ctxt, + uuid=uuids.fake1, + project_id='fake', + created_at=datetime.datetime(1955, 11, 5) + ) + ] + + req = self.req('/fake/servers') + output = self.view_builder.index(req, self.instances, True) + + self.assertEqual(2, len(output['servers'])) + + expected = { + "servers": [{ + "id": self.uuid, + "name": "test_server", + "links": [ + { + "rel": "self", + "href": self.self_link, + }, + { + "rel": "bookmark", + "href": self.bookmark_link, + }, + ] + }, + { + 'id': uuids.fake1, + "status": "UNKNOWN", + "links": [ + { + "rel": "self", + "href": "http://localhost/v2/fake/servers/%s" % + uuids.fake1, + }, + { + "rel": "bookmark", + "href": "http://localhost/fake/servers/%s" % + uuids.fake1, + }, + ], + }] + } + + self.assertThat(output, matchers.DictMatches(expected)) + + def test_get_server_with_down_cells(self): + # Fake out 1 partially constructued instance. + self.instance = objects.Instance( + context=self.ctxt, + uuid=self.uuid, + project_id=self.instance.project_id, + created_at=datetime.datetime(1955, 11, 5), + user_id=self.instance.user_id, + image_ref=self.instance.image_ref, + power_state=0, + flavor=self.instance.flavor, + availability_zone=self.instance.availability_zone + ) + + req = self.req('/fake/servers/%s' % FAKE_UUID) + output = self.view_builder.show(req, self.instance, + cell_down_support=True) + # nine fields from request_spec and instance_mapping + self.assertEqual(9, len(output['server'])) + image_bookmark = "http://localhost/fake/images/5" + expected = { + "server": { + "id": self.uuid, + "user_id": "fake_user", + "tenant_id": "fake_project", + "created": '1955-11-05T00:00:00Z', + "status": "UNKNOWN", + "image": { + "id": "5", + "links": [ + { + "rel": "bookmark", + "href": image_bookmark, + }, + ], + }, + "flavor": { + 'disk': 1, + 'ephemeral': 1, + 'vcpus': 1, + 'ram': 256, + 'original_name': 'flavor1', + 'extra_specs': {}, + 'swap': 0 + }, + "OS-EXT-AZ:availability_zone": "nova", + "OS-EXT-STS:power_state": 0 + } + } + self.assertThat(output, matchers.DictMatches(expected)) + + def test_get_server_without_image_avz_user_id_set_from_down_cells(self): + # Fake out 1 partially constructued instance. + self.instance = objects.Instance( + context=self.ctxt, + uuid=self.uuid, + project_id=self.instance.project_id, + created_at=datetime.datetime(1955, 11, 5), + user_id=None, + image_ref=None, + power_state=0, + flavor=self.instance.flavor, + availability_zone=None + ) + + req = self.req('/fake/servers/%s' % FAKE_UUID) + output = self.view_builder.show(req, self.instance, + cell_down_support=True) + # nine fields from request_spec and instance_mapping + self.assertEqual(9, len(output['server'])) + expected = { + "server": { + "id": self.uuid, + "user_id": "UNKNOWN", + "tenant_id": "fake_project", + "created": '1955-11-05T00:00:00Z', + "status": "UNKNOWN", + "image": "", + "flavor": { + 'disk': 1, + 'ephemeral': 1, + 'vcpus': 1, + 'ram': 256, + 'original_name': 'flavor1', + 'extra_specs': {}, + 'swap': 0 + }, + "OS-EXT-AZ:availability_zone": "UNKNOWN", + "OS-EXT-STS:power_state": 0 + } + } + self.assertThat(output, matchers.DictMatches(expected)) + + class ServersAllExtensionsTestCase(test.TestCase): """Servers tests using default API router with all extensions enabled.