From 29d956c0cee09465730f304cee7cc69dd137f2be Mon Sep 17 00:00:00 2001 From: LeopardMa Date: Thu, 20 Dec 2018 20:24:34 -0500 Subject: [PATCH] Rollback the quota_usages table when failed to create a incremental backup If we firstly create a incremental backup without parent backup or using unavailable backup file, like execute the command: ``cinder backup-create --name test_backup --incremental 2e3d9d0b-6b33-4cfe-a7c2-656415ee6f61``. To satisfy one of the above reasons, we will be received an error info: "Invalid backup: No backups available to do an incremental backup. (HTTP 400)". But the reserved value of the ``quota_usages`` table has been updated, so we need to roll back the ``quota_usages`` table. Otherwise, after repetitive operation for many times, create backup will be failed. Co-Authored-By: Brin Zhang Closes-Bug: #1809323 Change-Id: I1870633a5505bdb60d529fbda3147378b75e2b07 (cherry picked from commit a62fabfa65bf02ab91ff61ad2b74a6eb7509ee24) (cherry picked from commit ef07bc95471101e42d531908a00c242710834784) --- cinder/backup/api.py | 2 + cinder/tests/unit/backup/test_backup.py | 57 +++++++++++++++++++ ...3-fix-invalid-backup-4a341dc362ded88e.yaml | 6 ++ 3 files changed, 65 insertions(+) create mode 100644 releasenotes/notes/bug-1809323-fix-invalid-backup-4a341dc362ded88e.yaml diff --git a/cinder/backup/api.py b/cinder/backup/api.py index 6142d210e3f..62f5eed859f 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 8628e2c6386..dea2de43a01 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 @@ -2175,3 +2176,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.