Uniquify horizon messages returned in a single response

A network operation can fail because of different
reasons - yet in many places just one error message
is provided. Combined with too broad exception clause,
and incorrect assumptions on the reasons of failure
(e.g. Neutron service being unavailable causes all
other sorts of errors like inability to Allocate IP
or Associate it) this leads to multiple errors when just one
would suffice. The fix aims to provide sensible error
messages in case the network service is down. This includes
returning only unique messages in a single response.

Partial-Bug: #1301374
Change-Id: Id6c620ca51f7703f35c0c172e39fdf237fa42278
Co-Authored-By: Timur Sufiev <tsufiev@mirantis.com>
This commit is contained in:
Vlad Okhrimenko 2014-08-22 14:04:16 +03:00
parent cdd64c2f61
commit a212ef399f
6 changed files with 55 additions and 15 deletions

View File

@ -23,20 +23,34 @@ from django.utils.encoding import force_text
from django.utils.safestring import SafeData # noqa
def horizon_message_already_queued(request, message):
_message = force_text(message)
if request.is_ajax():
for tag, msg, extra in request.horizon['async_messages']:
if _message == msg:
return True
else:
for msg in _messages.get_messages(request)._queued_messages:
if msg.message == _message:
return True
return False
def add_message(request, level, message, extra_tags='', fail_silently=False):
"""Attempts to add a message to the request using the 'messages' app."""
if request.is_ajax():
tag = constants.DEFAULT_TAGS[level]
# if message is marked as safe, pass "safe" tag as extra_tags so that
# client can skip HTML escape for the message when rendering
if isinstance(message, SafeData):
extra_tags = extra_tags + ' safe'
request.horizon['async_messages'].append([tag,
force_text(message),
extra_tags])
else:
return _messages.add_message(request, level, message,
extra_tags, fail_silently)
if not horizon_message_already_queued(request, message):
if request.is_ajax():
tag = constants.DEFAULT_TAGS[level]
# if message is marked as safe, pass "safe" tag as extra_tags so
# that client can skip HTML escape for the message when rendering
if isinstance(message, SafeData):
extra_tags = extra_tags + ' safe'
request.horizon['async_messages'].append([tag,
force_text(message),
extra_tags])
else:
return _messages.add_message(request, level, message,
extra_tags, fail_silently)
def debug(request, message, extra_tags='', fail_silently=False):

View File

@ -118,7 +118,10 @@ class InstanceViewTest(test.BaseAdminViewTests):
res = self.client.get(INDEX_URL)
instances = res.context['table'].data
self.assertTemplateUsed(res, 'admin/instances/index.html')
self.assertMessageCount(res, error=len(servers))
# Since error messages produced for each instance are identical,
# there will be only one error message for all instances
# (messages de-duplication).
self.assertMessageCount(res, error=1)
self.assertItemsEqual(instances, servers)
@test.create_stubs({api.nova: ('server_list',)})

View File

@ -24,6 +24,8 @@ Views for managing floating IPs.
from django.core.urlresolvers import reverse_lazy
from django.utils.translation import ugettext_lazy as _
from neutronclient.common import exceptions as neutron_exc
from horizon import exceptions
from horizon import forms
from horizon import workflows
@ -60,6 +62,9 @@ class AllocateView(forms.ModalFormView):
def get_initial(self):
try:
pools = api.network.floating_ip_pools_list(self.request)
except neutron_exc.ConnectionFailed:
pools = []
exceptions.handle(self.request)
except Exception:
pools = []
exceptions.handle(self.request,

View File

@ -15,6 +15,8 @@
from django.core.urlresolvers import reverse
from django.utils.translation import ugettext_lazy as _
from neutronclient.common import exceptions as neutron_exc
from horizon import exceptions
from horizon import forms
from horizon import workflows
@ -59,10 +61,12 @@ class AssociateIPAction(workflows.Action):
def populate_ip_id_choices(self, request, context):
ips = []
redirect = reverse('horizon:project:access_and_security:index')
try:
ips = api.network.tenant_floating_ip_list(self.request)
except neutron_exc.ConnectionFailed:
exceptions.handle(self.request, redirect=redirect)
except Exception:
redirect = reverse('horizon:project:access_and_security:index')
exceptions.handle(self.request,
_('Unable to retrieve floating IP addresses.'),
redirect=redirect)

View File

@ -22,6 +22,8 @@ from django.utils.translation import ugettext_lazy as _
from horizon import exceptions
from horizon import tabs
from neutronclient.common import exceptions as neutron_exc
from openstack_dashboard.api import keystone
from openstack_dashboard.api import network
from openstack_dashboard.api import nova
@ -46,6 +48,9 @@ class SecurityGroupsTab(tabs.TableTab):
def get_security_groups_data(self):
try:
security_groups = network.security_group_list(self.request)
except neutron_exc.ConnectionFailed:
security_groups = []
exceptions.handle(self.request)
except Exception:
security_groups = []
exceptions.handle(self.request,
@ -80,6 +85,9 @@ class FloatingIPsTab(tabs.TableTab):
def get_floating_ips_data(self):
try:
floating_ips = network.tenant_floating_ip_list(self.request)
except neutron_exc.ConnectionFailed:
floating_ips = []
exceptions.handle(self.request)
except Exception:
floating_ips = []
exceptions.handle(self.request,
@ -87,6 +95,9 @@ class FloatingIPsTab(tabs.TableTab):
try:
floating_ip_pools = network.floating_ip_pools_list(self.request)
except neutron_exc.ConnectionFailed:
floating_ip_pools = []
exceptions.handle(self.request)
except Exception:
floating_ip_pools = []
exceptions.handle(self.request,

View File

@ -198,7 +198,10 @@ class InstanceTests(helpers.TestCase):
instances = res.context['instances_table'].data
self.assertTemplateUsed(res, 'project/instances/index.html')
self.assertMessageCount(res, error=len(servers))
# Since error messages produced for each instance are identical,
# there will be only one error message for all instances
# (messages de-duplication)
self.assertMessageCount(res, error=1)
self.assertItemsEqual(instances, self.servers.list())
@helpers.create_stubs({