From 68a8d513dd0cd6c2f24642f2f02e4c61f3a863a7 Mon Sep 17 00:00:00 2001 From: Monty Taylor Date: Fri, 6 Jan 2017 09:51:28 -0600 Subject: [PATCH] Handle pagination for glance images The default glance image list pagination seems to be about 20, which means for v2 you really need to deal with pagination every time. It also seems that the limit parameter does _not_ allow you to get more items than the server default, so you can't just say "limit 100000" and be done with it. In order to accomplish this, we need to have the adapter stop trying to return only the image list when there are other top level keys (so the code can read the next link) and then do a loop requesting the next link. To make us even happier, glance returns the next link as '/v2/images' but we have already set the adapter to 'https://example.com/v2' due to version discovery. Since we're setting the endpoint_override on the adapater, it treats that as the root, leaving us with https://example.com/v2/v2/images. To deal with that, introduce a 'raw' adapter which is bound to whatever is in the catalog, rather than whatever we found through version discovery. Change-Id: I030147e0275d0c4ee89588e21b5970f7d81800d3 Story: 2000837 --- ...nce-image-pagination-0b4dfef22b25852b.yaml | 4 ++++ shade/_adapter.py | 16 ++++---------- shade/openstackcloud.py | 22 +++++++++++++++++-- shade/tests/unit/test_image.py | 18 +++++++++++++++ 4 files changed, 46 insertions(+), 14 deletions(-) create mode 100644 releasenotes/notes/glance-image-pagination-0b4dfef22b25852b.yaml diff --git a/releasenotes/notes/glance-image-pagination-0b4dfef22b25852b.yaml b/releasenotes/notes/glance-image-pagination-0b4dfef22b25852b.yaml new file mode 100644 index 000000000..3b134fcb5 --- /dev/null +++ b/releasenotes/notes/glance-image-pagination-0b4dfef22b25852b.yaml @@ -0,0 +1,4 @@ +--- +issues: + - Fixed an issue where glance image list pagination was being ignored, + leading to truncated image lists. diff --git a/shade/_adapter.py b/shade/_adapter.py index d15214a00..ac184e0d4 100644 --- a/shade/_adapter.py +++ b/shade/_adapter.py @@ -121,18 +121,10 @@ class ShadeAdapter(adapter.Adapter): elif len(json_keys) == 1: result = result_json[json_keys[0]] else: - # Yay for inferrence! - path = urllib.parse.urlparse(response.url).path.strip() - object_type = path.split('/')[-1] - if object_type in json_keys: - result = result_json[object_type] - elif (object_type.startswith('os-') - and object_type[3:] in json_keys): - result = result_json[object_type[3:]] - else: - # Passthrough the whole body - sometimes (hi glance) things - # come through without a top-level container - result = result_json + # Passthrough the whole body - sometimes (hi glance) things + # come through without a top-level container. Also, sometimes + # you need to deal with pagination + result = result_json if task_manager._is_listlike(result): return meta.obj_list_to_dict(result, request_id=request_id) diff --git a/shade/openstackcloud.py b/shade/openstackcloud.py index ce0bdde04..d8d2b7e4f 100644 --- a/shade/openstackcloud.py +++ b/shade/openstackcloud.py @@ -387,6 +387,13 @@ class OpenStackCloud(_normalize.Normalizer): self._raw_clients['object-store'] = raw_client return self._raw_clients['object-store'] + @property + def _raw_image_client(self): + if 'raw-image' not in self._raw_clients: + image_client = self._get_raw_client('image') + self._raw_clients['raw-image'] = image_client + return self._raw_clients['raw-image'] + @property def _image_client(self): if 'image' not in self._raw_clients: @@ -1773,18 +1780,29 @@ class OpenStackCloud(_normalize.Normalizer): """ # First, try to actually get images from glance, it's more efficient images = [] + image_list = [] try: if self.cloud_config.get_api_version('image') == '2': endpoint = '/images' else: endpoint = '/images/detail' - image_list = self._image_client.get(endpoint) + response = self._image_client.get(endpoint) except keystoneauth1.exceptions.catalog.EndpointNotFound: # We didn't have glance, let's try nova # If this doesn't work - we just let the exception propagate - image_list = self._compute_client.get('/images/detail') + response = self._compute_client.get('/images/detail') + while 'next' in response: + image_list.extend(meta.obj_list_to_dict(response['images'])) + endpoint = response['next'] + # Use the raw endpoint from the catalog not the one from + # version discovery so that the next links will work right + response = self._raw_image_client.get(endpoint) + if 'images' in response: + image_list.extend(meta.obj_list_to_dict(response['images'])) + else: + image_list.extend(response) for image in image_list: # The cloud might return DELETED for invalid images. diff --git a/shade/tests/unit/test_image.py b/shade/tests/unit/test_image.py index 309cd3ef4..0b6ea254a 100644 --- a/shade/tests/unit/test_image.py +++ b/shade/tests/unit/test_image.py @@ -147,6 +147,24 @@ class TestImage(base.RequestsMockTestCase): self.cloud._normalize_images([self.fake_image_dict]), self.cloud.list_images()) + def test_list_images_paginated(self): + marker = str(uuid.uuid4()) + self.adapter.register_uri( + 'GET', 'https://image.example.com/v2/images', + json={ + 'images': [self.fake_image_dict], + 'next': '/v2/images?marker={marker}'.format(marker=marker), + }) + self.adapter.register_uri( + 'GET', + 'https://image.example.com/v2/images?marker={marker}'.format( + marker=marker), + json=self.fake_search_return) + self.assertEqual( + self.cloud._normalize_images([ + self.fake_image_dict, self.fake_image_dict]), + self.cloud.list_images()) + def test_create_image_put_v2(self): self.cloud.image_api_use_tasks = False