Ceph driver should respect the `--incremental` option for backups

Volume backup service with Ceph driver should respect the `--incremental`
option for backups. In this way we can keep consistency between backups.

If the incremental flag is not specified then a new
full backup will always be created, allowing for subsequent deletion of
older incremental backups.

Change-Id: I47f936e538810fcea8bffea0cf5c6065f6a3aa98
Closes-Bug: #1790713
Signed-off-by: Laura Sofia Enriquez <lsofia.enriquez@gmail.com>

cinder/backup/drivers/ceph.py

@@ -169,9 +169,9 @@ class CephBackupDriver(driver.BackupDriver):
     Backups may be stored in their own pool or even cluster. Store location is
     defined by the Ceph conf file and service config options supplied.
 
-    If the source volume is itself an RBD volume, the backup will be performed
-    using incremental differential backups which *should* give a performance
-    gain.
+    Only if the incremental option is specified and the source volume is itself
+    an RBD volume, the backup will be performed using incremental differential
+    backups which *should* give a performance gain.
     """
 
     def __init__(self, context, db=None, execute=None):
@@ -910,8 +910,9 @@ class CephBackupDriver(driver.BackupDriver):
     def backup(self, backup, volume_file, backup_metadata=True):
         """Backup volume and metadata (if available) to Ceph object store.
 
-        If the source volume is an RBD we will attempt to do an
-        incremental/differential backup, otherwise a full copy is performed.
+        If the source volume is an RBD and we use the `incremental`
+        option we will attempt to do an incremental/differential backup,
+        otherwise a full copy is performed.
         If this fails we will attempt to fall back to full copy.
         """
         volume = self.db.volume_get(self.context, backup.volume_id)
@@ -926,13 +927,13 @@ class CephBackupDriver(driver.BackupDriver):
         volume_file.seek(0)
         length = self._get_volume_size_gb(volume)
 
-        do_full_backup = False
-        if self._file_is_rbd(volume_file):
+        if backup.parent_id and self._file_is_rbd(volume_file):
             # If volume an RBD, attempt incremental backup.
             LOG.debug("Volume file is RBD: attempting incremental backup.")
             try:
                 updates = self._backup_rbd(backup, volume_file,
                                            volume.name, length)
+                do_full_backup = False
             except exception.BackupRBDOperationFailed:
                 LOG.debug("Forcing full backup of volume %s.", volume.id)
                 do_full_backup = True

cinder/tests/unit/backup/drivers/test_backup_ceph.py

@@ -483,8 +483,15 @@ class BackupCephTestCase(test.TestCase):
                                  {'name': 'backup.mock.snap.15341241.90'},
                                  {'name': 'backup.mock.snap.199994362.10'}])
 
+                            self.backup.parent_id = fake.UUID1
                             output = self.service.backup(self.backup, rbdio)
-                            self.assertDictEqual({}, output)
+
+                            # Confirm that the backup process is not able to
+                            # find the parent backup for incremental (creates
+                            # a full backup) and requests the DB to be changed
+                            # to reflect that there's no parent (is full
+                            # backup).
+                            self.assertIsNone(output['parent_id'])
 
                             self.assertEqual(['popen_init',
                                               'read',
@@ -502,10 +509,16 @@ class BackupCephTestCase(test.TestCase):
 
     @common_mocks
     def test_backup_volume_from_rbd_set_parent_id(self):
+        """Test volume from rbd with parent id.
+
+        If the backup has parent_id an incremental backup should
+        be performance.
+        """
         with mock.patch.object(self.service, '_backup_rbd') as \
                 mock_backup_rbd, mock.patch.object(self.service,
                                                    '_backup_metadata'):
-            mock_backup_rbd.return_value = {'parent_id': 'mock'}
+            self.backup.parent_id = fake.UUID1
+            mock_backup_rbd.return_value = {'parent_id': fake.UUID1}
             image = self.service.rbd.Image()
             meta = linuxrbd.RBDImageMetadata(image,
                                              'pool_foo',
@@ -513,7 +526,7 @@ class BackupCephTestCase(test.TestCase):
                                              'conf_foo')
             rbdio = linuxrbd.RBDVolumeIOWrapper(meta)
             output = self.service.backup(self.backup, rbdio)
-            self.assertDictEqual({'parent_id': 'mock'}, output)
+            self.assertDictEqual({'parent_id': fake.UUID1}, output)
 
     @common_mocks
     def test_backup_volume_from_rbd_set_parent_id_none(self):
@@ -596,6 +609,7 @@ class BackupCephTestCase(test.TestCase):
         """
         backup_name = self.service._get_backup_base_name(self.backup_id,
                                                          diff_format=True)
+        self.backup.parent_id = fake.UUID1
 
         def mock_write_data():
             self.volume_file.seek(0)

releasenotes/notes/ceph-driver-full-backups-by-default-c0677d4b6358cf14.yaml

@@ -0,0 +1,10 @@
+---
+upgrade:
+  - Default behavior of ceph backups upgraded (no longer incremental by
+    default) to keep consistency between drivers.
+fixes:
+  - Volume backup service with Ceph driver will now respect the `--incremental`
+    option for backups (this new behavior will be only available for Ceph
+    volumes, we still don't support incremental for non Ceph volumes).
+    If the incremental flag is not specified then a new full backup will
+    always be created.