diff --git a/cinder/backup/drivers/ceph.py b/cinder/backup/drivers/ceph.py index 8a839a26d05..999f932d2ec 100644 --- a/cinder/backup/drivers/ceph.py +++ b/cinder/backup/drivers/ceph.py @@ -646,6 +646,7 @@ class CephBackupDriver(driver.BackupDriver): rbd_conf = volume_file.rbd_conf 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 " @@ -730,6 +731,14 @@ class CephBackupDriver(driver.BackupDriver): "source volume='%(volume)s'.", {'snapshot': new_snap, 'volume': volume_id}) source_rbd_image.remove_snap(new_snap) + # We update the parent_id here. The from_snap is of the format: + # backup.BACKUP_ID.snap.TIMESTAMP. So we need to extract the + # backup_id of the parent only from from_snap and set it as + # parent_id + if from_snap: + parent_id = from_snap.split('.') + updates = {'parent_id': parent_id[1]} + return updates def _file_is_rbd(self, volume_file): """Returns True if the volume_file is actually an RBD image.""" @@ -895,6 +904,7 @@ class CephBackupDriver(driver.BackupDriver): If this fails we will attempt to fall back to full copy. """ volume = self.db.volume_get(self.context, backup.volume_id) + updates = {} if not backup.container: backup.container = self._ceph_backup_pool backup.save() @@ -910,7 +920,8 @@ class CephBackupDriver(driver.BackupDriver): # If volume an RBD, attempt incremental backup. LOG.debug("Volume file is RBD: attempting incremental backup.") try: - self._backup_rbd(backup, volume_file, volume.name, length) + updates = self._backup_rbd(backup, volume_file, + volume.name, length) except exception.BackupRBDOperationFailed: LOG.debug("Forcing full backup of volume %s.", volume.id) do_full_backup = True @@ -936,6 +947,13 @@ class CephBackupDriver(driver.BackupDriver): LOG.debug("Backup '%(backup_id)s' of volume %(volume_id)s finished.", {'backup_id': backup.id, 'volume_id': volume.id}) + # If updates is empty then set parent_id to None. This will + # take care if --incremental flag is used in CLI but a full + # backup is performed instead + if not updates and backup.parent_id: + updates = {'parent_id': None} + return updates + def _full_restore(self, backup, dest_file, dest_name, length, src_snap=None): """Restore volume using full copy i.e. all extents. diff --git a/cinder/backup/manager.py b/cinder/backup/manager.py index 2c29e5d8b60..9ab8e993524 100644 --- a/cinder/backup/manager.py +++ b/cinder/backup/manager.py @@ -284,6 +284,7 @@ class BackupManager(manager.ThreadPoolManager): snapshot = objects.Snapshot.get_by_id( context, snapshot_id) if snapshot_id else None previous_status = volume.get('previous_status', None) + updates = {} if snapshot_id: log_message = ('Create backup started, backup: %(backup_id)s ' 'volume: %(volume_id)s snapshot: %(snapshot_id)s.' @@ -343,7 +344,7 @@ class BackupManager(manager.ThreadPoolManager): err = _('Create backup aborted due to backup service is down') self._update_backup_error(backup, err) raise exception.InvalidBackup(reason=err) - self._run_backup(context, backup, volume) + updates = self._run_backup(context, backup, volume) except Exception as err: with excutils.save_and_reraise_exception(): if snapshot_id: @@ -366,6 +367,9 @@ class BackupManager(manager.ThreadPoolManager): 'previous_status': 'backing-up'}) backup.status = fields.BackupStatus.AVAILABLE backup.size = volume['size'] + + if updates: + backup.update(updates) backup.save() # Handle the num_dependent_backups of parent backup when child backup @@ -396,14 +400,17 @@ class BackupManager(manager.ThreadPoolManager): not os.path.isdir(device_path)): if backup_device.secure_enabled: with open(device_path) as device_file: - backup_service.backup(backup, device_file) + updates = backup_service.backup( + backup, device_file) else: with utils.temporary_chown(device_path): with open(device_path) as device_file: - backup_service.backup(backup, device_file) + updates = backup_service.backup( + backup, device_file) # device_path is already file-like so no need to open it else: - backup_service.backup(backup, device_path) + updates = backup_service.backup( + backup, device_path) finally: self._detach_device(context, attach_info, @@ -414,6 +421,7 @@ class BackupManager(manager.ThreadPoolManager): backup = objects.Backup.get_by_id(context, backup.id) self._cleanup_temp_volumes_snapshots_when_backup_created( context, backup) + return updates def restore_backup(self, context, backup, volume_id): """Restore volume backups from configured backup service.""" diff --git a/cinder/tests/unit/backup/drivers/test_backup_ceph.py b/cinder/tests/unit/backup/drivers/test_backup_ceph.py index 74371e5746c..5096b86baf4 100644 --- a/cinder/tests/unit/backup/drivers/test_backup_ceph.py +++ b/cinder/tests/unit/backup/drivers/test_backup_ceph.py @@ -470,8 +470,8 @@ class BackupCephTestCase(test.TestCase): 'user_foo', 'conf_foo') rbdio = linuxrbd.RBDVolumeIOWrapper(meta) - self.service.backup(self.backup, rbdio) - + output = self.service.backup(self.backup, rbdio) + self.assertDictEqual({}, output) self.assertEqual(['popen_init', 'read', 'popen_init', @@ -486,6 +486,84 @@ class BackupCephTestCase(test.TestCase): self.assertEqual(checksum.digest(), self.checksum.digest()) + @common_mocks + def test_backup_volume_from_rbd_set_parent_id(self): + 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'} + 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.assertDictEqual({'parent_id': 'mock'}, output) + + @common_mocks + def test_backup_volume_from_rbd_set_parent_id_none(self): + backup_name = self.service._get_backup_base_name( + self.backup_id, diff_format=True) + + self.mock_rbd.RBD().list.return_value = [backup_name] + self.backup.parent_id = 'mock_parent_id' + + 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')) + + # 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'): + 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']) + + @common_mocks + def test_backup_rbd_set_parent_id(self): + backup_name = self.service._get_backup_base_name( + self.backup_id, diff_format=True) + vol_name = self.volume.name + vol_length = self.volume.size + + self.mock_rbd.RBD().list.return_value = [backup_name] + + with mock.patch.object(self.service, '_snap_exists'), \ + mock.patch.object(self.service, '_get_backup_base_name') as \ + mock_get_backup_base_name, mock.patch.object( + self.service, '_get_most_recent_snap') as mock_get_most_recent_snap, \ + mock.patch.object(self.service, '_rbd_diff_transfer'): + mock_get_backup_base_name.return_value = backup_name + mock_get_most_recent_snap.return_value = ( + 'backup.mock.snap.153464362.12') + image = self.service.rbd.Image() + meta = linuxrbd.RBDImageMetadata(image, + 'pool_foo', + 'user_foo', + 'conf_foo') + rbdio = linuxrbd.RBDVolumeIOWrapper(meta) + rbdio.seek(0) + output = self.service._backup_rbd(self.backup, rbdio, + vol_name, vol_length) + self.assertDictEqual({'parent_id': 'mock'}, output) + @common_mocks @mock.patch('fcntl.fcntl', spec=True) @mock.patch('subprocess.Popen', spec=True) diff --git a/cinder/tests/unit/backup/test_backup.py b/cinder/tests/unit/backup/test_backup.py index d24634305b3..3442903601b 100644 --- a/cinder/tests/unit/backup/test_backup.py +++ b/cinder/tests/unit/backup/test_backup.py @@ -82,7 +82,8 @@ class BaseBackupTest(test.TestCase): temp_volume_id=None, temp_snapshot_id=None, snapshot_id=None, - metadata=None): + metadata=None, + parent_id=None): """Create a backup entry in the DB. Return the entry ID @@ -101,7 +102,7 @@ class BaseBackupTest(test.TestCase): kwargs['fail_reason'] = '' kwargs['service'] = service or CONF.backup_driver kwargs['snapshot_id'] = snapshot_id - kwargs['parent_id'] = None + kwargs['parent_id'] = parent_id kwargs['size'] = size kwargs['object_count'] = object_count kwargs['temp_volume_id'] = temp_volume_id @@ -637,6 +638,114 @@ class BackupTestCase(BaseBackupTest): self.assertEqual(fields.BackupStatus.AVAILABLE, backup['status']) self.assertEqual(vol_size, backup['size']) + @mock.patch('cinder.utils.brick_get_connector_properties') + @mock.patch('cinder.volume.rpcapi.VolumeAPI.get_backup_device') + @mock.patch('cinder.utils.temporary_chown') + @mock.patch('six.moves.builtins.open') + @mock.patch.object(os.path, 'isdir', return_value=True) + def test_create_backup_set_parent_id_to_none(self, mock_isdir, mock_open, + mock_chown, + mock_backup_device, + mock_brick): + vol_size = 1 + vol_id = self._create_volume_db_entry(size=vol_size) + backup = self._create_backup_db_entry(volume_id=vol_id, + parent_id = 'mock') + + with mock.patch.object(self.backup_mgr.service, 'get_backup_driver') as \ + mock_get_backup_driver: + mock_get_backup_driver.return_value.backup.return_value = ( + {'parent_id': None}) + with mock.patch.object(self.backup_mgr, '_detach_device'): + device_path = '/fake/disk/path/' + attach_info = {'device': {'path': device_path}} + mock_attach_device = self.mock_object(self.backup_mgr, + '_attach_device') + mock_attach_device.return_value = attach_info + properties = {} + mock_brick.return_value = properties + mock_open.return_value = open('/dev/null', 'rb') + mock_brick.return_value = properties + + self.backup_mgr.create_backup(self.ctxt, backup) + + backup = db.backup_get(self.ctxt, backup.id) + self.assertEqual(fields.BackupStatus.AVAILABLE, backup.status) + self.assertEqual(vol_size, backup.size) + self.assertIsNone(backup.parent_id) + + @mock.patch('cinder.utils.brick_get_connector_properties') + @mock.patch('cinder.volume.rpcapi.VolumeAPI.get_backup_device') + @mock.patch('cinder.utils.temporary_chown') + @mock.patch('six.moves.builtins.open') + @mock.patch.object(os.path, 'isdir', return_value=True) + def test_create_backup_set_parent_id(self, mock_isdir, mock_open, + mock_chown, mock_backup_device, + mock_brick): + vol_size = 1 + vol_id = self._create_volume_db_entry(size=vol_size) + backup = self._create_backup_db_entry(volume_id=vol_id) + parent_backup = self._create_backup_db_entry(size=vol_size) + + with mock.patch.object(self.backup_mgr.service, 'get_backup_driver') as \ + mock_get_backup_driver: + mock_get_backup_driver.return_value.backup.return_value = ( + {'parent_id': parent_backup.id}) + with mock.patch.object(self.backup_mgr, '_detach_device'): + device_path = '/fake/disk/path/' + attach_info = {'device': {'path': device_path}} + mock_attach_device = self.mock_object(self.backup_mgr, + '_attach_device') + mock_attach_device.return_value = attach_info + properties = {} + mock_brick.return_value = properties + mock_open.return_value = open('/dev/null', 'rb') + mock_brick.return_value = properties + + self.backup_mgr.create_backup(self.ctxt, backup) + + backup = db.backup_get(self.ctxt, backup.id) + self.assertEqual(fields.BackupStatus.AVAILABLE, backup.status) + self.assertEqual(vol_size, backup.size) + self.assertEqual(parent_backup.id, backup.parent_id) + + @mock.patch('cinder.utils.brick_get_connector_properties') + @mock.patch('cinder.volume.rpcapi.VolumeAPI.get_backup_device') + @mock.patch('cinder.utils.temporary_chown') + @mock.patch('six.moves.builtins.open') + @mock.patch.object(os.path, 'isdir', return_value=True) + def test_create_backup_fail_with_excep(self, mock_isdir, mock_open, + mock_chown, mock_backup_device, + mock_brick): + vol_id = self._create_volume_db_entry() + backup = self._create_backup_db_entry(volume_id=vol_id) + + with mock.patch.object(self.backup_mgr.service, 'get_backup_driver') as \ + mock_get_backup_driver: + mock_get_backup_driver.return_value.backup.side_effect = ( + FakeBackupException('fake')) + with mock.patch.object(self.backup_mgr, '_detach_device'): + device_path = '/fake/disk/path/' + attach_info = {'device': {'path': device_path}} + + mock_attach_device = self.mock_object(self.backup_mgr, + '_attach_device') + mock_attach_device.return_value = attach_info + properties = {} + mock_brick.return_value = properties + mock_open.return_value = open('/dev/null', 'rb') + mock_brick.return_value = properties + + self.assertRaises(FakeBackupException, + self.backup_mgr.create_backup, + self.ctxt, backup) + + vol = db.volume_get(self.ctxt, vol_id) + self.assertEqual('available', vol.status) + self.assertEqual('error_backing-up', vol.previous_status) + backup = db.backup_get(self.ctxt, backup.id) + self.assertEqual(fields.BackupStatus.ERROR, backup.status) + @mock.patch('cinder.utils.brick_get_connector_properties') @mock.patch('cinder.volume.rpcapi.VolumeAPI.get_backup_device') @mock.patch('cinder.utils.temporary_chown') @@ -648,7 +757,8 @@ class BackupTestCase(BaseBackupTest): mock_backup_device, mock_brick): backup_service = lambda: None - backup_service.backup = mock.Mock() + backup_service.backup = mock.Mock( + return_value=mock.sentinel.backup_update) self.backup_mgr.service.get_backup_driver = lambda x: backup_service vol_id = self._create_volume_db_entry() @@ -661,12 +771,13 @@ class BackupTestCase(BaseBackupTest): self.backup_mgr._attach_device = mock.Mock( return_value=attach_info) self.backup_mgr._detach_device = mock.Mock() - self.backup_mgr._run_backup(self.ctxt, backup, volume) + output = self.backup_mgr._run_backup(self.ctxt, backup, volume) mock_chown.assert_not_called() mock_open.assert_not_called() backup_service.backup.assert_called_once_with( backup, device_path) + self.assertEqual(mock.sentinel.backup_update, output) @mock.patch('cinder.backup.manager.BackupManager._run_backup') @ddt.data((fields.SnapshotStatus.BACKING_UP, 'available'),