From dfeb362b95f2b481ff3231b4821d4a5e94fa2e89 Mon Sep 17 00:00:00 2001 From: Masco Kaliyamoorthy Date: Mon, 17 Aug 2015 19:12:23 +0530 Subject: [PATCH] Adding informative message when deleting a used volume type Currently when you try to delete a volume type (which is in use) Horizon returns a generic message. This patch includes in this message the text from the exception, resulting on the following message: 'Error: Volume type '' is still in use.' Change-Id: I9577127909dca70a4c45129bf5df7e11b6ab1c2f Closes-bug: #1334523 --- horizon/exceptions.py | 8 ++++- openstack_dashboard/api/cinder.py | 8 +++-- .../admin/volumes/volume_types/tables.py | 7 +++- .../admin/volumes/volume_types/tests.py | 35 +++++++++++++++++++ 4 files changed, 54 insertions(+), 4 deletions(-) diff --git a/horizon/exceptions.py b/horizon/exceptions.py index c9d83853ad..68ac98cd7e 100644 --- a/horizon/exceptions.py +++ b/horizon/exceptions.py @@ -140,6 +140,11 @@ class Conflict(HorizonException): status_code = 409 +class BadRequest(HorizonException): + """Generic error to replace all "BadRequest"-type API errors.""" + status_code = 400 + + class RecoverableError(HorizonException): """Generic error to replace any "Recoverable"-type API errors.""" status_code = 100 # HTTP status code "Continue" @@ -224,7 +229,8 @@ UNAUTHORIZED = tuple(HORIZON_CONFIG['exceptions']['unauthorized']) UNAUTHORIZED += (NotAuthorized,) NOT_FOUND = tuple(HORIZON_CONFIG['exceptions']['not_found']) NOT_FOUND += (GetFileError,) -RECOVERABLE = (AlreadyExists, Conflict, NotAvailable, ServiceCatalogException) +RECOVERABLE = (AlreadyExists, Conflict, NotAvailable, ServiceCatalogException, + BadRequest) RECOVERABLE += tuple(HORIZON_CONFIG['exceptions']['recoverable']) diff --git a/openstack_dashboard/api/cinder.py b/openstack_dashboard/api/cinder.py index e4fd2062d1..d62d159f19 100644 --- a/openstack_dashboard/api/cinder.py +++ b/openstack_dashboard/api/cinder.py @@ -26,7 +26,7 @@ from django.conf import settings from django.utils.translation import pgettext_lazy from django.utils.translation import ugettext_lazy as _ -from cinderclient.exceptions import ClientException # noqa +from cinderclient import exceptions as cinder_exception from cinderclient.v2.contrib import list_extensions as cinder_list_extensions from horizon import exceptions @@ -497,7 +497,11 @@ def volume_type_default(request): def volume_type_delete(request, volume_type_id): - return cinderclient(request).volume_types.delete(volume_type_id) + try: + return cinderclient(request).volume_types.delete(volume_type_id) + except cinder_exception.BadRequest: + raise exceptions.BadRequest(_( + "This volume type is used by one or more volumes.")) def volume_type_get(request, volume_type_id): diff --git a/openstack_dashboard/dashboards/admin/volumes/volume_types/tables.py b/openstack_dashboard/dashboards/admin/volumes/volume_types/tables.py index 12b2c83a5b..924925d180 100644 --- a/openstack_dashboard/dashboards/admin/volumes/volume_types/tables.py +++ b/openstack_dashboard/dashboards/admin/volumes/volume_types/tables.py @@ -10,6 +10,7 @@ # License for the specific language governing permissions and limitations # under the License. +from django.core.urlresolvers import reverse from django.template import defaultfilters as filters from django.utils.translation import ugettext_lazy as _ from django.utils.translation import ungettext_lazy @@ -77,7 +78,11 @@ class DeleteVolumeType(tables.DeleteAction): policy_rules = (("volume", "volume_extension:types_manage"),) def delete(self, request, obj_id): - cinder.volume_type_delete(request, obj_id) + try: + cinder.volume_type_delete(request, obj_id) + except exceptions.BadRequest as e: + redirect_url = reverse("horizon:admin:volumes:index") + exceptions.handle(request, e, redirect=redirect_url) class CreateVolumeTypeEncryption(tables.LinkAction): diff --git a/openstack_dashboard/dashboards/admin/volumes/volume_types/tests.py b/openstack_dashboard/dashboards/admin/volumes/volume_types/tests.py index 68ee75784d..88411ea71c 100644 --- a/openstack_dashboard/dashboards/admin/volumes/volume_types/tests.py +++ b/openstack_dashboard/dashboards/admin/volumes/volume_types/tests.py @@ -14,6 +14,8 @@ from django.core.urlresolvers import reverse from django import http from mox3.mox import IsA # noqa +from horizon import exceptions + from openstack_dashboard import api from openstack_dashboard.api import cinder from openstack_dashboard.api import keystone @@ -93,6 +95,39 @@ class VolumeTypeTests(test.BaseAdminViewTests): self.assertNoFormErrors(res) self.assertRedirectsNoFollow(res, redirect) + @test.create_stubs({api.nova: ('server_list',), + cinder: ('volume_list', + 'volume_type_list_with_qos_associations', + 'qos_spec_list', + 'volume_type_delete', + 'volume_encryption_type_list'), + keystone: ('tenant_list',)}) + def test_delete_volume_type_exception(self): + volume_type = self.volume_types.first() + formData = {'action': 'volume_types__delete__%s' % volume_type.id} + encryption_list = (self.cinder_volume_encryption_types.list()[0], + self.cinder_volume_encryption_types.list()[1]) + + cinder.volume_type_list_with_qos_associations( + IsA(http.HttpRequest)).\ + AndReturn(self.volume_types.list()) + cinder.qos_spec_list(IsA(http.HttpRequest)).\ + AndReturn(self.cinder_qos_specs.list()) + cinder.volume_encryption_type_list(IsA(http.HttpRequest))\ + .AndReturn(encryption_list) + cinder.volume_type_delete(IsA(http.HttpRequest), + str(volume_type.id))\ + .AndRaise(exceptions.BadRequest()) + self.mox.ReplayAll() + + res = self.client.post( + reverse('horizon:admin:volumes:volumes_tab'), + formData) + + redirect = reverse('horizon:admin:volumes:volumes_tab') + self.assertNoFormErrors(res) + self.assertRedirectsNoFollow(res, redirect) + @test.create_stubs({cinder: ('volume_encryption_type_create', 'volume_type_list',)}) def test_create_volume_type_encryption(self):