From 42d73e85a686e95ddd8c8b589635be7231d424cc Mon Sep 17 00:00:00 2001 From: Bradley Jones Date: Thu, 21 Aug 2014 14:20:37 +0100 Subject: [PATCH] Fix inconsistent behaviour exceeding quota limit Added quota checks for security groups, networks and routers. If quotas are exceeded then buttons to create are disabled and feedback is given to the user. Appropriate tests are also in place Change-Id: Ie49db5397d87a0c9a583b64e5de34460144b5956 Closes-bug: 1278449 --- .../access_and_security/floating_ips/tests.py | 27 ++++-- .../security_groups/tables.py | 10 +++ .../project/access_and_security/tests.py | 86 ++++++++++++++++++- .../dashboards/project/networks/tables.py | 14 +++ .../dashboards/project/networks/tests.py | 66 ++++++++++++-- .../dashboards/project/routers/tables.py | 15 ++++ .../dashboards/project/routers/tests.py | 80 +++++++++++++++-- openstack_dashboard/usage/quotas.py | 51 ++++++++++- 8 files changed, 323 insertions(+), 26 deletions(-) diff --git a/openstack_dashboard/dashboards/project/access_and_security/floating_ips/tests.py b/openstack_dashboard/dashboards/project/access_and_security/floating_ips/tests.py index 97ce4e2631..4ac11e8849 100644 --- a/openstack_dashboard/dashboards/project/access_and_security/floating_ips/tests.py +++ b/openstack_dashboard/dashboards/project/access_and_security/floating_ips/tests.py @@ -182,42 +182,51 @@ class FloatingIpNeutronViewTests(FloatingIpViewTests): @test.create_stubs({api.nova: ('tenant_quota_get', 'flavor_list', 'server_list'), - api.cinder: ('tenant_quota_get', 'volume_list', - 'volume_snapshot_list',), api.network: ('floating_ip_pools_list', 'floating_ip_supported', + 'security_group_list', 'tenant_floating_ip_list'), api.neutron: ('is_extension_supported', - 'tenant_quota_get')}) + 'tenant_quota_get', + 'network_list', + 'router_list'), + api.base: ('is_service_enabled',)}) @test.update_settings(OPENSTACK_NEUTRON_NETWORK={'enable_quotas': True}) def test_correct_quotas_displayed(self): + quota_data = self.quota_usages.first() + quota_data['floating_ips']['quota'] = 50 + servers = [s for s in self.servers.list() if s.tenant_id == self.request.user.tenant_id] + api.base.is_service_enabled(IsA(http.HttpRequest), 'volume') \ + .AndReturn(False) + api.base.is_service_enabled(IsA(http.HttpRequest), 'network') \ + .MultipleTimes().AndReturn(True) api.nova.tenant_quota_get(IsA(http.HttpRequest), '1') \ .AndReturn(self.quotas.first()) api.nova.flavor_list(IsA(http.HttpRequest)) \ .AndReturn(self.flavors.list()) api.nova.server_list(IsA(http.HttpRequest)) \ .AndReturn([servers, False]) - api.cinder.volume_list(IsA(http.HttpRequest)) \ - .AndReturn(self.volumes.list()) - api.cinder.volume_snapshot_list(IsA(http.HttpRequest)) \ - .AndReturn(self.snapshots.list()) - api.cinder.tenant_quota_get(IsA(http.HttpRequest), '1') \ - .AndReturn(self.cinder_quotas.first()) api.neutron.is_extension_supported( IsA(http.HttpRequest), 'security-group').AndReturn(True) api.neutron.is_extension_supported(IsA(http.HttpRequest), 'quotas') \ .AndReturn(True) api.neutron.tenant_quota_get(IsA(http.HttpRequest), self.tenant.id) \ .AndReturn(self.neutron_quotas.first()) + api.neutron.router_list(IsA(http.HttpRequest)) \ + .AndReturn(self.routers.list()) + api.neutron.network_list(IsA(http.HttpRequest), shared=False) \ + .AndReturn(self.networks.list()) api.network.floating_ip_supported(IsA(http.HttpRequest)) \ .AndReturn(True) api.network.tenant_floating_ip_list(IsA(http.HttpRequest)) \ .MultipleTimes().AndReturn(self.floating_ips.list()) api.network.floating_ip_pools_list(IsA(http.HttpRequest)) \ .AndReturn(self.pools.list()) + api.network.security_group_list(IsA(http.HttpRequest)) \ + .AndReturn(self.security_groups.list()) self.mox.ReplayAll() url = reverse('%s:allocate' % NAMESPACE) diff --git a/openstack_dashboard/dashboards/project/access_and_security/security_groups/tables.py b/openstack_dashboard/dashboards/project/access_and_security/security_groups/tables.py index ba107e8fe4..f3af030af8 100644 --- a/openstack_dashboard/dashboards/project/access_and_security/security_groups/tables.py +++ b/openstack_dashboard/dashboards/project/access_and_security/security_groups/tables.py @@ -21,6 +21,7 @@ from horizon import tables from openstack_dashboard import api from openstack_dashboard import policy +from openstack_dashboard.usage import quotas from openstack_dashboard.utils import filters @@ -77,6 +78,15 @@ class CreateGroup(tables.LinkAction): else: policy = (("compute", "compute_extension:security_groups"),) + usages = quotas.tenant_quota_usages(request) + if usages['security_groups']['available'] <= 0: + if "disabled" not in self.classes: + self.classes = [c for c in self.classes] + ["disabled"] + self.verbose_name = _("Create Security Group (Quota exceeded)") + else: + self.verbose_name = _("Create Security Group") + self.classes = [c for c in self.classes if c != "disabled"] + return POLICY_CHECK(policy, request, target={}) diff --git a/openstack_dashboard/dashboards/project/access_and_security/tests.py b/openstack_dashboard/dashboards/project/access_and_security/tests.py index 5661933952..40625af178 100644 --- a/openstack_dashboard/dashboards/project/access_and_security/tests.py +++ b/openstack_dashboard/dashboards/project/access_and_security/tests.py @@ -26,9 +26,13 @@ from horizon.workflows import views from openstack_dashboard import api from openstack_dashboard.dashboards.project.access_and_security \ import api_access +from openstack_dashboard.dashboards.project.access_and_security \ + .security_groups import tables from openstack_dashboard.test import helpers as test from openstack_dashboard.usage import quotas +INDEX_URL = reverse('horizon:project:access_and_security:index') + class AccessAndSecurityTests(test.TestCase): def setUp(self): @@ -46,6 +50,7 @@ class AccessAndSecurityTests(test.TestCase): sec_groups = self.security_groups.list() floating_ips = self.floating_ips.list() quota_data = self.quota_usages.first() + quota_data['security_groups']['available'] = 10 api.nova.server_list( IsA(http.HttpRequest)) \ @@ -77,8 +82,7 @@ class AccessAndSecurityTests(test.TestCase): self.mox.ReplayAll() - url = reverse('horizon:project:access_and_security:index') - res = self.client.get(url) + res = self.client.get(INDEX_URL) self.assertTemplateUsed(res, 'project/access_and_security/index.html') self.assertItemsEqual(res.context['keypairs_table'].data, keypairs) @@ -140,3 +144,81 @@ class AccessAndSecurityNeutronProxyTests(AccessAndSecurityTests): def setUp(self): super(AccessAndSecurityNeutronProxyTests, self).setUp() self.floating_ips = self.floating_ips_uuid + + +class SecurityGroupTabTests(test.TestCase): + def setUp(self): + super(SecurityGroupTabTests, self).setUp() + + @test.create_stubs({api.network: ('floating_ip_supported', + 'tenant_floating_ip_list', + 'security_group_list', + 'floating_ip_pools_list',), + api.nova: ('keypair_list', + 'server_list',), + quotas: ('tenant_quota_usages',), + api.base: ('is_service_enabled',)}) + def _test_create_button_disabled_when_quota_exceeded(self, + network_enabled): + keypairs = self.keypairs.list() + floating_ips = self.floating_ips.list() + floating_pools = self.pools.list() + sec_groups = self.security_groups.list() + quota_data = self.quota_usages.first() + quota_data['security_groups']['available'] = 0 + + api.network.floating_ip_supported( + IsA(http.HttpRequest)) \ + .AndReturn(True) + api.network.tenant_floating_ip_list( + IsA(http.HttpRequest)) \ + .AndReturn(floating_ips) + api.network.floating_ip_pools_list( + IsA(http.HttpRequest)) \ + .AndReturn(floating_pools) + api.network.security_group_list( + IsA(http.HttpRequest)) \ + .AndReturn(sec_groups) + api.nova.keypair_list( + IsA(http.HttpRequest)) \ + .AndReturn(keypairs) + api.nova.server_list( + IsA(http.HttpRequest)) \ + .AndReturn([self.servers.list(), False]) + quotas.tenant_quota_usages( + IsA(http.HttpRequest)).MultipleTimes() \ + .AndReturn(quota_data) + + api.base.is_service_enabled( + IsA(http.HttpRequest), 'network').MultipleTimes() \ + .AndReturn(network_enabled) + api.base.is_service_enabled( + IsA(http.HttpRequest), 'ec2').MultipleTimes() \ + .AndReturn(False) + + self.mox.ReplayAll() + + res = self.client.get(INDEX_URL + + "?tab=access_security_tabs__security_groups_tab") + + security_groups = res.context['security_groups_table'].data + self.assertItemsEqual(security_groups, self.security_groups.list()) + + create_link = tables.CreateGroup() + url = create_link.get_link_url() + classes = list(create_link.get_default_classes())\ + + list(create_link.classes) + link_name = "%s (%s)" % (unicode(create_link.verbose_name), + "Quota exceeded") + expected_string = "" \ + "%s" \ + % (url, link_name, " ".join(classes), link_name) + self.assertContains(res, expected_string, html=True, + msg_prefix="The create button is not disabled") + + def test_create_button_disabled_when_quota_exceeded_neutron_disabled(self): + self._test_create_button_disabled_when_quota_exceeded(False) + + def test_create_button_disabled_when_quota_exceeded_neutron_enabled(self): + self._test_create_button_disabled_when_quota_exceeded(True) diff --git a/openstack_dashboard/dashboards/project/networks/tables.py b/openstack_dashboard/dashboards/project/networks/tables.py index fcd662a2f5..2f436865c4 100644 --- a/openstack_dashboard/dashboards/project/networks/tables.py +++ b/openstack_dashboard/dashboards/project/networks/tables.py @@ -23,6 +23,8 @@ from horizon import tables from openstack_dashboard import api from openstack_dashboard import policy +from openstack_dashboard.usage import quotas + LOG = logging.getLogger(__name__) @@ -70,6 +72,18 @@ class CreateNetwork(tables.LinkAction): icon = "plus" policy_rules = (("network", "create_network"),) + def allowed(self, request, datum=None): + usages = quotas.tenant_quota_usages(request) + if usages['networks']['available'] <= 0: + if "disabled" not in self.classes: + self.classes = [c for c in self.classes] + ["disabled"] + self.verbose_name = _("Create Network (Quota exceeded)") + else: + self.verbose_name = _("Create Network") + self.classes = [c for c in self.classes if c != "disabled"] + + return True + class EditNetwork(policy.PolicyTargetMixin, CheckNetworkEditable, tables.LinkAction): diff --git a/openstack_dashboard/dashboards/project/networks/tests.py b/openstack_dashboard/dashboards/project/networks/tests.py index 802f5df5fd..492c1cbfbc 100644 --- a/openstack_dashboard/dashboards/project/networks/tests.py +++ b/openstack_dashboard/dashboards/project/networks/tests.py @@ -21,10 +21,10 @@ from horizon.workflows import views from mox import IsA # noqa from openstack_dashboard import api -from openstack_dashboard.test import helpers as test - +from openstack_dashboard.dashboards.project.networks import tables from openstack_dashboard.dashboards.project.networks import workflows - +from openstack_dashboard.test import helpers as test +from openstack_dashboard.usage import quotas INDEX_URL = reverse('horizon:project:networks:index') @@ -95,8 +95,11 @@ def _str_host_routes(host_routes): class NetworkTests(test.TestCase): - @test.create_stubs({api.neutron: ('network_list',)}) + @test.create_stubs({api.neutron: ('network_list',), + quotas: ('tenant_quota_usages',)}) def test_index(self): + quota_data = self.quota_usages.first() + quota_data['networks']['available'] = 5 api.neutron.network_list( IsA(http.HttpRequest), tenant_id=self.tenant.id, @@ -104,21 +107,29 @@ class NetworkTests(test.TestCase): api.neutron.network_list( IsA(http.HttpRequest), shared=True).AndReturn([]) + quotas.tenant_quota_usages( + IsA(http.HttpRequest)) \ + .MultipleTimes().AndReturn(quota_data) self.mox.ReplayAll() res = self.client.get(INDEX_URL) - self.assertTemplateUsed(res, 'project/networks/index.html') networks = res.context['networks_table'].data self.assertItemsEqual(networks, self.networks.list()) - @test.create_stubs({api.neutron: ('network_list',)}) + @test.create_stubs({api.neutron: ('network_list',), + quotas: ('tenant_quota_usages',)}) def test_index_network_list_exception(self): + quota_data = self.quota_usages.first() + quota_data['networks']['available'] = 5 api.neutron.network_list( IsA(http.HttpRequest), tenant_id=self.tenant.id, - shared=False).AndRaise(self.exceptions.neutron) + shared=False).MultipleTimes().AndRaise(self.exceptions.neutron) + quotas.tenant_quota_usages( + IsA(http.HttpRequest)) \ + .MultipleTimes().AndReturn(quota_data) self.mox.ReplayAll() res = self.client.get(INDEX_URL) @@ -1741,3 +1752,44 @@ class NetworkPortTests(test.TestCase): redir_url = reverse('horizon:project:networks:detail', args=[port.network_id]) self.assertRedirectsNoFollow(res, redir_url) + + +class NetworkViewTests(test.TestCase): + + @test.create_stubs({api.neutron: ('network_list',), + quotas: ('tenant_quota_usages',)}) + def test_create_button_disabled_when_quota_exceeded(self): + quota_data = self.quota_usages.first() + quota_data['networks']['available'] = 0 + + api.neutron.network_list( + IsA(http.HttpRequest), + tenant_id=self.tenant.id, + shared=False).AndReturn(self.networks.list()) + api.neutron.network_list( + IsA(http.HttpRequest), + shared=True).AndReturn([]) + quotas.tenant_quota_usages( + IsA(http.HttpRequest)) \ + .MultipleTimes().AndReturn(quota_data) + + self.mox.ReplayAll() + + res = self.client.get(INDEX_URL) + self.assertTemplateUsed(res, 'project/networks/index.html') + + networks = res.context['networks_table'].data + self.assertItemsEqual(networks, self.networks.list()) + + create_link = tables.CreateNetwork() + url = create_link.get_link_url() + classes = list(create_link.get_default_classes())\ + + list(create_link.classes) + link_name = "%s (%s)" % (unicode(create_link.verbose_name), + "Quota exceeded") + expected_string = "" \ + "%s" \ + % (url, link_name, " ".join(classes), link_name) + self.assertContains(res, expected_string, html=True, + msg_prefix="The create button is not disabled") diff --git a/openstack_dashboard/dashboards/project/routers/tables.py b/openstack_dashboard/dashboards/project/routers/tables.py index 7924f06001..a0ca486197 100644 --- a/openstack_dashboard/dashboards/project/routers/tables.py +++ b/openstack_dashboard/dashboards/project/routers/tables.py @@ -23,8 +23,11 @@ from neutronclient.common import exceptions as q_ext from horizon import exceptions from horizon import messages from horizon import tables + from openstack_dashboard import api from openstack_dashboard import policy +from openstack_dashboard.usage import quotas + LOG = logging.getLogger(__name__) @@ -77,6 +80,18 @@ class CreateRouter(tables.LinkAction): icon = "plus" policy_rules = (("network", "create_router"),) + def allowed(self, request, datum=None): + usages = quotas.tenant_quota_usages(request) + if usages['routers']['available'] <= 0: + if "disabled" not in self.classes: + self.classes = [c for c in self.classes] + ["disabled"] + self.verbose_name = _("Create Router (Quota exceeded)") + else: + self.verbose_name = _("Create Router") + self.classes = [c for c in self.classes if c != "disabled"] + + return True + class EditRouter(policy.PolicyTargetMixin, tables.LinkAction): name = "update" diff --git a/openstack_dashboard/dashboards/project/routers/tests.py b/openstack_dashboard/dashboards/project/routers/tests.py index 5706de0a3a..2b6a6fcf56 100644 --- a/openstack_dashboard/dashboards/project/routers/tests.py +++ b/openstack_dashboard/dashboards/project/routers/tests.py @@ -21,7 +21,9 @@ from mox import IsA # noqa from openstack_dashboard import api from openstack_dashboard.dashboards.project.routers.extensions.routerrules\ import rulemanager +from openstack_dashboard.dashboards.project.routers import tables from openstack_dashboard.test import helpers as test +from openstack_dashboard.usage import quotas class RouterTests(test.TestCase): @@ -45,12 +47,18 @@ class RouterTests(test.TestCase): api.neutron.network_get(IsA(http.HttpRequest), ext_net_id, expand_subnet=False).AndReturn(ext_net) - @test.create_stubs({api.neutron: ('router_list', 'network_list')}) + @test.create_stubs({api.neutron: ('router_list', 'network_list'), + quotas: ('tenant_quota_usages',)}) def test_index(self): + quota_data = self.quota_usages.first() + quota_data['routers']['available'] = 5 api.neutron.router_list( IsA(http.HttpRequest), tenant_id=self.tenant.id, search_opts=None).AndReturn(self.routers.list()) + quotas.tenant_quota_usages( + IsA(http.HttpRequest)) \ + .MultipleTimes().AndReturn(quota_data) self._mock_external_network_list() self.mox.ReplayAll() @@ -60,12 +68,18 @@ class RouterTests(test.TestCase): routers = res.context['table'].data self.assertItemsEqual(routers, self.routers.list()) - @test.create_stubs({api.neutron: ('router_list', 'network_list')}) + @test.create_stubs({api.neutron: ('router_list', 'network_list'), + quotas: ('tenant_quota_usages',)}) def test_index_router_list_exception(self): + quota_data = self.quota_usages.first() + quota_data['routers']['available'] = 5 api.neutron.router_list( IsA(http.HttpRequest), tenant_id=self.tenant.id, - search_opts=None).AndRaise(self.exceptions.neutron) + search_opts=None).MultipleTimes().AndRaise(self.exceptions.neutron) + quotas.tenant_quota_usages( + IsA(http.HttpRequest)) \ + .MultipleTimes().AndReturn(quota_data) self._mock_external_network_list() self.mox.ReplayAll() @@ -75,13 +89,19 @@ class RouterTests(test.TestCase): self.assertEqual(len(res.context['table'].data), 0) self.assertMessageCount(res, error=1) - @test.create_stubs({api.neutron: ('router_list', 'network_list')}) + @test.create_stubs({api.neutron: ('router_list', 'network_list'), + quotas: ('tenant_quota_usages',)}) def test_set_external_network_empty(self): router = self.routers.first() + quota_data = self.quota_usages.first() + quota_data['routers']['available'] = 5 api.neutron.router_list( IsA(http.HttpRequest), tenant_id=self.tenant.id, - search_opts=None).AndReturn([router]) + search_opts=None).MultipleTimes().AndReturn([router]) + quotas.tenant_quota_usages( + IsA(http.HttpRequest)) \ + .MultipleTimes().AndReturn(quota_data) self._mock_external_network_list(alter_ids=True) self.mox.ReplayAll() @@ -679,3 +699,53 @@ class RouterRuleTests(test.TestCase): url = reverse(self.DETAIL_PATH, args=[pre_router.id]) res = self.client.post(url, form_data) self.assertNoFormErrors(res) + + +class RouterViewTests(test.TestCase): + DASHBOARD = 'project' + INDEX_URL = reverse('horizon:%s:routers:index' % DASHBOARD) + + def _mock_external_network_list(self, alter_ids=False): + search_opts = {'router:external': True} + ext_nets = [n for n in self.networks.list() if n['router:external']] + if alter_ids: + for ext_net in ext_nets: + ext_net.id += 'some extra garbage' + api.neutron.network_list( + IsA(http.HttpRequest), + **search_opts).AndReturn(ext_nets) + + @test.create_stubs({api.neutron: ('router_list', 'network_list'), + quotas: ('tenant_quota_usages',)}) + def test_create_button_disabled_when_quota_exceeded(self): + quota_data = self.quota_usages.first() + quota_data['routers']['available'] = 0 + api.neutron.router_list( + IsA(http.HttpRequest), + tenant_id=self.tenant.id, + search_opts=None).AndReturn(self.routers.list()) + quotas.tenant_quota_usages( + IsA(http.HttpRequest)) \ + .MultipleTimes().AndReturn(quota_data) + + self._mock_external_network_list() + self.mox.ReplayAll() + + res = self.client.get(self.INDEX_URL) + self.assertTemplateUsed(res, 'project/routers/index.html') + + routers = res.context['Routers_table'].data + self.assertItemsEqual(routers, self.routers.list()) + + create_link = tables.CreateRouter() + url = create_link.get_link_url() + classes = list(create_link.get_default_classes())\ + + list(create_link.classes) + link_name = "%s (%s)" % (unicode(create_link.verbose_name), + "Quota exceeded") + expected_string = "" \ + "%s" \ + % (url, link_name, " ".join(classes), link_name) + self.assertContains(res, expected_string, html=True, + msg_prefix="The create button is not disabled") diff --git a/openstack_dashboard/usage/quotas.py b/openstack_dashboard/usage/quotas.py index 9bd67807c1..5edc1837ea 100644 --- a/openstack_dashboard/usage/quotas.py +++ b/openstack_dashboard/usage/quotas.py @@ -167,18 +167,48 @@ 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. - if disabled_quotas and 'floating_ips' in disabled_quotas: + if not disabled_quotas: + return qs + + # Check if neutron is enabled by looking for network and router + if 'network' and 'router' not in disabled_quotas: + 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: - tenant_id = tenant_id or request.user.tenant_id - neutron_quotas = neutron.tenant_quota_get(request, tenant_id) # 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: + # 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': + qs.items.remove(item) + break + else: + net_quota = neutron_quotas.get('network').limit + qs.add(base.QuotaSet({'networks': net_quota})) + if 'router' in disabled_quotas: + for item in qs.items: + if item.name == 'routers': + qs.items.remove(item) + break + else: + router_quota = neutron_quotas.get('router').limit + qs.add(base.QuotaSet({'routers': router_quota})) return qs @@ -247,6 +277,21 @@ def tenant_quota_usages(request): usages.tally('instances', len(instances)) usages.tally('floating_ips', len(floating_ips)) + if 'security_group' not in disabled_quotas: + security_groups = [] + security_groups = network.security_group_list(request) + usages.tally('security_groups', len(security_groups)) + + if 'network' not in disabled_quotas: + networks = [] + networks = neutron.network_list(request, shared=False) + usages.tally('networks', len(networks)) + + if 'router' not in disabled_quotas: + routers = [] + routers = neutron.router_list(request) + usages.tally('routers', len(routers)) + if 'volumes' not in disabled_quotas: volumes = cinder.volume_list(request) snapshots = cinder.volume_snapshot_list(request)