openstack/cinder
Code Issues Proposed changes

Add context to cloning snapshots in remotefs driver

Browse Source 
Adds context to _create_cloned_volume method in order
to allow taking a snapshot during cloning. Also saves
the temp_snapshot in order to enable Nova to update the
snapshot state. Destroys the temp_snapshot after usage.
This affects the NFS, WindowsSMBFS, VZStorage and
Quobyte drivers.

Closes-Bug: #1744692

Change-Id: I328a02d0f26e8c3d41ec18a0487da6fd20b39b04
This commit is contained in:
Silvan Kaiser 2018-05-29 12:17:43 +02:00
parent b8a41b8d42
commit 78f2101fc5
7 changed files with 83 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
cinder/tests/unit/fake_snapshot.py
View File

@ -52,6 +52,8 @@ def fake_snapshot_obj(context, **updates):
expected_attrs = updates.pop('expected_attrs', None) or []
if 'volume' in updates and 'volume' not in expected_attrs:
expected_attrs.append('volume')
if 'context' in updates and 'context' not in expected_attrs:
expected_attrs.append('context')
return snapshot.Snapshot._from_db_object(context, snapshot.Snapshot(),
fake_db_snapshot(**updates),
expected_attrs=expected_attrs)

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

@ -16,6 +16,7 @@ import collections
import copy
import os
import re
import sys
import ddt
import mock
@ -23,6 +24,7 @@ import mock
from cinder import context
from cinder import exception
from cinder.image import image_utils
from cinder.objects import fields
from cinder import test
from cinder.tests.unit import fake_snapshot
from cinder.tests.unit import fake_volume
@ -327,17 +329,19 @@ class RemoteFsSnapDriverTestCase(test.TestCase):
{'status': 'creating', 'progress': '45%'},
{'status': 'deleting'},
]
fake_snapshot = self._fake_snapshot
fake_snapshot.context = self.context
with mock.patch.object(self._driver, '_do_create_snapshot') as \
mock_do_create_snapshot:
self.assertRaises(exception.RemoteFSConcurrentRequest,
self._driver._create_snapshot_online,
self._fake_snapshot,
fake_snapshot,
self._fake_volume.name,
self._fake_snapshot_path)
mock_do_create_snapshot.assert_called_once_with(
self._fake_snapshot, self._fake_volume.name,
fake_snapshot, self._fake_volume.name,
self._fake_snapshot_path)
self.assertEqual([mock.call(1), mock.call(1)],
mock_sleep.call_args_list)
@ -586,6 +590,7 @@ class RemoteFsSnapDriverTestCase(test.TestCase):
{'snapshots_exist': True},
{'force_temp_snap': True})
@ddt.unpack
@mock.patch.object(sys.modules['cinder.objects'], "Snapshot")
@mock.patch.object(remotefs.RemoteFSSnapDriver, 'local_path')
@mock.patch.object(remotefs.RemoteFSSnapDriver, '_snapshots_exist')
@mock.patch.object(remotefs.RemoteFSSnapDriver, '_copy_volume_image')
@ -603,16 +608,19 @@ class RemoteFsSnapDriverTestCase(test.TestCase):
mock_copy_volume_image,
mock_snapshots_exist,
mock_local_path,
mock_obj_snap,
snapshots_exist=False,
force_temp_snap=False):
drv = self._driver
# prepare test
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)
src_vref.context = self.context
mock_snapshots_exist.return_value = snapshots_exist
drv._always_use_temp_snap_when_cloning = force_temp_snap
@ -621,27 +629,38 @@ class RemoteFsSnapDriverTestCase(test.TestCase):
'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)
volume_ref = Volume(id=volume.id,
metadata=volume.metadata,
name=volume.name,
status=volume.status,
provider_location=volume.provider_location,
status=volume.status,
size=volume.size,
volume_type=volume.volume_type,
metadata=volume.metadata)
volume_type=volume.volume_type,)
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_args_creation = {
'volume_id': src_vref.id,
'user_id': None,
'project_id': None,
'status': fields.SnapshotStatus.CREATING,
'progress': '0%',
'volume_size': src_vref.size,
'display_name': 'tmp-snap-%s' % src_vref['id'],
'display_description': None,
'volume_type_id': src_vref.volume_type_id,
'encryption_key_id': None,
}
snap_args_deletion = snap_args_creation.copy()
snap_args_deletion["status"] = fields.SnapshotStatus.DELETED
snap_args_deletion["deleted"] = True
mock_obj_snap.return_value = mock.Mock()
mock_obj_snap.return_value.create = mock.Mock()
# end of prepare test
# run test
drv.create_cloned_volume(volume, src_vref)
# evaluate test
exp_acceptable_states = ['available', 'backing-up', 'downloading']
mock_validate_state.assert_called_once_with(
src_vref.status,
@ -649,10 +668,14 @@ class RemoteFsSnapDriverTestCase(test.TestCase):
obj_description='source volume')
if snapshots_exist or force_temp_snap:
mock_create_snapshot.assert_called_once_with(snap_ref)
mock_obj_snap.return_value.create.assert_called_once_with()
mock_obj_snap.assert_called_once_with(
context=self.context, **snap_args_creation)
mock_create_snapshot.assert_called_once_with(
mock_obj_snap.return_value)
mock_copy_volume_from_snapshot.assert_called_once_with(
snap_ref, volume_ref, volume['size'])
self.assertTrue(mock_delete_snapshot.called)
mock_obj_snap.return_value, volume_ref, volume['size'])
mock_delete_snapshot.called_once_with(snap_args_deletion)
else:
self.assertFalse(mock_create_snapshot.called)
@ -663,8 +686,7 @@ class RemoteFsSnapDriverTestCase(test.TestCase):
mock_local_path.return_value)
mock_local_path.assert_has_calls(
[mock.call(src_vref), mock.call(volume_ref)])
mock_extend_volume.assert_called_once_with(volume_ref,
volume.size)
mock_extend_volume.assert_called_once_with(volume_ref, volume.size)
@mock.patch('shutil.copyfile')
@mock.patch.object(remotefs.RemoteFSSnapDriver, '_set_rw_permissions')

2
cinder/tests/unit/volume/drivers/test_vzstorage.py
View File

@ -415,6 +415,7 @@ class VZStorageTestCase(test.TestCase):
id=src_vref_id,
name='volume-%s' % src_vref_id,
provider_location=self._FAKE_SHARE)
src_vref.context = self.context
mock_remotefs_create_cloned_volume.return_value = {
'provider_location': self._FAKE_SHARE}
@ -446,6 +447,7 @@ class VZStorageTestCase(test.TestCase):
id=src_vref_id,
name='volume-%s' % src_vref_id,
provider_location=self._FAKE_SHARE)
src_vref.context = self.context
snap_attrs = ['volume_name', 'size', 'volume_size', 'name',
'volume_id', 'id', 'volume']

3
cinder/volume/drivers/quobyte.py
View File

@ -325,7 +325,8 @@ class QuobyteDriver(remotefs_drv.RemoteFSSnapDriverDistributed):
@utils.synchronized('quobyte', external=False)
def create_cloned_volume(self, volume, src_vref):
"""Creates a clone of the specified volume."""
return self._create_cloned_volume(volume, src_vref)
return self._create_cloned_volume(volume, src_vref,
src_vref.obj_context)
@coordination.synchronized(
'{self.driver_prefix}-{snapshot.id}-{volume.id}')

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

@ -37,6 +37,7 @@ from cinder import db
from cinder import exception
from cinder.i18n import _
from cinder.image import image_utils
from cinder import objects
from cinder.objects import fields
from cinder import utils
from cinder.volume import configuration
@ -1024,7 +1025,7 @@ class RemoteFSSnapDriverBase(RemoteFSDriver):
def _is_volume_attached(self, volume):
return volume.attach_status == fields.VolumeAttachStatus.ATTACHED
def _create_cloned_volume(self, volume, src_vref):
def _create_cloned_volume(self, volume, src_vref, context):
LOG.info('Cloning volume %(src)s to volume %(dst)s',
{'src': src_vref.id,
'dst': volume.id})
@ -1040,7 +1041,6 @@ class RemoteFSSnapDriverBase(RemoteFSDriver):
vol_attrs = ['provider_location', 'size', 'id', 'name', 'status',
'volume_type', 'metadata']
Volume = collections.namedtuple('Volume', vol_attrs)
volume_info = Volume(provider_location=src_vref.provider_location,
size=src_vref.size,
id=volume.id,
@ -1051,25 +1051,34 @@ class RemoteFSSnapDriverBase(RemoteFSDriver):
if (self._always_use_temp_snap_when_cloning or
self._snapshots_exist(src_vref)):
snap_attrs = ['volume_name', 'volume_size', 'name',
'volume_id', 'id', 'volume']
Snapshot = collections.namedtuple('Snapshot', snap_attrs)
temp_snapshot = Snapshot(volume_name=volume_name,
volume_size=src_vref.size,
name='clone-snap-%s' % src_vref.id,
volume_id=src_vref.id,
id='tmp-snap-%s' % src_vref.id,
volume=src_vref)
kwargs = {
'volume_id': src_vref.id,
'user_id': context.user_id,
'project_id': context.project_id,
'status': fields.SnapshotStatus.CREATING,
'progress': '0%',
'volume_size': src_vref.size,
'display_name': 'tmp-snap-%s' % src_vref.id,
'display_description': None,
'volume_type_id': src_vref.volume_type_id,
'encryption_key_id': src_vref.encryption_key_id,
}
temp_snapshot = objects.Snapshot(context=context,
**kwargs)
temp_snapshot.create()
self._create_snapshot(temp_snapshot)
try:
self._copy_volume_from_snapshot(temp_snapshot,
volume_info,
volume.size)
# remove temp snapshot after the cloning is done
temp_snapshot.status = fields.SnapshotStatus.DELETING
temp_snapshot.context = context.elevated()
temp_snapshot.save()
finally:
self._delete_snapshot(temp_snapshot)
temp_snapshot.destroy()
else:
self._copy_volume_image(self.local_path(src_vref),
self.local_path(volume_info))
@ -1459,8 +1468,6 @@ class RemoteFSSnapDriverBase(RemoteFSDriver):
def _create_snapshot_online(self, snapshot, backing_filename,
new_snap_path):
# Perform online snapshot via Nova
context = snapshot._context
self._do_create_snapshot(snapshot,
backing_filename,
new_snap_path)
@ -1473,7 +1480,7 @@ class RemoteFSSnapDriverBase(RemoteFSDriver):
try:
result = self._nova.create_volume_snapshot(
context,
snapshot.obj_context,
snapshot.volume_id,
connection_info)
LOG.debug('nova call result: %s', result)
@ -1488,7 +1495,7 @@ class RemoteFSSnapDriverBase(RemoteFSDriver):
increment = 1
timeout = 600
while True:
s = db.snapshot_get(context, snapshot.id)
s = db.snapshot_get(snapshot.obj_context, snapshot.id)
LOG.debug('Status of snapshot %(id)s is now %(status)s',
{'id': snapshot['id'],
@ -1651,7 +1658,8 @@ class RemoteFSSnapDriver(RemoteFSSnapDriverBase):
def create_cloned_volume(self, volume, src_vref):
"""Creates a clone of the specified volume."""
return self._create_cloned_volume(volume, src_vref)
return self._create_cloned_volume(volume, src_vref,
src_vref.obj_context)
@locked_volume_id_operation
def copy_volume_to_image(self, context, volume, image_service, image_meta):
@ -1692,7 +1700,8 @@ class RemoteFSSnapDriverDistributed(RemoteFSSnapDriverBase):
def create_cloned_volume(self, volume, src_vref):
"""Creates a clone of the specified volume."""
return self._create_cloned_volume(volume, src_vref)
return self._create_cloned_volume(volume, src_vref,
src_vref.obj_context)
@coordination.synchronized('{self.driver_prefix}-{volume.id}')
def copy_volume_to_image(self, context, volume, image_service, image_meta):

2
cinder/volume/drivers/vzstorage.py
View File

@ -718,7 +718,7 @@ class VZStorageDriver(remotefs_drv.RemoteFSSnapDriver):
return {'provider_location': src_vref.provider_location}
def _create_cloned_volume(self, volume, src_vref):
def _create_cloned_volume(self, volume, src_vref, context):
"""Creates a clone of the specified volume."""
volume_format = self.get_volume_format(src_vref)
if volume_format == DISK_FORMAT_PLOOP:

6
releasenotes/notes/bugfix-1744692-5aebd0c97ae66407.yaml Normal file
View File

@ -0,0 +1,6 @@
---
fixes:
- |
Fixes a bug that prevented distributed file system drivers from creating
snapshots during volume clone operations (NFS, WindowsSMBFS, VZstorage
and Quobyte drivers). Fixing this allows creating snapshot based backups.