RemoteFS: enable image volume cache
At the moment, the RemoteFS based drivers cannot leverage the generic image volume cache feature. The reason is that this involves cloning a newly created volume while still being in 'downloading' state, which is an unacceptable state from the driver's point of view. This change adds 'downloading' as an acceptable state for the volume clone operation as well as for creating/deleting snapshots used internally by the volume clone operation. Note that 'regular' volume clones will be rejected before reaching the driver when having a source volume in 'downloading' state. Change-Id: I84bb3f062637031f7f46b414d6cbbff0bb0292ea Closes-Bug: #1685277
This commit is contained in:
Lucian Petrut
parent
ad6d14b6d4
commit
63b9f0fb8e
|
|
@ -52,9 +52,26 @@ class RemoteFsSnapDriverTestCase(test.TestCase):
|
|||
self._fake_snapshot.id)
|
||||
self._fake_snapshot.volume = self._fake_volume
|
||||
|
||||
@ddt.data({'current_state': 'in-use',
|
||||
'acceptable_states': ['available', 'in-use']},
|
||||
{'current_state': 'in-use',
|
||||
'acceptable_states': ['available'],
|
||||
'expected_exception': exception.InvalidVolume})
|
||||
@ddt.unpack
|
||||
def test_validate_state(self, current_state, acceptable_states,
|
||||
expected_exception=None):
|
||||
if expected_exception:
|
||||
self.assertRaises(expected_exception,
|
||||
self._driver._validate_state,
|
||||
current_state,
|
||||
acceptable_states)
|
||||
else:
|
||||
self._driver._validate_state(current_state, acceptable_states)
|
||||
|
||||
def _test_delete_snapshot(self, volume_in_use=False,
|
||||
stale_snapshot=False,
|
||||
is_active_image=True):
|
||||
is_active_image=True,
|
||||
is_tmp_snap=False):
|
||||
# If the snapshot is not the active image, it is guaranteed that
|
||||
# another snapshot exists having it as backing file.
|
||||
|
||||
|
|
@ -78,6 +95,7 @@ class RemoteFsSnapDriverTestCase(test.TestCase):
|
|||
self._driver._local_volume_dir = mock.Mock(
|
||||
return_value=self._FAKE_MNT_POINT)
|
||||
|
||||
self._driver._validate_state = mock.Mock()
|
||||
self._driver._read_info_file = mock.Mock()
|
||||
self._driver._write_info_file = mock.Mock()
|
||||
self._driver._img_commit = mock.Mock()
|
||||
|
|
@ -91,12 +109,18 @@ class RemoteFsSnapDriverTestCase(test.TestCase):
|
|||
self._fake_snapshot.id: fake_snapshot_name
|
||||
}
|
||||
|
||||
exp_acceptable_states = ['available', 'in-use', 'backing-up',
|
||||
'deleting', 'downloading']
|
||||
|
||||
if volume_in_use:
|
||||
self._fake_snapshot.volume.status = 'in-use'
|
||||
|
||||
self._driver._read_info_file.return_value = fake_info
|
||||
|
||||
self._driver._delete_snapshot(self._fake_snapshot)
|
||||
self._driver._validate_state.assert_called_once_with(
|
||||
self._fake_snapshot.volume.status,
|
||||
exp_acceptable_states)
|
||||
if stale_snapshot:
|
||||
self._driver._delete_stale_snapshot.assert_called_once_with(
|
||||
self._fake_snapshot)
|
||||
|
|
@ -228,7 +252,7 @@ class RemoteFsSnapDriverTestCase(test.TestCase):
|
|||
mock.call(*command3, run_as_root=True)]
|
||||
self._driver._execute.assert_has_calls(calls)
|
||||
|
||||
def _test_create_snapshot(self, volume_in_use=False):
|
||||
def _test_create_snapshot(self, volume_in_use=False, tmp_snap=False):
|
||||
fake_snapshot_info = {}
|
||||
fake_snapshot_file_name = os.path.basename(self._fake_snapshot_path)
|
||||
|
||||
|
|
@ -243,11 +267,16 @@ class RemoteFsSnapDriverTestCase(test.TestCase):
|
|||
return_value=self._fake_volume.name)
|
||||
self._driver._get_new_snap_path = mock.Mock(
|
||||
return_value=self._fake_snapshot_path)
|
||||
self._driver._validate_state = mock.Mock()
|
||||
|
||||
expected_snapshot_info = {
|
||||
'active': fake_snapshot_file_name,
|
||||
self._fake_snapshot.id: fake_snapshot_file_name
|
||||
}
|
||||
exp_acceptable_states = ['available', 'in-use', 'backing-up']
|
||||
if tmp_snap:
|
||||
exp_acceptable_states.append('downloading')
|
||||
self._fake_snapshot.id = 'tmp-snap-%s' % self._fake_snapshot.id
|
||||
|
||||
if volume_in_use:
|
||||
self._fake_snapshot.volume.status = 'in-use'
|
||||
|
|
@ -258,6 +287,9 @@ class RemoteFsSnapDriverTestCase(test.TestCase):
|
|||
|
||||
self._driver._create_snapshot(self._fake_snapshot)
|
||||
|
||||
self._driver._validate_state.assert_called_once_with(
|
||||
self._fake_snapshot.volume.status,
|
||||
exp_acceptable_states)
|
||||
fake_method = getattr(self._driver, expected_method_called)
|
||||
fake_method.assert_called_with(
|
||||
self._fake_snapshot, self._fake_volume.name,
|
||||
|
|
@ -428,52 +460,59 @@ class RemoteFsSnapDriverTestCase(test.TestCase):
|
|||
basedir=basedir,
|
||||
valid_backing_file=False)
|
||||
|
||||
def test_create_cloned_volume(self):
|
||||
@mock.patch.object(remotefs.RemoteFSSnapDriver,
|
||||
'_validate_state')
|
||||
@mock.patch.object(remotefs.RemoteFSSnapDriver, '_create_snapshot')
|
||||
@mock.patch.object(remotefs.RemoteFSSnapDriver, '_delete_snapshot')
|
||||
@mock.patch.object(remotefs.RemoteFSSnapDriver,
|
||||
'_copy_volume_from_snapshot')
|
||||
def test_create_cloned_volume(self, mock_copy_volume_from_snapshot,
|
||||
mock_delete_snapshot,
|
||||
mock_create_snapshot,
|
||||
mock_validate_state):
|
||||
drv = self._driver
|
||||
|
||||
with mock.patch.object(drv, '_create_snapshot') as \
|
||||
mock_create_snapshot,\
|
||||
mock.patch.object(drv, '_delete_snapshot') as \
|
||||
mock_delete_snapshot,\
|
||||
mock.patch.object(drv, '_copy_volume_from_snapshot') as \
|
||||
mock_copy_volume_from_snapshot:
|
||||
volume = fake_volume.fake_volume_obj(self.context)
|
||||
src_vref_id = '375e32b2-804a-49f2-b282-85d1d5a5b9e1'
|
||||
src_vref = fake_volume.fake_volume_obj(
|
||||
self.context,
|
||||
id=src_vref_id,
|
||||
name='volume-%s' % src_vref_id)
|
||||
|
||||
volume = fake_volume.fake_volume_obj(self.context)
|
||||
src_vref_id = '375e32b2-804a-49f2-b282-85d1d5a5b9e1'
|
||||
src_vref = fake_volume.fake_volume_obj(
|
||||
self.context,
|
||||
id=src_vref_id,
|
||||
name='volume-%s' % src_vref_id)
|
||||
vol_attrs = ['provider_location', 'size', 'id', 'name', 'status',
|
||||
'volume_type', 'metadata']
|
||||
Volume = collections.namedtuple('Volume', vol_attrs)
|
||||
|
||||
vol_attrs = ['provider_location', 'size', 'id', 'name', 'status',
|
||||
'volume_type', 'metadata']
|
||||
Volume = collections.namedtuple('Volume', vol_attrs)
|
||||
snap_attrs = ['volume_name', 'volume_size', 'name',
|
||||
'volume_id', 'id', 'volume']
|
||||
Snapshot = collections.namedtuple('Snapshot', snap_attrs)
|
||||
|
||||
snap_attrs = ['volume_name', 'volume_size', 'name',
|
||||
'volume_id', 'id', 'volume']
|
||||
Snapshot = collections.namedtuple('Snapshot', snap_attrs)
|
||||
volume_ref = Volume(id=volume.id,
|
||||
name=volume.name,
|
||||
status=volume.status,
|
||||
provider_location=volume.provider_location,
|
||||
size=volume.size,
|
||||
volume_type=volume.volume_type,
|
||||
metadata=volume.metadata)
|
||||
|
||||
volume_ref = Volume(id=volume.id,
|
||||
name=volume.name,
|
||||
status=volume.status,
|
||||
provider_location=volume.provider_location,
|
||||
size=volume.size,
|
||||
volume_type=volume.volume_type,
|
||||
metadata=volume.metadata)
|
||||
snap_ref = Snapshot(volume_name=volume.name,
|
||||
name='clone-snap-%s' % src_vref.id,
|
||||
volume_size=src_vref.size,
|
||||
volume_id=src_vref.id,
|
||||
id='tmp-snap-%s' % src_vref.id,
|
||||
volume=src_vref)
|
||||
|
||||
snap_ref = Snapshot(volume_name=volume.name,
|
||||
name='clone-snap-%s' % src_vref.id,
|
||||
volume_size=src_vref.size,
|
||||
volume_id=src_vref.id,
|
||||
id='tmp-snap-%s' % src_vref.id,
|
||||
volume=src_vref)
|
||||
drv.create_cloned_volume(volume, src_vref)
|
||||
|
||||
drv.create_cloned_volume(volume, src_vref)
|
||||
|
||||
mock_create_snapshot.assert_called_once_with(snap_ref)
|
||||
mock_copy_volume_from_snapshot.assert_called_once_with(
|
||||
snap_ref, volume_ref, volume['size'])
|
||||
self.assertTrue(mock_delete_snapshot.called)
|
||||
exp_acceptable_states = ['available', 'backing-up', 'downloading']
|
||||
mock_validate_state.assert_called_once_with(
|
||||
src_vref.status,
|
||||
exp_acceptable_states,
|
||||
obj_description='source volume')
|
||||
mock_create_snapshot.assert_called_once_with(snap_ref)
|
||||
mock_copy_volume_from_snapshot.assert_called_once_with(
|
||||
snap_ref, volume_ref, volume['size'])
|
||||
self.assertTrue(mock_delete_snapshot.called)
|
||||
|
||||
def test_create_regular_file(self):
|
||||
self._driver._create_regular_file('/path', 1)
|
||||
|
|
|
|||
|
|
@ -233,6 +233,23 @@ class RemoteFSDriver(driver.BaseVD):
|
|||
" mount_point_base.")
|
||||
return None
|
||||
|
||||
@staticmethod
|
||||
def _validate_state(current_state,
|
||||
acceptable_states,
|
||||
obj_description='volume',
|
||||
invalid_exc=exception.InvalidVolume):
|
||||
if current_state not in acceptable_states:
|
||||
message = _('Invalid %(obj_description)s state. '
|
||||
'Acceptable states for this operation: '
|
||||
'%(acceptable_states)s. '
|
||||
'Current %(obj_description)s state: '
|
||||
'%(current_state)s.')
|
||||
raise invalid_exc(
|
||||
message=message %
|
||||
dict(obj_description=obj_description,
|
||||
acceptable_states=acceptable_states,
|
||||
current_state=current_state))
|
||||
|
||||
@utils.trace
|
||||
def create_volume(self, volume):
|
||||
"""Creates a volume.
|
||||
|
|
@ -941,11 +958,10 @@ class RemoteFSSnapDriverBase(RemoteFSDriver):
|
|||
{'src': src_vref.id,
|
||||
'dst': volume.id})
|
||||
|
||||
if src_vref.status not in ['available', 'backing-up']:
|
||||
msg = _("Source volume status must be 'available', or "
|
||||
"'backing-up' but is: "
|
||||
"%(status)s.") % {'status': src_vref.status}
|
||||
raise exception.InvalidVolume(msg)
|
||||
acceptable_states = ['available', 'backing-up', 'downloading']
|
||||
self._validate_state(src_vref.status,
|
||||
acceptable_states,
|
||||
obj_description='source volume')
|
||||
|
||||
volume_name = CONF.volume_name_template % volume.id
|
||||
|
||||
|
|
@ -1021,13 +1037,9 @@ class RemoteFSSnapDriverBase(RemoteFSDriver):
|
|||
else 'offline')})
|
||||
|
||||
volume_status = snapshot.volume.status
|
||||
if volume_status not in ['available', 'in-use',
|
||||
'backing-up', 'deleting']:
|
||||
msg = _("Volume status must be 'available', 'in-use', "
|
||||
"'backing-up' or 'deleting' but is: "
|
||||
"%(status)s.") % {'status': volume_status}
|
||||
|
||||
raise exception.InvalidVolume(msg)
|
||||
acceptable_states = ['available', 'in-use', 'backing-up', 'deleting',
|
||||
'downloading']
|
||||
self._validate_state(volume_status, acceptable_states)
|
||||
|
||||
vol_path = self._local_volume_dir(snapshot.volume)
|
||||
self._ensure_share_writable(vol_path)
|
||||
|
|
@ -1332,12 +1344,15 @@ class RemoteFSSnapDriverBase(RemoteFSDriver):
|
|||
else 'offline')})
|
||||
|
||||
status = snapshot.volume.status
|
||||
if status not in ['available', 'in-use', 'backing-up']:
|
||||
msg = _("Volume status must be 'available', 'in-use' or "
|
||||
"'backing-up' but is: "
|
||||
"%(status)s.") % {'status': status}
|
||||
|
||||
raise exception.InvalidVolume(msg)
|
||||
acceptable_states = ['available', 'in-use', 'backing-up']
|
||||
if snapshot.id.startswith('tmp-snap-'):
|
||||
# This is an internal volume snapshot. In order to support
|
||||
# image caching, we'll allow creating/deleting such snapshots
|
||||
# while having volumes in 'downloading' state.
|
||||
acceptable_states.append('downloading')
|
||||
|
||||
self._validate_state(status, acceptable_states)
|
||||
|
||||
info_path = self._local_path_volume_info(snapshot.volume)
|
||||
snap_info = self._read_info_file(info_path, empty_if_missing=True)
|
||||
|
|
|
|||
Loading…
Reference in New Issue