diff --git a/cinder/backup/api.py b/cinder/backup/api.py index 07385386661..b6cc0192bde 100644 --- a/cinder/backup/api.py +++ b/cinder/backup/api.py @@ -274,6 +274,7 @@ class API(base.Base): < snapshot['created_at'])) else datetime(1, 1, 1, 1, 1, 1, tzinfo=timezone('UTC'))) else: + QUOTAS.rollback(context, reservations) msg = _('No backups available to do an incremental backup.') raise exception.InvalidBackup(reason=msg) @@ -284,6 +285,7 @@ class API(base.Base): parent = latest_backup parent_id = latest_backup.id if latest_backup['status'] != fields.BackupStatus.AVAILABLE: + QUOTAS.rollback(context, reservations) msg = _('The parent backup must be available for ' 'incremental backup.') raise exception.InvalidBackup(reason=msg) diff --git a/cinder/tests/unit/backup/test_backup.py b/cinder/tests/unit/backup/test_backup.py index 288db592b50..ffd0c335e8f 100644 --- a/cinder/tests/unit/backup/test_backup.py +++ b/cinder/tests/unit/backup/test_backup.py @@ -38,6 +38,7 @@ from cinder.objects import fields from cinder import quota from cinder import test from cinder.tests import fake_driver +from cinder.tests.unit.api.v2 import fakes as v2_fakes from cinder.tests.unit import fake_constants as fake from cinder.tests.unit import utils from cinder.volume import rpcapi as volume_rpcapi @@ -2152,3 +2153,59 @@ class BackupAPITestCase(BaseBackupTest): mock_reserve.assert_called_with( self.ctxt, backups=1, backup_gigabytes=1) mock_rollback.assert_called_with(self.ctxt, "fake_reservation") + + @mock.patch('cinder.db.backup_get_all_by_volume') + @mock.patch('cinder.backup.rpcapi.BackupAPI.create_backup') + @mock.patch.object(api.API, '_get_available_backup_service_host', + return_value='fake_host') + @mock.patch.object(quota.QUOTAS, 'rollback') + @mock.patch.object(quota.QUOTAS, 'reserve') + def test_create_backup_failed_with_empty_backup_objects( + self, mock_reserve, mock_rollback, mock_get_service, + mock_create, mock_get_backups): + mock_get_backups.return_value = [v2_fakes.fake_backup('fake-1')] + backups = objects.BackupList.get_all_by_volume(self.ctxt, + fake.VOLUME_ID) + backups.objects = [] + is_incremental = True + self.ctxt.user_id = 'fake_user' + self.ctxt.project_id = 'fake_project' + mock_reserve.return_value = 'fake_reservation' + + volume_id = self._create_volume_db_entry(status='available', + host='testhost#rbd', + size=1) + self.assertRaises(exception.InvalidBackup, + self.api.create, + self.ctxt, + None, None, + volume_id, None, + incremental=is_incremental) + mock_rollback.assert_called_with(self.ctxt, "fake_reservation") + + @mock.patch('cinder.db.backup_get_all_by_volume', + return_value=[v2_fakes.fake_backup('fake-1')]) + @mock.patch('cinder.backup.rpcapi.BackupAPI.create_backup') + @mock.patch.object(api.API, '_get_available_backup_service_host', + return_value='fake_host') + @mock.patch.object(quota.QUOTAS, 'rollback') + @mock.patch.object(quota.QUOTAS, 'reserve') + def test_create_backup_failed_with_backup_status_not_available( + self, mock_reserve, mock_rollback, mock_get_service, + mock_createi, mock_get_backups): + + is_incremental = True + self.ctxt.user_id = 'fake_user' + self.ctxt.project_id = 'fake_project' + mock_reserve.return_value = 'fake_reservation' + + volume_id = self._create_volume_db_entry(status='available', + host='testhost#rbd', + size=1) + self.assertRaises(exception.InvalidBackup, + self.api.create, + self.ctxt, + None, None, + volume_id, None, + incremental=is_incremental) + mock_rollback.assert_called_with(self.ctxt, "fake_reservation") diff --git a/releasenotes/notes/bug-1809323-fix-invalid-backup-4a341dc362ded88e.yaml b/releasenotes/notes/bug-1809323-fix-invalid-backup-4a341dc362ded88e.yaml new file mode 100644 index 00000000000..1868873add5 --- /dev/null +++ b/releasenotes/notes/bug-1809323-fix-invalid-backup-4a341dc362ded88e.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Now cinder will be rollback the ``quota_usages`` table when failed + to create an incremental backup if there doesn't exist a parent + backup or the backup is not in available state.