From: Nate Potter Date: Tue, 8 Dec 2015 22:12:27 +0000 (+0000) Subject: Move retype quota checks to API X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=d78399e614a90b062bcc1ff10aa00a037b158cad;p=openstack-build%2Fcinder-build.git Move retype quota checks to API Currently the retype quota check is done in the manager, where it will raise an exception if the check fails. This patch moves the check to the API so that the user is given feedback before the RPC cast is made. APIImpact Closes-bug: #1508249 Change-Id: Iba24edd8ad824837028353b52c90742df55c9173 --- diff --git a/cinder/scheduler/manager.py b/cinder/scheduler/manager.py index fa06628aa..ced49c315 100644 --- a/cinder/scheduler/manager.py +++ b/cinder/scheduler/manager.py @@ -216,6 +216,7 @@ class SchedulerManager(manager.Manager): context, ex, request_spec, msg) reservations = request_spec.get('quota_reservations') + old_reservations = request_spec.get('old_reservations', None) new_type = request_spec.get('volume_type') if new_type is None: msg = _('New volume type not specified in request_spec.') @@ -245,7 +246,9 @@ class SchedulerManager(manager.Manager): else: volume_rpcapi.VolumeAPI().retype(context, volume, new_type['id'], tgt_host, - migration_policy, reservations) + migration_policy, + reservations, + old_reservations) def manage_existing(self, context, topic, volume_id, request_spec, filter_properties=None): diff --git a/cinder/tests/unit/test_volume.py b/cinder/tests/unit/test_volume.py index 08c9010b7..0e45eb18a 100644 --- a/cinder/tests/unit/test_volume.py +++ b/cinder/tests/unit/test_volume.py @@ -4800,6 +4800,14 @@ class VolumeMigrationTestCase(VolumeTestCase): project_id=project_id, **reserve_opts) + old_reserve_opts = {'volumes': -1, 'gigabytes': -volume.size} + QUOTAS.add_volume_type_opts(self.context, + old_reserve_opts, + old_vol_type['id']) + old_reservations = QUOTAS.reserve(self.context, + project_id=project_id, + **old_reserve_opts) + with mock.patch.object(self.volume.driver, 'retype') as _retype,\ mock.patch.object(volume_types, 'volume_types_diff') as _diff,\ mock.patch.object(self.volume, 'migrate_volume') as _mig,\ @@ -4817,6 +4825,7 @@ class VolumeMigrationTestCase(VolumeTestCase): vol_type['id'], host_obj, migration_policy=policy, reservations=reservations, + old_reservations=old_reservations, volume=volume) else: self.assertRaises(exc, self.volume.retype, @@ -4824,6 +4833,7 @@ class VolumeMigrationTestCase(VolumeTestCase): vol_type['id'], host_obj, migration_policy=policy, reservations=reservations, + old_reservations=old_reservations, volume=volume) # get volume/quota properties diff --git a/cinder/tests/unit/test_volume_rpcapi.py b/cinder/tests/unit/test_volume_rpcapi.py index 22f0c906c..c192ee791 100644 --- a/cinder/tests/unit/test_volume_rpcapi.py +++ b/cinder/tests/unit/test_volume_rpcapi.py @@ -19,6 +19,7 @@ import copy import mock from oslo_config import cfg +import oslo_messaging as messaging from oslo_serialization import jsonutils from cinder import context @@ -459,27 +460,52 @@ class VolumeRpcAPITestCase(test.TestCase): new_type_id='fake', dest_host=dest_host, migration_policy='never', - reservations=None, - version='1.34') - can_send_version.assert_called_once_with('1.34') + reservations=self.fake_reservations, + old_reservations=self.fake_reservations, + version='1.37') + can_send_version.assert_called_once_with('1.37') - @mock.patch('oslo_messaging.RPCClient.can_send_version', - return_value=False) - def test_retype_old(self, can_send_version): + def test_retype_version_134(self): class FakeHost(object): def __init__(self): self.host = 'host' self.capabilities = {} dest_host = FakeHost() - self._test_volume_api('retype', - rpc_method='cast', - volume=self.fake_volume_obj, - new_type_id='fake', - dest_host=dest_host, - migration_policy='never', - reservations=None, - version='1.12') - can_send_version.assert_called_once_with('1.34') + with mock.patch.object(messaging.RPCClient, + 'can_send_version', + side_effect=[False, True]) as can_send_version: + self._test_volume_api('retype', + rpc_method='cast', + volume=self.fake_volume_obj, + new_type_id='fake', + dest_host=dest_host, + migration_policy='never', + reservations=self.fake_reservations, + old_reservations=self.fake_reservations, + version='1.34') + can_send_version.assert_any_call('1.37') + can_send_version.assert_any_call('1.34') + + def test_retype_version_112(self): + class FakeHost(object): + def __init__(self): + self.host = 'host' + self.capabilities = {} + dest_host = FakeHost() + with mock.patch.object(messaging.RPCClient, + 'can_send_version', + side_effect=[False, False]) as can_send_version: + self._test_volume_api('retype', + rpc_method='cast', + volume=self.fake_volume_obj, + new_type_id='fake', + dest_host=dest_host, + migration_policy='never', + reservations=self.fake_reservations, + old_reservations=self.fake_reservations, + version='1.12') + can_send_version.assert_any_call('1.37') + can_send_version.assert_any_call('1.34') def test_manage_existing(self): self._test_volume_api('manage_existing', diff --git a/cinder/volume/api.py b/cinder/volume/api.py index 9341399a4..d3eac8087 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -1505,6 +1505,22 @@ class API(base.Base): reservations = quota_utils.get_volume_type_reservation(context, volume, vol_type_id) + # Get old reservations + try: + reserve_opts = {'volumes': -1, 'gigabytes': -volume.size} + QUOTAS.add_volume_type_opts(context, + reserve_opts, + old_vol_type_id) + old_reservations = QUOTAS.reserve(context, + project_id=volume.project_id, + **reserve_opts) + except Exception: + volume.status = volume.previous_status + volume.save() + msg = _("Failed to update quota usage while retyping volume.") + LOG.exception(msg, resource=volume) + raise exception.CinderException(msg) + self.update(context, volume, {'status': 'retyping', 'previous_status': volume.status}) @@ -1512,7 +1528,8 @@ class API(base.Base): 'volume_id': volume.id, 'volume_type': vol_type, 'migration_policy': migration_policy, - 'quota_reservations': reservations} + 'quota_reservations': reservations, + 'old_reservations': old_reservations} self.scheduler_rpcapi.retype(context, CONF.volume_topic, volume.id, request_spec=request_spec, diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py index d63e2360c..4a9a9aa79 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -197,7 +197,7 @@ def locked_snapshot_operation(f): class VolumeManager(manager.SchedulerDependentManager): """Manages attachable block storage devices.""" - RPC_API_VERSION = '1.36' + RPC_API_VERSION = '1.37' target = messaging.Target(version=RPC_API_VERSION) @@ -2063,7 +2063,8 @@ class VolumeManager(manager.SchedulerDependentManager): resource=volume) def retype(self, ctxt, volume_id, new_type_id, host, - migration_policy='never', reservations=None, volume=None): + migration_policy='never', reservations=None, + volume=None, old_reservations=None): def _retype_error(context, volume, old_reservations, new_reservations, status_update): @@ -2102,22 +2103,26 @@ class VolumeManager(manager.SchedulerDependentManager): volume.update(status_update) volume.save() - # Get old reservations - try: - reserve_opts = {'volumes': -1, 'gigabytes': -volume.size} - QUOTAS.add_volume_type_opts(context, - reserve_opts, - volume.volume_type_id) - old_reservations = QUOTAS.reserve(context, - project_id=project_id, - **reserve_opts) - except Exception: - volume.update(status_update) - volume.save() - LOG.exception(_LE("Failed to update usages " - "while retyping volume.")) - raise exception.CinderException(_("Failed to get old volume type" - " quota reservations")) + # If old_reservations has been passed in from the API, we should + # skip quotas. + # TODO(ntpttr): These reservation checks are left in to be backwards + # compatible with Liberty and can be removed in N. + if not old_reservations: + # Get old reservations + try: + reserve_opts = {'volumes': -1, 'gigabytes': -volume.size} + QUOTAS.add_volume_type_opts(context, + reserve_opts, + volume.volume_type_id) + old_reservations = QUOTAS.reserve(context, + project_id=project_id, + **reserve_opts) + except Exception: + volume.update(status_update) + volume.save() + msg = _("Failed to update quota usage while retyping volume.") + LOG.exception(msg, resource=volume) + raise exception.CinderException(msg) # We already got the new reservations new_reservations = reservations diff --git a/cinder/volume/rpcapi.py b/cinder/volume/rpcapi.py index 035b4d595..89879c56f 100644 --- a/cinder/volume/rpcapi.py +++ b/cinder/volume/rpcapi.py @@ -85,6 +85,8 @@ class VolumeAPI(object): 1.35 - Adds support for sending objects over RPC in extend_volume(). 1.36 - Adds support for sending objects over RPC in migrate_volume(), migrate_volume_completion(), and update_migrated_volume(). + 1.37 - Adds old_reservations parameter to retype to support quota + checks in the API. """ BASE_RPC_API_VERSION = '1.0' @@ -279,17 +281,22 @@ class VolumeAPI(object): return cctxt.call(ctxt, 'migrate_volume_completion', **msg_args) def retype(self, ctxt, volume, new_type_id, dest_host, - migration_policy='never', reservations=None): + migration_policy='never', reservations=None, + old_reservations=None): host_p = {'host': dest_host.host, 'capabilities': dest_host.capabilities} msg_args = {'volume_id': volume.id, 'new_type_id': new_type_id, 'host': host_p, 'migration_policy': migration_policy, 'reservations': reservations} - if self.client.can_send_version('1.34'): - version = '1.34' - msg_args['volume'] = volume + if self.client.can_send_version('1.37'): + version = '1.37' + msg_args.update(volume=volume, old_reservations=old_reservations) else: - version = '1.12' + if self.client.can_send_version('1.34'): + version = '1.34' + msg_args['volume'] = volume + else: + version = '1.12' new_host = utils.extract_host(volume.host) cctxt = self.client.prepare(server=new_host, version=version)