diff --git a/cinder/tests/unit/fake_snapshot.py b/cinder/tests/unit/fake_snapshot.py index d7459c516e7..051809ee000 100644 --- a/cinder/tests/unit/fake_snapshot.py +++ b/cinder/tests/unit/fake_snapshot.py @@ -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) diff --git a/cinder/tests/unit/volume/drivers/test_remotefs.py b/cinder/tests/unit/volume/drivers/test_remotefs.py index 290e64949f3..b9d0552e2cd 100644 --- a/cinder/tests/unit/volume/drivers/test_remotefs.py +++ b/cinder/tests/unit/volume/drivers/test_remotefs.py @@ -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') diff --git a/cinder/tests/unit/volume/drivers/test_vzstorage.py b/cinder/tests/unit/volume/drivers/test_vzstorage.py index 3ea1313e568..0322b2accf1 100644 --- a/cinder/tests/unit/volume/drivers/test_vzstorage.py +++ b/cinder/tests/unit/volume/drivers/test_vzstorage.py @@ -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'] diff --git a/cinder/volume/drivers/quobyte.py b/cinder/volume/drivers/quobyte.py index e64b1856877..ede5e3f488d 100644 --- a/cinder/volume/drivers/quobyte.py +++ b/cinder/volume/drivers/quobyte.py @@ -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}') diff --git a/cinder/volume/drivers/remotefs.py b/cinder/volume/drivers/remotefs.py index b6128b71587..bceb640f4dd 100644 --- a/cinder/volume/drivers/remotefs.py +++ b/cinder/volume/drivers/remotefs.py @@ -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 @@ -1025,7 +1026,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}) @@ -1041,7 +1042,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, @@ -1052,25 +1052,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)) @@ -1460,8 +1469,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) @@ -1474,7 +1481,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) @@ -1489,7 +1496,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'], @@ -1652,7 +1659,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): @@ -1693,7 +1701,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): diff --git a/cinder/volume/drivers/vzstorage.py b/cinder/volume/drivers/vzstorage.py index 29f59e2b8db..6612ba5d83e 100644 --- a/cinder/volume/drivers/vzstorage.py +++ b/cinder/volume/drivers/vzstorage.py @@ -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: diff --git a/releasenotes/notes/bugfix-1744692-5aebd0c97ae66407.yaml b/releasenotes/notes/bugfix-1744692-5aebd0c97ae66407.yaml new file mode 100644 index 00000000000..81d676c2526 --- /dev/null +++ b/releasenotes/notes/bugfix-1744692-5aebd0c97ae66407.yaml @@ -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.