Fixes a series of bugs related to Floating IPs.

- Fixes KeyErrors when accessing 'floatingip' values in usages, which
  broken Floating IP allocation.
- The quota display in the bottom right of the Allocation dialog are
  only displayed if 'enabled_quotas' is True
- Adds security group rule tallying for the usages overview page, which
  fixes a KeyError crash for installations where Horizon 'enable_quotas'
  is set to true, but the 'quota_details' extension is not installed on
  in Neutron
- Adds a policy check to show and hide The plus/add button in
  Instances->Associate Floating IP to match the Allocate IP To Project
  button in Floating IPs
- Fixed the page title not being set for the non-modal version of the
  modal allocation dialog/form
- Added an 'allowed' functionality for network usage overview charts to
  allow for them to be dynamically disabled
- Added tests and mocks for the above
- Added tests for non-legacy quota tallying for networks
- Added test for disabled network quotas in overview

Change-Id: I47f73ff94664d315a2400feb8ce8a25f4e6beced
closes-bug: #1838522
This commit is contained in:
Marek 2019-10-30 09:38:29 +01:00 committed by Akihiro Motoki
parent f7313b74c9
commit 161b4ae5d4
10 changed files with 196 additions and 55 deletions

View File

@ -56,7 +56,8 @@ class FloatingIpAllocate(forms.SelfHandlingForm):
# Prevent allocating more IP than the quota allows
usages = quotas.tenant_quota_usages(request,
targets=('floatingip', ))
if usages['floatingip']['available'] <= 0:
if ('floatingip' in usages and
usages['floatingip']['available'] <= 0):
error_message = _('You are already using all of your available'
' floating IPs.')
self.api_error(error_message)

View File

@ -49,7 +49,7 @@ class AllocateIP(tables.LinkAction):
def allowed(self, request, fip=None):
usages = quotas.tenant_quota_usages(request,
targets=('floatingip', ))
if usages['floatingip']['available'] <= 0:
if 'floatingip' in usages and usages['floatingip']['available'] <= 0:
if "disabled" not in self.classes:
self.classes = [c for c in self.classes] + ['disabled']
self.verbose_name = format_lazy(

View File

@ -5,26 +5,30 @@
<div class="quota-dynamic">
<h3>{% trans "Description:" %}</h3>
<p>{% trans "Allocate a floating IP from a given floating IP pool." %}</p>
{% if usages %}
<h3>{% trans "Project Quotas" %}</h3>
{% if usages.floatingip %}
<div class="quota_title">
<div class="pull-left">
<strong>{% trans "Floating IP" %}</strong>
</div>
<span class="pull-right">
{% blocktrans with used=usages.floatingip.used quota=usages.floatingip.quota|quotainf %}{{ used }} of {{ quota }} Used{% endblocktrans %}
</span>
</div>
<div id="floating_ip_progress"
class="quota_bar"
data-quota-used="{{ usages.floatingip.used }}"
data-quota-limit="{{ usages.floatingip.quota }}">
{% widthratio usages.floatingip.used usages.floatingip.quota 100 as ip_percent %}
{% widthratio 100 usages.floatingip.quota 1 as single_step %}
{% bs_progress_bar ip_percent single_step %}
</div>
{% endif %}
{% endif %}
<h3>{% trans "Project Quotas" %}</h3>
<div class="quota_title">
<div class="pull-left">
<strong>{% trans "Floating IP" %}</strong>
</div>
<span class="pull-right">
{% blocktrans with used=usages.floatingip.used quota=usages.floatingip.quota|quotainf %}{{ used }} of {{ quota }} Used{% endblocktrans %}
</span>
</div>
<div id="floating_ip_progress"
class="quota_bar"
data-quota-used="{{ usages.floatingip.used }}"
data-quota-limit="{{ usages.floatingip.quota }}">
{% widthratio usages.floatingip.used usages.floatingip.quota 100 as ip_percent %}
{% widthratio 100 usages.floatingip.quota 1 as single_step %}
{% bs_progress_bar ip_percent single_step %}
</div>
</div>
<script type="text/javascript" charset="utf-8">
if(typeof horizon.Quota !== 'undefined') {
horizon.Quota.init();

View File

@ -1,6 +1,8 @@
{% extends 'base.html' %}
{% load i18n %}
{% block title %}{{ page_title }}{% endblock %}
{% block main %}
{% include 'project/floating_ips/_allocate.html' %}
{% endblock %}

View File

@ -62,7 +62,8 @@ class AllocateView(forms.ModalFormView):
context = super(AllocateView, self).get_context_data(**kwargs)
try:
context['usages'] = quotas.tenant_quota_usages(
self.request, targets=('floatingip', ))
self.request, targets=('floatingip', )
)
except Exception:
exceptions.handle(self.request)
return context

View File

@ -23,9 +23,9 @@ from horizon.utils import memoized
from horizon import workflows
from openstack_dashboard import api
from openstack_dashboard import policy
from openstack_dashboard.utils import filters
ALLOCATE_URL = "horizon:project:floating_ips:allocate"
@ -34,8 +34,7 @@ class AssociateIPAction(workflows.Action):
ip_id = forms.ThemableDynamicTypedChoiceField(
label=_("IP Address"),
coerce=filters.get_int_or_uuid,
empty_value=None,
add_item_link=ALLOCATE_URL
empty_value=None
)
instance_id = forms.ThemableChoiceField(
label=_("Port to be associated")
@ -56,6 +55,11 @@ class AssociateIPAction(workflows.Action):
# and set the initial value of instance_id ChoiceField.
q_instance_id = self.request.GET.get('instance_id')
q_port_id = self.request.GET.get('port_id')
if policy.check((("network", "create_floatingip"),),
request=self.request):
self.fields['ip_id'].widget.add_item_link = ALLOCATE_URL
if q_instance_id:
targets = self._get_target_list(q_instance_id)
# Setting the initial value here is required to avoid a situation

View File

@ -35,13 +35,16 @@ INDEX_URL = reverse('horizon:project:overview:index')
class UsageViewTests(test.TestCase):
@test.create_mocks({api.nova: (
'usage_get',
'extension_supported',
)})
@test.create_mocks({
api.nova: ('usage_get', 'extension_supported',),
api.neutron: ('is_quotas_extension_supported',)
})
def _stub_api_calls(self, nova_stu_enabled=True,
stu_exception=False, overview_days_range=1,
quota_usage_overrides=None):
quota_usage_overrides=None,
quota_extension_support=True):
self.mock_is_quotas_extension_supported.return_value = \
quota_extension_support
self.mock_extension_supported.side_effect = [nova_stu_enabled,
nova_stu_enabled]
if nova_stu_enabled:
@ -255,9 +258,11 @@ class UsageViewTests(test.TestCase):
self._check_api_calls(nova_stu_enabled=True)
def _test_usage_charts(self, quota_usage_overrides=None):
def _test_usage_charts(self, quota_usage_overrides=None,
quota_extension_support=True):
self._stub_api_calls(nova_stu_enabled=False,
quota_usage_overrides=quota_usage_overrides)
quota_usage_overrides=quota_usage_overrides,
quota_extension_support=quota_extension_support)
res = self.client.get(reverse('horizon:project:overview:index'))
@ -301,6 +306,14 @@ class UsageViewTests(test.TestCase):
self.assertEqual(1234, chart_fip['used'])
self.assertEqual('1,234', chart_fip['used_display'])
def test_disallowed_network_chart(self):
res = self._test_usage_charts(
quota_usage_overrides={'floatingip': {'quota': -1, 'used': 1234}},
quota_extension_support=False)
charts = res.context['charts']
self.assertEqual(['Compute', 'Volume'],
[c['title'] for c in charts])
def test_usage_charts_infinite_quota(self):
res = self._test_usage_charts(
quota_usage_overrides={'floatingip': {'quota': -1}})

View File

@ -113,12 +113,13 @@ class QuotaTests(test.APITestCase):
usages[item]['quota'] = float('inf')
return usages
def assertAvailableQuotasEqual(self, expected_usages, actual_usages):
def assertAvailableQuotasEqual(self, expected_usages, actual_usages,
msg=None):
expected_available = {key: value['available'] for key, value in
expected_usages.items() if 'available' in value}
actual_available = {key: value['available'] for key, value in
actual_usages.items() if 'available' in value}
self.assertEqual(expected_available, actual_available)
self.assertEqual(expected_available, actual_available, msg=msg)
@test.create_mocks({
api.nova: (('tenant_absolute_limits', 'nova_tenant_absolute_limits'),),
@ -370,20 +371,20 @@ class QuotaTests(test.APITestCase):
def test_tenant_quota_usages_with_target_instances(self):
self._test_tenant_quota_usages_with_target(
targets=('instances', ), use_cinder_call=False)
targets=('instances',), use_cinder_call=False)
def test_tenant_quota_usages_with_target_ram(self):
self._test_tenant_quota_usages_with_target(
targets=('ram', ), use_flavor_list=True, use_cinder_call=False)
targets=('ram',), use_flavor_list=True, use_cinder_call=False)
def test_tenant_quota_usages_with_target_volume(self):
self._test_tenant_quota_usages_with_target(
targets=('volumes', ), use_compute_call=False,
targets=('volumes',), use_compute_call=False,
use_cinder_call=True)
def test_tenant_quota_usages_with_target_compute_volume(self):
self._test_tenant_quota_usages_with_target(
targets=('instances', 'cores', 'ram', 'volumes', ),
targets=('instances', 'cores', 'ram', 'volumes',),
use_flavor_list=True, use_cinder_call=True)
@test.create_mocks({
@ -435,15 +436,81 @@ class QuotaTests(test.APITestCase):
def test_tenant_quota_usages_neutron_with_target_network_resources(self):
self._test_tenant_quota_usages_neutron_with_target(
targets=('network', 'subnet', 'router', ))
targets=('network', 'subnet', 'router',))
def test_tenant_quota_usages_neutron_with_target_security_groups(self):
self._test_tenant_quota_usages_neutron_with_target(
targets=('security_group', ))
targets=('security_group',))
def test_tenant_quota_usages_neutron_with_target_floating_ips(self):
self._test_tenant_quota_usages_neutron_with_target(
targets=('floatingip', ))
targets=('floatingip',))
def test_tenant_quota_usages_neutron_with_target_security_group_rule(self):
self._test_tenant_quota_usages_neutron_with_target(
targets=('security_group_rule',)
)
def _list_security_group_rules(self):
security_groups = self.security_groups.list()
security_group_rules = []
for group in security_groups:
security_group_rules += group.security_group_rules
return security_group_rules
# Tests network quota retrieval via neutron.tenant_quota_detail_get
# rather than quotas._get_tenant_network_usages_legacy (see
# quotas._get_tenant_network_usages)
@test.create_mocks({api.base: ('is_service_enabled',),
cinder: ('is_volume_service_enabled',),
api.neutron: ('floating_ip_supported',
'is_extension_supported',
'is_quotas_extension_supported',
'tenant_quota_detail_get')})
def test_tenant_quota_usages_non_legacy(self):
self._mock_service_enabled(network_enabled=True)
self.mock_is_extension_supported.return_value = True
test_data = [
("network", self.networks.list(), 10),
("subnet", self.subnets.list(), 10),
("port", self.ports.list(), 100),
("router", self.routers.list(), 10),
("floatingip", self.floating_ips.list(), 50),
("security_group", self.security_groups.list(), 20),
("security_group_rule", self._list_security_group_rules(), 100)
]
for datum in test_data:
target = datum[0]
used = len(datum[1])
limit = datum[2]
expected = {
target: {
'used': used,
'quota': limit,
'available': limit - used
}
}
self.mock_tenant_quota_detail_get.return_value = {
target: {
'reserved': 0,
'used': used,
'limit': limit
}
}
quota_usages = quotas.tenant_quota_usages(self.request,
targets=(target,))
msg = "Test failure for resource: '{}'".format(target)
# Compare internal structure of usages to expected.
self.assertEqual(expected, quota_usages.usages, msg=msg)
# Compare available resources
self.assertAvailableQuotasEqual(expected, quota_usages.usages,
msg=msg)
@test.create_mocks({api.base: ('is_service_enabled',),
cinder: ('is_volume_service_enabled',),
@ -459,7 +526,7 @@ class QuotaTests(test.APITestCase):
'router_list')})
def _test_tenant_quota_usages_neutron_with_target(self, targets):
self._mock_service_enabled(network_enabled=True)
if 'security_group' in targets:
if 'security_group' in targets or 'security_group_rule' in targets:
self.mock_is_extension_supported.side_effect = [True, False]
else:
self.mock_is_extension_supported.side_effect = [False]
@ -476,7 +543,7 @@ class QuotaTests(test.APITestCase):
if 'floatingip' in targets:
self.mock_tenant_floating_ip_list.return_value = \
self.floating_ips.list()
if 'security_group' in targets:
if 'security_group' in targets or 'security_group_rule' in targets:
self.mock_security_group_list.return_value = \
self.security_groups.list()
@ -487,7 +554,14 @@ class QuotaTests(test.APITestCase):
subnet_used = len(self.subnets.list())
router_used = len(self.routers.list())
fip_used = len(self.floating_ips.list())
sg_used = len(self.security_groups.list())
security_groups = self.security_groups.list()
sg_used = len(security_groups)
sgr_used = sum(map(
lambda group: len(group.security_group_rules),
security_groups
))
expected = {
'network': {'used': network_used, 'quota': 10,
'available': 10 - network_used},
@ -497,6 +571,9 @@ class QuotaTests(test.APITestCase):
'available': 10 - router_used},
'security_group': {'used': sg_used, 'quota': 20,
'available': 20 - sg_used},
'security_group_rule': {
'quota': 100, 'used': sgr_used, 'available': 100 - sgr_used
},
'floatingip': {'used': fip_used, 'quota': 50,
'available': 50 - fip_used},
}
@ -508,7 +585,8 @@ class QuotaTests(test.APITestCase):
self.assertAvailableQuotasEqual(expected, quota_usages.usages)
self._check_service_enabled({'network': 1})
if 'security_group' in targets:
if 'security_group' in targets or 'security_group_rule' in targets:
self.mock_is_extension_supported.assert_has_calls([
mock.call(test.IsHttpRequest(), 'security-group'),
mock.call(test.IsHttpRequest(), 'quota_details'),
@ -549,7 +627,7 @@ class QuotaTests(test.APITestCase):
test.IsHttpRequest())
else:
self.mock_tenant_floating_ip_list.assert_not_called()
if 'security_group' in targets:
if 'security_group' in targets or 'security_group_rule' in targets:
self.mock_security_group_list.assert_called_once_with(
test.IsHttpRequest())
else:

View File

@ -135,6 +135,9 @@ class QuotaUsage(dict):
def __repr__(self):
return repr(dict(self.usages))
def __bool__(self):
return bool(self.usages)
def get(self, key, default=None):
return self.usages.get(key, default)
@ -365,14 +368,12 @@ def _get_tenant_network_usages_legacy(request, usages, disabled_quotas,
for quota in qs:
usages.add_quota(quota)
# TODO(amotoki): Add security_group_rule?
resource_lister = {
'network': (neutron.network_list, {'tenant_id': tenant_id}),
'subnet': (neutron.subnet_list, {'tenant_id': tenant_id}),
'port': (neutron.port_list, {'tenant_id': tenant_id}),
'router': (neutron.router_list, {'tenant_id': tenant_id}),
'floatingip': (neutron.tenant_floating_ip_list, {}),
'security_group': (neutron.security_group_list, {}),
}
for quota_name, lister_info in resource_lister.items():
@ -385,6 +386,26 @@ def _get_tenant_network_usages_legacy(request, usages, disabled_quotas,
resources = []
usages.tally(quota_name, len(resources))
# Security groups have to be processed separately so that rules may be
# processed in the same api call and in a single pass
add_sg = 'security_group' not in disabled_quotas
add_sgr = 'security_group_rule' not in disabled_quotas
if add_sg or add_sgr:
try:
security_groups = neutron.security_group_list(request)
num_rules = sum(len(group['security_group_rules'])
for group in security_groups)
except Exception:
security_groups = []
num_rules = 0
if add_sg:
usages.tally('security_group', len(security_groups))
if add_sgr:
usages.tally('security_group_rule', num_rules)
@profiler.trace
def _get_tenant_volume_usages(request, usages, disabled_quotas, tenant_id):

View File

@ -87,6 +87,10 @@ class UsageView(tables.DataTableView):
return resp
def _check_network_allowed(request):
return api.neutron.is_quotas_extension_supported(request)
ChartDef = collections.namedtuple(
'ChartDef',
('quota_key', 'label', 'used_phrase', 'filters'))
@ -99,6 +103,10 @@ ChartDef = collections.namedtuple(
# - filters to be applied to the value
# If None is specified, the default filter 'intcomma' will be applied.
# if you want to apply no filters, specify an empty tuple or list.
# - allowed:
# An optional argument used to determine if the chart section should be
# displayed. Can be a static value or a function, which is called dynamically
# with the request as it's first parameter.
CHART_DEFS = [
{
'title': _("Compute"),
@ -106,7 +114,7 @@ CHART_DEFS = [
ChartDef("instances", _("Instances"), None, None),
ChartDef("cores", _("VCPUs"), None, None),
ChartDef("ram", _("RAM"), None, (sizeformat.mb_float_format,)),
]
],
},
{
'title': _("Volume"),
@ -115,7 +123,7 @@ CHART_DEFS = [
ChartDef("snapshots", _("Volume Snapshots"), None, None),
ChartDef("gigabytes", _("Volume Storage"), None,
(sizeformat.diskgbformat,)),
]
],
},
{
'title': _("Network"),
@ -129,7 +137,8 @@ CHART_DEFS = [
ChartDef("network", _("Networks"), None, None),
ChartDef("port", _("Ports"), None, None),
ChartDef("router", _("Routers"), None, None),
]
],
'allowed': _check_network_allowed,
},
]
@ -147,13 +156,21 @@ class ProjectUsageView(UsageView):
def _get_charts_data(self):
chart_sections = []
for section in CHART_DEFS:
chart_data = self._process_chart_section(section['charts'])
chart_sections.append({
'title': section['title'],
'charts': chart_data
})
if self._check_chart_allowed(section):
chart_data = self._process_chart_section(section['charts'])
chart_sections.append({
'title': section['title'],
'charts': chart_data
})
return chart_sections
def _check_chart_allowed(self, chart_def):
result = True
if 'allowed' in chart_def:
allowed = chart_def['allowed']
result = allowed(self.request) if callable(allowed) else allowed
return result
def _process_chart_section(self, chart_defs):
charts = []
for t in chart_defs: