diff --git a/cinder/backup/drivers/ceph.py b/cinder/backup/drivers/ceph.py index f00d27abf1b..ab79c899296 100644 --- a/cinder/backup/drivers/ceph.py +++ b/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 diff --git a/cinder/tests/unit/backup/drivers/test_backup_ceph.py b/cinder/tests/unit/backup/drivers/test_backup_ceph.py index c5b163b86bd..3ebc0d482c3 100644 --- a/cinder/tests/unit/backup/drivers/test_backup_ceph.py +++ b/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) diff --git a/releasenotes/notes/ceph-driver-full-backups-by-default-c0677d4b6358cf14.yaml b/releasenotes/notes/ceph-driver-full-backups-by-default-c0677d4b6358cf14.yaml new file mode 100644 index 00000000000..7e760ca6ace --- /dev/null +++ b/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.