From 2c4c4aa77a092d673f90ea5e73599fc43b6ec7be Mon Sep 17 00:00:00 2001 From: David Lyle Date: Fri, 6 Oct 2017 10:44:03 -0600 Subject: [PATCH] Correct quota usage check for instances Previously, updating the quota value for instance on a project that the user was not currently scoped to would validate the new quota value by comparing with the usage of the currently scoped project. The cause of this error was an incorrect use of the server list parameters. According to the comment in nova source: https://github.com/openstack/nova/blob/d43d1d673403c1bf9e2ffd94d7a711956a3506de/nova/api/openstack/compute/servers.py#L277-L280 and the comments in this bug: https://bugs.launchpad.net/nova/+bug/1185290 indicate that the tenant_id or project_id flag cannot be used without the --all-tenants flag. The fix involves passing the --all-tenants flag when querying instance usage for any project the user is not currently scoped to. It also removes the passing of the tenant_id flag when querying the current project. Tests were also updated to match the new behavior by not passing the tenant_id to the API call to list servers where the target project is the currently scoped project. Change-Id: Iee06bc1c8ccd50f595f4cb274f956c13495c8494 Closes-Bug: #1713724 (cherry picked from commit 33dc94079cc25128998d2f339e664a775ce82396) --- openstack_dashboard/test/tests/quotas.py | 85 +++++++++++++++++++----- openstack_dashboard/usage/quotas.py | 6 +- 2 files changed, 71 insertions(+), 20 deletions(-) diff --git a/openstack_dashboard/test/tests/quotas.py b/openstack_dashboard/test/tests/quotas.py index 0518689ecb..8e01e4292f 100644 --- a/openstack_dashboard/test/tests/quotas.py +++ b/openstack_dashboard/test/tests/quotas.py @@ -34,7 +34,7 @@ from openstack_dashboard.usage import quotas class QuotaTests(test.APITestCase): def get_usages(self, with_volume=True, with_compute=True, - nova_quotas_enabled=True): + nova_quotas_enabled=True, tenant_id=None): usages = {} if with_compute: # These are all nova fields; the neutron ones are named slightly @@ -54,6 +54,14 @@ class QuotaTests(test.APITestCase): 'key_pairs': {'quota': 100}, 'injected_file_path_bytes': {'quota': 255} }) + if tenant_id == 3: + usages.update({ + 'ram': {'available': 10000, + 'used': 0, + 'quota': 10000}, + 'instances': {'available': 10, 'used': 2, 'quota': 10}, + 'cores': {'available': 10, 'used': 2, 'quota': 10} + }) if with_volume: usages.update({'volumes': {'available': 0, 'used': 4, 'quota': 1}, @@ -70,6 +78,58 @@ class QuotaTests(test.APITestCase): actual_usages.items() if 'available' in value} self.assertEqual(expected_available, actual_available) + @test.create_stubs({api.nova: ('server_list', + 'flavor_list', + 'tenant_quota_get',), + api.neutron: ('tenant_floating_ip_list', + 'floating_ip_supported'), + api.base: ('is_service_enabled',), + cinder: ('volume_list', 'volume_snapshot_list', + 'tenant_quota_get', + 'is_volume_service_enabled')}) + def test_tenant_quota_usages_with_id(self): + tenant_id = 3 + cinder.is_volume_service_enabled(IsA(http.HttpRequest)).AndReturn(True) + api.base.is_service_enabled(IsA(http.HttpRequest), + 'network').AndReturn(False) + api.base.is_service_enabled(IsA(http.HttpRequest), 'compute') \ + .MultipleTimes().AndReturn(True) + servers = [s for s in self.servers.list() if s.tenant_id == tenant_id] + api.nova.flavor_list(IsA(http.HttpRequest)) \ + .AndReturn(self.flavors.list()) + api.neutron.floating_ip_supported(IsA(http.HttpRequest)) \ + .AndReturn(True) + api.neutron.tenant_floating_ip_list(IsA(http.HttpRequest)) \ + .AndReturn(self.floating_ips.list()) + opts = {'tenant_id': tenant_id, + 'all_tenants': 1} + api.nova.server_list(IsA(http.HttpRequest), search_opts=opts) \ + .AndReturn([servers, False]) + api.nova.tenant_quota_get(IsA(http.HttpRequest), tenant_id) \ + .AndReturn(self.quotas.first()) + + opts = {'all_tenants': 1, + 'project_id': tenant_id} + cinder.volume_list(IsA(http.HttpRequest), opts) \ + .AndReturn(self.volumes.list()) + cinder.volume_snapshot_list(IsA(http.HttpRequest), opts) \ + .AndReturn(self.cinder_volume_snapshots.list()) + cinder.tenant_quota_get(IsA(http.HttpRequest), tenant_id) \ + .AndReturn(self.cinder_quotas.first()) + + self.mox.ReplayAll() + + quota_usages = quotas.tenant_quota_usages(self.request, + tenant_id=tenant_id) + expected_output = self.get_usages( + nova_quotas_enabled=True, with_volume=True, + with_compute=True, tenant_id=tenant_id) + + # Compare internal structure of usages to expected. + self.assertItemsEqual(expected_output, quota_usages.usages) + # Compare available resources + self.assertAvailableQuotasEqual(expected_output, quota_usages.usages) + @test.create_stubs({api.nova: ('server_list', 'flavor_list', 'tenant_quota_get',), @@ -99,9 +159,7 @@ class QuotaTests(test.APITestCase): .AndReturn(True) api.neutron.tenant_floating_ip_list(IsA(http.HttpRequest)) \ .AndReturn(self.floating_ips.list()) - search_opts = {'tenant_id': self.request.user.tenant_id} - api.nova.server_list(IsA(http.HttpRequest), - search_opts=search_opts) \ + api.nova.server_list(IsA(http.HttpRequest)) \ .AndReturn([servers, False]) api.nova.tenant_quota_get(IsA(http.HttpRequest), '1') \ .AndReturn(self.quotas.first()) @@ -182,8 +240,7 @@ class QuotaTests(test.APITestCase): .AndReturn(True) api.neutron.tenant_floating_ip_list(IsA(http.HttpRequest)) \ .AndReturn(self.floating_ips.list()) - search_opts = {'tenant_id': self.request.user.tenant_id} - api.nova.server_list(IsA(http.HttpRequest), search_opts=search_opts) \ + api.nova.server_list(IsA(http.HttpRequest)) \ .AndReturn([servers, False]) self.mox.ReplayAll() @@ -222,9 +279,7 @@ class QuotaTests(test.APITestCase): .AndReturn(True) api.neutron.tenant_floating_ip_list(IsA(http.HttpRequest)) \ .AndReturn([]) - search_opts = {'tenant_id': self.request.user.tenant_id} - api.nova.server_list(IsA(http.HttpRequest), search_opts=search_opts) \ - .AndReturn([[], False]) + api.nova.server_list(IsA(http.HttpRequest)).AndReturn([[], False]) self.mox.ReplayAll() @@ -270,9 +325,7 @@ class QuotaTests(test.APITestCase): .AndReturn(True) api.neutron.tenant_floating_ip_list(IsA(http.HttpRequest)) \ .AndReturn(self.floating_ips.list()) - search_opts = {'tenant_id': self.request.user.tenant_id} - api.nova.server_list(IsA(http.HttpRequest), search_opts=search_opts) \ - .AndReturn([servers, False]) + api.nova.server_list(IsA(http.HttpRequest)).AndReturn([servers, False]) opts = {'all_tenants': 1, 'project_id': self.request.user.tenant_id} cinder.volume_list(IsA(http.HttpRequest), opts) \ .AndReturn(self.volumes.list()) @@ -318,9 +371,7 @@ class QuotaTests(test.APITestCase): .AndReturn(self.quotas.first()) api.neutron.floating_ip_supported(IsA(http.HttpRequest)) \ .AndReturn(False) - search_opts = {'tenant_id': self.request.user.tenant_id} - api.nova.server_list(IsA(http.HttpRequest), search_opts=search_opts) \ - .AndReturn([servers, False]) + api.nova.server_list(IsA(http.HttpRequest)).AndReturn([servers, False]) opts = {'all_tenants': 1, 'project_id': self.request.user.tenant_id} cinder.volume_list(IsA(http.HttpRequest), opts) \ .AndReturn(self.volumes.list()) @@ -453,9 +504,7 @@ class QuotaTests(test.APITestCase): if use_flavor_list: api.nova.flavor_list(IsA(http.HttpRequest)) \ .AndReturn(self.flavors.list()) - search_opts = {'tenant_id': self.request.user.tenant_id} - api.nova.server_list(IsA(http.HttpRequest), - search_opts=search_opts) \ + api.nova.server_list(IsA(http.HttpRequest)) \ .AndReturn([servers, False]) api.nova.tenant_quota_get(IsA(http.HttpRequest), '1') \ .AndReturn(self.quotas.first()) diff --git a/openstack_dashboard/usage/quotas.py b/openstack_dashboard/usage/quotas.py index 3d72e6f564..4333a247d2 100644 --- a/openstack_dashboard/usage/quotas.py +++ b/openstack_dashboard/usage/quotas.py @@ -310,9 +310,11 @@ def _get_tenant_compute_usages(request, usages, disabled_quotas, tenant_id): if not base.is_service_enabled(request, 'compute'): return - if tenant_id: + if tenant_id and tenant_id != request.user.project_id: + # all_tenants is required when querying about any project the user is + # not currently scoped to instances, has_more = nova.server_list( - request, search_opts={'tenant_id': tenant_id}) + request, search_opts={'tenant_id': tenant_id, 'all_tenants': True}) else: instances, has_more = nova.server_list(request)