From 81891697c6c8d2cf1ee0bfceb3512e174624bc00 Mon Sep 17 00:00:00 2001 From: Akihiro Motoki Date: Wed, 12 Apr 2017 16:34:06 +0000 Subject: [PATCH] Retrieve quota and usage only for quota-enabled resources Previously _get_tenant_(compute|network|volume)_usage are callled regardless of resources are listed in enabled_quotas. This commit retrives usage information only for resources listed in eanbled_quotas. floating_ips and security_groups were returned even when they are included in disabled_quotas. They are not needed and they are cleanup in this commit. All (*_)QUOTA_FILEDS were previously a list but it is always used after converting into a set, so this commit changes (*_)QUOTA_FIELDS into a set in the initial definitions. This is a preparation for fix of bug 1675504 Partial-Bug: #1675504 Change-Id: I41cdabde477d87aa8b35d1c908e18a69454286c3 --- .../dashboards/identity/projects/tests.py | 10 +- .../dashboards/identity/projects/workflows.py | 10 +- .../test/api_tests/cinder_rest_tests.py | 4 +- .../test/api_tests/neutron_rest_tests.py | 4 +- .../test/api_tests/nova_rest_tests.py | 21 ++-- openstack_dashboard/test/tests/quotas.py | 43 +++---- openstack_dashboard/usage/quotas.py | 105 +++++++++++------- 7 files changed, 104 insertions(+), 93 deletions(-) diff --git a/openstack_dashboard/dashboards/identity/projects/tests.py b/openstack_dashboard/dashboards/identity/projects/tests.py index 4b0dc2f95c..a91e8177f8 100644 --- a/openstack_dashboard/dashboards/identity/projects/tests.py +++ b/openstack_dashboard/dashboards/identity/projects/tests.py @@ -385,7 +385,7 @@ class CreateProjectWorkflowTests(test.BaseAdminViewTests): project=self.tenant.id) nova_updated_quota = {key: quota_data[key] for key in - set(quotas.NOVA_QUOTA_FIELDS) - disabled_quotas} + quotas.NOVA_QUOTA_FIELDS - disabled_quotas} quotas.get_disabled_quotas(IsA(http.HttpRequest)) \ .AndReturn(disabled_quotas) api.nova.tenant_quota_update(IsA(http.HttpRequest), @@ -604,7 +604,7 @@ class CreateProjectWorkflowTests(test.BaseAdminViewTests): project=self.tenant.id) nova_updated_quota = {key: quota_data[key] for key in - set(quotas.NOVA_QUOTA_FIELDS) - disabled_quotas} + quotas.NOVA_QUOTA_FIELDS - disabled_quotas} quotas.get_disabled_quotas(IsA(http.HttpRequest)) \ .AndReturn(disabled_quotas) api.nova.tenant_quota_update(IsA(http.HttpRequest), @@ -688,7 +688,7 @@ class CreateProjectWorkflowTests(test.BaseAdminViewTests): break nova_updated_quota = {key: quota_data[key] for key in - set(quotas.NOVA_QUOTA_FIELDS) - disabled_quotas} + quotas.NOVA_QUOTA_FIELDS - disabled_quotas} quotas.get_disabled_quotas(IsA(http.HttpRequest)) \ .AndReturn(disabled_quotas) api.nova.tenant_quota_update(IsA(http.HttpRequest), @@ -1075,7 +1075,7 @@ class UpdateProjectWorkflowTests(test.BaseAdminViewTests): .AndReturn(quota_usages) nova_updated_quota = {key: updated_quota[key] for key in - set(quotas.NOVA_QUOTA_FIELDS) - disabled_quotas} + quotas.NOVA_QUOTA_FIELDS - disabled_quotas} quotas.get_disabled_quotas(IsA(http.HttpRequest)) \ .AndReturn(disabled_quotas) api.nova.tenant_quota_update(IsA(http.HttpRequest), @@ -1366,7 +1366,7 @@ class UpdateProjectWorkflowTests(test.BaseAdminViewTests): .AndReturn(quota_usages) nova_updated_quota = {key: updated_quota[key] for key in - set(quotas.NOVA_QUOTA_FIELDS) - disabled_quotas} + quotas.NOVA_QUOTA_FIELDS - disabled_quotas} quotas.get_disabled_quotas(IsA(http.HttpRequest)) \ .AndReturn(disabled_quotas) api.nova.tenant_quota_update(IsA(http.HttpRequest), diff --git a/openstack_dashboard/dashboards/identity/projects/workflows.py b/openstack_dashboard/dashboards/identity/projects/workflows.py index 6b3e7179fb..4a98baf196 100644 --- a/openstack_dashboard/dashboards/identity/projects/workflows.py +++ b/openstack_dashboard/dashboards/identity/projects/workflows.py @@ -411,24 +411,20 @@ class CommonQuotaWorkflow(workflows.Workflow): # Update the project quotas. if api.base.is_service_enabled(request, 'compute'): nova_data = {key: data[key] for key in - quotas.NOVA_QUOTA_FIELDS - if key not in disabled_quotas} + quotas.NOVA_QUOTA_FIELDS - disabled_quotas} if nova_data: nova.tenant_quota_update(request, project_id, **nova_data) if cinder.is_volume_service_enabled(request): cinder_data = {key: data[key] for key in - quotas.CINDER_QUOTA_FIELDS - if key not in disabled_quotas and - data[key] is not None} + quotas.CINDER_QUOTA_FIELDS - disabled_quotas} if cinder_data: cinder.tenant_quota_update(request, project_id, **cinder_data) if (api.base.is_service_enabled(request, 'network') and api.neutron.is_quotas_extension_supported(request)): neutron_data = {key: data[key] for key in - quotas.NEUTRON_QUOTA_FIELDS - if key not in disabled_quotas} + quotas.NEUTRON_QUOTA_FIELDS - disabled_quotas} if neutron_data: api.neutron.tenant_quota_update(request, project_id, **neutron_data) diff --git a/openstack_dashboard/test/api_tests/cinder_rest_tests.py b/openstack_dashboard/test/api_tests/cinder_rest_tests.py index 925dffa6ba..0126bd5221 100644 --- a/openstack_dashboard/test/api_tests/cinder_rest_tests.py +++ b/openstack_dashboard/test/api_tests/cinder_rest_tests.py @@ -446,7 +446,7 @@ class CinderRestTestCase(test.TestCase): ''') qc.get_disabled_quotas.return_value = [] - qc.CINDER_QUOTA_FIELDS = (n for n in quota_data) + qc.CINDER_QUOTA_FIELDS = {n for n in quota_data} cc.is_volume_service_enabled.return_value = True response = cinder.QuotaSets().patch(request, 'spam123') @@ -473,7 +473,7 @@ class CinderRestTestCase(test.TestCase): ''') qc.get_disabled_quotas.return_value = [] - qc.CINDER_QUOTA_FIELDS = (n for n in quota_data) + qc.CINDER_QUOTA_FIELDS = {n for n in quota_data} cc.is_volume_service_enabled.return_value = False response = cinder.QuotaSets().patch(request, 'spam123') diff --git a/openstack_dashboard/test/api_tests/neutron_rest_tests.py b/openstack_dashboard/test/api_tests/neutron_rest_tests.py index ee82ec2464..6186fa7fae 100644 --- a/openstack_dashboard/test/api_tests/neutron_rest_tests.py +++ b/openstack_dashboard/test/api_tests/neutron_rest_tests.py @@ -240,7 +240,7 @@ class NeutronQuotaSetsTestCase(test.TestCase): ''') qc.get_disabled_quotas.return_value = [] - qc.NEUTRON_QUOTA_FIELDS = (n for n in self._quota_data) + qc.NEUTRON_QUOTA_FIELDS = {n for n in self._quota_data} bc.is_service_enabled.return_value = True nc.is_extension_supported.return_value = True @@ -266,7 +266,7 @@ class NeutronQuotaSetsTestCase(test.TestCase): ''') qc.get_disabled_quotas.return_value = [] - qc.NEUTRON_QUOTA_FIELDS = (n for n in self._quota_data) + qc.NEUTRON_QUOTA_FIELDS = {n for n in self._quota_data} bc.is_service_enabled.return_value = False response = neutron.QuotasSets().patch(request, 'spam123') diff --git a/openstack_dashboard/test/api_tests/nova_rest_tests.py b/openstack_dashboard/test/api_tests/nova_rest_tests.py index 539f620098..c3d0cbfa8e 100644 --- a/openstack_dashboard/test/api_tests/nova_rest_tests.py +++ b/openstack_dashboard/test/api_tests/nova_rest_tests.py @@ -947,16 +947,19 @@ class NovaRestTestCase(test.TestCase): @mock.patch.object(nova, 'quotas') @mock.patch.object(nova.api, 'nova') def test_editable_quotas_get(self, nc, qc): - disabled_quotas = ['floating_ips', 'fixed_ips', - 'security_groups', 'security_group_rules'] - editable_quotas = ['cores', 'volumes', 'network', 'fixed_ips'] + disabled_quotas = {'floating_ips', 'fixed_ips', + 'security_groups', 'security_group_rules'} + editable_quotas = {'cores', 'volumes', 'network', 'fixed_ips'} qc.get_disabled_quotas.return_value = disabled_quotas qc.QUOTA_FIELDS = editable_quotas request = self.mock_rest_request() response = nova.EditableQuotaSets().get(request) self.assertStatusCode(response, 200) - self.assertItemsCollectionEqual(response, - ['cores', 'volumes', 'network']) + # NOTE(amotoki): assertItemsCollectionEqual cannot be used below + # since the item list is generated from a set and the order of items + # is unpredictable. + self.assertEqual(set(response.json['items']), + {'cores', 'volumes', 'network'}) @mock.patch.object(nova.api, 'nova') @mock.patch.object(nova.api, 'base') @@ -978,8 +981,8 @@ class NovaRestTestCase(test.TestCase): "security_group_rules": "100", "volumes": "10"} ''') - qc.get_disabled_quotas.return_value = [] - qc.NOVA_QUOTA_FIELDS = (n for n in quota_data) + qc.get_disabled_quotas.return_value = set() + qc.NOVA_QUOTA_FIELDS = {n for n in quota_data} bc.is_service_enabled.return_value = True response = nova.QuotaSets().patch(request, 'spam123') @@ -1009,8 +1012,8 @@ class NovaRestTestCase(test.TestCase): "security_group_rules": "100", "volumes": "10"} ''') - qc.get_disabled_quotas.return_value = [] - qc.NOVA_QUOTA_FIELDS = (n for n in quota_data) + qc.get_disabled_quotas.return_value = {} + qc.NOVA_QUOTA_FIELDS = {n for n in quota_data} bc.is_service_enabled.return_value = False response = nova.QuotaSets().patch(request, 'spam123') diff --git a/openstack_dashboard/test/tests/quotas.py b/openstack_dashboard/test/tests/quotas.py index 624a5fce54..24d447bf19 100644 --- a/openstack_dashboard/test/tests/quotas.py +++ b/openstack_dashboard/test/tests/quotas.py @@ -54,16 +54,6 @@ class QuotaTests(test.APITestCase): 'key_pairs': {'quota': 100}, 'injected_file_path_bytes': {'quota': 255} }) - else: - inf = float('inf') - usages.update({ - 'security_groups': {'available': inf, 'quota': inf}, - 'ram': {'available': inf, 'used': 1024, 'quota': inf}, - 'floating_ips': {'available': inf, 'used': 2, - 'quota': inf}, - 'instances': {'available': inf, 'used': 2, 'quota': inf}, - 'cores': {'available': inf, 'used': 2, 'quota': inf} - }) if with_volume: usages.update({'volumes': {'available': 0, 'used': 4, 'quota': 1}, @@ -100,20 +90,19 @@ class QuotaTests(test.APITestCase): IsA(http.HttpRequest), 'compute' ).MultipleTimes().AndReturn(with_compute) if with_compute: - servers = [s for s in self.servers.list() - if s.tenant_id == self.request.user.tenant_id] - api.nova.flavor_list(IsA(http.HttpRequest)) \ - .AndReturn(self.flavors.list()) - api.network.floating_ip_supported(IsA(http.HttpRequest)) \ - .AndReturn(True) - api.network.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]) - if nova_quotas_enabled: + servers = [s for s in self.servers.list() + if s.tenant_id == self.request.user.tenant_id] + api.nova.flavor_list(IsA(http.HttpRequest)) \ + .AndReturn(self.flavors.list()) + api.network.floating_ip_supported(IsA(http.HttpRequest)) \ + .AndReturn(True) + api.network.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.tenant_quota_get(IsA(http.HttpRequest), '1') \ .AndReturn(self.quotas.first()) @@ -162,9 +151,9 @@ class QuotaTests(test.APITestCase): self.mox.ReplayAll() result_quotas = quotas.get_disabled_quotas(self.request) - expected_quotas = list(quotas.CINDER_QUOTA_FIELDS) + \ - list(quotas.NEUTRON_QUOTA_FIELDS) + \ - list(quotas.NOVA_QUOTA_FIELDS) + expected_quotas = (quotas.CINDER_QUOTA_FIELDS | + quotas.NEUTRON_QUOTA_FIELDS | + quotas.NOVA_QUOTA_FIELDS) self.assertItemsEqual(result_quotas, expected_quotas) @test.create_stubs({api.nova: ('server_list', @@ -359,7 +348,7 @@ class QuotaTests(test.APITestCase): _("Unable to retrieve volume limit information.")) self.mox.ReplayAll() - quotas._get_tenant_volume_usages(self.request, {}, [], None) + quotas._get_tenant_volume_usages(self.request, {}, set(), None) @test.create_stubs({api.base: ('is_service_enabled',), api.cinder: ('tenant_quota_get', diff --git a/openstack_dashboard/usage/quotas.py b/openstack_dashboard/usage/quotas.py index d30723d544..1c2dcfeff9 100644 --- a/openstack_dashboard/usage/quotas.py +++ b/openstack_dashboard/usage/quotas.py @@ -30,35 +30,40 @@ from openstack_dashboard.contrib.developer.profiler import api as profiler LOG = logging.getLogger(__name__) -NOVA_QUOTA_FIELDS = ("metadata_items", - "cores", - "instances", - "injected_files", - "injected_file_content_bytes", - "ram", - "floating_ips", - "fixed_ips", - "security_groups", - "security_group_rules", - "key_pairs", - "injected_file_path_bytes", - ) +NOVA_COMPUTE_QUOTA_FIELDS = { + "metadata_items", + "cores", + "instances", + "injected_files", + "injected_file_content_bytes", + "injected_file_path_bytes", + "ram", + "key_pairs", +} +NOVA_NETWORK_QUOTA_FIELDS = { + "floating_ips", + "fixed_ips", + "security_groups", + "security_group_rules", +} -CINDER_QUOTA_FIELDS = ("volumes", +NOVA_QUOTA_FIELDS = NOVA_COMPUTE_QUOTA_FIELDS | NOVA_NETWORK_QUOTA_FIELDS + +CINDER_QUOTA_FIELDS = {"volumes", "snapshots", - "gigabytes",) + "gigabytes"} -NEUTRON_QUOTA_FIELDS = ("network", +NEUTRON_QUOTA_FIELDS = {"network", "subnet", "port", "router", "floatingip", "security_group", "security_group_rule", - ) + } -QUOTA_FIELDS = NOVA_QUOTA_FIELDS + CINDER_QUOTA_FIELDS + NEUTRON_QUOTA_FIELDS +QUOTA_FIELDS = NOVA_QUOTA_FIELDS | CINDER_QUOTA_FIELDS | NEUTRON_QUOTA_FIELDS QUOTA_NAMES = { "metadata_items": _('Metadata Items'), @@ -147,13 +152,13 @@ def _get_quota_data(request, tenant_mode=True, disabled_quotas=None, qs = base.QuotaSet() - if 'instances' not in disabled_quotas: + if NOVA_QUOTA_FIELDS - disabled_quotas: if tenant_mode: quotasets.append(nova.tenant_quota_get(request, tenant_id)) else: quotasets.append(nova.default_quota_get(request, tenant_id)) - if 'volumes' not in disabled_quotas: + if CINDER_QUOTA_FIELDS - disabled_quotas: try: if tenant_mode: quotasets.append(cinder.tenant_quota_get(request, tenant_id)) @@ -163,6 +168,7 @@ def _get_quota_data(request, tenant_mode=True, disabled_quotas=None, disabled_quotas.update(CINDER_QUOTA_FIELDS) msg = _("Unable to retrieve volume limit information.") exceptions.handle(request, msg) + for quota in itertools.chain(*quotasets): if quota.name not in disabled_quotas: qs[quota.name] = quota.limit @@ -187,32 +193,35 @@ def get_tenant_quota_data(request, disabled_quotas=None, tenant_id=None): # TODO(jpichon): There is no API to get the default system quotas # in Neutron (cf. LP#1204956), so for now handle tenant quotas here. # This should be handled in _get_quota_data() eventually. + + # TODO(amotoki): Purge this tricky usage. + # openstack_dashboard/dashboards/identity/projects/views.py + # calls get_tenant_quota_data directly and it expects + # neutron data is not returned. if not disabled_quotas: return qs # Check if neutron is enabled by looking for network - if 'network' not in disabled_quotas: - tenant_id = tenant_id or request.user.tenant_id - neutron_quotas = neutron.tenant_quota_get(request, tenant_id) + if not (NEUTRON_QUOTA_FIELDS - disabled_quotas): + return qs + + tenant_id = tenant_id or request.user.tenant_id + neutron_quotas = neutron.tenant_quota_get(request, tenant_id) + if 'floating_ips' in disabled_quotas: - # Neutron with quota extension disabled - if 'floatingip' in disabled_quotas: - qs.add(base.QuotaSet({'floating_ips': -1})) - # Neutron with quota extension enabled - else: + if 'floatingip' not in disabled_quotas: # Rename floatingip to floating_ips since that's how it's # expected in some places (e.g. Security & Access' Floating IPs) fips_quota = neutron_quotas.get('floatingip').limit qs.add(base.QuotaSet({'floating_ips': fips_quota})) + if 'security_groups' in disabled_quotas: - if 'security_group' in disabled_quotas: - qs.add(base.QuotaSet({'security_groups': -1})) - # Neutron with quota extension enabled - else: + if 'security_group' not in disabled_quotas: # Rename security_group to security_groups since that's how it's # expected in some places (e.g. Security & Access' Security Groups) sec_quota = neutron_quotas.get('security_group').limit qs.add(base.QuotaSet({'security_groups': sec_quota})) + if 'network' in disabled_quotas: for item in qs.items: if item.name == 'networks': @@ -221,6 +230,7 @@ def get_tenant_quota_data(request, disabled_quotas=None, tenant_id=None): else: net_quota = neutron_quotas.get('network').limit qs.add(base.QuotaSet({'networks': net_quota})) + if 'subnet' in disabled_quotas: for item in qs.items: if item.name == 'subnets': @@ -229,6 +239,7 @@ def get_tenant_quota_data(request, disabled_quotas=None, tenant_id=None): else: net_quota = neutron_quotas.get('subnet').limit qs.add(base.QuotaSet({'subnets': net_quota})) + if 'router' in disabled_quotas: for item in qs.items: if item.name == 'routers': @@ -284,6 +295,10 @@ def get_disabled_quotas(request): @profiler.trace def _get_tenant_compute_usages(request, usages, disabled_quotas, tenant_id): + enabled_compute_quotas = NOVA_COMPUTE_QUOTA_FIELDS - disabled_quotas + if not enabled_compute_quotas: + return + # Unlike the other services it can be the case that nova is enabled but # doesn't support quotas, in which case we still want to get usage info, # so don't rely on '"instances" in disabled_quotas' as elsewhere @@ -323,13 +338,21 @@ def _get_tenant_compute_usages(request, usages, disabled_quotas, tenant_id): @profiler.trace def _get_tenant_network_usages(request, usages, disabled_quotas, tenant_id): - floating_ips = [] - try: - if network.floating_ip_supported(request): - floating_ips = network.tenant_floating_ip_list(request) - except Exception: - pass - usages.tally('floating_ips', len(floating_ips)) + enabled_quotas = ((NOVA_NETWORK_QUOTA_FIELDS | NEUTRON_QUOTA_FIELDS) + - disabled_quotas) + if not enabled_quotas: + return + + # NOTE(amotoki): floatingip is Neutron quota and floating_ips is + # Nova quota. We need to check both. + if {'floatingip', 'floating_ips'} & enabled_quotas: + floating_ips = [] + try: + if network.floating_ip_supported(request): + floating_ips = network.tenant_floating_ip_list(request) + except Exception: + pass + usages.tally('floating_ips', len(floating_ips)) if 'security_group' not in disabled_quotas: security_groups = [] @@ -351,7 +374,7 @@ def _get_tenant_network_usages(request, usages, disabled_quotas, tenant_id): @profiler.trace def _get_tenant_volume_usages(request, usages, disabled_quotas, tenant_id): - if 'volumes' not in disabled_quotas: + if CINDER_QUOTA_FIELDS - disabled_quotas: try: if tenant_id: opts = {'all_tenants': 1, 'project_id': tenant_id} @@ -431,4 +454,4 @@ def tenant_limit_usages(request): def enabled_quotas(request): """Returns the list of quotas available minus those that are disabled""" - return set(QUOTA_FIELDS) - get_disabled_quotas(request) + return QUOTA_FIELDS - get_disabled_quotas(request)