Browse Source

Add context to cloning snapshots in remotefs driver

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
(cherry picked from commit 78f2101fc5)
(cherry picked from commit 247c660715)
(cherry picked from commit b886d093d7)
tags/12.0.10^0
Silvan Kaiser Alan Bishop 1 year ago
parent
commit
772897cad7
7 changed files with 83 additions and 41 deletions
  1. +2
    -0
      cinder/tests/unit/fake_snapshot.py
  2. +42
    -20
      cinder/tests/unit/volume/drivers/test_remotefs.py
  3. +2
    -0
      cinder/tests/unit/volume/drivers/test_vzstorage.py
  4. +2
    -1
      cinder/volume/drivers/quobyte.py
  5. +28
    -19
      cinder/volume/drivers/remotefs.py
  6. +1
    -1
      cinder/volume/drivers/vzstorage.py
  7. +6
    -0
      releasenotes/notes/bugfix-1744692-5aebd0c97ae66407.yaml

+ 2
- 0
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)

+ 42
- 20
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)
@@ -518,6 +522,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')
@@ -535,16 +540,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
@@ -553,27 +561,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_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

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)
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,
@@ -581,10 +600,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)

@@ -595,8 +618,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
- 0
cinder/tests/unit/volume/drivers/test_vzstorage.py View File

@@ -426,6 +426,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}
@@ -457,6 +458,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']


+ 2
- 1
cinder/volume/drivers/quobyte.py View File

@@ -187,7 +187,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)

def _create_volume_from_snapshot(self, volume, snapshot):
"""Creates a volume from a snapshot.


+ 28
- 19
cinder/volume/drivers/remotefs.py View File

@@ -36,6 +36,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
@@ -983,7 +984,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})
@@ -999,7 +1000,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,
@@ -1010,25 +1010,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))
@@ -1418,8 +1427,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)
@@ -1432,7 +1439,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)
@@ -1447,7 +1454,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'],
@@ -1610,7 +1617,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):
@@ -1651,7 +1659,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):


+ 1
- 1
cinder/volume/drivers/vzstorage.py View File

@@ -720,7 +720,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
- 0
releasenotes/notes/bugfix-1744692-5aebd0c97ae66407.yaml 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.

Loading…
Cancel
Save