openstack
/
cinder
Code Issues Proposed changes

Merge "cinder backup sets incorrect parent_id"

This commit is contained in:
Jenkins 2017-08-09 06:31:35 +00:00 committed by Gerrit Code Review
parent 2ec202c719 f6cbcf4636
commit 4b45a359fb
4 changed files with 226 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
cinder/backup/drivers/ceph.py
View File

@ -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.

16
cinder/backup/manager.py
View File

@ -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."""

82
cinder/tests/unit/backup/drivers/test_backup_ceph.py
View File

@ -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)

119
cinder/tests/unit/backup/test_backup.py
View File

@ -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'),