From 3a1b9abe39ef0bf4fab67a9f1144adf76ffd1b85 Mon Sep 17 00:00:00 2001 From: Surya Seetharaman Date: Tue, 12 Feb 2019 17:34:48 +0100 Subject: [PATCH] Plumbing required in servers ViewBuilder to construct partial results This patch specifically adds the returning of the partial results in the ViewBuilder. Note that this will be enabled in the microversion bump patch. Related to blueprint handling-down-cell Change-Id: Ie101678f8b9f0e624c84a355e6a7249dfe0530c4 --- nova/api/openstack/compute/servers.py | 23 +- nova/api/openstack/compute/views/servers.py | 89 +++++- .../api/openstack/compute/test_serversV21.py | 301 ++++++++++++++++++ 3 files changed, 396 insertions(+), 17 deletions(-) diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index ecaf77de0f..d67e8e1980 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.