Consume quota when importing backup resource

Now cinder will validate&consume backup quotas when
importing new backup resource.

Change-Id: I1825600dc6e01499c8fc4ede05400a180672e130
Closes-Bug:#1783525

cinder/backup/api.py

@@ -485,6 +485,8 @@ class API(base.Base):
         :raises InvalidBackup:
         :raises InvalidInput:
         """
+        reservations = None
+        backup = None
         # Deserialize string backup record into a dictionary
         backup_record = objects.Backup.decode_record(backup_url)
 
@@ -493,6 +495,22 @@ class API(base.Base):
             msg = _('Provided backup record is missing an id')
             raise exception.InvalidInput(reason=msg)
 
+        # Since we use size to reserve&commit quota, size is another required
+        # field.
+        if 'size' not in backup_record:
+            msg = _('Provided backup record is missing size attribute')
+            raise exception.InvalidInput(reason=msg)
+
+        try:
+            reserve_opts = {'backups': 1,
+                            'backup_gigabytes': backup_record['size']}
+            reservations = QUOTAS.reserve(context, **reserve_opts)
+        except exception.OverQuota as e:
+            quota_utils.process_reserve_over_quota(
+                context, e,
+                resource='backups',
+                size=backup_record['size'])
+
         kwargs = {
             'user_id': context.user_id,
             'project_id': context.project_id,
@@ -504,29 +522,37 @@ class API(base.Base):
         }
 
         try:
-            # Try to get the backup with that ID in all projects even among
+            try:
-            # deleted entries.
+                # Try to get the backup with that ID in all projects even among
-            backup = objects.BackupImport.get_by_id(
+                # deleted entries.
-                context.elevated(read_deleted='yes'),
+                backup = objects.BackupImport.get_by_id(
-                backup_record['id'],
+                    context.elevated(read_deleted='yes'),
-                project_only=False)
+                    backup_record['id'],
+                    project_only=False)
 
-            # If record exists and it's not deleted we cannot proceed with the
+                # If record exists and it's not deleted we cannot proceed
-            # import
+                # with the import
-            if backup.status != fields.BackupStatus.DELETED:
+                if backup.status != fields.BackupStatus.DELETED:
-                msg = _('Backup already exists in database.')
+                    msg = _('Backup already exists in database.')
-                raise exception.InvalidBackup(reason=msg)
+                    raise exception.InvalidBackup(reason=msg)
-
-            # Otherwise we'll "revive" delete backup record
-            backup.update(kwargs)
-            backup.save()
-
-        except exception.BackupNotFound:
-            # If record doesn't exist create it with the specific ID
-            backup = objects.BackupImport(context=context,
-                                          id=backup_record['id'], **kwargs)
-            backup.create()
 
+                # Otherwise we'll "revive" delete backup record
+                backup.update(kwargs)
+                backup.save()
+                QUOTAS.commit(context, reservations)
+            except exception.BackupNotFound:
+                # If record doesn't exist create it with the specific ID
+                backup = objects.BackupImport(context=context,
+                                              id=backup_record['id'], **kwargs)
+                backup.create()
+                QUOTAS.commit(context, reservations)
+        except Exception:
+            with excutils.save_and_reraise_exception():
+                try:
+                    if backup and 'id' in backup:
+                        backup.destroy()
+                finally:
+                    QUOTAS.rollback(context, reservations)
         return backup
 
     def import_record(self, context, backup_service, backup_url):

cinder/tests/unit/api/contrib/test_backups.py

@@ -37,6 +37,7 @@ from cinder import exception
 from cinder.i18n import _
 from cinder import objects
 from cinder.objects import fields
+from cinder import quota
 from cinder import test
 from cinder.tests.unit.api import fakes
 from cinder.tests.unit import fake_constants as fake
@@ -2053,17 +2054,25 @@ class BackupsAPITestCase(test.TestCase):
         # request is not authorized
         self.assertEqual(http_client.FORBIDDEN, res.status_int)
 
+    @mock.patch.object(quota.QUOTAS, 'commit')
+    @mock.patch.object(quota.QUOTAS, 'rollback')
+    @mock.patch.object(quota.QUOTAS, 'reserve')
     @mock.patch('cinder.backup.api.API._list_backup_hosts')
     @mock.patch('cinder.backup.rpcapi.BackupAPI.import_record')
     def test_import_record_volume_id_specified_json(self,
                                                     _mock_import_record_rpc,
-                                                    _mock_list_services):
+                                                    _mock_list_services,
+                                                    mock_reserve,
+                                                    mock_rollback,
+                                                    mock_commit):
         utils.replace_obj_loader(self, objects.Backup)
+        mock_reserve.return_value = "fake_reservation"
         project_id = fake.PROJECT_ID
         backup_service = 'fake'
         ctx = context.RequestContext(fake.USER_ID, project_id, is_admin=True)
         backup = objects.Backup(ctx, id=fake.BACKUP_ID, user_id=fake.USER_ID,
                                 project_id=project_id,
+                                size=1,
                                 status=fields.BackupStatus.AVAILABLE)
         backup_url = backup.encode_record()
         _mock_import_record_rpc.return_value = None
@@ -2091,19 +2100,31 @@ class BackupsAPITestCase(test.TestCase):
         self.assertEqual(ctx.user_id, db_backup.user_id)
         self.assertEqual(backup_api.IMPORT_VOLUME_ID, db_backup.volume_id)
         self.assertEqual(fields.BackupStatus.CREATING, db_backup.status)
+        mock_reserve.assert_called_with(
+            ctx, backups=1, backup_gigabytes=1)
+        mock_commit.assert_called_with(ctx, "fake_reservation")
 
+    @mock.patch.object(quota.QUOTAS, 'commit')
+    @mock.patch.object(quota.QUOTAS, 'rollback')
+    @mock.patch.object(quota.QUOTAS, 'reserve')
     @mock.patch('cinder.backup.api.API._list_backup_hosts')
     @mock.patch('cinder.backup.rpcapi.BackupAPI.import_record')
     def test_import_record_volume_id_exists_deleted(self,
                                                     _mock_import_record_rpc,
-                                                    _mock_list_services):
+                                                    _mock_list_services,
+                                                    mock_reserve,
+                                                    mock_rollback,
+                                                    mock_commit,
+                                                    ):
         ctx = context.RequestContext(fake.USER_ID, fake.PROJECT_ID,
                                      is_admin=True)
+        mock_reserve.return_value = 'fake_reservation'
         utils.replace_obj_loader(self, objects.Backup)
 
         # Original backup belonged to a different user_id and project_id
         backup = objects.Backup(ctx, id=fake.BACKUP_ID, user_id=fake.USER2_ID,
                                 project_id=fake.PROJECT2_ID,
+                                size=1,
                                 status=fields.BackupStatus.AVAILABLE)
         backup_url = backup.encode_record()
 
@@ -2136,6 +2157,8 @@ class BackupsAPITestCase(test.TestCase):
         self.assertEqual(ctx.user_id, db_backup.user_id)
         self.assertEqual(backup_api.IMPORT_VOLUME_ID, db_backup.volume_id)
         self.assertEqual(fields.BackupStatus.CREATING, db_backup.status)
+        mock_reserve.assert_called_with(ctx, backups=1, backup_gigabytes=1)
+        mock_commit.assert_called_with(ctx, "fake_reservation")
 
         backup_del.destroy()
 
@@ -2188,12 +2211,19 @@ class BackupsAPITestCase(test.TestCase):
         self.assertEqual("Invalid input received: Can't parse backup record.",
                          res_dict['badRequest']['message'])
 
+    @mock.patch.object(quota.QUOTAS, 'commit')
+    @mock.patch.object(quota.QUOTAS, 'rollback')
+    @mock.patch.object(quota.QUOTAS, 'reserve')
     @mock.patch('cinder.backup.api.API._list_backup_hosts')
     def test_import_backup_with_existing_backup_record(self,
-                                                       _mock_list_services):
+                                                       _mock_list_services,
+                                                       mock_reserve,
+                                                       mock_rollback,
+                                                       mock_commit):
         ctx = context.RequestContext(fake.USER_ID, fake.PROJECT_ID,
                                      is_admin=True)
-        backup = utils.create_backup(self.context, fake.VOLUME_ID)
+        mock_reserve.return_value = "fake_reservation"
+        backup = utils.create_backup(self.context, fake.VOLUME_ID, size=1)
         backup_service = 'fake'
         backup_url = backup.encode_record()
         _mock_list_services.return_value = ['no-match1', 'no-match2']
@@ -2212,12 +2242,20 @@ class BackupsAPITestCase(test.TestCase):
                          res_dict['badRequest']['code'])
         self.assertEqual('Invalid backup: Backup already exists in database.',
                          res_dict['badRequest']['message'])
-
+        mock_reserve.assert_called_with(
+            ctx, backups=1, backup_gigabytes=1)
+        mock_rollback.assert_called_with(ctx, "fake_reservation")
         backup.destroy()
 
+    @mock.patch.object(quota.QUOTAS, 'commit')
+    @mock.patch.object(quota.QUOTAS, 'rollback')
+    @mock.patch.object(quota.QUOTAS, 'reserve')
     @mock.patch('cinder.backup.api.API._list_backup_hosts')
     @mock.patch('cinder.backup.rpcapi.BackupAPI.import_record')
     def test_import_backup_with_missing_backup_services(self,
+                                                        mock_reserve,
+                                                        mock_rollback,
+                                                        mock_commit,
                                                         _mock_import_record,
                                                         _mock_list_services):
         ctx = context.RequestContext(fake.USER_ID, fake.PROJECT_ID,

cinder/tests/unit/backup/test_backup.py

@@ -35,6 +35,7 @@ from cinder import db
 from cinder import exception
 from cinder import objects
 from cinder.objects import fields
+from cinder import quota
 from cinder import test
 from cinder.tests import fake_driver
 from cinder.tests.unit import fake_constants as fake
@@ -2128,3 +2129,26 @@ class BackupAPITestCase(BaseBackupTest):
         self.api.restore(self.ctxt, backup.id, volume_id)
         backup = objects.Backup.get_by_id(self.ctxt, backup.id)
         self.assertEqual(volume_id, backup.restore_volume_id)
+
+    @mock.patch.object(objects.Backup, 'decode_record')
+    @mock.patch.object(quota.QUOTAS, 'commit')
+    @mock.patch.object(quota.QUOTAS, 'rollback')
+    @mock.patch.object(quota.QUOTAS, 'reserve')
+    def test__get_import_backup_invalid_backup(
+            self, mock_reserve, mock_rollback, mock_commit, mock_decode):
+
+        backup = self._create_backup_db_entry(size=1,
+                                              status='available')
+        mock_decode.return_value = {'id': backup.id,
+                                    'project_id': backup.project_id,
+                                    'user_id': backup.user_id,
+                                    'volume_id': backup.volume_id,
+                                    'size': 1}
+        mock_reserve.return_value = 'fake_reservation'
+
+        self.assertRaises(exception.InvalidBackup,
+                          self.api._get_import_backup,
+                          self.ctxt, 'fake_backup_url')
+        mock_reserve.assert_called_with(
+            self.ctxt, backups=1, backup_gigabytes=1)
+        mock_rollback.assert_called_with(self.ctxt, "fake_reservation")

releasenotes/notes/fix-import-backup-quota-issue-8yh69hd19u7tuu23.yaml

@@ -0,0 +1,3 @@
+---
+fixes:
+  - Cinder will now consume quota when importing new backup resource.