]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Prevent that all backup objects are deleted
authorMasaki Kanno <kanno.masaki@jp.fujitsu.com>
Tue, 25 Aug 2015 23:56:55 +0000 (08:56 +0900)
committerMasaki Kanno <kanno.masaki@jp.fujitsu.com>
Tue, 25 Aug 2015 23:56:55 +0000 (08:56 +0900)
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
cinder/tests/unit/test_backup_swift.py

index 774ecefde7385caebdb0faa95e732371f3a079a4..e9f56954e7af0dc8e343ab227d679a335f8aa9c4 100644 (file)
@@ -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)
index c0fb78690dfe35b899b6b070fb12ff966ce6a0b8..c5d639923a8305d35c468c6420bf457413529221 100644 (file)
@@ -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')