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
(cherry picked from commit 161b4ae5d4)
This commit is contained in:
Marek
Marek Lyčka
parent
e4269fb88b
commit
ebc9ee920e
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -1,6 +1,8 @@
|
|||
{% extends 'base.html' %}
|
||||
{% load i18n %}
|
||||
|
||||
{% block title %}{{ page_title }}{% endblock %}
|
||||
|
||||
{% block main %}
|
||||
{% include 'project/floating_ips/_allocate.html' %}
|
||||
{% endblock %}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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}})
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
Loading…
Reference in New Issue