openstack/cinder
Code Issues Proposed changes

Merge "RemoteFS: revert snapshot support"

Browse Source
This commit is contained in:
Zuul 2017-11-15 12:56:18 +00:00 committed by Gerrit Code Review
commit df6eb31580
5 changed files with 191 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

91
cinder/tests/unit/volume/drivers/test_remotefs.py
View File

@ -702,3 +702,94 @@ class RemoteFSPoolMixinTestCase(test.TestCase):
mock.sentinel.share)
self._driver.configuration.safe_get.assert_called_once_with(
'volume_backend_name')
@ddt.ddt
class RevertToSnapshotMixinTestCase(test.TestCase):
_FAKE_MNT_POINT = '/mnt/fake_hash'
def setUp(self):
super(RevertToSnapshotMixinTestCase, self).setUp()
self._driver = remotefs.RevertToSnapshotMixin()
self._driver._remotefsclient = mock.Mock()
self._driver._execute = mock.Mock()
self._driver._delete = mock.Mock()
self.context = context.get_admin_context()
self._fake_volume = fake_volume.fake_volume_obj(
self.context, provider_location='fake_share')
self._fake_volume_path = os.path.join(self._FAKE_MNT_POINT,
self._fake_volume.name)
self._fake_snapshot = fake_snapshot.fake_snapshot_obj(self.context)
self._fake_snapshot_path = (self._fake_volume_path + '.' +
self._fake_snapshot.id)
self._fake_snapshot_name = os.path.basename(
self._fake_snapshot_path)
self._fake_snapshot.volume = self._fake_volume
@ddt.data(True, False)
@mock.patch.object(remotefs.RevertToSnapshotMixin, '_validate_state',
create=True)
@mock.patch.object(remotefs.RevertToSnapshotMixin, '_read_info_file',
create=True)
@mock.patch.object(remotefs.RevertToSnapshotMixin,
'_local_path_volume_info', create=True)
@mock.patch.object(remotefs.RevertToSnapshotMixin, '_qemu_img_info',
create=True)
@mock.patch.object(remotefs.RevertToSnapshotMixin, '_do_create_snapshot',
create=True)
@mock.patch.object(remotefs.RevertToSnapshotMixin, '_local_volume_dir',
create=True)
def test_revert_to_snapshot(self,
is_latest_snapshot,
mock_local_vol_dir,
mock_do_create_snapshot,
mock_qemu_img_info,
mock_local_path_vol_info,
mock_read_info_file,
mock_validate_state):
active_file = (self._fake_snapshot_name if is_latest_snapshot
else 'fake_latest_snap')
fake_snapshot_info = {
'active': active_file,
self._fake_snapshot.id: self._fake_snapshot_name
}
mock_read_info_file.return_value = fake_snapshot_info
fake_snap_img_info = mock.Mock()
fake_snap_img_info.backing_file = self._fake_volume.name
mock_qemu_img_info.return_value = fake_snap_img_info
mock_local_vol_dir.return_value = self._FAKE_MNT_POINT
if is_latest_snapshot:
self._driver._revert_to_snapshot(self.context, self._fake_volume,
self._fake_snapshot)
self._driver._delete.assert_called_once_with(
self._fake_snapshot_path)
mock_do_create_snapshot.assert_called_once_with(
self._fake_snapshot,
fake_snap_img_info.backing_file,
self._fake_snapshot_path)
mock_qemu_img_info.assert_called_once_with(
self._fake_snapshot_path,
self._fake_volume.name)
elif not is_latest_snapshot:
self.assertRaises(exception.InvalidSnapshot,
self._driver._revert_to_snapshot,
self.context, self._fake_volume,
self._fake_snapshot)
self._driver._delete.assert_not_called()
exp_acceptable_states = ['available', 'reverting']
mock_validate_state.assert_called_once_with(
self._fake_snapshot.volume.status,
exp_acceptable_states)
mock_local_path_vol_info.assert_called_once_with(
self._fake_snapshot.volume)
mock_read_info_file.assert_called_once_with(
mock_local_path_vol_info.return_value)

27
cinder/tests/unit/volume/test_volume.py
View File

@ -1779,8 +1779,13 @@ class VolumeTestCase(base.BaseVolumeTestCase):
if driver_error:
generic_revert.assert_called_once_with(self.context, {}, {})
@ddt.data(True, False)
def test_revert_to_snapshot(self, has_snapshot):
@ddt.data({},
{'has_snapshot': True},
{'use_temp_snapshot': True},
{'use_temp_snapshot': True, 'has_snapshot': True})
@ddt.unpack
def test_revert_to_snapshot(self, has_snapshot=False,
use_temp_snapshot=False):
fake_volume = tests_utils.create_volume(self.context,
status='reverting',
project_id='123',
@ -1794,8 +1799,13 @@ class VolumeTestCase(base.BaseVolumeTestCase):
mock.patch.object(self.volume,
'_create_backup_snapshot') as _create_snapshot,\
mock.patch.object(self.volume,
'delete_snapshot') as _delete_snapshot:
'delete_snapshot') as _delete_snapshot, \
mock.patch.object(self.volume.driver,
'snapshot_revert_use_temp_snapshot') as \
_use_temp_snap:
_revert.return_value = None
_use_temp_snap.return_value = use_temp_snapshot
if has_snapshot:
_create_snapshot.return_value = {'id': 'fake_snapshot'}
else:
@ -1804,12 +1814,19 @@ class VolumeTestCase(base.BaseVolumeTestCase):
fake_snapshot)
_revert.assert_called_once_with(self.context, fake_volume,
fake_snapshot)
_create_snapshot.assert_called_once_with(self.context, fake_volume)
if has_snapshot:
if not use_temp_snapshot:
_create_snapshot.assert_not_called()
else:
_create_snapshot.assert_called_once_with(self.context,
fake_volume)
if use_temp_snapshot and has_snapshot:
_delete_snapshot.assert_called_once_with(
self.context, {'id': 'fake_snapshot'}, handle_quota=False)
else:
_delete_snapshot.assert_not_called()
fake_volume.refresh()
fake_snapshot.refresh()
self.assertEqual('available', fake_volume['status'])

7
cinder/volume/driver.py
View File

@ -1102,6 +1102,13 @@ class BaseVD(object):
"""
return self.configuration.safe_get("backup_use_temp_snapshot")
def snapshot_revert_use_temp_snapshot(self):
# Specify whether a temporary backup snapshot should be used when
# reverting a snapshot. For some backends, this operation is not
# needed or not supported, in which case the driver should override
# this method.
return True
def snapshot_remote_attachable(self):
# TODO(lixiaoy1): the method will be deleted later when remote
# attach snapshot is implemented.

67
cinder/volume/drivers/remotefs.py
View File

@ -690,6 +690,13 @@ class RemoteFSSnapDriverBase(RemoteFSDriver):
self._nova = compute.API()
def snapshot_revert_use_temp_snapshot(self):
# Considering that RemoteFS based drivers use COW images
# for storing snapshots, having chains of such images,
# creating a backup snapshot when reverting one is not
# actutally helpful.
return False
def _local_volume_dir(self, volume):
share = volume.provider_location
local_dir = self._get_mount_point_for_share(share)
@ -1580,6 +1587,9 @@ class RemoteFSSnapDriverBase(RemoteFSDriver):
def _extend_volume(self, volume, size_gb):
raise NotImplementedError()
def _revert_to_snapshot(self, context, volume, snapshot):
raise NotImplementedError()
class RemoteFSSnapDriver(RemoteFSSnapDriverBase):
@locked_volume_id_operation
@ -1615,6 +1625,12 @@ class RemoteFSSnapDriver(RemoteFSSnapDriverBase):
def extend_volume(self, volume, size_gb):
return self._extend_volume(volume, size_gb)
@locked_volume_id_operation
def revert_to_snapshot(self, context, volume, snapshot):
"""Revert to specified snapshot."""
return self._revert_to_snapshot(context, volume, snapshot)
class RemoteFSSnapDriverDistributed(RemoteFSSnapDriverBase):
@coordination.synchronized('{self.driver_prefix}-{snapshot.volume.id}')
@ -1650,6 +1666,12 @@ class RemoteFSSnapDriverDistributed(RemoteFSSnapDriverBase):
def extend_volume(self, volume, size_gb):
return self._extend_volume(volume, size_gb)
@coordination.synchronized('{self.driver_prefix}-{volume.id}')
def revert_to_snapshot(self, context, volume, snapshot):
"""Revert to specified snapshot."""
return self._revert_to_snapshot(context, volume, snapshot)
class RemoteFSPoolMixin(object):
"""Drivers inheriting this will report each share as a pool."""
@ -1710,3 +1732,48 @@ class RemoteFSPoolMixin(object):
data['pools'] = pools
self._stats = data
class RevertToSnapshotMixin(object):
def _revert_to_snapshot(self, context, volume, snapshot):
"""Revert a volume to specified snapshot
The volume must not be attached. Only the latest snapshot
can be used.
"""
status = snapshot.volume.status
acceptable_states = ['available', 'reverting']
self._validate_state(status, acceptable_states)
LOG.debug('Reverting volume %(vol)s to snapshot %(snap)s',
{'vol': snapshot.volume.id, 'snap': snapshot.id})
info_path = self._local_path_volume_info(snapshot.volume)
snap_info = self._read_info_file(info_path)
snapshot_file = snap_info[snapshot.id]
active_file = snap_info['active']
if not utils.paths_normcase_equal(snapshot_file, active_file):
msg = _("Could not revert volume '%(volume_id)s' to snapshot "
"'%(snapshot_id)s' as it does not "
"appear to be the latest snapshot. Current active "
"image: %(active_file)s.")
raise exception.InvalidSnapshot(
msg % dict(snapshot_id=snapshot.id,
active_file=active_file,
volume_id=volume.id))
snapshot_path = os.path.join(
self._local_volume_dir(snapshot.volume), snapshot_file)
backing_filename = self._qemu_img_info(
snapshot_path, volume.name).backing_file
# We revert the volume to the latest snapshot by recreating the top
# image from the chain.
# This workflow should work with most (if not all) drivers inheriting
# this class.
self._delete(snapshot_path)
self._do_create_snapshot(snapshot, backing_filename, snapshot_path)

5
cinder/volume/manager.py
View File

@ -949,7 +949,10 @@ class VolumeManager(manager.CleanableManager,
# Create a snapshot which can be used to restore the volume
# data by hand if revert process failed.
backup_snapshot = self._create_backup_snapshot(context, volume)
if self.driver.snapshot_revert_use_temp_snapshot():
backup_snapshot = self._create_backup_snapshot(context,
volume)
self._revert_to_snapshot(context, volume, snapshot)
except Exception as error:
with excutils.save_and_reraise_exception():