From b4b1cdeb0250a25592d68f4a9d1bf7a90a9f2eea Mon Sep 17 00:00:00 2001 From: Vipin Balachandran Date: Thu, 25 Aug 2016 17:04:07 +0530 Subject: [PATCH] Fix quota rollback on retype failure Currently quota rollback is not happening if retype fails with no valid host. We are passing reservation as None and the exception message is set to the actual reservation. Hence, a cryptic message is printed and the rollback is skipped. Change-Id: I80644330d2d7bf905c4a4800d54efa6951f07496 Closes-bug: #1616875 --- cinder/scheduler/manager.py | 2 +- cinder/tests/unit/scheduler/test_scheduler.py | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/cinder/scheduler/manager.py b/cinder/scheduler/manager.py index 950b16cc2c9..f32df031bad 100644 --- a/cinder/scheduler/manager.py +++ b/cinder/scheduler/manager.py @@ -253,7 +253,7 @@ class SchedulerManager(manager.Manager): reraise = not isinstance(ex, exception.NoValidHost) with excutils.save_and_reraise_exception(reraise=reraise): _retype_volume_set_error(self, context, ex, request_spec, - volume, None, reservations) + volume, reservations) else: volume_rpcapi.VolumeAPI().retype(context, volume, new_type['id'], tgt_host, diff --git a/cinder/tests/unit/scheduler/test_scheduler.py b/cinder/tests/unit/scheduler/test_scheduler.py index b5c5cf5807d..4055a41975f 100644 --- a/cinder/tests/unit/scheduler/test_scheduler.py +++ b/cinder/tests/unit/scheduler/test_scheduler.py @@ -266,8 +266,9 @@ class SchedulerManagerTestCase(test.TestCase): @mock.patch('cinder.db.volume_update') @mock.patch('cinder.db.volume_attachment_get_all_by_volume_id') + @mock.patch('cinder.quota.QUOTAS.rollback') def test_retype_volume_exception_returns_volume_state( - self, _mock_vol_attachment_get, _mock_vol_update): + self, quota_rollback, _mock_vol_attachment_get, _mock_vol_update): # Test NoValidHost exception behavior for retype. # Puts the volume in original state and eats the exception. volume = tests_utils.create_volume(self.context, @@ -279,8 +280,10 @@ class SchedulerManagerTestCase(test.TestCase): '/dev/fake') _mock_vol_attachment_get.return_value = [volume_attach] topic = 'fake_topic' + reservations = mock.sentinel.reservations request_spec = {'volume_id': volume.id, 'volume_type': {'id': 3}, - 'migration_policy': 'on-demand'} + 'migration_policy': 'on-demand', + 'quota_reservations': reservations} _mock_vol_update.return_value = {'status': 'in-use'} _mock_find_retype_host = mock.Mock( side_effect=exception.NoValidHost(reason="")) @@ -295,6 +298,7 @@ class SchedulerManagerTestCase(test.TestCase): _mock_find_retype_host.assert_called_once_with(self.context, request_spec, {}, 'on-demand') + quota_rollback.assert_called_once_with(self.context, reservations) _mock_vol_update.assert_called_once_with(self.context, volume.id, {'status': 'in-use'}) self.manager.driver.find_retype_host = orig_retype