From eff607ccef91d09052d58f6798f68d67404f51ce Mon Sep 17 00:00:00 2001 From: Jacek Tomasiak Date: Tue, 16 Jan 2018 11:09:26 +0100 Subject: [PATCH] Fix listing of instances above API max_limit Output of `nova list` command contained only `max_limit` entries even when there were more. The list needs to be obtained in multiple requests and the paging loop was terminated prematurely. Change-Id: I4194e6d5e34ecc0ac704851c08bb895e223aa850 Closes-Bug: 1743532 --- novaclient/tests/unit/fixture_data/servers.py | 32 ++- novaclient/tests/unit/utils.py | 12 +- novaclient/tests/unit/v2/fakes.py | 4 + novaclient/tests/unit/v2/test_servers.py | 64 +++-- novaclient/tests/unit/v2/test_shell.py | 265 +++++++++++++----- novaclient/v2/servers.py | 5 +- novaclient/v2/shell.py | 4 +- 7 files changed, 283 insertions(+), 103 deletions(-) diff --git a/novaclient/tests/unit/fixture_data/servers.py b/novaclient/tests/unit/fixture_data/servers.py index 2402cd106..3950158d7 100644 --- a/novaclient/tests/unit/fixture_data/servers.py +++ b/novaclient/tests/unit/fixture_data/servers.py @@ -32,6 +32,16 @@ class Base(base.Fixture): self.requests_mock.get(self.url(), json=get_servers, + headers=self.json_headers, + complete_qs=True) + + self.requests_mock.get(self.url(name='sample-server'), + json=get_servers, + headers=self.json_headers, + complete_qs=True) + + self.requests_mock.get(self.url(marker='5678'), + json={"servers": []}, headers=self.json_headers) self.server_1234 = { @@ -160,14 +170,32 @@ class Base(base.Fixture): self.requests_mock.get( self.url('detail', marker=self.server_1234["id"]), + json={"servers": [self.server_5678, self.server_9012]}, + headers=self.json_headers, complete_qs=True) + + self.requests_mock.get( + self.url('detail', marker=self.server_1234["id"], limit=2), + json={"servers": [self.server_5678, self.server_9012]}, + headers=self.json_headers, complete_qs=True) + + # simulate max_limit=2 by returning 2 items when limit=3 + # another request should be triggered with limit=1 to get complete + # result + self.requests_mock.get( + self.url('detail', limit=3), json={"servers": [self.server_1234, self.server_5678]}, headers=self.json_headers, complete_qs=True) self.requests_mock.get( - self.url('detail', marker=self.server_5678["id"]), - json={"servers": []}, + self.url('detail', marker=self.server_5678["id"], limit=1), + json={"servers": [self.server_9012]}, headers=self.json_headers, complete_qs=True) + self.requests_mock.get( + self.url('detail', marker=self.server_9012["id"]), + json={"servers": []}, + headers=self.json_headers) + self.server_1235 = self.server_1234.copy() self.server_1235['id'] = 1235 self.server_1235['status'] = 'error' diff --git a/novaclient/tests/unit/utils.py b/novaclient/tests/unit/utils.py index 49d3631f3..f036497a9 100644 --- a/novaclient/tests/unit/utils.py +++ b/novaclient/tests/unit/utils.py @@ -93,12 +93,16 @@ class FixturedTestCase(testscenarios.TestWithScenarios, TestCase): fix = self.data_fixture_class(self.requests_mock) self.data_fixture = self.useFixture(fix) - def assert_called(self, method, path, body=None): - self.assertEqual(self.requests_mock.last_request.method, method) - self.assertEqual(self.requests_mock.last_request.path_url, path) + def assert_called(self, method, path, body=None, pos=-1): + self.assertEqual( + self.requests_mock.request_history[pos].method, + method) + self.assertEqual( + self.requests_mock.request_history[pos].path_url, + path) if body: - req_data = self.requests_mock.last_request.body + req_data = self.requests_mock.request_history[pos].body if isinstance(req_data, six.binary_type): req_data = req_data.decode('utf-8') if not isinstance(body, six.string_types): diff --git a/novaclient/tests/unit/v2/fakes.py b/novaclient/tests/unit/v2/fakes.py index 803dc41ec..e853ef223 100644 --- a/novaclient/tests/unit/v2/fakes.py +++ b/novaclient/tests/unit/v2/fakes.py @@ -392,6 +392,8 @@ class FakeSessionClient(base_client.SessionClient): # def get_servers(self, **kw): + if kw.get('marker') == '9014': + return (200, {}, {"servers": []}) return (200, {}, {"servers": [ {'id': '1234', 'name': 'sample-server'}, {'id': '5678', 'name': 'sample-server2'}, @@ -399,6 +401,8 @@ class FakeSessionClient(base_client.SessionClient): ]}) def get_servers_detail(self, **kw): + if kw.get('marker') == '9014': + return (200, {}, {"servers": []}) return (200, {}, {"servers": [ { "id": '1234', diff --git a/novaclient/tests/unit/v2/test_servers.py b/novaclient/tests/unit/v2/test_servers.py index 07ad1497e..dfa1b6fd5 100644 --- a/novaclient/tests/unit/v2/test_servers.py +++ b/novaclient/tests/unit/v2/test_servers.py @@ -47,39 +47,44 @@ class ServersTest(utils.FixturedTestCase): """ return None - def test_list_servers(self): + def test_list_all_servers(self): sl = self.cs.servers.list() self.assert_request_id(sl, fakes.FAKE_REQUEST_ID_LIST) - self.assert_called('GET', '/servers/detail') + self.assert_called('GET', '/servers/detail', pos=-2) + self.assert_called('GET', '/servers/detail?marker=9012') for s in sl: self.assertIsInstance(s, servers.Server) def test_filter_servers_unicode(self): sl = self.cs.servers.list(search_opts={'name': u't€sting'}) self.assert_request_id(sl, fakes.FAKE_REQUEST_ID_LIST) - self.assert_called('GET', '/servers/detail?name=t%E2%82%ACsting') - for s in sl: - self.assertIsInstance(s, servers.Server) - - def test_list_all_servers(self): - # use marker just to identify this call in fixtures - sl = self.cs.servers.list(limit=-1, marker=1234) - self.assert_request_id(sl, fakes.FAKE_REQUEST_ID_LIST) - - self.assertEqual(2, len(sl)) - - self.assertEqual(self.requests_mock.request_history[-2].method, 'GET') - self.assertEqual(self.requests_mock.request_history[-2].path_url, - '/servers/detail?marker=1234') - self.assert_called('GET', '/servers/detail?marker=5678') - + self.assert_called( + 'GET', + '/servers/detail?name=t%E2%82%ACsting', + pos=-2) + self.assert_called( + 'GET', + '/servers/detail?marker=9012&name=t%E2%82%ACsting') for s in sl: self.assertIsInstance(s, servers.Server) def test_list_servers_undetailed(self): sl = self.cs.servers.list(detailed=False) self.assert_request_id(sl, fakes.FAKE_REQUEST_ID_LIST) - self.assert_called('GET', '/servers') + self.assert_called('GET', '/servers', pos=-2) + self.assert_called('GET', '/servers?marker=5678') + for s in sl: + self.assertIsInstance(s, servers.Server) + + def test_list_servers_with_marker(self): + sl = self.cs.servers.list(marker=1234) + self.assert_request_id(sl, fakes.FAKE_REQUEST_ID_LIST) + + self.assertEqual(2, len(sl)) + + self.assert_called('GET', '/servers/detail?marker=1234', pos=-2) + self.assert_called('GET', '/servers/detail?marker=9012') + for s in sl: self.assertIsInstance(s, servers.Server) @@ -89,6 +94,17 @@ class ServersTest(utils.FixturedTestCase): self.assert_called('GET', '/servers/detail?limit=2&marker=1234') for s in sl: self.assertIsInstance(s, servers.Server) + self.assertEqual(2, len(sl)) + + def test_list_servers_with_limit_above_max_limit(self): + # use limit=3 to trigger paging simulation on backend fixture side + sl = self.cs.servers.list(limit=3) + self.assert_request_id(sl, fakes.FAKE_REQUEST_ID_LIST) + self.assert_called('GET', '/servers/detail?limit=3', pos=-2) + self.assert_called('GET', '/servers/detail?limit=1&marker=5678') + for s in sl: + self.assertIsInstance(s, servers.Server) + self.assertEqual(3, len(sl)) def test_list_servers_sort_single(self): sl = self.cs.servers.list(sort_keys=['display_name'], @@ -96,7 +112,10 @@ class ServersTest(utils.FixturedTestCase): self.assert_request_id(sl, fakes.FAKE_REQUEST_ID_LIST) self.assert_called( 'GET', - '/servers/detail?sort_dir=asc&sort_key=display_name') + '/servers/detail?sort_dir=asc&sort_key=display_name', pos=-2) + self.assert_called( + 'GET', + '/servers/detail?marker=9012&sort_dir=asc&sort_key=display_name') for s in sl: self.assertIsInstance(s, servers.Server) @@ -107,6 +126,11 @@ class ServersTest(utils.FixturedTestCase): self.assert_called( 'GET', ('/servers/detail?sort_dir=asc&sort_dir=desc&' + 'sort_key=display_name&sort_key=id'), + pos=-2) + self.assert_called( + 'GET', + ('/servers/detail?marker=9012&sort_dir=asc&sort_dir=desc&' 'sort_key=display_name&sort_key=id')) for s in sl: self.assertIsInstance(s, servers.Server) diff --git a/novaclient/tests/unit/v2/test_shell.py b/novaclient/tests/unit/v2/test_shell.py index 785b9683f..dfa798b85 100644 --- a/novaclient/tests/unit/v2/test_shell.py +++ b/novaclient/tests/unit/v2/test_shell.py @@ -1324,63 +1324,99 @@ class ShellTest(utils.TestCase): def test_list(self): self.run_command('list') - self.assert_called('GET', '/servers/detail') + self.assert_called('GET', '/servers/detail', pos=0) + self.assert_called('GET', '/servers/detail?marker=9014') def test_list_minimal(self): self.run_command('list --minimal') - self.assert_called('GET', '/servers') + self.assert_called('GET', '/servers', pos=0) + self.assert_called('GET', '/servers?marker=9014') def test_list_deleted(self): self.run_command('list --deleted') - self.assert_called('GET', '/servers/detail?deleted=True') + self.assert_called( + 'GET', + '/servers/detail?deleted=True', + pos=0) + self.assert_called( + 'GET', + '/servers/detail?deleted=True&marker=9014') def test_list_with_images(self): self.run_command('list --image %s' % FAKE_UUID_1) - self.assert_called('GET', '/servers/detail?image=%s' % FAKE_UUID_1) + self.assert_called( + 'GET', + '/servers/detail?image=%s' % FAKE_UUID_1, + pos=1) + self.assert_called( + 'GET', + '/servers/detail?image=%s&marker=9014' % FAKE_UUID_1) def test_list_with_flavors(self): self.run_command('list --flavor 1') - self.assert_called('GET', '/servers/detail?flavor=1') + self.assert_called('GET', '/servers/detail?flavor=1', pos=1) + self.assert_called('GET', '/servers/detail?flavor=1&marker=9014') def test_list_by_tenant(self): self.run_command('list --tenant fake_tenant') self.assert_called( 'GET', - '/servers/detail?all_tenants=1&tenant_id=fake_tenant') + '/servers/detail?all_tenants=1&tenant_id=fake_tenant', pos=0) + self.assert_called( + 'GET', + '/servers/detail?all_tenants=1&marker=9014&tenant_id=fake_tenant') def test_list_by_user(self): self.run_command('list --user fake_user') self.assert_called( 'GET', - '/servers/detail?all_tenants=1&user_id=fake_user') + '/servers/detail?all_tenants=1&user_id=fake_user', pos=0) + self.assert_called( + 'GET', + '/servers/detail?all_tenants=1&marker=9014&user_id=fake_user') def test_list_with_single_sort_key_no_dir(self): self.run_command('list --sort 1') self.assert_called( - 'GET', ('/servers/detail?sort_dir=desc&sort_key=1')) + 'GET', ('/servers/detail?sort_dir=desc&sort_key=1'), pos=0) + self.assert_called( + 'GET', + '/servers/detail?marker=9014&sort_dir=desc&sort_key=1') def test_list_with_single_sort_key_and_dir(self): self.run_command('list --sort 1:asc') self.assert_called( - 'GET', ('/servers/detail?sort_dir=asc&sort_key=1')) + 'GET', ('/servers/detail?sort_dir=asc&sort_key=1'), pos=0) + self.assert_called( + 'GET', + '/servers/detail?marker=9014&sort_dir=asc&sort_key=1') def test_list_with_sort_keys_no_dir(self): self.run_command('list --sort 1,2') self.assert_called( 'GET', ('/servers/detail?sort_dir=desc&sort_dir=desc&' + 'sort_key=1&sort_key=2'), pos=0) + self.assert_called( + 'GET', ('/servers/detail?marker=9014&sort_dir=desc&sort_dir=desc&' 'sort_key=1&sort_key=2')) def test_list_with_sort_keys_and_dirs(self): self.run_command('list --sort 1:asc,2:desc') self.assert_called( 'GET', ('/servers/detail?sort_dir=asc&sort_dir=desc&' + 'sort_key=1&sort_key=2'), pos=0) + self.assert_called( + 'GET', ('/servers/detail?marker=9014&sort_dir=asc&sort_dir=desc&' 'sort_key=1&sort_key=2')) def test_list_with_sort_keys_and_some_dirs(self): self.run_command('list --sort 1,2:asc') self.assert_called( 'GET', ('/servers/detail?sort_dir=desc&sort_dir=asc&' - 'sort_key=1&sort_key=2')) + 'sort_key=1&sort_key=2'), pos=0) + self.assert_called( + 'GET', ('/servers/detail?marker=9014&sort_dir=desc&' + 'sort_dir=asc&sort_key=1&sort_key=2')) def test_list_with_invalid_sort_dir_one(self): cmd = 'list --sort 1:foo' @@ -1412,7 +1448,8 @@ class ShellTest(utils.TestCase): output, _err = self.run_command( 'list --fields ' 'host,security_groups,OS-EXT-MOD:some_thing') - self.assert_called('GET', '/servers/detail') + self.assert_called('GET', '/servers/detail', pos=0) + self.assert_called('GET', '/servers/detail?marker=9014') self.assertIn('computenode1', output) self.assertIn('securitygroup1', output) self.assertIn('OS-EXT-MOD: Some Thing', output) @@ -1454,7 +1491,8 @@ class ShellTest(utils.TestCase): def test_list_with_marker(self): self.run_command('list --marker some-uuid') - self.assert_called('GET', '/servers/detail?marker=some-uuid') + self.assert_called('GET', '/servers/detail?marker=some-uuid', pos=0) + self.assert_called('GET', '/servers/detail?marker=9014') def test_list_with_limit(self): self.run_command('list --limit 3') @@ -1463,7 +1501,13 @@ class ShellTest(utils.TestCase): def test_list_with_changes_since(self): self.run_command('list --changes-since 2016-02-29T06:23:22') self.assert_called( - 'GET', '/servers/detail?changes-since=2016-02-29T06%3A23%3A22') + 'GET', + '/servers/detail?changes-since=2016-02-29T06%3A23%3A22', + pos=0) + self.assert_called( + 'GET', + ('/servers/detail?changes-since=2016-02-29T06%3A23%3A22&' + 'marker=9014')) def test_list_with_changes_since_invalid_value(self): self.assertRaises(exceptions.CommandError, @@ -1501,12 +1545,14 @@ class ShellTest(utils.TestCase): output, _err = self.run_command('rebuild sample-server %s' % FAKE_UUID_1) self.assert_called('GET', '/servers?name=sample-server', pos=0) - self.assert_called('GET', '/servers/1234', pos=1) - self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_1, pos=2) + self.assert_called('GET', '/servers?marker=9014&name=sample-server', + pos=1) + self.assert_called('GET', '/servers/1234', pos=2) + self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_1, pos=3) self.assert_called('POST', '/servers/1234/action', - {'rebuild': {'imageRef': FAKE_UUID_1}}, pos=3) - self.assert_called('GET', '/flavors/1', pos=4) - self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_2, pos=5) + {'rebuild': {'imageRef': FAKE_UUID_1}}, pos=4) + self.assert_called('GET', '/flavors/1', pos=5) + self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_2, pos=6) self.assertIn('adminPass', output) def test_rebuild_password(self): @@ -1514,63 +1560,73 @@ class ShellTest(utils.TestCase): ' --rebuild-password asdf' % FAKE_UUID_1) self.assert_called('GET', '/servers?name=sample-server', pos=0) - self.assert_called('GET', '/servers/1234', pos=1) - self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_1, pos=2) + self.assert_called('GET', '/servers?marker=9014&name=sample-server', + pos=1) + self.assert_called('GET', '/servers/1234', pos=2) + self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_1, pos=3) self.assert_called('POST', '/servers/1234/action', {'rebuild': {'imageRef': FAKE_UUID_1, - 'adminPass': 'asdf'}}, pos=3) - self.assert_called('GET', '/flavors/1', pos=4) - self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_2, pos=5) + 'adminPass': 'asdf'}}, pos=4) + self.assert_called('GET', '/flavors/1', pos=5) + self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_2, pos=6) self.assertIn('adminPass', output) def test_rebuild_preserve_ephemeral(self): self.run_command('rebuild sample-server %s --preserve-ephemeral' % FAKE_UUID_1) self.assert_called('GET', '/servers?name=sample-server', pos=0) - self.assert_called('GET', '/servers/1234', pos=1) - self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_1, pos=2) + self.assert_called('GET', '/servers?marker=9014&name=sample-server', + pos=1) + self.assert_called('GET', '/servers/1234', pos=2) + self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_1, pos=3) self.assert_called('POST', '/servers/1234/action', {'rebuild': {'imageRef': FAKE_UUID_1, - 'preserve_ephemeral': True}}, pos=3) - self.assert_called('GET', '/flavors/1', pos=4) - self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_2, pos=5) + 'preserve_ephemeral': True}}, pos=4) + self.assert_called('GET', '/flavors/1', pos=5) + self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_2, pos=6) def test_rebuild_name_meta(self): self.run_command('rebuild sample-server %s --name asdf --meta ' 'foo=bar' % FAKE_UUID_1) self.assert_called('GET', '/servers?name=sample-server', pos=0) - self.assert_called('GET', '/servers/1234', pos=1) - self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_1, pos=2) + self.assert_called('GET', '/servers?marker=9014&name=sample-server', + pos=1) + self.assert_called('GET', '/servers/1234', pos=2) + self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_1, pos=3) self.assert_called('POST', '/servers/1234/action', {'rebuild': {'imageRef': FAKE_UUID_1, 'name': 'asdf', - 'metadata': {'foo': 'bar'}}}, pos=3) - self.assert_called('GET', '/flavors/1', pos=4) - self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_2, pos=5) + 'metadata': {'foo': 'bar'}}}, pos=4) + self.assert_called('GET', '/flavors/1', pos=5) + self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_2, pos=6) def test_rebuild_reset_keypair(self): self.run_command('rebuild sample-server %s --key-name test_keypair' % FAKE_UUID_1, api_version='2.54') self.assert_called('GET', '/servers?name=sample-server', pos=0) - self.assert_called('GET', '/servers/1234', pos=1) - self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_1, pos=2) + self.assert_called('GET', '/servers?marker=9014&name=sample-server', + pos=1) + self.assert_called('GET', '/servers/1234', pos=2) + self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_1, pos=3) self.assert_called('POST', '/servers/1234/action', {'rebuild': {'imageRef': FAKE_UUID_1, 'key_name': 'test_keypair', - 'description': None}}, pos=3) - self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_2, pos=4) + 'description': None}}, pos=4) + self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_2, pos=5) def test_rebuild_unset_keypair(self): self.run_command('rebuild sample-server %s --key-unset' % FAKE_UUID_1, api_version='2.54') self.assert_called('GET', '/servers?name=sample-server', pos=0) - self.assert_called('GET', '/servers/1234', pos=1) - self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_1, pos=2) + self.assert_called('GET', '/servers?marker=9014&name=sample-server', + pos=1) + self.assert_called('GET', '/servers/1234', pos=2) + self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_1, pos=3) self.assert_called('POST', '/servers/1234/action', {'rebuild': {'imageRef': FAKE_UUID_1, 'key_name': None, - 'description': None}}, pos=3) - self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_2, pos=4) + 'description': None}}, pos=4) + self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_2, pos=5) def test_rebuild_unset_keypair_with_key_name(self): ex = self.assertRaises( @@ -1612,25 +1668,29 @@ class ShellTest(utils.TestCase): FAKE_UUID_1, api_version='2.57') user_data = servers.ServerManager.transform_userdata('test') self.assert_called('GET', '/servers?name=sample-server', pos=0) - self.assert_called('GET', '/servers/1234', pos=1) - self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_1, pos=2) + self.assert_called('GET', '/servers?marker=9014&name=sample-server', + pos=1) + self.assert_called('GET', '/servers/1234', pos=2) + self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_1, pos=3) self.assert_called('POST', '/servers/1234/action', {'rebuild': {'imageRef': FAKE_UUID_1, 'user_data': user_data, - 'description': None}}, pos=3) - self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_2, pos=4) + 'description': None}}, pos=4) + self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_2, pos=5) def test_rebuild_unset_user_data(self): self.run_command('rebuild sample-server %s --user-data-unset' % FAKE_UUID_1, api_version='2.57') self.assert_called('GET', '/servers?name=sample-server', pos=0) - self.assert_called('GET', '/servers/1234', pos=1) - self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_1, pos=2) + self.assert_called('GET', '/servers?marker=9014&name=sample-server', + pos=1) + self.assert_called('GET', '/servers/1234', pos=2) + self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_1, pos=3) self.assert_called('POST', '/servers/1234/action', {'rebuild': {'imageRef': FAKE_UUID_1, 'user_data': None, - 'description': None}}, pos=3) - self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_2, pos=4) + 'description': None}}, pos=4) + self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_2, pos=5) def test_rebuild_user_data_and_unset_user_data(self): """Tests that trying to set --user-data and --unset-user-data in the @@ -1651,7 +1711,11 @@ class ShellTest(utils.TestCase): self.run_command('start sample-server --all-tenants') self.assert_called('GET', '/servers?all_tenants=1&name=sample-server', pos=0) - self.assert_called('GET', '/servers/1234', pos=1) + self.assert_called('GET', + ('/servers?all_tenants=1&marker=9014&' + 'name=sample-server'), + pos=1) + self.assert_called('GET', '/servers/1234', pos=2) self.assert_called('POST', '/servers/1234/action', {'os-start': None}) def test_stop(self): @@ -1662,7 +1726,11 @@ class ShellTest(utils.TestCase): self.run_command('stop sample-server --all-tenants') self.assert_called('GET', '/servers?all_tenants=1&name=sample-server', pos=0) - self.assert_called('GET', '/servers/1234', pos=1) + self.assert_called('GET', + ('/servers?all_tenants=1&marker=9014&' + 'name=sample-server'), + pos=1) + self.assert_called('GET', '/servers/1234', pos=2) self.assert_called('POST', '/servers/1234/action', {'os-stop': None}) def test_pause(self): @@ -1765,10 +1833,12 @@ class ShellTest(utils.TestCase): def test_show(self): self.run_command('show 1234') self.assert_called('GET', '/servers?name=1234', pos=0) - self.assert_called('GET', '/servers?name=1234', pos=1) - self.assert_called('GET', '/servers/1234', pos=2) - self.assert_called('GET', '/flavors/1', pos=3) - self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_2, pos=4) + self.assert_called('GET', '/servers?marker=9014&name=1234', pos=1) + self.assert_called('GET', '/servers?name=1234', pos=2) + self.assert_called('GET', '/servers?marker=9014&name=1234', pos=3) + self.assert_called('GET', '/servers/1234', pos=4) + self.assert_called('GET', '/flavors/1', pos=5) + self.assert_called('GET', '/v2/images/%s' % FAKE_UUID_2, pos=6) def test_show_no_image(self): self.run_command('show 9012') @@ -1827,21 +1897,31 @@ class ShellTest(utils.TestCase): self.run_command('restore sample-server') self.assert_called('GET', '/servers?deleted=True&name=sample-server', pos=0) - self.assert_called('GET', '/servers/1234', pos=1) + self.assert_called('GET', + ('/servers?deleted=True&marker=9014&' + 'name=sample-server'), + pos=1) + self.assert_called('GET', '/servers/1234', pos=2) self.assert_called('POST', '/servers/1234/action', {'restore': None}, - pos=2) + pos=3) def test_delete_two_with_two_existent(self): self.run_command('delete 1234 5678') - self.assert_called('DELETE', '/servers/1234', pos=-5) + self.assert_called('DELETE', '/servers/1234', pos=-7) self.assert_called('DELETE', '/servers/5678', pos=-1) self.run_command('delete sample-server sample-server2') self.assert_called('GET', - '/servers?name=sample-server', pos=-6) - self.assert_called('GET', '/servers/1234', pos=-5) - self.assert_called('DELETE', '/servers/1234', pos=-4) + '/servers?name=sample-server', pos=-8) + self.assert_called('GET', + '/servers?marker=9014&name=sample-server', + pos=-7) + self.assert_called('GET', '/servers/1234', pos=-6) + self.assert_called('DELETE', '/servers/1234', pos=-5) self.assert_called('GET', '/servers?name=sample-server2', + pos=-4) + self.assert_called('GET', + '/servers?marker=9014&name=sample-server2', pos=-3) self.assert_called('GET', '/servers/5678', pos=-2) self.assert_called('DELETE', '/servers/5678', pos=-1) @@ -1850,13 +1930,21 @@ class ShellTest(utils.TestCase): self.run_command('delete sample-server sample-server2 --all-tenants') self.assert_called('GET', '/servers?all_tenants=1&name=sample-server', pos=0) - self.assert_called('GET', '/servers/1234', pos=1) - self.assert_called('DELETE', '/servers/1234', pos=2) + self.assert_called('GET', + ('/servers?all_tenants=1&marker=9014&' + 'name=sample-server'), + pos=1) + self.assert_called('GET', '/servers/1234', pos=2) + self.assert_called('DELETE', '/servers/1234', pos=3) self.assert_called('GET', '/servers?all_tenants=1&name=sample-server2', - pos=3) - self.assert_called('GET', '/servers/5678', pos=4) - self.assert_called('DELETE', '/servers/5678', pos=5) + pos=4) + self.assert_called('GET', + ('/servers?all_tenants=1&marker=9014&' + 'name=sample-server2'), + pos=5) + self.assert_called('GET', '/servers/5678', pos=6) + self.assert_called('DELETE', '/servers/5678', pos=7) def test_delete_two_with_one_nonexistent(self): cmd = 'delete 1234 123456789' @@ -2503,21 +2591,25 @@ class ShellTest(utils.TestCase): self.run_command('reset-state sample-server --all-tenants') self.assert_called('GET', '/servers?all_tenants=1&name=sample-server', pos=0) - self.assert_called('GET', '/servers/1234', pos=1) + self.assert_called('GET', + ('/servers?all_tenants=1&marker=9014&' + 'name=sample-server'), + pos=1) + self.assert_called('GET', '/servers/1234', pos=2) self.assert_called('POST', '/servers/1234/action', {'os-resetState': {'state': 'error'}}) def test_reset_state_multiple(self): self.run_command('reset-state sample-server sample-server2') self.assert_called('POST', '/servers/1234/action', - {'os-resetState': {'state': 'error'}}, pos=-4) + {'os-resetState': {'state': 'error'}}, pos=-5) self.assert_called('POST', '/servers/5678/action', {'os-resetState': {'state': 'error'}}, pos=-1) def test_reset_state_active_multiple(self): self.run_command('reset-state --active sample-server sample-server2') self.assert_called('POST', '/servers/1234/action', - {'os-resetState': {'state': 'active'}}, pos=-4) + {'os-resetState': {'state': 'active'}}, pos=-5) self.assert_called('POST', '/servers/5678/action', {'os-resetState': {'state': 'active'}}, pos=-1) @@ -3533,7 +3625,8 @@ class ShellTest(utils.TestCase): def test_list_v2_10(self): self.run_command('list', api_version='2.10') - self.assert_called('GET', '/servers/detail') + self.assert_called('GET', '/servers/detail', pos=0) + self.assert_called('GET', '/servers/detail?marker=9014') def test_server_tag_add(self): self.run_command('server-tag-add sample-server tag', @@ -3576,19 +3669,43 @@ class ShellTest(utils.TestCase): def test_list_v2_26_tags(self): self.run_command('list --tags tag1,tag2', api_version='2.26') - self.assert_called('GET', '/servers/detail?tags=tag1%2Ctag2') + self.assert_called( + 'GET', + '/servers/detail?tags=tag1%2Ctag2', + pos=0) + self.assert_called( + 'GET', + '/servers/detail?marker=9014&tags=tag1%2Ctag2') def test_list_v2_26_tags_any(self): self.run_command('list --tags-any tag1,tag2', api_version='2.26') - self.assert_called('GET', '/servers/detail?tags-any=tag1%2Ctag2') + self.assert_called( + 'GET', + '/servers/detail?tags-any=tag1%2Ctag2', + pos=0) + self.assert_called( + 'GET', + '/servers/detail?marker=9014&tags-any=tag1%2Ctag2') def test_list_v2_26_not_tags(self): self.run_command('list --not-tags tag1,tag2', api_version='2.26') - self.assert_called('GET', '/servers/detail?not-tags=tag1%2Ctag2') + self.assert_called( + 'GET', + '/servers/detail?not-tags=tag1%2Ctag2', + pos=0) + self.assert_called( + 'GET', + '/servers/detail?marker=9014¬-tags=tag1%2Ctag2') def test_list_v2_26_not_tags_any(self): self.run_command('list --not-tags-any tag1,tag2', api_version='2.26') - self.assert_called('GET', '/servers/detail?not-tags-any=tag1%2Ctag2') + self.assert_called( + 'GET', + '/servers/detail?not-tags-any=tag1%2Ctag2', + pos=0) + self.assert_called( + 'GET', + '/servers/detail?marker=9014¬-tags-any=tag1%2Ctag2') class PollForStatusTestCase(utils.TestCase): diff --git a/novaclient/v2/servers.py b/novaclient/v2/servers.py index 79ab0cdce..805dd4541 100644 --- a/novaclient/v2/servers.py +++ b/novaclient/v2/servers.py @@ -855,7 +855,10 @@ class ServerManager(base.BootingManagerWithFind): result.extend(servers) result.append_request_ids(servers.request_ids) - if not servers or limit != -1: + if limit and limit != -1: + limit = max(limit - len(servers), 0) + + if not servers or limit == 0: break marker = result[-1].id return result diff --git a/novaclient/v2/shell.py b/novaclient/v2/shell.py index e73bd5040..532306454 100644 --- a/novaclient/v2/shell.py +++ b/novaclient/v2/shell.py @@ -1441,8 +1441,8 @@ def _print_flavor(flavor): default=None, help=_("Maximum number of servers to display. If limit == -1, all servers " "will be displayed. If limit is bigger than 'CONF.api.max_limit' " - "option of Nova API, limit 'CONF.api.max_limit' will be used " - "instead.")) + "option of Nova API, multiple requests will be sent and results " + "will be merged.")) @utils.arg( '--changes-since', dest='changes_since',