diff --git a/cinderclient/base.py b/cinderclient/base.py index a193fa3a7..5542e0a2e 100644 --- a/cinderclient/base.py +++ b/cinderclient/base.py @@ -54,8 +54,11 @@ class Manager(utils.HookableMixin): def __init__(self, api): self.api = api - def _list(self, url, response_key, obj_class=None, body=None): + def _list(self, url, response_key, obj_class=None, body=None, + limit=None, items=None): resp = None + if items is None: + items = [] if body: resp, body = self.api.client.post(url, body=body) else: @@ -75,8 +78,37 @@ class Manager(utils.HookableMixin): with self.completion_cache('human_id', obj_class, mode="w"): with self.completion_cache('uuid', obj_class, mode="w"): - return [obj_class(self, res, loaded=True) - for res in data if res] + items_new = [obj_class(self, res, loaded=True) + for res in data if res] + if limit: + limit = int(limit) + margin = limit - len(items) + if margin <= len(items_new): + # If the limit is reached, return the items. + items = items + items_new[:margin] + return items + else: + items = items + items_new + else: + items = items + items_new + + # It is possible that the length of the list we request is longer + # than osapi_max_limit, so we have to retrieve multiple times to + # get the complete list. + next = None + if 'volumes_links' in body: + volumes_links = body['volumes_links'] + if volumes_links: + for volumes_link in volumes_links: + if 'rel' in volumes_link and 'next' == volumes_link['rel']: + next = volumes_link['href'] + break + if next: + # As long as the 'next' link is not empty, keep requesting it + # till there is no more items. + items = self._list(next, response_key, obj_class, None, + limit, items) + return items @contextlib.contextmanager def completion_cache(self, cache_type, obj_class, mode): diff --git a/cinderclient/tests/v2/fakes.py b/cinderclient/tests/v2/fakes.py index 8962f9354..8ca75a6fc 100644 --- a/cinderclient/tests/v2/fakes.py +++ b/cinderclient/tests/v2/fakes.py @@ -239,6 +239,8 @@ class FakeHTTPClient(base_client.HTTPClient): self.auth_url = 'auth_url' self.callstack = [] self.management_url = 'http://10.0.2.15:8776/v2/fake' + self.osapi_max_limit = 1000 + self.marker = None def _cs_request(self, url, method, **kwargs): # Check that certain things are called correctly @@ -250,7 +252,16 @@ class FakeHTTPClient(base_client.HTTPClient): # Call the method args = urlparse.parse_qsl(urlparse.urlparse(url)[4]) kwargs.update(args) - munged_url = url.rsplit('?', 1)[0] + url_split = url.rsplit('?', 1) + munged_url = url_split[0] + if len(url_split) > 1: + parameters = url_split[1] + if 'marker' in parameters: + self.marker = int(parameters.rsplit('marker=', 1)[1]) + else: + self.marker = None + else: + self.marker = None munged_url = munged_url.strip('/').replace('/', '_').replace('.', '_') munged_url = munged_url.replace('-', '_') @@ -340,10 +351,21 @@ class FakeHTTPClient(base_client.HTTPClient): return (200, {}, {'volume': volume}) def get_volumes(self, **kw): - return (200, {}, {"volumes": [ - {'id': 1234, 'name': 'sample-volume'}, - {'id': 5678, 'name': 'sample-volume2'} - ]}) + if self.marker == 1234: + return (200, {}, {"volumes": [ + {'id': 5678, 'name': 'sample-volume2'} + ]}) + elif self.osapi_max_limit == 1: + return (200, {}, {"volumes": [ + {'id': 1234, 'name': 'sample-volume'} + ], "volumes_links": [ + {'href': "/volumes?limit=1&marker=1234", 'rel': 'next'} + ]}) + else: + return (200, {}, {"volumes": [ + {'id': 1234, 'name': 'sample-volume'}, + {'id': 5678, 'name': 'sample-volume2'} + ]}) # TODO(jdg): This will need to change # at the very least it's not complete diff --git a/cinderclient/tests/v2/test_volumes.py b/cinderclient/tests/v2/test_volumes.py index aeceff460..beda5fa3e 100644 --- a/cinderclient/tests/v2/test_volumes.py +++ b/cinderclient/tests/v2/test_volumes.py @@ -16,7 +16,7 @@ from cinderclient.tests import utils from cinderclient.tests.v2 import fakes - +from cinderclient.v2.volumes import Volume cs = fakes.FakeClient() @@ -39,6 +39,33 @@ class VolumesTest(utils.TestCase): self.assertRaises(ValueError, cs.volumes.list, sort_key='id', sort_dir='invalid') + def test__list(self): + # There only 2 volumes available for our tests, so we set limit to 2. + limit = 2 + url = "/volumes?limit=%s" % limit + response_key = "volumes" + fake_volume1234 = Volume(self, {'id': 1234, + 'name': 'sample-volume'}, + loaded=True) + fake_volume5678 = Volume(self, {'id': 5678, + 'name': 'sample-volume2'}, + loaded=True) + fake_volumes = [fake_volume1234, fake_volume5678] + # osapi_max_limit is 1000 by default. If limit is less than + # osapi_max_limit, we can get 2 volumes back. + volumes = cs.volumes._list(url, response_key, limit=limit) + cs.assert_called('GET', url) + self.assertEqual(fake_volumes, volumes) + + # When we change the osapi_max_limit to 1, the next link should be + # generated. If limit equals 2 and id passed as an argument, we can + # still get 2 volumes back, because the method _list will fetch the + # volume from the next link. + cs.client.osapi_max_limit = 1 + volumes = cs.volumes._list(url, response_key, limit=limit) + self.assertEqual(fake_volumes, volumes) + cs.client.osapi_max_limit = 1000 + def test_delete_volume(self): v = cs.volumes.list()[0] v.delete() diff --git a/cinderclient/v2/volumes.py b/cinderclient/v2/volumes.py index ae8ed2909..59853adb7 100644 --- a/cinderclient/v2/volumes.py +++ b/cinderclient/v2/volumes.py @@ -281,7 +281,7 @@ class VolumeManager(base.ManagerWithFind): detail = "/detail" return self._list("/volumes%s%s" % (detail, query_string), - "volumes") + "volumes", limit=limit) def delete(self, volume): """Delete a volume.