diff --git a/manila/releasenotes/notes/bug-1475351-handle-successful-deletion-of-snapshot-if-quota-commit-fails-4d150bf0b71a2fd9.yaml b/manila/releasenotes/notes/bug-1475351-handle-successful-deletion-of-snapshot-if-quota-commit-fails-4d150bf0b71a2fd9.yaml new file mode 100644 index 0000000000..a23714a802 --- /dev/null +++ b/manila/releasenotes/notes/bug-1475351-handle-successful-deletion-of-snapshot-if-quota-commit-fails-4d150bf0b71a2fd9.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixed an issue during snapshot creation where a database error was being + mishandled with dead code. See `Launchpad bug 1475351 + `_ for more details. diff --git a/manila/share/api.py b/manila/share/api.py index d8325b39c6..754c4cc8f3 100644 --- a/manila/share/api.py +++ b/manila/share/api.py @@ -1359,6 +1359,7 @@ class API(base.Base): 'share_proto': share['share_proto']} try: + snapshot = None snapshot = self.db.share_snapshot_create(context, options) QUOTAS.commit( context, reservations, @@ -1366,7 +1367,9 @@ class API(base.Base): except Exception: with excutils.save_and_reraise_exception(): try: - self.db.snapshot_delete(context, share['id']) + if snapshot and snapshot['instance']: + self.db.share_snapshot_instance_delete( + context, snapshot['instance']['id']) finally: QUOTAS.rollback( context, reservations, diff --git a/manila/tests/share/test_api.py b/manila/tests/share/test_api.py index a0765554a4..c7a301111a 100644 --- a/manila/tests/share/test_api.py +++ b/manila/tests/share/test_api.py @@ -2235,6 +2235,50 @@ class ShareAPITestCase(test.TestCase): share_api.policy.check_policy.assert_called_once_with( self.context, 'share', 'create_snapshot', share) + def test_create_snapshot_fail(self): + share = fakes.fake_share( + has_replicas=False, status=constants.STATUS_AVAILABLE) + mock_db_share_snapshot_create = self.mock_object( + db_api, 'share_snapshot_create', mock.Mock( + side_effect=exception.NotFound)) + + self.mock_object(quota.QUOTAS, 'rollback') + + self.assertRaises(exception.NotFound, + self.api.create_snapshot, + self.context, share, + 'fake_name', 'fake_desc') + + self.assertTrue(mock_db_share_snapshot_create.called) + quota.QUOTAS.rollback.assert_called_once_with( + self.context, + mock.ANY, + share_type_id=share['instance']['share_type_id']) + + def test_create_snapshot_quota_commit_fail(self): + share = fakes.fake_share( + has_replicas=False, status=constants.STATUS_AVAILABLE) + snapshot = fakes.fake_snapshot( + create_instance=True, share_instance_id='id2') + self.mock_object( + quota.QUOTAS, 'commit', mock.Mock( + side_effect=exception.QuotaError('fake'))) + + self.mock_object(db_api, 'share_snapshot_create', mock.Mock( + return_value=snapshot)) + self.mock_object(db_api, 'share_snapshot_instance_delete') + self.mock_object(quota.QUOTAS, 'rollback') + + self.assertRaises(exception.QuotaError, + self.api.create_snapshot, + self.context, share, + 'fake_name', 'fake_desc') + + quota.QUOTAS.rollback.assert_called_once_with( + self.context, + mock.ANY, + share_type_id=share['instance']['share_type_id']) + @ddt.data({'use_scheduler': False, 'valid_host': 'fake', 'az': None}, {'use_scheduler': True, 'valid_host': None,