diff --git a/doc/api_samples/os-simple-tenant-usage/v2.40/simple-tenant-usage-get-all.json b/doc/api_samples/os-simple-tenant-usage/v2.40/simple-tenant-usage-get-all.json new file mode 100644 index 0000000000..d6a1be5cc6 --- /dev/null +++ b/doc/api_samples/os-simple-tenant-usage/v2.40/simple-tenant-usage-get-all.json @@ -0,0 +1,68 @@ +{ + "tenant_usages": [ + { + "server_usages": [ + { + "ended_at": null, + "flavor": "m1.tiny", + "hours": 1.0, + "instance_id": "1f1deceb-17b5-4c04-84c7-e0d4499c8f06", + "local_gb": 1, + "memory_mb": 512, + "name": "instance-3", + "started_at": "2018-10-09T11:29:04.166194", + "state": "active", + "tenant_id": "0000000e737461636b20342065000000", + "uptime": 3600, + "vcpus": 1 + } + ], + "start": "2018-10-09T11:29:04.166194", + "stop": "2018-10-09T12:29:04.166194", + "tenant_id": "0000000e737461636b20342065000000", + "total_hours": 1.0, + "total_local_gb_usage": 1.0, + "total_memory_mb_usage": 512.0, + "total_vcpus_usage": 1.0 + }, + { + "server_usages": [ + { + "ended_at": null, + "flavor": "m1.tiny", + "hours": 1.0, + "instance_id": "1f1deceb-17b5-4c04-84c7-e0d4499c8f00", + "local_gb": 1, + "memory_mb": 512, + "name": "instance-1", + "started_at": "2018-10-09T11:29:04.166194", + "state": "active", + "tenant_id": "6f70656e737461636b20342065766572", + "uptime": 3600, + "vcpus": 1 + }, + { + "ended_at": null, + "flavor": "m1.tiny", + "hours": 1.0, + "instance_id": "1f1deceb-17b5-4c04-84c7-e0d4499c8f03", + "local_gb": 1, + "memory_mb": 512, + "name": "instance-2", + "started_at": "2018-10-09T11:29:04.166194", + "state": "active", + "tenant_id": "6f70656e737461636b20342065766572", + "uptime": 3600, + "vcpus": 1 + } + ], + "start": "2018-10-09T11:29:04.166194", + "stop": "2018-10-09T12:29:04.166194", + "tenant_id": "6f70656e737461636b20342065766572", + "total_hours": 2.0, + "total_local_gb_usage": 2.0, + "total_memory_mb_usage": 1024.0, + "total_vcpus_usage": 2.0 + } + ] +} \ No newline at end of file diff --git a/nova/api/openstack/compute/simple_tenant_usage.py b/nova/api/openstack/compute/simple_tenant_usage.py index 7aa4dff1a6..7bcc084117 100644 --- a/nova/api/openstack/compute/simple_tenant_usage.py +++ b/nova/api/openstack/compute/simple_tenant_usage.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. +import collections import datetime import iso8601 @@ -142,7 +143,7 @@ class SimpleTenantUsageController(wsgi.Controller): instances = self._get_instances_all_cells(context, period_start, period_stop, tenant_id, limit, marker) - rval = {} + rval = collections.OrderedDict() flavors = {} all_server_usages = [] diff --git a/nova/tests/functional/api_sample_tests/api_sample_base.py b/nova/tests/functional/api_sample_tests/api_sample_base.py index 89d9d806aa..394c39a8fa 100644 --- a/nova/tests/functional/api_sample_tests/api_sample_base.py +++ b/nova/tests/functional/api_sample_tests/api_sample_base.py @@ -62,7 +62,7 @@ class ApiSampleTestBaseV21(testscenarios.WithScenarios, # any additional fixtures needed for this scenario _additional_fixtures = [] sample_dir = None - _project_id = True + _use_project_id = True scenarios = [ # test v2 with the v2.1 compatibility stack @@ -74,7 +74,7 @@ class ApiSampleTestBaseV21(testscenarios.WithScenarios, # test v2.18 code without project id ('v2_1_noproject_id', { 'api_major_version': 'v2.1', - '_project_id': False, + '_use_project_id': False, '_additional_fixtures': [ api_paste_fixture.ApiPasteNoProjectId]}) ] diff --git a/nova/tests/functional/api_sample_tests/api_samples/os-simple-tenant-usage/v2.40/simple-tenant-usage-get-all.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/os-simple-tenant-usage/v2.40/simple-tenant-usage-get-all.json.tpl new file mode 100644 index 0000000000..fde1ac3519 --- /dev/null +++ b/nova/tests/functional/api_sample_tests/api_samples/os-simple-tenant-usage/v2.40/simple-tenant-usage-get-all.json.tpl @@ -0,0 +1,68 @@ +{ + "tenant_usages": [ + { + "server_usages": [ + { + "ended_at": null, + "flavor": "m1.tiny", + "hours": 1.0, + "instance_id": "1f1deceb-17b5-4c04-84c7-e0d4499c8f06", + "local_gb": 1, + "memory_mb": 512, + "name": "instance-3", + "started_at": "%(strtime)s", + "state": "active", + "tenant_id": "0000000e737461636b20342065000000", + "uptime": 3600, + "vcpus": 1 + } + ], + "start": "%(strtime)s", + "stop": "%(strtime)s", + "tenant_id": "0000000e737461636b20342065000000", + "total_hours": 1.0, + "total_local_gb_usage": 1.0, + "total_memory_mb_usage": 512.0, + "total_vcpus_usage": 1.0 + }, + { + "server_usages": [ + { + "ended_at": null, + "flavor": "m1.tiny", + "hours": 1.0, + "instance_id": "1f1deceb-17b5-4c04-84c7-e0d4499c8f00", + "local_gb": 1, + "memory_mb": 512, + "name": "instance-1", + "started_at": "%(strtime)s", + "state": "active", + "tenant_id": "6f70656e737461636b20342065766572", + "uptime": 3600, + "vcpus": 1 + }, + { + "ended_at": null, + "flavor": "m1.tiny", + "hours": 1.0, + "instance_id": "1f1deceb-17b5-4c04-84c7-e0d4499c8f03", + "local_gb": 1, + "memory_mb": 512, + "name": "instance-2", + "started_at": "%(strtime)s", + "state": "active", + "tenant_id": "6f70656e737461636b20342065766572", + "uptime": 3600, + "vcpus": 1 + } + ], + "start": "%(strtime)s", + "stop": "%(strtime)s", + "tenant_id": "6f70656e737461636b20342065766572", + "total_hours": 2.0, + "total_local_gb_usage": 2.0, + "total_memory_mb_usage": 1024.0, + "total_vcpus_usage": 2.0 + } + ] +} diff --git a/nova/tests/functional/api_sample_tests/test_simple_tenant_usage.py b/nova/tests/functional/api_sample_tests/test_simple_tenant_usage.py index 3f71248a4e..9f67dbd6d1 100644 --- a/nova/tests/functional/api_sample_tests/test_simple_tenant_usage.py +++ b/nova/tests/functional/api_sample_tests/test_simple_tenant_usage.py @@ -79,6 +79,9 @@ class SimpleTenantUsageV240Test(test_servers.ServersSampleBase): super(SimpleTenantUsageV240Test, self).setUp() self.api.microversion = self.microversion + self.project_id_0 = astb.PROJECT_ID + self.project_id_1 = '0000000e737461636b20342065000000' + started = timeutils.utcnow() now = started + datetime.timedelta(hours=1) @@ -87,8 +90,11 @@ class SimpleTenantUsageV240Test(test_servers.ServersSampleBase): # make uuids incrementing, so that sort order is deterministic uuid_format = '1f1deceb-17b5-4c04-84c7-e0d4499c8f%02d' mock_uuids.side_effect = [uuid_format % x for x in range(100)] + self.project_id = self.project_id_0 self.instance1_uuid = self._post_server(name='instance-1') self.instance2_uuid = self._post_server(name='instance-2') + + self.project_id = self.project_id_1 self.instance3_uuid = self._post_server(name='instance-3') timeutils.set_time_override(now) @@ -118,9 +124,27 @@ class SimpleTenantUsageV240Test(test_servers.ServersSampleBase): self._verify_response(template_name, {}, response, 200) def test_get_tenant_usage_details(self): - tenant_id = astb.PROJECT_ID + tenant_id = self.project_id_0 url = 'os-simple-tenant-usage/{tenant}?%s'.format(tenant=tenant_id) response = self._do_get(url % (parse.urlencode(self.query))) template_name = 'simple-tenant-usage-get-specific' - subs = {'tenant_id': self.api.project_id} + subs = {'tenant_id': self.project_id_0} self._verify_response(template_name, subs, response, 200) + + def test_get_tenants_usage_end_marker(self): + # When using the last server retrieved as a marker, + # the subsequent usages list should be empty (bug #1796689). + url = 'os-simple-tenant-usage?%s' + query = dict(detailed=1, + start=self.query['start'], + end=self.query['end']) + + response = self._do_get(url % (parse.urlencode(query))) + template_name = 'simple-tenant-usage-get-all' + self._verify_response(template_name, {}, response, 200) + + last_server = response.json()['tenant_usages'][-1]['server_usages'][-1] + query['marker'] = last_server['instance_id'] + + response = self._do_get(url % (parse.urlencode(query))) + self.assertEqual(0, len(response.json()['tenant_usages'])) diff --git a/nova/tests/functional/api_samples_test_base.py b/nova/tests/functional/api_samples_test_base.py index 130c064a86..c1a6ab3798 100644 --- a/nova/tests/functional/api_samples_test_base.py +++ b/nova/tests/functional/api_samples_test_base.py @@ -303,6 +303,24 @@ class ApiSampleTestBase(integrated_helpers._IntegratedTestBase): return matched_value + @property + def project_id(self): + # We'll allow test cases to override the default project id. This is + # useful when using multiple tenants. + project_id = None + try: + project_id = self.api.project_id + except AttributeError: + pass + + return project_id or PROJECT_ID + + @project_id.setter + def project_id(self, project_id): + self.api.project_id = project_id + # Reset cached credentials + self.api.auth_result = None + def generalize_subs(self, subs, vanilla_regexes): """Give the test a chance to modify subs after the server response was verified, and before the on-disk doc/api_samples file is checked. @@ -317,17 +335,20 @@ class ApiSampleTestBase(integrated_helpers._IntegratedTestBase): def _update_links(self, sample_data): """Process sample data and update version specific links.""" # replace version urls - url_re = self._get_host() + "/v(2|2\.1)/" + PROJECT_ID + + project_id_exp = '(%s|%s)' % (PROJECT_ID, self.project_id) + + url_re = self._get_host() + "/v(2|2\.1)/" + project_id_exp new_url = self._get_host() + "/" + self.api_major_version - if self._project_id: - new_url += "/" + PROJECT_ID + if self._use_project_id: + new_url += "/" + self.project_id updated_data = re.sub(url_re, new_url, sample_data) # replace unversioned urls - url_re = self._get_host() + "/" + PROJECT_ID + url_re = self._get_host() + "/" + project_id_exp new_url = self._get_host() - if self._project_id: - new_url += "/" + PROJECT_ID + if self._use_project_id: + new_url += "/" + self.project_id updated_data = re.sub(url_re, new_url, updated_data) return updated_data @@ -461,17 +482,17 @@ class ApiSampleTestBase(integrated_helpers._IntegratedTestBase): def _get_compute_endpoint(self): # NOTE(sdague): "openstack" is stand in for project_id, it # should be more generic in future. - if self._project_id: - return '%s/%s' % (self._get_host(), PROJECT_ID) + if self._use_project_id: + return '%s/%s' % (self._get_host(), self.project_id) else: return self._get_host() def _get_vers_compute_endpoint(self): # NOTE(sdague): "openstack" is stand in for project_id, it # should be more generic in future. - if self._project_id: + if self._use_project_id: return '%s/%s/%s' % (self._get_host(), self.api_major_version, - PROJECT_ID) + self.project_id) else: return '%s/%s' % (self._get_host(), self.api_major_version) diff --git a/nova/tests/unit/api_samples_test_base/test_compare_result.py b/nova/tests/unit/api_samples_test_base/test_compare_result.py index de7cdce767..6066cf186d 100644 --- a/nova/tests/unit/api_samples_test_base/test_compare_result.py +++ b/nova/tests/unit/api_samples_test_base/test_compare_result.py @@ -43,7 +43,7 @@ class TestCompareResult(test.NoDBTestCase): # required by ApiSampleTestBase ast_instance.api_major_version = 'v2' - ast_instance._project_id = 'True' + ast_instance._use_project_id = 'True' # automagically create magic methods usually handled by test classes ast_instance.compute = mock.MagicMock() diff --git a/releasenotes/notes/fix-simple-tenant-usage-pagination-393ed6e7d0e31594.yaml b/releasenotes/notes/fix-simple-tenant-usage-pagination-393ed6e7d0e31594.yaml new file mode 100644 index 0000000000..b68db5e457 --- /dev/null +++ b/releasenotes/notes/fix-simple-tenant-usage-pagination-393ed6e7d0e31594.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + The ``os-simple-tenant-usage`` pagination has been fixed. In some cases, + nova usage-list would have returned incorrect results because of this. + See bug https://launchpad.net/bugs/1796689 for details.