From 2655e8bba9279dec7c4fbbea28e6a011272794e2 Mon Sep 17 00:00:00 2001 From: haobing1 Date: Wed, 7 Dec 2016 21:50:18 +0800 Subject: [PATCH] Cinder consistency group returning generic error message When trying to create a consistency group with the quota reached already, the error raised to the user is the generic "ERROR: The server has either erred or is incapable of performing the requested operation. (HTTP 500)" message. This patch will make the error message return to the user with a 413 error code. Such as "ERROR: GroupLimitExceeded: Maximum number of groups allowed (10) exceeded'.(HTTP 413)" Change-Id: I0dd86dbc84d3dc75568c39aca8150c8fa12c4811 Closes-Bug: #1610295 --- cinder/consistencygroup/api.py | 6 +++- cinder/exception.py | 4 +++ cinder/group/api.py | 6 +++- cinder/quota_utils.py | 3 +- cinder/tests/unit/api/v3/test_groups.py | 16 +++++++++ cinder/tests/unit/group/test_groups_api.py | 39 ++++++++++++++++++++++ cinder/tests/unit/test_quota_utils.py | 9 +++++ 7 files changed, 80 insertions(+), 3 deletions(-) diff --git a/cinder/consistencygroup/api.py b/cinder/consistencygroup/api.py index ffd7d503573..fbd3b698a60 100644 --- a/cinder/consistencygroup/api.py +++ b/cinder/consistencygroup/api.py @@ -33,6 +33,7 @@ from cinder import objects from cinder.objects import fields as c_fields import cinder.policy from cinder import quota +from cinder import quota_utils from cinder.scheduler import rpcapi as scheduler_rpcapi from cinder.volume import api as volume_api from cinder.volume import rpcapi as volume_rpcapi @@ -405,10 +406,13 @@ class API(base.Base): **reserve_opts) if reservations: CGQUOTAS.commit(context, reservations) - except Exception: + except Exception as e: with excutils.save_and_reraise_exception(): try: group.destroy() + if isinstance(e, exception.OverQuota): + quota_utils.process_reserve_over_quota( + context, e, resource='groups') finally: LOG.error(_LE("Failed to update quota for " "consistency group %s."), group.id) diff --git a/cinder/exception.py b/cinder/exception.py index 39d26ddd951..8e1fd456a7f 100644 --- a/cinder/exception.py +++ b/cinder/exception.py @@ -659,6 +659,10 @@ class GroupTypeUpdateFailed(CinderException): message = _("Cannot update group_type %(id)s") +class GroupLimitExceeded(QuotaError): + message = _("Maximum number of groups allowed (%(allowed)d) exceeded") + + class UnknownCmd(VolumeDriverException): message = _("Unknown or unsupported command %(cmd)s") diff --git a/cinder/group/api.py b/cinder/group/api.py index 5e450eee9c6..1a86a32dc3e 100644 --- a/cinder/group/api.py +++ b/cinder/group/api.py @@ -35,6 +35,7 @@ from cinder.objects import base as objects_base from cinder.objects import fields as c_fields import cinder.policy from cinder import quota +from cinder import quota_utils from cinder.scheduler import rpcapi as scheduler_rpcapi from cinder.volume import api as volume_api from cinder.volume import rpcapi as volume_rpcapi @@ -469,10 +470,13 @@ class API(base.Base): **reserve_opts) if reservations: GROUP_QUOTAS.commit(context, reservations) - except Exception: + except Exception as e: with excutils.save_and_reraise_exception(): try: group.destroy() + if isinstance(e, exception.OverQuota): + quota_utils.process_reserve_over_quota( + context, e, resource='groups') finally: LOG.error(_LE("Failed to update quota for " "group %s."), group.id) diff --git a/cinder/quota_utils.py b/cinder/quota_utils.py index 6faa323bc5b..eee4ddf2809 100644 --- a/cinder/quota_utils.py +++ b/cinder/quota_utils.py @@ -238,7 +238,8 @@ def _keystone_client(context, version=(3, 0)): OVER_QUOTA_RESOURCE_EXCEPTIONS = {'snapshots': exception.SnapshotLimitExceeded, 'backups': exception.BackupLimitExceeded, - 'volumes': exception.VolumeLimitExceeded, } + 'volumes': exception.VolumeLimitExceeded, + 'groups': exception.GroupLimitExceeded} def process_reserve_over_quota(context, over_quota_exception, diff --git a/cinder/tests/unit/api/v3/test_groups.py b/cinder/tests/unit/api/v3/test_groups.py index 3ceb17d9a77..b491193ee1f 100644 --- a/cinder/tests/unit/api/v3/test_groups.py +++ b/cinder/tests/unit/api/v3/test_groups.py @@ -491,6 +491,22 @@ class GroupsAPITestCase(test.TestCase): group.id) self.assertEqual(fields.GroupStatus.DELETED, group.status) + @mock.patch('cinder.group.api.API.create') + def test_create_group_failed_exceeded_quota(self, mock_group_create): + mock_group_create.side_effect = exception.GroupLimitExceeded(allowed=1) + name = 'group1' + body = {"group": {"group_type": fake.GROUP_TYPE_ID, + "volume_types": [fake.VOLUME_TYPE_ID], + "name": name, + "description": + "Group 1", }} + req = fakes.HTTPRequest.blank('/v3/%s/groups' % fake.PROJECT_ID, + version=GROUP_MICRO_VERSION) + ex = self.assertRaises(exception.GroupLimitExceeded, + self.controller.create, + req, body) + self.assertEqual(413, ex.code) + def test_delete_group_with_invalid_body(self): self.group1.status = fields.GroupStatus.AVAILABLE self.group1.save() diff --git a/cinder/tests/unit/group/test_groups_api.py b/cinder/tests/unit/group/test_groups_api.py index 0b4ddcab2b9..5ffac7cb397 100644 --- a/cinder/tests/unit/group/test_groups_api.py +++ b/cinder/tests/unit/group/test_groups_api.py @@ -22,14 +22,19 @@ import mock from cinder import context from cinder import db +from cinder import exception import cinder.group from cinder import objects from cinder.objects import fields +from cinder import quota from cinder import test from cinder.tests.unit import fake_constants as fake from cinder.tests.unit import utils +GROUP_QUOTAS = quota.GROUP_QUOTAS + + @ddt.ddt class GroupAPITestCase(test.TestCase): """Test Case for group API.""" @@ -168,6 +173,40 @@ class GroupAPITestCase(test.TestCase): mock_volume_types_get.assert_called_once_with(mock.ANY, volume_type_names) + @mock.patch.object(GROUP_QUOTAS, "reserve") + @mock.patch('cinder.objects.Group') + @mock.patch('cinder.db.group_type_get_by_name') + @mock.patch('cinder.db.volume_types_get_by_name_or_id') + @mock.patch('cinder.group.api.check_policy') + def test_create_group_failed_update_quota(self, mock_policy, + mock_volume_types_get, + mock_group_type_get, mock_group, + mock_group_quota_reserve): + mock_volume_types_get.return_value = [{'id': fake.VOLUME_TYPE_ID}] + mock_group_type_get.return_value = {'id': fake.GROUP_TYPE_ID} + fake_overs = ['groups'] + fake_quotas = {'groups': 1} + fake_usages = {'groups': {'reserved': 0, 'in_use': 1}} + mock_group_quota_reserve.side_effect = exception.OverQuota( + overs=fake_overs, + quotas=fake_quotas, + usages=fake_usages) + name = "test_group" + description = "this is a test group" + grp = utils.create_group(self.ctxt, group_type_id=fake.GROUP_TYPE_ID, + volume_type_ids=[fake.VOLUME_TYPE_ID], + availability_zone='nova', host=None, + name=name, description=description, + status=fields.GroupStatus.CREATING) + mock_group.return_value = grp + + self.assertRaises(exception.GroupLimitExceeded, + self.group_api.create, + self.ctxt, name, description, + "fake-grouptype-name", + [fake.VOLUME_TYPE_ID], + availability_zone='nova') + @mock.patch('cinder.volume.rpcapi.VolumeAPI.update_group') @mock.patch('cinder.db.volume_get_all_by_generic_group') @mock.patch('cinder.group.api.API._cast_create_group') diff --git a/cinder/tests/unit/test_quota_utils.py b/cinder/tests/unit/test_quota_utils.py index 782a82f014b..d1b4b4b3781 100644 --- a/cinder/tests/unit/test_quota_utils.py +++ b/cinder/tests/unit/test_quota_utils.py @@ -225,6 +225,15 @@ class QuotaUtilsTest(test.TestCase): overs, usages, quotas, exception.VolumeLimitExceeded) + def test_groups_limit_quota(self): + overs = ['groups'] + usages = {'groups': {'reserved': 1, 'in_use': 9}} + quotas = {'groups': 9} + self._process_reserve_over_quota( + overs, usages, quotas, + exception.GroupLimitExceeded, + resource='groups') + def test_unknown_quota(self): overs = ['unknown'] usages = {'volumes': {'reserved': 1, 'in_use': 9}}