diff --git a/cinder/backup/drivers/ceph.py b/cinder/backup/drivers/ceph.py index 999f932d2ec..8769d684630 100644 --- a/cinder/backup/drivers/ceph.py +++ b/cinder/backup/drivers/ceph.py @@ -647,12 +647,6 @@ class CephBackupDriver(driver.BackupDriver): source_rbd_image = volume_file.rbd_image volume_id = backup.volume_id updates = {} - # Identify our --from-snap point (if one exists) - from_snap = self._get_most_recent_snap(source_rbd_image) - LOG.debug("Using --from-snap '%(snap)s' for incremental backup of " - "volume %(volume)s.", - {'snap': from_snap, 'volume': volume_id}) - base_name = self._get_backup_base_name(volume_id, diff_format=True) image_created = False with rbd_driver.RADOSClient(self, backup.container) as client: @@ -663,30 +657,34 @@ class CephBackupDriver(driver.BackupDriver): # TODO(dosaboy): find a way to repair the broken backup # if base_name not in self.rbd.RBD().list(ioctx=client.ioctx): - # If a from_snap is defined but the base does not exist, we - # ignore it since it is stale and waiting to be cleaned up. - if from_snap: - LOG.debug("Source snapshot '%(snapshot)s' of volume " - "%(volume)s is stale so deleting.", - {'snapshot': from_snap, 'volume': volume_id}) + src_vol_snapshots = self.get_backup_snaps(source_rbd_image) + if src_vol_snapshots: + # If there are source volume snapshots but base does not + # exist then we delete it and set from_snap to None + LOG.debug("Volume '%(volume)s' has stale source " + "snapshots so deleting them.", + {'volume': volume_id}) + for snap in src_vol_snapshots: + from_snap = snap['name'] source_rbd_image.remove_snap(from_snap) - from_snap = None + from_snap = None # Create new base image self._create_base_image(base_name, length, client) image_created = True else: - # If a from_snap is defined but does not exist in the back base - # then we cannot proceed (see above) - if not self._snap_exists(base_name, from_snap, client): - errmsg = (_("Snapshot='%(snap)s' does not exist in base " - "image='%(base)s' - aborting incremental " - "backup") % - {'snap': from_snap, 'base': base_name}) - LOG.info(errmsg) - # Raise this exception so that caller can try another - # approach - raise exception.BackupRBDOperationFailed(errmsg) + # If a from_snap is defined and is present in the source volume + # image but does not exist in the backup base then we look down + # the list of source volume snapshots and find the latest one + # for which a backup snapshot exist in the backup base. Until + # that snapshot is reached, we delete all the other snapshots + # for which backup snapshot does not exist. + from_snap = self._get_most_recent_snap(source_rbd_image, + base_name, client) + + LOG.debug("Using --from-snap '%(snap)s' for incremental backup of " + "volume %(volume)s.", + {'snap': from_snap, 'volume': volume_id}) # Snapshot source volume so that we have a new point-in-time new_snap = self._get_new_snap_name(backup.id) @@ -713,11 +711,6 @@ class CephBackupDriver(driver.BackupDriver): LOG.debug("Differential backup transfer completed in %.4fs", (time.time() - before)) - # We don't need the previous snapshot (if there was one) anymore so - # delete it. - if from_snap: - source_rbd_image.remove_snap(from_snap) - except exception.BackupRBDOperationFailed: with excutils.save_and_reraise_exception(): LOG.debug("Differential backup transfer failed") @@ -851,17 +844,22 @@ class CephBackupDriver(driver.BackupDriver): LOG.debug("Found snapshot '%s'", snaps[0]) return snaps[0] - def _get_most_recent_snap(self, rbd_image): + def _get_most_recent_snap(self, rbd_image, base_name, client): """Get the most recent backup snapshot of the provided image. Returns name of most recent backup snapshot or None if there are no backup snapshots. """ - backup_snaps = self.get_backup_snaps(rbd_image, sort=True) - if not backup_snaps: - return None + src_vol_backup_snaps = self.get_backup_snaps(rbd_image, sort=True) + from_snap = None - return backup_snaps[0]['name'] + for snap in src_vol_backup_snaps: + if self._snap_exists(base_name, snap['name'], client): + from_snap = snap['name'] + break + rbd_image.remove_snap(snap['name']) + + return from_snap def _get_volume_size_gb(self, volume): """Return the size in gigabytes of the given volume. diff --git a/cinder/tests/unit/backup/drivers/test_backup_ceph.py b/cinder/tests/unit/backup/drivers/test_backup_ceph.py index 5096b86baf4..a1089a8116c 100644 --- a/cinder/tests/unit/backup/drivers/test_backup_ceph.py +++ b/cinder/tests/unit/backup/drivers/test_backup_ceph.py @@ -259,13 +259,17 @@ class BackupCephTestCase(test.TestCase): last = 'backup.%s.snap.9824923.1212' % (uuid.uuid4()) image = self.mock_rbd.Image.return_value - image.list_snaps.return_value = \ - [{'name': 'backup.%s.snap.6423868.2342' % (uuid.uuid4())}, - {'name': 'backup.%s.snap.1321319.3235' % (uuid.uuid4())}, - {'name': last}, - {'name': 'backup.%s.snap.3824923.1412' % (uuid.uuid4())}] - - snap = self.service._get_most_recent_snap(image) + with mock.patch.object(self.service, '_snap_exists') as \ + mock_snap_exists: + mock_snap_exists.return_value = True + image.list_snaps.return_value = \ + [{'name': 'backup.%s.snap.6423868.2342' % (uuid.uuid4())}, + {'name': 'backup.%s.snap.1321319.3235' % (uuid.uuid4())}, + {'name': last}, + {'name': 'backup.%s.snap.3824923.1412' % (uuid.uuid4())}] + base_name = "mock_base" + client = mock.Mock() + snap = self.service._get_most_recent_snap(image, base_name, client) self.assertEqual(last, snap) @common_mocks @@ -470,8 +474,14 @@ class BackupCephTestCase(test.TestCase): 'user_foo', 'conf_foo') rbdio = linuxrbd.RBDVolumeIOWrapper(meta) + mock_get_backup_snaps.return_value = ( + [{'name': 'backup.mock.snap.153464362.12'}, + {'name': 'backup.mock.snap.15341241.90'}, + {'name': 'backup.mock.snap.199994362.10'}]) + output = self.service.backup(self.backup, rbdio) self.assertDictEqual({}, output) + self.assertEqual(['popen_init', 'read', 'popen_init', @@ -527,14 +537,19 @@ class BackupCephTestCase(test.TestCase): mock.patch.object(self.service, '_try_delete_base_image'): with mock.patch.object(self.service, '_backup_metadata'): - image = self.service.rbd.Image() - meta = linuxrbd.RBDImageMetadata(image, - 'pool_foo', - 'user_foo', - 'conf_foo') - rbdio = linuxrbd.RBDVolumeIOWrapper(meta) - output = self.service.backup(self.backup, rbdio) - self.assertIsNone(output['parent_id']) + with mock.patch.object(self.service, 'get_backup_snaps') as \ + mock_get_backup_snaps: + image = self.service.rbd.Image() + meta = linuxrbd.RBDImageMetadata(image, + 'pool_foo', + 'user_foo', + 'conf_foo') + rbdio = linuxrbd.RBDVolumeIOWrapper(meta) + mock_get_backup_snaps.return_value = ( + [{'name': 'backup.mock.snap.153464362.12'}, + {'name': 'backup.mock.snap.199994362.10'}]) + output = self.service.backup(self.backup, rbdio) + self.assertIsNone(output['parent_id']) @common_mocks def test_backup_rbd_set_parent_id(self): @@ -597,47 +612,50 @@ class BackupCephTestCase(test.TestCase): self.mock_rbd.RBD.list = mock.Mock() self.mock_rbd.RBD.list.return_value = [backup_name] - with mock.patch.object(self.service, 'get_backup_snaps'), \ - mock.patch.object(self.service, '_rbd_diff_transfer') as \ - mock_rbd_diff_transfer: - def mock_rbd_diff_transfer_side_effect(src_name, src_pool, - dest_name, dest_pool, - src_user, src_conf, - dest_user, dest_conf, - src_snap, from_snap): - raise exception.BackupRBDOperationFailed(_('mock')) + with mock.patch.object(self.service, 'get_backup_snaps') as \ + mock_get_backup_snaps: + mock_get_backup_snaps.return_value = ( + [{'name': 'backup.mock.snap.153464362.12'}, + {'name': 'backup.mock.snap.199994362.10'}]) + with mock.patch.object(self.service, '_rbd_diff_transfer') as \ + mock_rbd_diff_transfer: + def mock_rbd_diff_transfer_side_effect(src_name, src_pool, + dest_name, dest_pool, + src_user, src_conf, + dest_user, dest_conf, + src_snap, from_snap): + raise exception.BackupRBDOperationFailed(_('mock')) - # Raise a pseudo exception.BackupRBDOperationFailed. - mock_rbd_diff_transfer.side_effect \ - = mock_rbd_diff_transfer_side_effect + # Raise a pseudo exception.BackupRBDOperationFailed. + mock_rbd_diff_transfer.side_effect \ + = mock_rbd_diff_transfer_side_effect - with mock.patch.object(self.service, '_full_backup'), \ - mock.patch.object(self.service, - '_try_delete_base_image') as \ - mock_try_delete_base_image: - def mock_try_delete_base_image_side_effect(backup_id, - base_name): - raise self.service.rbd.ImageNotFound(_('mock')) + with mock.patch.object(self.service, '_full_backup'), \ + mock.patch.object(self.service, '_try_delete_base_image') as \ + mock_try_delete_base_image: + def mock_try_delete_base_image_side_effect(backup_id, + base_name): + raise self.service.rbd.ImageNotFound(_('mock')) - # Raise a pesudo exception rbd.ImageNotFound. - mock_try_delete_base_image.side_effect \ - = mock_try_delete_base_image_side_effect - with mock.patch.object(self.service, '_backup_metadata'): - with tempfile.NamedTemporaryFile() as test_file: - checksum = hashlib.sha256() - image = self.service.rbd.Image() - meta = linuxrbd.RBDImageMetadata(image, - 'pool_foo', - 'user_foo', - 'conf_foo') - rbdio = linuxrbd.RBDVolumeIOWrapper(meta) + # Raise a pesudo exception rbd.ImageNotFound. + mock_try_delete_base_image.side_effect \ + = mock_try_delete_base_image_side_effect + with mock.patch.object(self.service, '_backup_metadata'): + with tempfile.NamedTemporaryFile() as test_file: + checksum = hashlib.sha256() + image = self.service.rbd.Image() + meta = linuxrbd.RBDImageMetadata(image, + 'pool_foo', + 'user_foo', + 'conf_foo') + rbdio = linuxrbd.RBDVolumeIOWrapper(meta) - # We expect that the second exception is - # notified. - self.assertRaises( - self.service.rbd.ImageNotFound, - self.service.backup, - self.backup, rbdio) + # We expect that the second exception is + # notified. + self.assertRaises( + self.service.rbd.ImageNotFound, + self.service.backup, + self.backup, rbdio) @common_mocks @mock.patch('fcntl.fcntl', spec=True) @@ -671,39 +689,126 @@ class BackupCephTestCase(test.TestCase): self.mock_rbd.RBD.list = mock.Mock() self.mock_rbd.RBD.list.return_value = [backup_name] - with mock.patch.object(self.service, 'get_backup_snaps'), \ - mock.patch.object(self.service, '_rbd_diff_transfer'), \ + with mock.patch.object(self.service, 'get_backup_snaps') as \ + mock_get_backup_snaps: + mock_get_backup_snaps.return_value = ( + [{'name': 'backup.mock.snap.153464362.12'}, + {'name': 'backup.mock.snap.199994362.10'}]) + with mock.patch.object(self.service, '_rbd_diff_transfer'), \ mock.patch.object(self.service, '_full_backup'), \ mock.patch.object(self.service, '_backup_metadata') as \ - mock_backup_metadata: + mock_backup_metadata: - def mock_backup_metadata_side_effect(backup): - raise exception.BackupOperationError(_('mock')) + def mock_backup_metadata_side_effect(backup): + raise exception.BackupOperationError(_('mock')) - # Raise a pseudo exception.BackupOperationError. - mock_backup_metadata.side_effect = mock_backup_metadata_side_effect - with mock.patch.object(self.service, 'delete_backup') as \ - mock_delete: - def mock_delete_side_effect(backup): - raise self.service.rbd.ImageBusy() + # Raise a pseudo exception.BackupOperationError. + mock_backup_metadata.side_effect = ( + mock_backup_metadata_side_effect) + with mock.patch.object(self.service, 'delete_backup') as \ + mock_delete: + def mock_delete_side_effect(backup): + raise self.service.rbd.ImageBusy() - # Raise a pseudo exception rbd.ImageBusy. - mock_delete.side_effect = mock_delete_side_effect - with tempfile.NamedTemporaryFile() as test_file: - checksum = hashlib.sha256() - image = self.service.rbd.Image() - meta = linuxrbd.RBDImageMetadata(image, - 'pool_foo', - 'user_foo', - 'conf_foo') - rbdio = linuxrbd.RBDVolumeIOWrapper(meta) + # Raise a pseudo exception rbd.ImageBusy. + mock_delete.side_effect = mock_delete_side_effect + with tempfile.NamedTemporaryFile() as test_file: + checksum = hashlib.sha256() + image = self.service.rbd.Image() + meta = linuxrbd.RBDImageMetadata(image, + 'pool_foo', + 'user_foo', + 'conf_foo') + rbdio = linuxrbd.RBDVolumeIOWrapper(meta) - # We expect that the second exception is - # notified. - self.assertRaises( - self.service.rbd.ImageBusy, - self.service.backup, - self.backup, rbdio) + # We expect that the second exception is + # notified. + self.assertRaises( + self.service.rbd.ImageBusy, + self.service.backup, + self.backup, rbdio) + + @common_mocks + def test_backup_rbd_from_snap(self): + backup_name = self.service._get_backup_base_name(self.backup_id, + diff_format=True) + vol_name = self.volume['name'] + vol_length = self.service._get_volume_size_gb(self.volume) + + self.mock_rbd.RBD().list = mock.Mock() + self.mock_rbd.RBD().list.return_value = ['mock'] + + with mock.patch.object(self.service, '_get_new_snap_name') as \ + mock_get_new_snap_name: + with mock.patch.object(self.service, 'get_backup_snaps') as \ + mock_get_backup_snaps: + with mock.patch.object(self.service, '_rbd_diff_transfer') as \ + mock_rbd_diff_transfer: + with mock.patch.object(self.service, '_get_backup_base_name') as \ + mock_get_backup_base_name: + mock_get_backup_base_name.return_value = ( + backup_name) + mock_get_backup_snaps.return_value = ( + [{'name': 'backup.mock.snap.153464362.12'}, + {'name': 'backup.mock.snap.15341241.90'}, + {'name': 'backup.mock.snap.199994362.10'}]) + mock_get_new_snap_name.return_value = 'new_snap' + image = self.service.rbd.Image() + meta = linuxrbd.RBDImageMetadata(image, + 'pool_foo', + 'user_foo', + 'conf_foo') + rbdio = linuxrbd.RBDVolumeIOWrapper(meta) + rbdio.seek(0) + self.service._backup_rbd(self.backup, rbdio, + vol_name, vol_length) + mock_rbd_diff_transfer.assert_called_with( + vol_name, 'pool_foo', backup_name, + self.backup.container, src_user='user_foo', + src_conf='conf_foo', + dest_conf='/etc/ceph/ceph.conf', + dest_user='cinder', src_snap='new_snap', + from_snap=None) + + @common_mocks + def test_backup_rbd_from_snap2(self): + backup_name = self.service._get_backup_base_name(self.backup_id, + diff_format=True) + vol_name = self.volume['name'] + vol_length = self.service._get_volume_size_gb(self.volume) + + self.mock_rbd.RBD().list = mock.Mock() + self.mock_rbd.RBD().list.return_value = [backup_name] + + with mock.patch.object(self.service, '_get_most_recent_snap') as \ + mock_get_most_recent_snap: + with mock.patch.object(self.service, '_get_backup_base_name') as \ + mock_get_backup_base_name: + with mock.patch.object(self.service, '_rbd_diff_transfer') as \ + mock_rbd_diff_transfer: + with mock.patch.object(self.service, '_get_new_snap_name') as \ + mock_get_new_snap_name: + mock_get_backup_base_name.return_value = ( + backup_name) + mock_get_most_recent_snap.return_value = ( + 'backup.mock.snap.153464362.12') + mock_get_new_snap_name.return_value = 'new_snap' + image = self.service.rbd.Image() + meta = linuxrbd.RBDImageMetadata(image, + 'pool_foo', + 'user_foo', + 'conf_foo') + rbdio = linuxrbd.RBDVolumeIOWrapper(meta) + rbdio.seek(0) + self.service._backup_rbd(self.backup, rbdio, + vol_name, vol_length) + mock_rbd_diff_transfer.assert_called_with( + vol_name, 'pool_foo', backup_name, + self.backup.container, src_user='user_foo', + src_conf='conf_foo', + dest_conf='/etc/ceph/ceph.conf', + dest_user='cinder', src_snap='new_snap', + from_snap='backup.mock.snap.153464362.12') @common_mocks def test_backup_vol_length_0(self):