From: Vincent Hou Date: Wed, 8 Jan 2014 07:52:54 +0000 (-0500) Subject: Validate the quota in the API layer for volume extend X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=24d6b57d1cb0c4f91545f85ea9c766d1e0ec50a0;p=openstack-build%2Fcinder-build.git 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 caeea99bd..8576a19c9 100644 --- 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 f1f438cd1..c204b498b 100644 --- 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 cc4dc5510..2e4a746d8 100644 --- 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 0b4e7a9d3..1e080d05e 100644 --- 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 01011527d..df7f69363 100644 --- 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,