Merge "Address pylint error"

cinder/backup/api.py

@@ -327,24 +327,28 @@ class API(base.Base):
                                   {'status': 'backing-up',
                                    'previous_status': previous_status})
 
-        backup = None
+        kwargs = {
+            'user_id': context.user_id,
+            'project_id': context.project_id,
+            'display_name': name,
+            'display_description': description,
+            'volume_id': volume_id,
+            'status': fields.BackupStatus.CREATING,
+            'container': container,
+            'parent_id': parent_id,
+            'size': volume['size'],
+            'snapshot_id': snapshot_id,
+            'data_timestamp': data_timestamp,
+            'parent': parent,
+            'metadata': metadata or {}
+        }
         try:
-            kwargs = {
-                'user_id': context.user_id,
-                'project_id': context.project_id,
-                'display_name': name,
-                'display_description': description,
-                'volume_id': volume_id,
-                'status': fields.BackupStatus.CREATING,
-                'container': container,
-                'parent_id': parent_id,
-                'size': volume['size'],
-                'snapshot_id': snapshot_id,
-                'data_timestamp': data_timestamp,
-                'parent': parent,
-                'metadata': metadata or {}
-            }
             backup = objects.Backup(context=context, **kwargs)
+        except Exception:
+            with excutils.save_and_reraise_exception():
+                QUOTAS.rollback(context, reservations)
+
+        try:
             backup.create()
             if not snapshot_id:
                 backup.data_timestamp = backup.created_at
@@ -353,7 +357,7 @@ class API(base.Base):
         except Exception:
             with excutils.save_and_reraise_exception():
                 try:
-                    if backup and 'id' in backup:
+                    if 'id' in backup:
                         backup.destroy()
                 finally:
                     QUOTAS.rollback(context, reservations)

cinder/tests/unit/backup/test_backup.py

@@ -1988,10 +1988,13 @@ class BackupAPITestCase(BaseBackupTest):
 
     @mock.patch.object(api.API, '_get_available_backup_service_host',
                        return_value='fake_host')
+    @mock.patch.object(quota.QUOTAS, 'commit')
+    @mock.patch.object(quota.QUOTAS, 'rollback')
+    @mock.patch.object(quota.QUOTAS, 'reserve')
     @mock.patch.object(db, 'backup_create',
                        side_effect=db_exc.DBError())
     def test_create_when_failed_to_create_backup_object(
-            self, mock_create,
+            self, mock_create, mock_reserve, mock_rollback, mock_commit,
             mock_get_service):
 
         # Create volume in admin context
@@ -2002,6 +2005,9 @@ class BackupAPITestCase(BaseBackupTest):
         new_context.user_id = fake.USER3_ID
         new_context.project_id = fake.USER3_ID
 
+        # name the reservation so we can check it later
+        mock_reserve.return_value = 'fake-reservation'
+
         # The opposite side of this test case is a "NotImplementedError:
         # Cannot load 'id' in the base class" being raised.
         # More detailed, in the try clause, if backup.create() failed
@@ -2015,15 +2021,27 @@ class BackupAPITestCase(BaseBackupTest):
                           volume_id=volume_id,
                           container='volumebackups')
 
+        # make sure quotas are behaving as expected when backup.create() fails
+        mock_reserve.assert_called_once()
+        mock_rollback.assert_called_with(new_context, 'fake-reservation')
+        mock_commit.assert_not_called()
+
     @mock.patch.object(api.API, '_get_available_backup_service_host',
                        return_value='fake_host')
+    @mock.patch.object(quota.QUOTAS, 'commit')
+    @mock.patch.object(quota.QUOTAS, 'rollback')
+    @mock.patch.object(quota.QUOTAS, 'reserve')
     @mock.patch.object(objects.Backup, '__init__',
                        side_effect=exception.InvalidInput(
                            reason='Failed to new'))
-    def test_create_when_failed_to_new_backup_object(self, mock_new,
-                                                     mock_get_service):
+    def test_create_when_failed_to_new_backup_object(
+            self, mock_new, mock_reserve, mock_rollback, mock_commit,
+            mock_get_service):
         volume_id = utils.create_volume(self.ctxt)['id']
 
+        # name the reservation so we can check it later
+        mock_reserve.return_value = 'fake-reservation'
+
         # The opposite side of this test case is that a "UnboundLocalError:
         # local variable 'backup' referenced before assignment" is raised.
         # More detailed, in the try clause, backup = objects.Backup(...)
@@ -2036,6 +2054,11 @@ class BackupAPITestCase(BaseBackupTest):
                           volume_id=volume_id,
                           container='volumebackups')
 
+        # make sure quotas are behaving as expected when objects.Backup() fails
+        mock_reserve.assert_called_once()
+        mock_rollback.assert_called_with(self.ctxt, 'fake-reservation')
+        mock_commit.assert_not_called()
+
     @mock.patch.object(api.API, '_get_available_backup_service_host',
                        return_value='fake_host')
     @mock.patch('cinder.backup.rpcapi.BackupAPI.create_backup')