From dc7c4969b2864f34c39275422d2934c868968a17 Mon Sep 17 00:00:00 2001 From: Masaki Kanno Date: Wed, 26 Aug 2015 08:56:55 +0900 Subject: [PATCH] Prevent that all backup objects are deleted All backup objects of a project are deleted by this bug. The issue occurs when Swift is a backend. The below is reproduction steps of the issue. Step 1: Create a backup from a volume. The creation of the backup may fail by some kind of causes. If the case is a failure in put_container() in ChunkedBackupDriver._create_container(), backup objects of the volume are not stored in Swift because 'volumebackups' container is not created. In the case, the following information is recorded in Cinder DB. backup.status : 'error' backup.container : 'volumebackups' backup.service_metadata : None Step 2: Investigate and solve causes in which put_container() failed. Step 3: Create another backup from a volume. When the creation of the backup succeeded, 'volumebackups' container is created in Swift, then backup objects of the volume are stored into the container. Step 4: Delete the backup created in step 1. Only a record of the backup in Cinder DB should be deleted because the backup objects were not stored in Swift. However, the backup objects created in step 3 also are deleted by the bug. Usually SwiftBackupDrier.get_container_entries() lists only object names of the backup. However, when backup.service_metadata of the backup is None, SwiftBackupDriver.get_container_entries() lists all object names in 'volumebackups' container. As a result, all the backup objects in the container are deleted. This fix prevents that all the backup objects are deleted. When a backup is deleted, deletion of the backup objects by delete_object() is not executed if backup.service_metadata of the backup is None. Change-Id: I6c1c57c1397b336c12cc49159cecd17b55577f6c Closes-Bug: #1463341 --- cinder/backup/chunkeddriver.py | 5 +++-- cinder/tests/unit/test_backup_swift.py | 25 ++++++++++++++++++++++--- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/cinder/backup/chunkeddriver.py b/cinder/backup/chunkeddriver.py index 774ecefde..e9f56954e 100644 --- a/cinder/backup/chunkeddriver.py +++ b/cinder/backup/chunkeddriver.py @@ -695,13 +695,14 @@ class ChunkedBackupDriver(driver.BackupDriver): def delete(self, backup): """Delete the given backup.""" container = backup['container'] + object_prefix = backup['service_metadata'] LOG.debug('delete started, backup: %(id)s, container: %(cont)s, ' 'prefix: %(pre)s.', {'id': backup['id'], 'cont': container, - 'pre': backup['service_metadata']}) + 'pre': object_prefix}) - if container is not None: + if container is not None and object_prefix is not None: object_names = [] try: object_names = self._generate_object_names(backup) diff --git a/cinder/tests/unit/test_backup_swift.py b/cinder/tests/unit/test_backup_swift.py index c0fb78690..c5d639923 100644 --- a/cinder/tests/unit/test_backup_swift.py +++ b/cinder/tests/unit/test_backup_swift.py @@ -62,7 +62,8 @@ class BackupSwiftTestCase(test.TestCase): return db.volume_create(self.ctxt, vol)['id'] def _create_backup_db_entry(self, container='test-container', - backup_id=123, parent_id=None): + backup_id=123, parent_id=None, + service_metadata=None): backup = {'id': backup_id, 'size': 1, 'container': container, @@ -70,6 +71,7 @@ class BackupSwiftTestCase(test.TestCase): 'parent_id': parent_id, 'user_id': 'user-id', 'project_id': 'project-id', + 'service_metadata': service_metadata, } return db.backup_create(self.ctxt, backup)['id'] @@ -534,20 +536,37 @@ class BackupSwiftTestCase(test.TestCase): backup, '1234-5678-1234-8888', volume_file) def test_delete(self): - self._create_backup_db_entry() + object_prefix = 'test_prefix' + self._create_backup_db_entry(service_metadata=object_prefix) service = swift_dr.SwiftBackupDriver(self.ctxt) backup = objects.Backup.get_by_id(self.ctxt, 123) service.delete(backup) def test_delete_wraps_socket_error(self): container_name = 'socket_error_on_delete' - self._create_backup_db_entry(container=container_name) + object_prefix = 'test_prefix' + self._create_backup_db_entry(container=container_name, + service_metadata=object_prefix) service = swift_dr.SwiftBackupDriver(self.ctxt) backup = objects.Backup.get_by_id(self.ctxt, 123) self.assertRaises(exception.SwiftConnectionFailed, service.delete, backup) + def test_delete_without_object_prefix(self): + + def _fake_delete_object(self, container, object_name): + raise AssertionError('delete_object method should not be called.') + + self.stubs.Set(swift_dr.SwiftBackupDriver, + 'delete_object', + _fake_delete_object) + + self._create_backup_db_entry() + service = swift_dr.SwiftBackupDriver(self.ctxt) + backup = objects.Backup.get_by_id(self.ctxt, 123) + service.delete(backup) + def test_get_compressor(self): service = swift_dr.SwiftBackupDriver(self.ctxt) compressor = service._get_compressor('None') -- 2.45.2