Validate the quota in the API layer for volume extend

The user needs a friendly message feedback if the quota exceeds when
the volume is about to extend.

Change-Id: Ifd523ac5e9039861cf87711dc5c4842b5cb524c2
Closes-Bug: #1256763

diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py
index caeea99bd9e57f7f6b334a9d6f12bddfe5dbd603..8576a19c928dc69f72f281c7a128b0da4afcd6f8 100644 (file)
--- a/cinder/tests/test_volume.py
+++ b/cinder/tests/test_volume.py
@@ -1862,7 +1862,8 @@ class VolumeTestCase(BaseVolumeTestCase):
         snap = db.snapshot_get(context.get_admin_context(), snapshot['id'])
         self.assertEqual(snap['display_name'], 'test update name')
 
-    def test_extend_volume(self):
+    @mock.patch.object(QUOTAS, 'reserve')
+    def test_extend_volume(self, reserve):
         """Test volume can be extended at API level."""
         # create a volume and assign to host
         volume = tests_utils.create_volume(self.context, size=2,
@@ -1896,17 +1897,29 @@ class VolumeTestCase(BaseVolumeTestCase):
                           2)
 
         # works when new_size > orig_size
+        reserve.return_value = ["RESERVATION"]
         volume_api.extend(self.context, volume, 3)
-
         volume = db.volume_get(context.get_admin_context(), volume['id'])
         self.assertEqual(volume['status'], 'extending')
 
+        # Test the quota exceeded
+        volume['status'] = 'available'
+        reserve.side_effect = exception.OverQuota(overs=['gigabytes'],
+                                                  quotas={'gigabytes': 20},
+                                                  usages={'gigabytes':
+                                                          {'reserved': 5,
+                                                           'in_use': 15}})
+        self.assertRaises(exception.VolumeSizeExceedsAvailableQuota,
+                          volume_api.extend, self.context,
+                          volume, 3)
+
         # clean up
         self.volume.delete_volume(self.context, volume['id'])
 
     def test_extend_volume_driver_not_initialized(self):
         """Test volume can be extended at API level."""
         # create a volume and assign to host
+        fake_reservations = ['RESERVATION']
         volume = tests_utils.create_volume(self.context, size=2,
                                            status='available',
                                            host=CONF.host)
@@ -1917,7 +1930,8 @@ class VolumeTestCase(BaseVolumeTestCase):
 
         self.assertRaises(exception.DriverNotInitialized,
                           self.volume.extend_volume,
-                          self.context, volume['id'], 3)
+                          self.context, volume['id'], 3,
+                          fake_reservations)
 
         volume = db.volume_get(context.get_admin_context(), volume['id'])
         self.assertEqual(volume.status, 'error_extending')
@@ -1929,49 +1943,36 @@ class VolumeTestCase(BaseVolumeTestCase):
 
     def test_extend_volume_manager(self):
         """Test volume can be extended at the manager level."""
-        def fake_reserve(context, expire=None, project_id=None, **deltas):
-            return ['RESERVATION']
-
-        def fake_reserve_exc(context, expire=None, project_id=None, **deltas):
-            raise exception.OverQuota(overs=['gigabytes'],
-                                      quotas={'gigabytes': 20},
-                                      usages={'gigabytes': {'reserved': 5,
-                                                            'in_use': 15}})
-
-        def fake_extend_exc(volume, new_size):
-            raise exception.CinderException('fake exception')
+        def fake_extend(volume, new_size):
+            volume['size'] = new_size
 
+        fake_reservations = ['RESERVATION']
         volume = tests_utils.create_volume(self.context, size=2,
                                            status='creating', host=CONF.host)
         self.volume.create_volume(self.context, volume['id'])
 
-        # Test quota exceeded
-        self.stubs.Set(QUOTAS, 'reserve', fake_reserve_exc)
-        self.stubs.Set(QUOTAS, 'commit', lambda x, y, project_id=None: True)
-        self.stubs.Set(QUOTAS, 'rollback', lambda x, y: True)
-        volume['status'] = 'extending'
-        self.volume.extend_volume(self.context, volume['id'], '4')
-        volume = db.volume_get(context.get_admin_context(), volume['id'])
-        self.assertEqual(volume['size'], 2)
-        self.assertEqual(volume['status'], 'error_extending')
-
         # Test driver exception
-        self.stubs.Set(QUOTAS, 'reserve', fake_reserve)
-        self.stubs.Set(self.volume.driver, 'extend_volume', fake_extend_exc)
-        volume['status'] = 'extending'
-        self.volume.extend_volume(self.context, volume['id'], '4')
-        volume = db.volume_get(context.get_admin_context(), volume['id'])
-        self.assertEqual(volume['size'], 2)
-        self.assertEqual(volume['status'], 'error_extending')
+        with mock.patch.object(self.volume.driver,
+                               'extend_volume') as extend_volume:
+            extend_volume.side_effect =\
+                exception.CinderException('fake exception')
+            volume['status'] = 'extending'
+            self.volume.extend_volume(self.context, volume['id'], '4',
+                                      fake_reservations)
+            volume = db.volume_get(context.get_admin_context(), volume['id'])
+            self.assertEqual(volume['size'], 2)
+            self.assertEqual(volume['status'], 'error_extending')
 
         # Test driver success
-        self.stubs.Set(self.volume.driver, 'extend_volume',
-                       lambda x, y: True)
-        volume['status'] = 'extending'
-        self.volume.extend_volume(self.context, volume['id'], '4')
-        volume = db.volume_get(context.get_admin_context(), volume['id'])
-        self.assertEqual(volume['size'], 4)
-        self.assertEqual(volume['status'], 'available')
+        with mock.patch.object(self.volume.driver,
+                               'extend_volume') as extend_volume:
+            extend_volume.return_value = fake_extend
+            volume['status'] = 'extending'
+            self.volume.extend_volume(self.context, volume['id'], '4',
+                                      fake_reservations)
+            volume = db.volume_get(context.get_admin_context(), volume['id'])
+            self.assertEqual(volume['size'], 4)
+            self.assertEqual(volume['status'], 'available')
 
         # clean up
         self.volume.delete_volume(self.context, volume['id'])

diff --git a/cinder/tests/test_volume_rpcapi.py b/cinder/tests/test_volume_rpcapi.py
index f1f438cd1ca21a717c55c4fcb99ab33c6185eea5..c204b498bb52e1938cacfd1becd712ca493463be 100644 (file)
--- a/cinder/tests/test_volume_rpcapi.py
+++ b/cinder/tests/test_volume_rpcapi.py
@@ -54,6 +54,7 @@ class VolumeRpcAPITestCase(test.TestCase):
         self.fake_volume = jsonutils.to_primitive(volume)
         self.fake_volume_metadata = volume["volume_metadata"]
         self.fake_snapshot = jsonutils.to_primitive(snapshot)
+        self.fake_reservations = ["RESERVATION"]
 
     def test_serialized_volume_has_id(self):
         self.assertIn('id', self.fake_volume)
@@ -229,7 +230,8 @@ class VolumeRpcAPITestCase(test.TestCase):
                               rpc_method='cast',
                               volume=self.fake_volume,
                               new_size=1,
-                              version='1.6')
+                              reservations=self.fake_reservations,
+                              version='1.14')
 
     def test_migrate_volume(self):
         class FakeHost(object):

diff --git a/cinder/volume/api.py b/cinder/volume/api.py
index cc4dc55101474b809e539ff6be249a2074cb6504..2e4a746d8df0733fc3e300dff12ef3a90ca32b6d 100644 (file)
--- a/cinder/volume/api.py
+++ b/cinder/volume/api.py
@@ -760,8 +760,32 @@ class API(base.Base):
                                                    'size': volume['size']})
             raise exception.InvalidInput(reason=msg)
 
+        try:
+            reservations = QUOTAS.reserve(context, gigabytes=+size_increase)
+        except exception.OverQuota as exc:
+            self.db.volume_update(context, volume['id'],
+                                  {'status': 'error_extending'})
+            usages = exc.kwargs['usages']
+            quotas = exc.kwargs['quotas']
+
+            def _consumed(name):
+                return (usages[name]['reserved'] + usages[name]['in_use'])
+
+            msg = _("Quota exceeded for %(s_pid)s, tried to extend volume by "
+                    "%(s_size)sG, (%(d_consumed)dG of %(d_quota)dG already "
+                    "consumed).")
+            LOG.error(msg % {'s_pid': context.project_id,
+                             's_size': size_increase,
+                             'd_consumed': _consumed('gigabytes'),
+                             'd_quota': quotas['gigabytes']})
+            raise exception.VolumeSizeExceedsAvailableQuota(
+                requested=size_increase,
+                consumed=_consumed('gigabytes'),
+                quota=quotas['gigabytes'])
+
         self.update(context, volume, {'status': 'extending'})
-        self.volume_rpcapi.extend_volume(context, volume, new_size)
+        self.volume_rpcapi.extend_volume(context, volume, new_size,
+                                         reservations)
 
     @wrap_check_policy
     def migrate_volume(self, context, volume, host, force_host_copy):

diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py
index 0b4e7a9d37bb7eca34561c8068e5d513aa34eb22..1e080d05ee21ff3489ae03a008254fa11bc778f5 100644 (file)
--- a/cinder/volume/manager.py
+++ b/cinder/volume/manager.py
@@ -182,7 +182,7 @@ def locked_snapshot_operation(f):
 class VolumeManager(manager.SchedulerDependentManager):
     """Manages attachable block storage devices."""
 
-    RPC_API_VERSION = '1.13'
+    RPC_API_VERSION = '1.14'
 
     def __init__(self, volume_driver=None, service_name=None,
                  *args, **kwargs):
@@ -1092,7 +1092,7 @@ class VolumeManager(manager.SchedulerDependentManager):
             context, snapshot, event_suffix,
             extra_usage_info=extra_usage_info, host=self.host)
 
-    def extend_volume(self, context, volume_id, new_size):
+    def extend_volume(self, context, volume_id, new_size, reservations):
         try:
             # NOTE(flaper87): Verify the driver is enabled
             # before going forward. The exception will be caught
@@ -1105,30 +1105,6 @@ class VolumeManager(manager.SchedulerDependentManager):
 
         volume = self.db.volume_get(context, volume_id)
         size_increase = (int(new_size)) - volume['size']
-
-        try:
-            reservations = QUOTAS.reserve(context, gigabytes=+size_increase)
-        except exception.OverQuota as exc:
-            self.db.volume_update(context, volume['id'],
-                                  {'status': 'error_extending'})
-            overs = exc.kwargs['overs']
-            usages = exc.kwargs['usages']
-            quotas = exc.kwargs['quotas']
-
-            def _consumed(name):
-                return (usages[name]['reserved'] + usages[name]['in_use'])
-
-            if 'gigabytes' in overs:
-                msg = _("Quota exceeded for %(s_pid)s, "
-                        "tried to extend volume by "
-                        "%(s_size)sG, (%(d_consumed)dG of %(d_quota)dG "
-                        "already consumed)")
-                LOG.error(msg % {'s_pid': context.project_id,
-                                 's_size': size_increase,
-                                 'd_consumed': _consumed('gigabytes'),
-                                 'd_quota': quotas['gigabytes']})
-            return
-
         self._notify_about_volume_usage(context, volume, "resize.start")
         try:
             LOG.info(_("volume %s: extending"), volume['id'])
@@ -1140,6 +1116,9 @@ class VolumeManager(manager.SchedulerDependentManager):
             try:
                 self.db.volume_update(context, volume['id'],
                                       {'status': 'error_extending'})
+                raise exception.CinderException(_("Volume %s: Error trying "
+                                                  "to extend volume") %
+                                                volume_id)
             finally:
                 QUOTAS.rollback(context, reservations)
                 return

diff --git a/cinder/volume/rpcapi.py b/cinder/volume/rpcapi.py
index 01011527d69a0f2aae47ff45e48c840a070d9292..df7f6936379da27bc99811d9893172989a69945b 100644 (file)
--- a/cinder/volume/rpcapi.py
+++ b/cinder/volume/rpcapi.py
@@ -48,6 +48,7 @@ class VolumeAPI(cinder.openstack.common.rpc.proxy.RpcProxy):
                to support volume read-only attaching.
         1.12 - Adds retype.
         1.13 - Adds create_export.
+        1.14 - Adds reservation parameter to extend_volume().
     '''
 
     BASE_RPC_API_VERSION = '1.0'
@@ -154,13 +155,14 @@ class VolumeAPI(cinder.openstack.common.rpc.proxy.RpcProxy):
                   topic=rpc.queue_get_for(ctxt, self.topic, volume['host']),
                   version='1.9')
 
-    def extend_volume(self, ctxt, volume, new_size):
+    def extend_volume(self, ctxt, volume, new_size, reservations):
         self.cast(ctxt,
                   self.make_msg('extend_volume',
                                 volume_id=volume['id'],
-                                new_size=new_size),
+                                new_size=new_size,
+                                reservations=reservations),
                   topic=rpc.queue_get_for(ctxt, self.topic, volume['host']),
-                  version='1.6')
+                  version='1.14')
 
     def migrate_volume(self, ctxt, volume, dest_host, force_host_copy):
         host_p = {'host': dest_host.host,