From 76d7a37afe386ae8ea272faed84d73a2538f72d3 Mon Sep 17 00:00:00 2001 From: Cao ShuFeng Date: Sat, 28 May 2016 05:33:41 -0400 Subject: [PATCH] check quota per_volume_gigabytes for transfer-accept Now user can accept a volume which is larger than quota "per_volume_gigabytes". With this change, an exception is returned to user if the accepting volume is larger than per_volume_gigabytes. Change-Id: Ia7809be2ec69b827b2b2a132d598f0cb3eaccff4 Closes-bug: #1586593 --- cinder/tests/unit/test_volume_transfer.py | 43 ++++++++++++++++++++++- cinder/transfer/api.py | 12 +++++-- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/cinder/tests/unit/test_volume_transfer.py b/cinder/tests/unit/test_volume_transfer.py index 39d2b3ccdd8..ccc7bfe7cfc 100644 --- a/cinder/tests/unit/test_volume_transfer.py +++ b/cinder/tests/unit/test_volume_transfer.py @@ -133,11 +133,12 @@ class VolumeTransferTestCase(test.TestCase): tx_api.accept, self.ctxt, transfer['id'], transfer['auth_key']) + @mock.patch.object(QUOTAS, "limit_check") @mock.patch.object(QUOTAS, "reserve") @mock.patch.object(QUOTAS, "add_volume_type_opts") @mock.patch('cinder.volume.utils.notify_about_volume_usage') def test_transfer_accept(self, mock_notify, mock_quota_voltype, - mock_quota_reserve): + mock_quota_reserve, mock_quota_limit): svc = self.start_service('volume', host='test_host') self.addCleanup(svc.stop) tx_api = transfer_api.API() @@ -181,6 +182,12 @@ class VolumeTransferTestCase(test.TestCase): **release_opt)] mock_quota_reserve.assert_has_calls(calls) + # QUOTAS.limit_check + values = {'per_volume_gigabytes': 1} + mock_quota_limit.assert_called_once_with(self.ctxt, + project_id=fake.PROJECT2_ID, + **values) + @mock.patch.object(QUOTAS, "reserve") @mock.patch.object(QUOTAS, "add_volume_type_opts") @mock.patch('cinder.volume.utils.notify_about_volume_usage') @@ -211,6 +218,40 @@ class VolumeTransferTestCase(test.TestCase): self.ctxt, transfer['id'], transfer['auth_key']) + # notification of transfer.accept is sent only after quota check + # passes + self.assertEqual(2, mock_notify.call_count) + + @mock.patch.object(QUOTAS, "limit_check") + @mock.patch('cinder.volume.utils.notify_about_volume_usage') + def test_transfer_accept_over_quota_check_limit(self, mock_notify, + mock_quota_limit): + svc = self.start_service('volume', host='test_host') + self.addCleanup(svc.stop) + tx_api = transfer_api.API() + volume = utils.create_volume(self.ctxt, + volume_type_id=fake.VOLUME_TYPE_ID, + updated_at=self.updated_at) + transfer = tx_api.create(self.ctxt, volume.id, 'Description') + fake_overs = ['per_volume_gigabytes'] + fake_quotas = {'per_volume_gigabytes': 1} + fake_usages = {} + + mock_quota_limit.side_effect = exception.OverQuota( + overs=fake_overs, + quotas=fake_quotas, + usages=fake_usages) + + self.ctxt.user_id = fake.USER2_ID + self.ctxt.project_id = fake.PROJECT2_ID + self.assertRaises(exception.VolumeSizeExceedsLimit, + tx_api.accept, + self.ctxt, + transfer['id'], + transfer['auth_key']) + # notification of transfer.accept is sent only after quota check + # passes + self.assertEqual(2, mock_notify.call_count) def test_transfer_get(self): tx_api = transfer_api.API() diff --git a/cinder/transfer/api.py b/cinder/transfer/api.py index 52d25ceb391..6800fbc526c 100644 --- a/cinder/transfer/api.py +++ b/cinder/transfer/api.py @@ -169,8 +169,14 @@ class API(base.Base): LOG.error(msg) raise exception.InvalidVolume(reason=msg) - volume_utils.notify_about_volume_usage(context, vol_ref, - "transfer.accept.start") + try: + values = {'per_volume_gigabytes': vol_ref.size} + QUOTAS.limit_check(context, project_id=context.project_id, + **values) + except exception.OverQuota as e: + quotas = e.kwargs['quotas'] + raise exception.VolumeSizeExceedsLimit( + size=vol_ref.size, limit=quotas['per_volume_gigabytes']) try: reserve_opts = {'volumes': 1, 'gigabytes': vol_ref.size} @@ -196,6 +202,8 @@ class API(base.Base): LOG.exception(_LE("Failed to update quota donating volume" " transfer id %s"), transfer_id) + volume_utils.notify_about_volume_usage(context, vol_ref, + "transfer.accept.start") try: # Transfer ownership of the volume now, must use an elevated # context.