From 369572f5b2cd9a832ceec0e55220be524f0c7592 Mon Sep 17 00:00:00 2001 From: Dmitry Guryanov Date: Thu, 21 Jan 2016 19:39:42 +0300 Subject: [PATCH] Use versionedobjects in remotefs.py Use versioned objects in remotefs.py, which is base classes for all drivers with an interface of remote filesystem. Update glusterfs, nfs, smbfs, quobyte, scality, vzstorage volume drivers to use versionedobjects instead of dicts. Please note that netapp_nfs driver wasn't fixed, just unit tests were adapted to pass. Change-Id: Iebd20544102672d7b7ca820c9e9005833ec2580e Depends-On: I661ef85755e9c75aaedb4f157165d2514de3e9ec --- cinder/tests/unit/test_glusterfs.py | 367 ++++++++++-------------- cinder/tests/unit/test_netapp_nfs.py | 83 +++--- cinder/tests/unit/test_nfs.py | 124 ++++---- cinder/tests/unit/test_quobyte.py | 121 ++++---- cinder/tests/unit/test_remotefs.py | 195 +++++++------ cinder/tests/unit/test_scality.py | 95 +++--- cinder/tests/unit/test_smbfs.py | 166 ++++++----- cinder/tests/unit/test_vzstorage.py | 63 ++-- cinder/tests/unit/windows/test_smbfs.py | 58 ++-- cinder/volume/drivers/glusterfs.py | 44 +-- cinder/volume/drivers/nfs.py | 20 +- cinder/volume/drivers/quobyte.py | 32 +-- cinder/volume/drivers/remotefs.py | 199 +++++++------ cinder/volume/drivers/scality.py | 22 +- cinder/volume/drivers/smbfs.py | 97 +++---- cinder/volume/drivers/vzstorage.py | 30 +- 16 files changed, 872 insertions(+), 844 deletions(-) diff --git a/cinder/tests/unit/test_glusterfs.py b/cinder/tests/unit/test_glusterfs.py index a2292e4d500..e15e362dc1c 100644 --- a/cinder/tests/unit/test_glusterfs.py +++ b/cinder/tests/unit/test_glusterfs.py @@ -34,8 +34,9 @@ from cinder import db from cinder import exception from cinder.i18n import _ 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 from cinder import utils from cinder.volume import driver as base_driver from cinder.volume.drivers import glusterfs @@ -45,16 +46,6 @@ from cinder.volume.drivers import remotefs as remotefs_drv CONF = cfg.CONF -class DumbVolume(object): - fields = {} - - def __setitem__(self, key, value): - self.fields[key] = value - - def __getitem__(self, item): - return self.fields[item] - - class FakeDb(object): msg = "Tests are broken: mock this out." @@ -66,6 +57,17 @@ class FakeDb(object): return [] +class SideEffectList(object): + def __init__(self, obj, **kwargs): + self.obj = obj + self.attrs = kwargs + + def __call__(self, *args, **kw): + for attr in self.attrs.keys(): + setattr(self.obj, attr, self.attrs[attr].pop(0)) + return self.obj + + class GlusterFsDriverTestCase(test.TestCase): """Test case for GlusterFS driver.""" @@ -75,12 +77,10 @@ class GlusterFsDriverTestCase(test.TestCase): TEST_SIZE_IN_GB = 1 TEST_MNT_POINT = '/mnt/glusterfs' TEST_MNT_POINT_BASE = '/mnt/test' - TEST_LOCAL_PATH = '/mnt/glusterfs/volume-123' TEST_FILE_NAME = 'test.txt' TEST_SHARES_CONFIG_FILE = '/etc/cinder/test-shares.conf' TEST_TMP_FILE = '/tmp/tempfile' VOLUME_UUID = 'abcdefab-cdef-abcd-efab-cdefabcdefab' - VOLUME_NAME = 'volume-%s' % VOLUME_UUID SNAP_UUID = 'bacadaca-baca-daca-baca-dacadacadaca' SNAP_UUID_2 = 'bebedede-bebe-dede-bebe-dedebebedede' @@ -102,6 +102,7 @@ class GlusterFsDriverTestCase(test.TestCase): db=FakeDb()) self._driver.shares = {} compute.API = mock.MagicMock() + self.context = context.get_admin_context() def assertRaisesAndMessageMatches( self, excClass, msg, callableObj, *args, **kwargs): @@ -126,13 +127,12 @@ class GlusterFsDriverTestCase(test.TestCase): self.TEST_MNT_POINT_BASE) drv = self._driver - volume = DumbVolume() - volume['id'] = self.VOLUME_UUID - volume['provider_location'] = self.TEST_EXPORT1 - volume['name'] = 'volume-123' + volume = fake_volume.fake_volume_obj( + self.context, + provider_location=self.TEST_EXPORT1) self.assertEqual( - '/mnt/test/ab03ab34eaca46a5fb81878f7e9b91fc/volume-123', + '/mnt/test/ab03ab34eaca46a5fb81878f7e9b91fc/%s' % volume.name, drv.local_path(volume)) def test_mount_glusterfs(self): @@ -470,17 +470,20 @@ class GlusterFsDriverTestCase(test.TestCase): drv._find_share, self.TEST_SIZE_IN_GB) - def _simple_volume(self, id=None): - volume = DumbVolume() - volume['provider_location'] = self.TEST_EXPORT1 + def _simple_volume(self, id=None, **updates): if id is None: - volume['id'] = self.VOLUME_UUID - else: - volume['id'] = id - # volume['name'] mirrors format from db/sqlalchemy/models.py - volume['name'] = 'volume-%s' % volume['id'] - volume['size'] = 10 - volume['status'] = 'available' + id = self.VOLUME_UUID + if 'id' not in updates: + updates['id'] = self.VOLUME_UUID + if 'name' not in updates: + updates['name'] = 'volume-%s' % id + if 'status' not in updates: + updates['status'] = 'available' + if 'provider_location' not in updates: + updates['provider_location'] = self.TEST_EXPORT1 + if 'size' not in updates: + updates['size'] = 10 + volume = fake_volume.fake_volume_obj(self.context, **updates) return volume @@ -497,7 +500,7 @@ class GlusterFsDriverTestCase(test.TestCase): drv._do_create_volume(volume) volume_path = drv.local_path(volume) - volume_size = volume['size'] + volume_size = volume.size mock_create_qcow2_file.assert_called_once_with(volume_path, volume_size) mock_set_rw_permissions_for_all.\ @@ -516,7 +519,7 @@ class GlusterFsDriverTestCase(test.TestCase): drv._do_create_volume(volume) volume_path = drv.local_path(volume) - volume_size = volume['size'] + volume_size = volume.size mock_fallocate.assert_called_once_with(volume_path, volume_size) mock_set_rw_permissions_for_all.\ @@ -539,7 +542,7 @@ class GlusterFsDriverTestCase(test.TestCase): drv._do_create_volume(volume) volume_path = drv.local_path(volume) - volume_size = volume['size'] + volume_size = volume.size mock_fallocate.assert_called_once_with(volume_path, volume_size) mock_create_regular_file.assert_called_once_with(volume_path, @@ -556,9 +559,10 @@ class GlusterFsDriverTestCase(test.TestCase): mock_do_create_volume,\ mock.patch.object(drv, '_ensure_shares_mounted') as \ mock_ensure_shares_mounted: - volume = DumbVolume() - volume['size'] = self.TEST_SIZE_IN_GB - volume['id'] = self.VOLUME_UUID + volume = fake_volume.fake_volume_obj(self.context, + size=self.TEST_SIZE_IN_GB, + id=self.VOLUME_UUID) + mock_find_share.return_value = self.TEST_EXPORT2 drv.create_volume(volume) self.assertTrue(mock_ensure_shares_mounted.called) self.assertTrue(mock_do_create_volume.called) @@ -575,9 +579,9 @@ class GlusterFsDriverTestCase(test.TestCase): mock_ensure_shares_mounted: mock_find_share.return_value = self.TEST_EXPORT1 - volume = DumbVolume() - volume['size'] = self.TEST_SIZE_IN_GB - volume['id'] = self.VOLUME_UUID + volume = fake_volume.fake_volume_obj(self.context, + size=self.TEST_SIZE_IN_GB, + id=self.VOLUME_UUID) result = drv.create_volume(volume) self.assertEqual(self.TEST_EXPORT1, result['provider_location']) self.assertTrue(mock_ensure_shares_mounted.called) @@ -610,7 +614,7 @@ class GlusterFsDriverTestCase(test.TestCase): self._driver.delete_volume(volume) mock_ensure_share_mounted.assert_called_once_with( - volume['provider_location']) + volume.provider_location) mock_local_volume_dir.assert_called_once_with(volume) mock_active_image_from_info.assert_called_once_with(volume) mock_execute.assert_called_once_with('rm', '-f', volume_path, @@ -827,10 +831,12 @@ class GlusterFsDriverTestCase(test.TestCase): with mock.patch.object(drv, '_execute') as mock_execute,\ mock.patch.object(drv, '_ensure_share_mounted') as \ mock_ensure_share_mounted: - volume = DumbVolume() - volume['id'] = self.VOLUME_UUID - volume['name'] = 'volume-123' - volume['provider_location'] = self.TEST_EXPORT1 + + volume = fake_volume.fake_volume_obj( + self.context, + id=self.VOLUME_UUID, + display_name='volume-123', + provider_location=self.TEST_EXPORT1) drv.delete_volume(volume) @@ -845,10 +851,10 @@ class GlusterFsDriverTestCase(test.TestCase): with mock.patch.object(drv, '_execute') as mock_execute,\ mock.patch.object(drv, '_ensure_share_mounted') as \ mock_ensure_share_mounted: - volume = DumbVolume() - volume['id'] = self.VOLUME_UUID - volume['name'] = 'volume-123' - volume['provider_location'] = None + volume = fake_volume.fake_volume_obj(self.context, + id=self.VOLUME_UUID, + name='volume-123', + provider_location=None) drv.delete_volume(volume) @@ -868,10 +874,6 @@ class GlusterFsDriverTestCase(test.TestCase): mock_read_file.return_value = '{"%(id)s": "volume-%(id)s"}' %\ {'id': self.VOLUME_UUID} - volume = DumbVolume() - volume['id'] = self.VOLUME_UUID - volume['name'] = 'volume-%s' % self.VOLUME_UUID - info = drv._read_info_file(info_path) self.assertEqual('volume-%s' % self.VOLUME_UUID, @@ -897,7 +899,7 @@ class GlusterFsDriverTestCase(test.TestCase): mock_qemu_img_info,\ mock.patch.object(image_utils, 'resize_image') as \ mock_resize_image: - mock_get_active_image_from_info.return_value = volume['name'] + mock_get_active_image_from_info.return_value = volume.name mock_local_volume_dir.return_value = self.TEST_MNT_POINT mock_qemu_img_info.return_value = img_info @@ -940,9 +942,6 @@ class GlusterFsDriverTestCase(test.TestCase): def test_create_snapshot_online(self): drv = self._driver - volume = self._simple_volume() - volume['status'] = 'in-use' - hashed = drv._get_hash_str(self.TEST_EXPORT1) volume_file = 'volume-%s' % self.VOLUME_UUID volume_path = '%s/%s/%s' % (self.TEST_MNT_POINT_BASE, @@ -951,13 +950,14 @@ class GlusterFsDriverTestCase(test.TestCase): ctxt = context.RequestContext('fake_user', 'fake_project') - snap_ref = {'name': 'test snap (online)', - 'volume_id': self.VOLUME_UUID, - 'volume': volume, - 'id': self.SNAP_UUID, - 'context': ctxt, - 'status': fields.SnapshotStatus.CREATING, - 'progress': 'asdf'} + snap_ref = fake_snapshot.fake_snapshot_obj( + ctxt, + display_name='test snap (online)', + volume_id=self.VOLUME_UUID, + id=self.SNAP_UUID, + status='creating', + progress='asdf') + snap_ref.context = ctxt snap_path = '%s.%s' % (volume_path, self.SNAP_UUID) snap_file = '%s.%s' % (volume_file, self.SNAP_UUID) @@ -967,26 +967,13 @@ class GlusterFsDriverTestCase(test.TestCase): mock.patch.object(db, 'snapshot_get') as mock_snapshot_get,\ mock.patch.object(drv, '_nova') as mock_nova,\ mock.patch.object(time, 'sleep') as mock_sleep: - create_info = {'snapshot_id': snap_ref['id'], + create_info = {'snapshot_id': snap_ref.id, 'type': 'qcow2', 'new_file': snap_file} - snap_ref_progress = snap_ref.copy() - snap_ref_progress['status'] = fields.SnapshotStatus.CREATING - - snap_ref_progress_0p = snap_ref_progress.copy() - snap_ref_progress_0p['progress'] = '0%' - - snap_ref_progress_50p = snap_ref_progress.copy() - snap_ref_progress_50p['progress'] = '50%' - - snap_ref_progress_90p = snap_ref_progress.copy() - snap_ref_progress_90p['progress'] = '90%' - - mock_snapshot_get.side_effect = [ - snap_ref_progress_0p, snap_ref_progress_50p, - snap_ref_progress_90p - ] + mock_snapshot_get.side_effect = SideEffectList( + snap_ref, + progress = ['0%', '50%', '90%']) drv._create_snapshot_online(snap_ref, snap_file, snap_path) mock_do_create_snapshot.\ @@ -999,7 +986,7 @@ class GlusterFsDriverTestCase(test.TestCase): drv = self._driver volume = self._simple_volume() - volume['status'] = 'in-use' + volume.status = 'in-use' hashed = drv._get_hash_str(self.TEST_EXPORT1) volume_file = 'volume-%s' % self.VOLUME_UUID @@ -1009,11 +996,12 @@ class GlusterFsDriverTestCase(test.TestCase): ctxt = context.RequestContext('fake_user', 'fake_project') - snap_ref = {'name': 'test snap (online)', - 'volume_id': self.VOLUME_UUID, - 'volume': volume, - 'id': self.SNAP_UUID, - 'context': ctxt} + snap_ref = fake_snapshot.fake_snapshot_obj( + ctxt, + display_name='test snap (online)', + volume_id=self.VOLUME_UUID, + id=self.SNAP_UUID) + snap_ref.context = ctxt snap_path = '%s.%s' % (volume_path, self.SNAP_UUID) snap_file = '%s.%s' % (volume_file, self.SNAP_UUID) @@ -1022,23 +1010,11 @@ class GlusterFsDriverTestCase(test.TestCase): mock.patch.object(db, 'snapshot_get') as mock_snapshot_get,\ mock.patch.object(drv, '_nova') as mock_nova,\ mock.patch.object(time, 'sleep') as mock_sleep: - snap_ref_progress = snap_ref.copy() - snap_ref_progress['status'] = fields.SnapshotStatus.CREATING - snap_ref_progress_0p = snap_ref_progress.copy() - snap_ref_progress_0p['progress'] = '0%' - - snap_ref_progress_50p = snap_ref_progress.copy() - snap_ref_progress_50p['progress'] = '50%' - - snap_ref_progress_99p = snap_ref_progress.copy() - snap_ref_progress_99p['progress'] = '99%' - snap_ref_progress_99p['status'] = fields.SnapshotStatus.ERROR - - mock_snapshot_get.side_effect = [ - snap_ref_progress_0p, snap_ref_progress_50p, - snap_ref_progress_99p - ] + mock_snapshot_get.side_effect = SideEffectList( + snap_ref, + progress = ['0%', '50%', '99%'], + status = ["creating", "creating", "error"]) self.assertRaisesAndMessageMatches( exception.RemoteFSException, @@ -1054,15 +1030,19 @@ class GlusterFsDriverTestCase(test.TestCase): drv = self._driver volume = self._simple_volume() - volume['status'] = 'in-use' + volume.status = 'in-use' ctxt = context.RequestContext('fake_user', 'fake_project') - snap_ref = {'name': 'test snap to delete (online)', - 'volume_id': self.VOLUME_UUID, - 'volume': volume, - 'id': self.SNAP_UUID, - 'context': ctxt} + snap_ref = fake_snapshot.fake_snapshot_obj( + ctxt, + display_name='test snap to delete (online)', + volume_id=self.VOLUME_UUID, + status="deleting", + id=self.SNAP_UUID) + snap_ref.context = ctxt + + snap_ref.volume = volume hashed = drv._get_hash_str(self.TEST_EXPORT1) volume_file = 'volume-%s' % self.VOLUME_UUID @@ -1120,22 +1100,8 @@ class GlusterFsDriverTestCase(test.TestCase): 'volume_id': self.VOLUME_UUID } - snap_ref_progress = snap_ref.copy() - snap_ref_progress['status'] = 'deleting' - - snap_ref_progress_0p = snap_ref_progress.copy() - snap_ref_progress_0p['progress'] = '0%' - - snap_ref_progress_50p = snap_ref_progress.copy() - snap_ref_progress_50p['progress'] = '50%' - - snap_ref_progress_90p = snap_ref_progress.copy() - snap_ref_progress_90p['progress'] = '90%' - - mock_snapshot_get.side_effect = [ - snap_ref_progress_0p, snap_ref_progress_50p, - snap_ref_progress_90p - ] + mock_snapshot_get.side_effect = SideEffectList( + snap_ref, progress = ['0%', '50%', '90%']) drv.delete_snapshot(snap_ref) @@ -1156,15 +1122,18 @@ class GlusterFsDriverTestCase(test.TestCase): drv = self._driver volume = self._simple_volume() - volume['status'] = 'in-use' + volume.status = 'in-use' ctxt = context.RequestContext('fake_user', 'fake_project') - snap_ref = {'name': 'test snap to delete (online)', - 'volume_id': self.VOLUME_UUID, - 'volume': volume, - 'id': self.SNAP_UUID, - 'context': ctxt} + snap_ref = fake_snapshot.fake_snapshot_obj( + ctxt, + display_name='test snap to delete (online)', + volume_id=self.VOLUME_UUID, + status='deleting', + id=self.SNAP_UUID) + snap_ref.volume = volume + snap_ref.context = ctxt hashed = drv._get_hash_str(self.TEST_EXPORT1) volume_file = 'volume-%s' % self.VOLUME_UUID @@ -1225,21 +1194,8 @@ class GlusterFsDriverTestCase(test.TestCase): 'file_to_merge': snap_file, 'volume_id': self.VOLUME_UUID} - snap_ref_progress = snap_ref.copy() - snap_ref_progress['status'] = 'deleting' - - snap_ref_progress_0p = snap_ref_progress.copy() - snap_ref_progress_0p['progress'] = '0%' - - snap_ref_progress_50p = snap_ref_progress.copy() - snap_ref_progress_50p['progress'] = '50%' - - snap_ref_progress_90p = snap_ref_progress.copy() - snap_ref_progress_90p['progress'] = '90%' - - mock_snapshot_get.side_effect = [ - snap_ref_progress_0p, snap_ref_progress_50p, - snap_ref_progress_90p] + mock_snapshot_get.side_effect = SideEffectList( + snap_ref, progress = ['0%', '50%', '90%']) drv.delete_snapshot(snap_ref) @@ -1260,15 +1216,17 @@ class GlusterFsDriverTestCase(test.TestCase): drv = self._driver volume = self._simple_volume() - volume['status'] = 'in-use' + volume.status = 'in-use' ctxt = context.RequestContext('fake_user', 'fake_project') - snap_ref = {'name': 'test snap to delete (online)', - 'volume_id': self.VOLUME_UUID, - 'volume': volume, - 'id': self.SNAP_UUID, - 'context': ctxt} + snap_ref = fake_snapshot.fake_snapshot_obj( + ctxt, + display_name='test snap to delete (online)', + volume_id=self.VOLUME_UUID, + id=self.SNAP_UUID) + snap_ref.volume = volume + snap_ref.context = ctxt hashed = drv._get_hash_str(self.TEST_EXPORT1) volume_file = 'volume-%s' % self.VOLUME_UUID @@ -1320,23 +1278,10 @@ class GlusterFsDriverTestCase(test.TestCase): return paths[args[0]] mock_qemu_img_info.side_effect = img_info_side_effect - - snap_ref_progress = snap_ref.copy() - snap_ref_progress['status'] = 'deleting' - - snap_ref_progress_0p = snap_ref_progress.copy() - snap_ref_progress_0p['progress'] = '0%' - - snap_ref_progress_50p = snap_ref_progress.copy() - snap_ref_progress_50p['progress'] = '50%' - - snap_ref_progress_90p = snap_ref_progress.copy() - snap_ref_progress_90p['status'] = 'error_deleting' - snap_ref_progress_90p['progress'] = '90%' - - mock_snapshot_get.side_effect = [ - snap_ref_progress_0p, snap_ref_progress_50p, - snap_ref_progress_90p] + mock_snapshot_get.side_effect = SideEffectList( + snap_ref, + progress = ['0%', '50%', '99%'], + status = ['deleting', 'deleting', 'error']) self.assertRaisesAndMessageMatches(exception.RemoteFSException, 'Unable to delete snapshot', drv.delete_snapshot, @@ -1355,9 +1300,9 @@ class GlusterFsDriverTestCase(test.TestCase): self.TEST_MNT_POINT_BASE) volume = self._simple_volume() - vol_filename = volume['name'] - vol_filename_2 = volume['name'] + '.abcd' - vol_filename_3 = volume['name'] + '.efef' + vol_filename = volume.name + vol_filename_2 = volume.name + '.abcd' + vol_filename_3 = volume.name + '.efef' hashed = drv._get_hash_str(self.TEST_EXPORT1) vol_dir = '%s/%s' % (self.TEST_MNT_POINT_BASE, hashed) vol_path = '%s/%s' % (vol_dir, vol_filename) @@ -1432,25 +1377,29 @@ class GlusterFsDriverTestCase(test.TestCase): vol_dir = os.path.join(self.TEST_MNT_POINT_BASE, drv._get_hash_str(self.TEST_EXPORT1)) - src_vol_path = os.path.join(vol_dir, src_volume['name']) - dest_vol_path = os.path.join(vol_dir, dest_volume['name']) - snapshot = {'volume_name': src_volume['name'], - 'name': 'clone-snap-%s' % src_volume['id'], - 'size': src_volume['size'], - 'volume_size': src_volume['size'], - 'volume_id': src_volume['id'], - 'id': 'tmp-snap-%s' % src_volume['id'], - 'volume': src_volume} - snap_file = dest_volume['name'] + '.' + snapshot['id'] - size = dest_volume['size'] + src_vol_path = os.path.join(vol_dir, src_volume.name) + dest_vol_path = os.path.join(vol_dir, dest_volume.name) + + snapshot = fake_snapshot.fake_snapshot_obj( + self.context, + volume_name=src_volume.name, + display_name='clone-snap-%s' % src_volume.id, + size=src_volume.size, + volume_size=src_volume.size, + volume_id=src_volume.id, + id=self.SNAP_UUID) + snapshot.volume = src_volume + + snap_file = dest_volume.name + '.' + snapshot.id + size = dest_volume.size mock_read_info_file.return_value = {'active': snap_file, - snapshot['id']: snap_file} + snapshot.id: snap_file} qemu_img_output = """image: %s file format: raw virtual size: 1.0G (1073741824 bytes) disk size: 173K backing file: %s - """ % (snap_file, src_volume['name']) + """ % (snap_file, src_volume.name) img_info = imageutils.QemuImgInfo(qemu_img_output) mock_qemu_img_info.return_value = img_info @@ -1464,18 +1413,20 @@ class GlusterFsDriverTestCase(test.TestCase): drv = self._driver src_volume = self._simple_volume() - snap_ref = {'volume_name': src_volume['name'], - 'name': 'clone-snap-%s' % src_volume['id'], - 'size': src_volume['size'], - 'volume_size': src_volume['size'], - 'volume_id': src_volume['id'], - 'id': 'tmp-snap-%s' % src_volume['id'], - 'volume': src_volume, - 'status': 'available'} + snap_ref = fake_snapshot.fake_snapshot_obj( + self.context, + volume_name=src_volume.name, + display_name='clone-snap-%s' % src_volume.id, + size=src_volume.size, + volume_size=src_volume.size, + volume_id=src_volume.id, + id=self.SNAP_UUID, + status='available') + snap_ref.volume = src_volume - new_volume = DumbVolume() - new_volume['id'] = self.VOLUME_UUID - new_volume['size'] = snap_ref['size'] + new_volume = fake_volume.fake_volume_obj(self.context, + id=self.VOLUME_UUID, + size=snap_ref.volume.size) with mock.patch.object(drv, '_ensure_shares_mounted') as \ mock_ensure_shares_mounted,\ @@ -1492,7 +1443,7 @@ class GlusterFsDriverTestCase(test.TestCase): mock_do_create_volume.assert_called_once_with(new_volume) mock_copy_volume.assert_called_once_with(snap_ref, new_volume, - new_volume['size']) + new_volume.size) def test_initialize_connection(self): drv = self._driver @@ -1502,14 +1453,14 @@ class GlusterFsDriverTestCase(test.TestCase): file format: raw virtual size: 1.0G (1073741824 bytes) disk size: 173K - """ % volume['name'] + """ % volume.name img_info = imageutils.QemuImgInfo(qemu_img_output) with mock.patch.object(drv, 'get_active_image_from_info') as \ mock_get_active_image_from_info,\ mock.patch.object(image_utils, 'qemu_img_info') as \ mock_qemu_img_info: - mock_get_active_image_from_info.return_value = volume['name'] + mock_get_active_image_from_info.return_value = volume.name mock_qemu_img_info.return_value = img_info conn_info = drv.initialize_connection(volume, None) @@ -1539,7 +1490,7 @@ class GlusterFsDriverTestCase(test.TestCase): mock_backup_volume: ctxt = context.RequestContext('fake_user', 'fake_project') volume = self._simple_volume() - backup = {'volume_id': volume['id']} + backup = {'volume_id': volume.id} mock_volume_get.return_value = volume mock_get_active_image_from_info.return_value = '/some/path' @@ -1566,7 +1517,7 @@ class GlusterFsDriverTestCase(test.TestCase): mock_backup_volume: ctxt = context.RequestContext('fake_user', 'fake_project') volume = self._simple_volume() - backup = {'volume_id': volume['id']} + backup = {'volume_id': volume.id} mock_volume_get.return_value = volume mock_get_active_image_from_info.return_value = '/some/file2' @@ -1586,7 +1537,7 @@ class GlusterFsDriverTestCase(test.TestCase): mock_snapshot_get_all_for_volume: ctxt = context.RequestContext('fake_user', 'fake_project') volume = self._simple_volume() - backup = {'volume_id': volume['id']} + backup = {'volume_id': volume.id} mock_snapshot_get_all_for_volume.return_value = [ {'snap1': 'a'}, {'snap2': 'b'} @@ -1606,7 +1557,7 @@ class GlusterFsDriverTestCase(test.TestCase): mock_qemu_img_info: ctxt = context.RequestContext('fake_user', 'fake_project') volume = self._simple_volume() - backup = {'volume_id': volume['id']} + backup = {'volume_id': volume.id} mock_volume_get.return_value = volume mock_get_active_image_from_info.return_value = '/some/path/file2' @@ -1629,7 +1580,7 @@ class GlusterFsDriverTestCase(test.TestCase): mock.patch.object(drv, '_qemu_img_info') as mock_qemu_img_info: ctxt = context.RequestContext('fake_user', 'fake_project') volume = self._simple_volume() - backup = {'volume_id': volume['id']} + backup = {'volume_id': volume.id} mock_volume_get.return_value = volume mock_get_active_image_from_info.return_value = '/some/path' @@ -1651,7 +1602,7 @@ class GlusterFsDriverTestCase(test.TestCase): drv = self._driver volume = self._simple_volume() - volume_path = '%s/%s' % (self.TEST_MNT_POINT, volume['name']) + volume_path = '%s/%s' % (self.TEST_MNT_POINT, volume.name) image_meta = {'id': '10958016-e196-42e3-9e7f-5d8927ae3099'} with mock.patch.object(drv, 'get_active_image_from_info') as \ @@ -1664,7 +1615,7 @@ class GlusterFsDriverTestCase(test.TestCase): mock_upload_volume, \ mock.patch.object(image_utils, 'create_temporary_file') as \ mock_create_temporary_file: - mock_get_active_image_from_info.return_value = volume['name'] + mock_get_active_image_from_info.return_value = volume.name mock_local_volume_dir.return_value = self.TEST_MNT_POINT @@ -1674,7 +1625,7 @@ class GlusterFsDriverTestCase(test.TestCase): file format: raw virtual size: 1.0G (1073741824 bytes) disk size: 173K - """ % volume['name'] + """ % volume.name img_info = imageutils.QemuImgInfo(qemu_img_output) mock_qemu_img_info.return_value = img_info @@ -1694,7 +1645,7 @@ class GlusterFsDriverTestCase(test.TestCase): drv = self._driver volume = self._simple_volume() - volume_path = '%s/%s' % (self.TEST_MNT_POINT, volume['name']) + volume_path = '%s/%s' % (self.TEST_MNT_POINT, volume.name) image_meta = {'id': '10958016-e196-42e3-9e7f-5d8927ae3099'} with mock.patch.object(drv, 'get_active_image_from_info') as \ @@ -1709,7 +1660,7 @@ class GlusterFsDriverTestCase(test.TestCase): mock_upload_volume, \ mock.patch.object(image_utils, 'create_temporary_file') as \ mock_create_temporary_file: - mock_get_active_image_from_info.return_value = volume['name'] + mock_get_active_image_from_info.return_value = volume.name mock_local_volume_dir.return_value = self.TEST_MNT_POINT @@ -1719,7 +1670,7 @@ class GlusterFsDriverTestCase(test.TestCase): file format: qcow2 virtual size: 1.0G (1073741824 bytes) disk size: 173K - """ % volume['name'] + """ % volume.name img_info = imageutils.QemuImgInfo(qemu_img_output) mock_qemu_img_info.return_value = img_info @@ -1757,7 +1708,7 @@ class GlusterFsDriverTestCase(test.TestCase): mock_upload_volume, \ mock.patch.object(image_utils, 'create_temporary_file') as \ mock_create_temporary_file: - mock_get_active_image_from_info.return_value = volume['name'] + mock_get_active_image_from_info.return_value = volume.name mock_local_volume_dir.return_value = self.TEST_MNT_POINT diff --git a/cinder/tests/unit/test_netapp_nfs.py b/cinder/tests/unit/test_netapp_nfs.py index 8931453a631..3bf0ed091c3 100644 --- a/cinder/tests/unit/test_netapp_nfs.py +++ b/cinder/tests/unit/test_netapp_nfs.py @@ -23,9 +23,11 @@ import mock from mox3 import mox as mox_lib import six +from cinder import context from cinder import exception from cinder.image import image_utils from cinder import test +from cinder.tests.unit import fake_volume from cinder import utils as cinder_utils from cinder.volume import configuration as conf from cinder.volume.drivers.netapp import common @@ -158,6 +160,7 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase): self._driver.ssc_library = mock.Mock() config = self._driver.configuration config.netapp_vserver = FAKE_VSERVER + self.context = context.get_admin_context() def test_create_snapshot(self): """Test snapshot can be created and deleted.""" @@ -519,7 +522,8 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase): def test_clone_image_cloneableshare_raw(self): drv = self._driver mox = self.mox - volume = {'name': 'vol', 'size': '20'} + volume = fake_volume.fake_volume_obj(self.context, size=20) + volume_name = 'volume-%s' % volume.id mox.StubOutWithMock(utils, 'get_volume_extra_specs') mox.StubOutWithMock(drv, '_find_image_in_cache') mox.StubOutWithMock(drv, '_is_cloneable_share') @@ -541,11 +545,11 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase): image_utils.qemu_img_info('/mnt/img-id', run_as_root=True).\ AndReturn(self.get_img_info('raw')) drv._clone_backing_file_for_volume( - 'img-id', 'vol', share='127.0.0.1:/share', volume_id=None) + 'img-id', volume_name, share='127.0.0.1:/share', volume_id=None) drv._get_mount_point_for_share(mox_lib.IgnoreArg()).AndReturn('/mnt') drv._discover_file_till_timeout(mox_lib.IgnoreArg()).AndReturn(True) - drv._set_rw_permissions('/mnt/vol') - drv._resize_image_file({'name': 'vol'}, mox_lib.IgnoreArg()) + drv._set_rw_permissions('/mnt/%s' % volume_name) + drv._resize_image_file({'name': volume_name}, mox_lib.IgnoreArg()) mox.ReplayAll() drv.clone_image( @@ -559,7 +563,8 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase): def test_clone_image_cloneableshare_notraw(self): drv = self._driver mox = self.mox - volume = {'name': 'vol', 'size': '20'} + volume = fake_volume.fake_volume_obj(self.context, size=20) + volume_name = 'volume-%s' % volume.id mox.StubOutWithMock(utils, 'get_volume_extra_specs') mox.StubOutWithMock(drv, '_find_image_in_cache') mox.StubOutWithMock(drv, '_is_cloneable_share') @@ -572,6 +577,7 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase): mox.StubOutWithMock(image_utils, 'convert_image') mox.StubOutWithMock(drv, '_register_image_in_cache') mox.StubOutWithMock(drv, '_is_share_clone_compatible') + mox.StubOutWithMock(drv, '_do_qos_for_volume') utils.get_volume_extra_specs(mox_lib.IgnoreArg()) drv._find_image_in_cache(mox_lib.IgnoreArg()).AndReturn([]) @@ -585,13 +591,15 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase): image_utils.convert_image(mox_lib.IgnoreArg(), mox_lib.IgnoreArg(), 'raw', run_as_root=True) - image_utils.qemu_img_info('/mnt/vol', run_as_root=True).\ + image_utils.qemu_img_info('/mnt/%s' % volume_name, run_as_root=True).\ AndReturn(self.get_img_info('raw')) drv._register_image_in_cache(mox_lib.IgnoreArg(), mox_lib.IgnoreArg()) + drv._do_qos_for_volume(mox_lib.IgnoreArg(), mox_lib.IgnoreArg()) + drv._get_mount_point_for_share('127.0.0.1:/share').AndReturn('/mnt') drv._discover_file_till_timeout(mox_lib.IgnoreArg()).AndReturn(True) - drv._set_rw_permissions('/mnt/vol') - drv._resize_image_file({'name': 'vol'}, mox_lib.IgnoreArg()) + drv._set_rw_permissions('/mnt/%s' % volume_name) + drv._resize_image_file({'name': volume_name}, mox_lib.IgnoreArg()) mox.ReplayAll() drv.clone_image( @@ -605,7 +613,8 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase): def test_clone_image_file_not_discovered(self): drv = self._driver mox = self.mox - volume = {'name': 'vol', 'size': '20'} + volume = fake_volume.fake_volume_obj(self.context, size=20) + volume_name = 'volume-%s' % volume.id mox.StubOutWithMock(utils, 'get_volume_extra_specs') mox.StubOutWithMock(drv, '_find_image_in_cache') mox.StubOutWithMock(drv, '_is_cloneable_share') @@ -631,12 +640,12 @@ class NetAppCmodeNfsDriverTestCase(test.TestCase): image_utils.convert_image(mox_lib.IgnoreArg(), mox_lib.IgnoreArg(), 'raw', run_as_root=True) - image_utils.qemu_img_info('/mnt/vol', run_as_root=True).\ + image_utils.qemu_img_info('/mnt/%s' % volume_name, run_as_root=True).\ AndReturn(self.get_img_info('raw')) drv._register_image_in_cache(mox_lib.IgnoreArg(), mox_lib.IgnoreArg()) drv._do_qos_for_volume(mox_lib.IgnoreArg(), mox_lib.IgnoreArg()) - drv.local_path(mox_lib.IgnoreArg()).AndReturn('/mnt/vol') + drv.local_path(mox_lib.IgnoreArg()).AndReturn('/mnt/%s' % volume_name) drv._discover_file_till_timeout(mox_lib.IgnoreArg()).AndReturn(False) mox.ReplayAll() @@ -1180,6 +1189,8 @@ class NetAppCmodeNfsDriverOnlyTestCase(test.TestCase): 'spec': None, } + self.context = context.get_admin_context() + @mock.patch.object(utils, 'LOG', mock.Mock()) def test_create_volume(self): drv = self._driver @@ -1234,47 +1245,48 @@ class NetAppCmodeNfsDriverOnlyTestCase(test.TestCase): def test_copy_img_to_vol_copyoffload_success(self): drv = self._driver - context = object() - volume = {'id': 'vol_id', 'name': 'name'} + volume = fake_volume.fake_volume_obj(self.context) image_service = object() image_id = 'image_id' drv.zapi_client.get_ontapi_version = mock.Mock(return_value=(1, 20)) drv._copy_from_img_service = mock.Mock() drv._get_provider_location = mock.Mock(return_value='share') - drv._get_vol_for_share = mock.Mock(return_value='vol') + drv._get_vol_for_share = mock.Mock(return_value=volume.id) drv._update_stale_vols = mock.Mock() - drv.copy_image_to_volume(context, volume, image_service, image_id) - drv._copy_from_img_service.assert_called_once_with(context, volume, + drv.copy_image_to_volume(self.context, volume, image_service, image_id) + drv._copy_from_img_service.assert_called_once_with(self.context, + volume, image_service, image_id) - drv._update_stale_vols.assert_called_once_with('vol') + drv._update_stale_vols.assert_called_once_with(volume.id) def test_copy_img_to_vol_copyoffload_failure(self): drv = self._driver - context = object() - volume = {'id': 'vol_id', 'name': 'name'} + volume = fake_volume.fake_volume_obj(self.context) + image_service = object() image_id = 'image_id' drv.zapi_client.get_ontapi_version = mock.Mock(return_value=(1, 20)) drv._copy_from_img_service = mock.Mock(side_effect=Exception()) nfs_base.NetAppNfsDriver.copy_image_to_volume = mock.Mock() drv._get_provider_location = mock.Mock(return_value='share') - drv._get_vol_for_share = mock.Mock(return_value='vol') + drv._get_vol_for_share = mock.Mock(return_value=volume.id) drv._update_stale_vols = mock.Mock() - drv.copy_image_to_volume(context, volume, image_service, image_id) - drv._copy_from_img_service.assert_called_once_with(context, volume, + drv.copy_image_to_volume(self.context, volume, image_service, image_id) + drv._copy_from_img_service.assert_called_once_with(self.context, + volume, image_service, image_id) nfs_base.NetAppNfsDriver.copy_image_to_volume. \ - assert_called_once_with(context, volume, image_service, image_id) - drv._update_stale_vols.assert_called_once_with('vol') + assert_called_once_with(self.context, volume, + image_service, image_id) + drv._update_stale_vols.assert_called_once_with(volume.id) def test_copy_img_to_vol_copyoffload_nonexistent_binary_path(self): drv = self._driver - context = object() - volume = {'id': 'vol_id', 'name': 'name'} + volume = fake_volume.fake_volume_obj(self.context) image_service = mock.Mock() image_service.get_location.return_value = (mock.Mock(), mock.Mock()) image_service.show.return_value = {'size': 0} @@ -1296,15 +1308,14 @@ class NetAppCmodeNfsDriverOnlyTestCase(test.TestCase): # Verify the original error is propagated self.assertRaises(OSError, drv._copy_from_img_service, - context, volume, image_service, image_id) + self.context, volume, image_service, image_id) @mock.patch.object(image_utils, 'qemu_img_info') def test_img_service_raw_copyoffload_workflow_success(self, mock_qemu_img_info): drv = self._driver - volume = {'id': 'vol_id', 'name': 'name', 'size': 1} + volume = fake_volume.fake_volume_obj(self.context, size=1) image_id = 'image_id' - context = object() image_service = mock.Mock() image_service.get_location.return_value = ('nfs://ip1/openstack/img', None) @@ -1329,9 +1340,10 @@ class NetAppCmodeNfsDriverOnlyTestCase(test.TestCase): drv._clone_file_dst_exists = mock.Mock() drv._post_clone_image = mock.Mock() - drv._copy_from_img_service(context, volume, image_service, image_id) + drv._copy_from_img_service(self.context, volume, + image_service, image_id) drv._get_ip_verify_on_cluster.assert_any_call('ip1') - drv._get_export_path.assert_called_with('vol_id') + drv._get_export_path.assert_called_with(volume.id) drv._check_share_can_hold_size.assert_called_with('share', 1) assert drv._execute.call_count == 1 @@ -1344,9 +1356,8 @@ class NetAppCmodeNfsDriverOnlyTestCase(test.TestCase): mock_qemu_img_info, mock_cvrt_image): drv = self._driver - volume = {'id': 'vol_id', 'name': 'name', 'size': 1} + volume = fake_volume.fake_volume_obj(self.context, size=1) image_id = 'image_id' - context = object() image_service = mock.Mock() image_service.get_location.return_value = ('nfs://ip1/openstack/img', None) @@ -1371,9 +1382,10 @@ class NetAppCmodeNfsDriverOnlyTestCase(test.TestCase): drv._clone_file_dst_exists = mock.Mock() drv._post_clone_image = mock.Mock() - drv._copy_from_img_service(context, volume, image_service, image_id) + drv._copy_from_img_service(self.context, volume, + image_service, image_id) drv._get_ip_verify_on_cluster.assert_any_call('ip1') - drv._get_export_path.assert_called_with('vol_id') + drv._get_export_path.assert_called_with(volume.id) drv._check_share_can_hold_size.assert_called_with('share', 1) assert mock_cvrt_image.call_count == 1 assert drv._execute.call_count == 1 @@ -1393,6 +1405,7 @@ class NetApp7modeNfsDriverTestCase(NetAppCmodeNfsDriverTestCase): self._driver = netapp_nfs_7mode.NetApp7modeNfsDriver( configuration=create_configuration()) self._driver.zapi_client = mock.Mock() + self.context = context.get_admin_context() def _prepare_delete_snapshot_mock(self, snapshot_exists): drv = self._driver diff --git a/cinder/tests/unit/test_nfs.py b/cinder/tests/unit/test_nfs.py index 82b10fcc810..bc1615f693f 100644 --- a/cinder/tests/unit/test_nfs.py +++ b/cinder/tests/unit/test_nfs.py @@ -21,25 +21,16 @@ import os import mock from oslo_utils import units +from cinder import context from cinder import exception from cinder.image import image_utils from cinder import test +from cinder.tests.unit import fake_volume from cinder.volume import configuration as conf from cinder.volume.drivers import nfs from cinder.volume.drivers import remotefs -class DumbVolume(object): - # TODO(eharney): replace this with an autospecced mock class - fields = {} - - def __setitem__(self, key, value): - self.fields[key] = value - - def __getitem__(self, item): - return self.fields[item] - - class RemoteFsDriverTestCase(test.TestCase): TEST_FILE_NAME = 'test.txt' TEST_EXPORT = 'nas-host1:/export' @@ -346,18 +337,19 @@ class NfsDriverTestCase(test.TestCase): mock_exc = mock.patch.object(self._driver, '_execute') self._execute = mock_exc.start() self.addCleanup(mock_exc.stop) + self.context = context.get_admin_context() def test_local_path(self): """local_path common use case.""" self.configuration.nfs_mount_point_base = self.TEST_MNT_POINT_BASE drv = self._driver - volume = DumbVolume() - volume['provider_location'] = self.TEST_NFS_EXPORT1 - volume['name'] = 'volume-123' + volume = fake_volume.fake_volume_obj( + self.context, + provider_location=self.TEST_NFS_EXPORT1) self.assertEqual( - '/mnt/test/2f4f60214cf43c595666dd815f0360a4/volume-123', + '/mnt/test/2f4f60214cf43c595666dd815f0360a4/%s' % volume.name, drv.local_path(volume)) @mock.patch.object(image_utils, 'qemu_img_info') @@ -366,8 +358,9 @@ class NfsDriverTestCase(test.TestCase): def test_copy_image_to_volume(self, mock_fetch, mock_resize, mock_qemu): """resize_image common case usage.""" drv = self._driver - TEST_IMG_SOURCE = 'foo.img' - volume = {'size': self.TEST_SIZE_IN_GB, 'name': TEST_IMG_SOURCE} + volume = fake_volume.fake_volume_obj(self.context, + size=self.TEST_SIZE_IN_GB) + TEST_IMG_SOURCE = 'volume-%s' % volume.id with mock.patch.object(drv, 'local_path', return_value=TEST_IMG_SOURCE): @@ -581,12 +574,10 @@ class NfsDriverTestCase(test.TestCase): self.assertEqual(2, mock_get_capacity_info.call_count) def _simple_volume(self): - volume = DumbVolume() - volume['provider_location'] = '127.0.0.1:/mnt' - volume['name'] = 'volume_name' - volume['size'] = 10 - - return volume + return fake_volume.fake_volume_obj(self.context, + display_name='volume_name', + provider_location='127.0.0.1:/mnt', + size=10) def test_create_sparsed_volume(self): drv = self._driver @@ -626,13 +617,14 @@ class NfsDriverTestCase(test.TestCase): """create_volume ensures shares provided in config are mounted.""" drv = self._driver drv._find_share = mock.Mock() + drv._find_share.return_value = self.TEST_NFS_EXPORT1 drv._do_create_volume = mock.Mock() with mock.patch.object( drv, '_ensure_share_mounted') as mock_ensure_share: drv._ensure_share_mounted() - volume = DumbVolume() - volume['size'] = self.TEST_SIZE_IN_GB + volume = fake_volume.fake_volume_obj(self.context, + size=self.TEST_SIZE_IN_GB) drv.create_volume(volume) mock_ensure_share.assert_called_once_with() @@ -646,8 +638,8 @@ class NfsDriverTestCase(test.TestCase): with mock.patch.object(drv, '_find_share') as mock_find_share: mock_find_share.return_value = self.TEST_NFS_EXPORT1 - volume = DumbVolume() - volume['size'] = self.TEST_SIZE_IN_GB + volume = fake_volume.fake_volume_obj(self.context, + size=self.TEST_SIZE_IN_GB) result = drv.create_volume(volume) self.assertEqual(self.TEST_NFS_EXPORT1, result['provider_location']) @@ -658,9 +650,10 @@ class NfsDriverTestCase(test.TestCase): drv = self._driver drv._ensure_share_mounted = mock.Mock() - volume = DumbVolume() - volume['name'] = 'volume-123' - volume['provider_location'] = self.TEST_NFS_EXPORT1 + volume = fake_volume.fake_volume_obj( + self.context, + display_name='volume-123', + provider_location=self.TEST_NFS_EXPORT1) with mock.patch.object(drv, 'local_path') as mock_local_path: mock_local_path.return_value = self.TEST_LOCAL_PATH @@ -673,9 +666,10 @@ class NfsDriverTestCase(test.TestCase): def test_delete_should_ensure_share_mounted(self): """delete_volume should ensure that corresponding share is mounted.""" drv = self._driver - volume = DumbVolume() - volume['name'] = 'volume-123' - volume['provider_location'] = self.TEST_NFS_EXPORT1 + volume = fake_volume.fake_volume_obj( + self.context, + display_name='volume-123', + provider_location=self.TEST_NFS_EXPORT1) with mock.patch.object( drv, '_ensure_share_mounted') as mock_ensure_share: @@ -685,9 +679,9 @@ class NfsDriverTestCase(test.TestCase): def test_delete_should_not_delete_if_provider_location_not_provided(self): """delete_volume shouldn't delete if provider_location missed.""" drv = self._driver - volume = DumbVolume() - volume['name'] = 'volume-123' - volume['provider_location'] = None + volume = fake_volume.fake_volume_obj(self.context, + name='volume-123', + provider_location=None) with mock.patch.object(drv, '_ensure_share_mounted'): drv.delete_volume(volume) @@ -846,8 +840,11 @@ class NfsDriverTestCase(test.TestCase): def test_extend_volume(self): """Extend a volume by 1.""" drv = self._driver - volume = {'id': '80ee16b6-75d2-4d54-9539-ffc1b4b0fb10', 'size': 1, - 'provider_location': 'nfs_share'} + volume = fake_volume.fake_volume_obj( + self.context, + id='80ee16b6-75d2-4d54-9539-ffc1b4b0fb10', + size=1, + provider_location='nfs_share') path = 'path' newSize = volume['size'] + 1 @@ -865,8 +862,11 @@ class NfsDriverTestCase(test.TestCase): def test_extend_volume_failure(self): """Error during extend operation.""" drv = self._driver - volume = {'id': '80ee16b6-75d2-4d54-9539-ffc1b4b0fb10', 'size': 1, - 'provider_location': 'nfs_share'} + volume = fake_volume.fake_volume_obj( + self.context, + id='80ee16b6-75d2-4d54-9539-ffc1b4b0fb10', + size=1, + provider_location='nfs_share') with mock.patch.object(image_utils, 'resize_image'): with mock.patch.object(drv, 'local_path', return_value='path'): @@ -880,8 +880,11 @@ class NfsDriverTestCase(test.TestCase): def test_extend_volume_insufficient_space(self): """Insufficient space on nfs_share during extend operation.""" drv = self._driver - volume = {'id': '80ee16b6-75d2-4d54-9539-ffc1b4b0fb10', 'size': 1, - 'provider_location': 'nfs_share'} + volume = fake_volume.fake_volume_obj( + self.context, + id='80ee16b6-75d2-4d54-9539-ffc1b4b0fb10', + size=1, + provider_location='nfs_share') with mock.patch.object(image_utils, 'resize_image'): with mock.patch.object(drv, 'local_path', return_value='path'): @@ -1125,8 +1128,8 @@ class NfsDriverDoSetupTestCase(test.TestCase): def _test_update_migrated_volume(self, volume_status, rename_volume, rename_exception=False): drv = nfs.NfsDriver(configuration=self.configuration) - fake_volume_id = 'vol1' - fake_new_volume_id = 'vol2' + fake_volume_id = 'f51b5730-13b7-11e6-a238-fa163e67a298' + fake_new_volume_id = '12341234-13b7-11e6-a238-fa163e67a298' fake_provider_source = 'fake_provider_source' fake_provider = 'fake_provider' base_dir = '/dir_base/' @@ -1135,27 +1138,34 @@ class NfsDriverDoSetupTestCase(test.TestCase): current_name = volume_name_template % fake_new_volume_id original_volume_path = base_dir + original_volume_name current_path = base_dir + current_name - fake_volume = {'size': 1, 'id': fake_volume_id, - 'provider_location': fake_provider_source, - '_name_id': None} - fake_new_volume = {'size': 1, 'id': fake_new_volume_id, - 'provider_location': fake_provider, - '_name_id': None} + volume = fake_volume.fake_volume_obj( + self.context, + id=fake_volume_id, + size=1, + provider_location=fake_provider_source, + _name_id=None) + + new_volume = fake_volume.fake_volume_obj( + self.context, + id=fake_new_volume_id, + size=1, + provider_location=fake_provider, + _name_id=None) with mock.patch.object(drv, 'local_path') as local_path: local_path.return_value = base_dir + current_name if volume_status == 'in-use': update = drv.update_migrated_volume(self.context, - fake_volume, - fake_new_volume, + volume, + new_volume, volume_status) self.assertEqual({'_name_id': fake_new_volume_id, 'provider_location': fake_provider}, update) elif rename_exception: rename_volume.side_effect = OSError update = drv.update_migrated_volume(self.context, - fake_volume, - fake_new_volume, + volume, + new_volume, volume_status) rename_volume.assert_called_once_with(current_path, original_volume_path) @@ -1163,8 +1173,8 @@ class NfsDriverDoSetupTestCase(test.TestCase): 'provider_location': fake_provider}, update) else: update = drv.update_migrated_volume(self.context, - fake_volume, - fake_new_volume, + volume, + new_volume, volume_status) rename_volume.assert_called_once_with(current_path, original_volume_path) @@ -1175,7 +1185,7 @@ class NfsDriverDoSetupTestCase(test.TestCase): "Ensure that driver.retype() is there.""" drv = nfs.NfsDriver(configuration=self.configuration) - v1 = DumbVolume() + v1 = fake_volume.fake_volume_obj(self.context) ret = drv.retype(self.context, v1, diff --git a/cinder/tests/unit/test_quobyte.py b/cinder/tests/unit/test_quobyte.py index 9f8bb2ab242..624349171de 100644 --- a/cinder/tests/unit/test_quobyte.py +++ b/cinder/tests/unit/test_quobyte.py @@ -30,6 +30,8 @@ from cinder import context from cinder import exception from cinder.image import image_utils from cinder import test +from cinder.tests.unit import fake_snapshot +from cinder.tests.unit import fake_volume from cinder.volume import configuration as conf from cinder.volume.drivers import quobyte @@ -37,16 +39,6 @@ from cinder.volume.drivers import quobyte CONF = cfg.CONF -class DumbVolume(object): - fields = {} - - def __setitem__(self, key, value): - self.fields[key] = value - - def __getitem__(self, item): - return self.fields[item] - - class FakeDb(object): msg = "Tests are broken: mock this out." @@ -66,7 +58,6 @@ class QuobyteDriverTestCase(test.TestCase): TEST_SIZE_IN_GB = 1 TEST_MNT_POINT = '/mnt/quobyte' TEST_MNT_POINT_BASE = '/mnt' - TEST_LOCAL_PATH = '/mnt/quobyte/volume-123' TEST_FILE_NAME = 'test.txt' TEST_SHARES_CONFIG_FILE = '/etc/cinder/test-shares.conf' TEST_TMP_FILE = '/tmp/tempfile' @@ -94,6 +85,7 @@ class QuobyteDriverTestCase(test.TestCase): db=FakeDb()) self._driver.shares = {} self._driver.set_nas_security_options(is_new_cinder_install=False) + self.context = context.get_admin_context() def assertRaisesAndMessageMatches( self, excClass, msg, callableObj, *args, **kwargs): @@ -115,13 +107,11 @@ class QuobyteDriverTestCase(test.TestCase): def test_local_path(self): """local_path common use case.""" drv = self._driver - - volume = DumbVolume() - volume['provider_location'] = self.TEST_QUOBYTE_VOLUME - volume['name'] = 'volume-123' + vol_id = self.VOLUME_UUID + volume = self._simple_volume(_name_id=vol_id) self.assertEqual( - '/mnt/1331538734b757ed52d0e18c0a7210cd/volume-123', + '/mnt/1331538734b757ed52d0e18c0a7210cd/volume-%s' % vol_id, drv.local_path(volume)) def test_mount_quobyte_should_mount_correctly(self): @@ -428,19 +418,18 @@ class QuobyteDriverTestCase(test.TestCase): # Future ones might do and therefore we mocked it. self.assertGreaterEqual(mock_get_available_capacity.call_count, 0) - def _simple_volume(self, uuid=None): - volume = DumbVolume() - volume['provider_location'] = self.TEST_QUOBYTE_VOLUME - if uuid is None: - volume['id'] = self.VOLUME_UUID - else: - volume['id'] = uuid - # volume['name'] mirrors format from db/sqlalchemy/models.py - volume['name'] = 'volume-%s' % volume['id'] - volume['size'] = 10 - volume['status'] = 'available' + def _simple_volume(self, **kwargs): + updates = {'id': self.VOLUME_UUID, + 'provider_location': self.TEST_QUOBYTE_VOLUME, + 'display_name': 'volume-%s' % self.VOLUME_UUID, + 'size': 10, + 'status': 'available'} - return volume + updates.update(kwargs) + if 'display_name' not in updates: + updates['display_name'] = 'volume-%s' % updates['id'] + + return fake_volume.fake_volume_obj(self.context, **updates) def test_create_sparsed_volume(self): drv = self._driver @@ -501,11 +490,11 @@ class QuobyteDriverTestCase(test.TestCase): drv.LOG = mock.Mock() drv._find_share = mock.Mock() + drv._find_share.return_value = self.TEST_QUOBYTE_VOLUME drv._do_create_volume = mock.Mock() drv._ensure_shares_mounted = mock.Mock() - volume = DumbVolume() - volume['size'] = self.TEST_SIZE_IN_GB + volume = self._simple_volume(size=self.TEST_SIZE_IN_GB) drv.create_volume(volume) drv._find_share.assert_called_once_with(mock.ANY) @@ -521,8 +510,7 @@ class QuobyteDriverTestCase(test.TestCase): drv._do_create_volume = mock.Mock() drv._find_share = mock.Mock(return_value=self.TEST_QUOBYTE_VOLUME) - volume = DumbVolume() - volume['size'] = self.TEST_SIZE_IN_GB + volume = self._simple_volume(size=self.TEST_SIZE_IN_GB) result = drv.create_volume(volume) self.assertEqual(self.TEST_QUOBYTE_VOLUME, result['provider_location']) @@ -575,9 +563,7 @@ class QuobyteDriverTestCase(test.TestCase): drv._execute = mock.Mock() - volume = DumbVolume() - volume['name'] = 'volume-123' - volume['provider_location'] = self.TEST_QUOBYTE_VOLUME + volume = self._simple_volume(display_name='volume-123') drv._ensure_share_mounted = mock.Mock() @@ -596,9 +582,8 @@ class QuobyteDriverTestCase(test.TestCase): drv._ensure_share_mounted = mock.Mock() drv._execute = mock.Mock() - volume = DumbVolume() - volume['name'] = 'volume-123' - volume['provider_location'] = None + volume = self._simple_volume(display_name='volume-123', + provider_location=None) drv.delete_volume(volume) @@ -638,7 +623,7 @@ class QuobyteDriverTestCase(test.TestCase): # lots of test vars to be prepared at first dest_volume = self._simple_volume( - 'c1073000-0000-0000-0000-0000000c1073') + id='c1073000-0000-0000-0000-0000000c1073') src_volume = self._simple_volume() vol_dir = os.path.join(self.TEST_MNT_POINT_BASE, @@ -647,13 +632,15 @@ class QuobyteDriverTestCase(test.TestCase): dest_vol_path = os.path.join(vol_dir, dest_volume['name']) info_path = os.path.join(vol_dir, src_volume['name']) + '.info' - snapshot = {'volume_name': src_volume['name'], - 'name': 'clone-snap-%s' % src_volume['id'], - 'size': src_volume['size'], - 'volume_size': src_volume['size'], - 'volume_id': src_volume['id'], - 'id': 'tmp-snap-%s' % src_volume['id'], - 'volume': src_volume} + snapshot = fake_snapshot.fake_snapshot_obj( + self.context, + volume_name=src_volume.name, + display_name='clone-snap-%s' % src_volume.id, + size=src_volume.size, + volume_size=src_volume.size, + volume_id=src_volume.id, + id=self.SNAP_UUID) + snapshot.volume = src_volume snap_file = dest_volume['name'] + '.' + snapshot['id'] snap_path = os.path.join(vol_dir, snap_file) @@ -692,17 +679,18 @@ class QuobyteDriverTestCase(test.TestCase): drv = self._driver src_volume = self._simple_volume() - snap_ref = {'volume_name': src_volume['name'], - 'name': 'clone-snap-%s' % src_volume['id'], - 'size': src_volume['size'], - 'volume_size': src_volume['size'], - 'volume_id': src_volume['id'], - 'id': 'tmp-snap-%s' % src_volume['id'], - 'volume': src_volume, - 'status': 'error'} - new_volume = DumbVolume() - new_volume['size'] = snap_ref['size'] + snap_ref = fake_snapshot.fake_snapshot_obj( + self.context, + volume_name=src_volume.name, + display_name='clone-snap-%s' % src_volume.id, + volume_size=src_volume.size, + volume_id=src_volume.id, + id=self.SNAP_UUID, + status='error') + snap_ref.volume = src_volume + + new_volume = self._simple_volume(size=snap_ref.volume_size) self.assertRaises(exception.InvalidSnapshot, drv.create_volume_from_snapshot, @@ -713,17 +701,18 @@ class QuobyteDriverTestCase(test.TestCase): drv = self._driver src_volume = self._simple_volume() - snap_ref = {'volume_name': src_volume['name'], - 'name': 'clone-snap-%s' % src_volume['id'], - 'size': src_volume['size'], - 'volume_size': src_volume['size'], - 'volume_id': src_volume['id'], - 'id': 'tmp-snap-%s' % src_volume['id'], - 'volume': src_volume, - 'status': 'available'} - new_volume = DumbVolume() - new_volume['size'] = snap_ref['size'] + snap_ref = fake_snapshot.fake_snapshot_obj( + self.context, + volume_name=src_volume.name, + display_name='clone-snap-%s' % src_volume.id, + volume_size=src_volume.size, + volume_id=src_volume.id, + id=self.SNAP_UUID, + status='available') + snap_ref.volume = src_volume + + new_volume = self._simple_volume(size=snap_ref.volume_size) drv._ensure_shares_mounted = mock.Mock() drv._find_share = mock.Mock(return_value=self.TEST_QUOBYTE_VOLUME) diff --git a/cinder/tests/unit/test_remotefs.py b/cinder/tests/unit/test_remotefs.py index d461d7abcc5..2bf9947a19a 100644 --- a/cinder/tests/unit/test_remotefs.py +++ b/cinder/tests/unit/test_remotefs.py @@ -12,15 +12,19 @@ # License for the specific language governing permissions and limitations # under the License. +import collections import copy import os import ddt import mock +from cinder import context from cinder import exception from cinder.image import image_utils from cinder import test +from cinder.tests.unit import fake_snapshot +from cinder.tests.unit import fake_volume from cinder import utils from cinder.volume.drivers import remotefs @@ -28,25 +32,7 @@ from cinder.volume.drivers import remotefs @ddt.ddt class RemoteFsSnapDriverTestCase(test.TestCase): - _FAKE_CONTEXT = 'fake_context' - _FAKE_VOLUME_ID = '4f711859-4928-4cb7-801a-a50c37ceaccc' - _FAKE_VOLUME_NAME = 'volume-%s' % _FAKE_VOLUME_ID - _FAKE_VOLUME = {'id': _FAKE_VOLUME_ID, - 'size': 1, - 'provider_location': 'fake_share', - 'name': _FAKE_VOLUME_NAME, - 'status': 'available'} _FAKE_MNT_POINT = '/mnt/fake_hash' - _FAKE_VOLUME_PATH = os.path.join(_FAKE_MNT_POINT, - _FAKE_VOLUME_NAME) - _FAKE_SNAPSHOT_ID = '5g811859-4928-4cb7-801a-a50c37ceacba' - _FAKE_SNAPSHOT = {'context': _FAKE_CONTEXT, - 'id': _FAKE_SNAPSHOT_ID, - 'volume': _FAKE_VOLUME, - 'volume_id': _FAKE_VOLUME_ID, - 'status': 'available', - 'volume_size': 1} - _FAKE_SNAPSHOT_PATH = (_FAKE_VOLUME_PATH + '.' + _FAKE_SNAPSHOT_ID) def setUp(self): super(RemoteFsSnapDriverTestCase, self).setUp() @@ -55,21 +41,32 @@ class RemoteFsSnapDriverTestCase(test.TestCase): 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.volume = self._fake_volume + def _test_delete_snapshot(self, volume_in_use=False, stale_snapshot=False, is_active_image=True): # If the snapshot is not the active image, it is guaranteed that # another snapshot exists having it as backing file. - fake_snapshot_name = os.path.basename(self._FAKE_SNAPSHOT_PATH) + fake_snapshot_name = os.path.basename(self._fake_snapshot_path) fake_info = {'active': fake_snapshot_name, - self._FAKE_SNAPSHOT['id']: fake_snapshot_name} + self._fake_snapshot.id: fake_snapshot_name} fake_snap_img_info = mock.Mock() fake_base_img_info = mock.Mock() if stale_snapshot: fake_snap_img_info.backing_file = None else: - fake_snap_img_info.backing_file = self._FAKE_VOLUME_NAME + fake_snap_img_info.backing_file = self._fake_volume.name fake_snap_img_info.file_format = 'qcow2' fake_base_img_info.backing_file = None fake_base_img_info.file_format = 'raw' @@ -91,72 +88,71 @@ class RemoteFsSnapDriverTestCase(test.TestCase): expected_info = { 'active': fake_snapshot_name, - self._FAKE_SNAPSHOT_ID: fake_snapshot_name + self._fake_snapshot.id: fake_snapshot_name } if volume_in_use: - fake_snapshot = copy.deepcopy(self._FAKE_SNAPSHOT) - fake_snapshot['volume']['status'] = 'in-use' + self._fake_snapshot.volume.status = 'in-use' self._driver._read_info_file.return_value = fake_info - self._driver._delete_snapshot(fake_snapshot) + self._driver._delete_snapshot(self._fake_snapshot) if stale_snapshot: self._driver._delete_stale_snapshot.assert_called_once_with( - fake_snapshot) + self._fake_snapshot) else: expected_online_delete_info = { 'active_file': fake_snapshot_name, 'snapshot_file': fake_snapshot_name, - 'base_file': self._FAKE_VOLUME_NAME, + 'base_file': self._fake_volume.name, 'base_id': None, 'new_base_file': None } self._driver._delete_snapshot_online.assert_called_once_with( - self._FAKE_CONTEXT, fake_snapshot, + self.context, self._fake_snapshot, expected_online_delete_info) elif is_active_image: self._driver._read_info_file.return_value = fake_info - self._driver._delete_snapshot(self._FAKE_SNAPSHOT) + self._driver._delete_snapshot(self._fake_snapshot) self._driver._img_commit.assert_called_once_with( - self._FAKE_SNAPSHOT_PATH) - self.assertNotIn(self._FAKE_SNAPSHOT_ID, fake_info) + self._fake_snapshot_path) + self.assertNotIn(self._fake_snapshot.id, fake_info) self._driver._write_info_file.assert_called_once_with( mock.sentinel.fake_info_path, fake_info) else: fake_upper_snap_id = 'fake_upper_snap_id' fake_upper_snap_path = ( - self._FAKE_VOLUME_PATH + '-snapshot' + fake_upper_snap_id) + self._fake_volume_path + '-snapshot' + fake_upper_snap_id) fake_upper_snap_name = os.path.basename(fake_upper_snap_path) fake_backing_chain = [ {'filename': fake_upper_snap_name, 'backing-filename': fake_snapshot_name}, {'filename': fake_snapshot_name, - 'backing-filename': self._FAKE_VOLUME_NAME}, - {'filename': self._FAKE_VOLUME_NAME, + 'backing-filename': self._fake_volume.name}, + {'filename': self._fake_volume.name, 'backing-filename': None}] fake_info[fake_upper_snap_id] = fake_upper_snap_name - fake_info[self._FAKE_SNAPSHOT_ID] = fake_snapshot_name + fake_info[self._fake_snapshot.id] = fake_snapshot_name fake_info['active'] = fake_upper_snap_name expected_info = copy.deepcopy(fake_info) - del expected_info[self._FAKE_SNAPSHOT_ID] + del expected_info[self._fake_snapshot.id] self._driver._read_info_file.return_value = fake_info self._driver._get_backing_chain_for_path = mock.Mock( return_value=fake_backing_chain) - self._driver._delete_snapshot(self._FAKE_SNAPSHOT) + self._driver._delete_snapshot(self._fake_snapshot) self._driver._img_commit.assert_called_once_with( - self._FAKE_SNAPSHOT_PATH) + self._fake_snapshot_path) self._driver._rebase_img.assert_called_once_with( - fake_upper_snap_path, self._FAKE_VOLUME_NAME, + fake_upper_snap_path, self._fake_volume.name, fake_base_img_info.file_format) self._driver._write_info_file.assert_called_once_with( mock.sentinel.fake_info_path, expected_info) @@ -175,12 +171,12 @@ class RemoteFsSnapDriverTestCase(test.TestCase): self._test_delete_snapshot(is_active_image=False) def test_delete_stale_snapshot(self): - fake_snapshot_name = os.path.basename(self._FAKE_SNAPSHOT_PATH) + fake_snapshot_name = os.path.basename(self._fake_snapshot_path) fake_snap_info = { - 'active': self._FAKE_VOLUME_NAME, - self._FAKE_SNAPSHOT_ID: fake_snapshot_name + 'active': self._fake_volume.name, + self._fake_snapshot.id: fake_snapshot_name } - expected_info = {'active': self._FAKE_VOLUME_NAME} + expected_info = {'active': self._fake_volume.name} self._driver._local_path_volume_info = mock.Mock( return_value=mock.sentinel.fake_info_path) @@ -190,42 +186,41 @@ class RemoteFsSnapDriverTestCase(test.TestCase): return_value=self._FAKE_MNT_POINT) self._driver._write_info_file = mock.Mock() - self._driver._delete_stale_snapshot(self._FAKE_SNAPSHOT) + self._driver._delete_stale_snapshot(self._fake_snapshot) - self._driver._delete.assert_called_once_with(self._FAKE_SNAPSHOT_PATH) + self._driver._delete.assert_called_once_with(self._fake_snapshot_path) self._driver._write_info_file.assert_called_once_with( mock.sentinel.fake_info_path, expected_info) def test_do_create_snapshot(self): self._driver._local_volume_dir = mock.Mock( - return_value=self._FAKE_VOLUME_PATH) + return_value=self._fake_volume_path) fake_backing_path = os.path.join( self._driver._local_volume_dir(), - self._FAKE_VOLUME_NAME) + self._fake_volume.name) self._driver._execute = mock.Mock() self._driver._set_rw_permissions = mock.Mock() self._driver._qemu_img_info = mock.Mock( return_value=mock.Mock(file_format=mock.sentinel.backing_fmt)) - self._driver._do_create_snapshot(self._FAKE_SNAPSHOT, - self._FAKE_VOLUME_NAME, - self._FAKE_SNAPSHOT_PATH) + self._driver._do_create_snapshot(self._fake_snapshot, + self._fake_volume.name, + self._fake_snapshot_path) command1 = ['qemu-img', 'create', '-f', 'qcow2', '-o', 'backing_file=%s' % fake_backing_path, - self._FAKE_SNAPSHOT_PATH] + self._fake_snapshot_path] command2 = ['qemu-img', 'rebase', '-u', - '-b', self._FAKE_VOLUME_NAME, + '-b', self._fake_volume.name, '-F', mock.sentinel.backing_fmt, - self._FAKE_SNAPSHOT_PATH] + self._fake_snapshot_path] self._driver._execute.assert_any_call(*command1, run_as_root=True) self._driver._execute.assert_any_call(*command2, run_as_root=True) def _test_create_snapshot(self, volume_in_use=False): - fake_snapshot = copy.deepcopy(self._FAKE_SNAPSHOT) fake_snapshot_info = {} - fake_snapshot_file_name = os.path.basename(self._FAKE_SNAPSHOT_PATH) + fake_snapshot_file_name = os.path.basename(self._fake_snapshot_path) self._driver._local_path_volume_info = mock.Mock( return_value=mock.sentinel.fake_info_path) @@ -235,28 +230,28 @@ class RemoteFsSnapDriverTestCase(test.TestCase): self._driver._create_snapshot_online = mock.Mock() self._driver._write_info_file = mock.Mock() self._driver.get_active_image_from_info = mock.Mock( - return_value=self._FAKE_VOLUME_NAME) + return_value=self._fake_volume.name) self._driver._get_new_snap_path = mock.Mock( - return_value=self._FAKE_SNAPSHOT_PATH) + return_value=self._fake_snapshot_path) expected_snapshot_info = { 'active': fake_snapshot_file_name, - self._FAKE_SNAPSHOT_ID: fake_snapshot_file_name + self._fake_snapshot.id: fake_snapshot_file_name } if volume_in_use: - fake_snapshot['volume']['status'] = 'in-use' + self._fake_snapshot.volume.status = 'in-use' expected_method_called = '_create_snapshot_online' else: - fake_snapshot['volume']['status'] = 'available' + self._fake_snapshot.volume.status = 'available' expected_method_called = '_do_create_snapshot' - self._driver._create_snapshot(fake_snapshot) + self._driver._create_snapshot(self._fake_snapshot) fake_method = getattr(self._driver, expected_method_called) fake_method.assert_called_with( - fake_snapshot, self._FAKE_VOLUME_NAME, - self._FAKE_SNAPSHOT_PATH) + self._fake_snapshot, self._fake_volume.name, + self._fake_snapshot_path) self._driver._write_info_file.assert_called_with( mock.sentinel.fake_info_path, expected_snapshot_info) @@ -268,12 +263,10 @@ class RemoteFsSnapDriverTestCase(test.TestCase): self._test_create_snapshot(volume_in_use=True) def test_create_snapshot_invalid_volume(self): - fake_snapshot = copy.deepcopy(self._FAKE_SNAPSHOT) - fake_snapshot['volume']['status'] = 'error' - + self._fake_snapshot.volume.status = 'error' self.assertRaises(exception.InvalidVolume, self._driver._create_snapshot, - fake_snapshot) + self._fake_snapshot) @mock.patch('cinder.db.snapshot_get') @mock.patch('time.sleep') @@ -294,18 +287,18 @@ class RemoteFsSnapDriverTestCase(test.TestCase): mock_do_create_snapshot: self.assertRaises(exception.RemoteFSConcurrentRequest, self._driver._create_snapshot_online, - self._FAKE_SNAPSHOT, - self._FAKE_VOLUME_NAME, - self._FAKE_SNAPSHOT_PATH) + self._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, - self._FAKE_SNAPSHOT_PATH) + self._fake_snapshot, self._fake_volume.name, + self._fake_snapshot_path) self.assertEqual([mock.call(1), mock.call(1)], mock_sleep.call_args_list) self.assertEqual(3, mock_snapshot_get.call_count) - mock_snapshot_get.assert_called_with(self._FAKE_SNAPSHOT['context'], - self._FAKE_SNAPSHOT['id']) + mock_snapshot_get.assert_called_with(self._fake_snapshot._context, + self._fake_snapshot.id) @mock.patch.object(utils, 'synchronized') def _locked_volume_operation_test_helper(self, mock_synchronized, func, @@ -318,7 +311,7 @@ class RemoteFsSnapDriverTestCase(test.TestCase): mock_synchronized.side_effect = mock_decorator expected_lock = '%s-%s' % (self._driver.driver_prefix, - self._FAKE_VOLUME_ID) + self._fake_volume.id) if expected_exception: self.assertRaises(expected_exception, func, @@ -332,7 +325,8 @@ class RemoteFsSnapDriverTestCase(test.TestCase): self.assertEqual(mock.sentinel.ret_val, ret_val) def test_locked_volume_id_operation(self): - mock_volume = {'id': self._FAKE_VOLUME_ID} + mock_volume = mock.Mock() + mock_volume.id = self._fake_volume.id @remotefs.locked_volume_id_operation def synchronized_func(inst, volume): @@ -342,7 +336,8 @@ class RemoteFsSnapDriverTestCase(test.TestCase): volume=mock_volume) def test_locked_volume_id_snapshot_operation(self): - mock_snapshot = {'volume': {'id': self._FAKE_VOLUME_ID}} + mock_snapshot = mock.Mock() + mock_snapshot.volume.id = self._fake_volume.id @remotefs.locked_volume_id_operation def synchronized_func(inst, snapshot): @@ -432,22 +427,36 @@ class RemoteFsSnapDriverTestCase(test.TestCase): mock.patch.object(drv, '_copy_volume_from_snapshot') as \ mock_copy_volume_from_snapshot: - volume = self._FAKE_VOLUME.copy() - src_vref = self._FAKE_VOLUME.copy() - src_vref['id'] = '375e32b2-804a-49f2-b282-85d1d5a5b9e1' - src_vref['name'] = 'volume-%s' % src_vref['id'] - volume_ref = {'id': volume['id'], - 'name': volume['name'], - 'status': volume['status'], - 'provider_location': volume['provider_location'], - 'size': volume['size']} - snap_ref = {'volume_name': volume['name'], - 'name': 'clone-snap-%s' % src_vref['id'], - 'size': src_vref['size'], - 'volume_size': src_vref['size'], - 'volume_id': src_vref['id'], - 'id': 'tmp-snap-%s' % src_vref['id'], - 'volume': src_vref} + 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) + + 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) + + 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) mock_create_snapshot.assert_called_once_with(snap_ref) diff --git a/cinder/tests/unit/test_scality.py b/cinder/tests/unit/test_scality.py index 1988230311f..34484336985 100644 --- a/cinder/tests/unit/test_scality.py +++ b/cinder/tests/unit/test_scality.py @@ -26,27 +26,26 @@ from six.moves import urllib from cinder import context from cinder import exception from cinder import test +from cinder.tests.unit import fake_snapshot +from cinder.tests.unit import fake_volume from cinder.volume import configuration as conf import cinder.volume.drivers.scality as driver -_FAKE_VOLUME = {'name': 'volume-a79d463e-1fd5-11e5-a6ff-5b81bfee8544', - 'id': 'a79d463e-1fd5-11e5-a6ff-5b81bfee8544', - 'provider_location': 'fake_share'} -_FAKE_SNAPSHOT = {'id': 'ae3d6da2-1fd5-11e5-967f-1b8cf3b401ab', - 'volume': _FAKE_VOLUME, - 'status': 'available', - 'provider_location': None, - 'volume_size': 1, - 'name': 'snapshot-ae3d6da2-1fd5-11e5-967f-1b8cf3b401ab'} +_FAKE_VOLUME_ID = 'a79d463e-1fd5-11e5-a6ff-5b81bfee8544' +_FAKE_VOLUME_NAME = 'volume-a79d463e-1fd5-11e5-a6ff-5b81bfee8544' + +_FAKE_SNAPSHOT_ID = 'ae3d6da2-1fd5-11e5-967f-1b8cf3b401ab' +_FAKE_SNAPSHOT_NAME = 'snapshot-ae3d6da2-1fd5-11e5-967f-1b8cf3b401ab' + _FAKE_BACKUP = {'id': '914849d2-2585-11e5-be54-d70ca0c343d6', - 'volume_id': _FAKE_VOLUME['id']} + 'volume_id': _FAKE_VOLUME_ID} _FAKE_MNT_POINT = '/tmp' _FAKE_SOFS_CONFIG = '/etc/sfused.conf' _FAKE_VOLUME_DIR = 'cinder/volumes' _FAKE_VOL_BASEDIR = os.path.join(_FAKE_MNT_POINT, _FAKE_VOLUME_DIR, '00') -_FAKE_VOL_PATH = os.path.join(_FAKE_VOL_BASEDIR, _FAKE_VOLUME['name']) -_FAKE_SNAP_PATH = os.path.join(_FAKE_VOL_BASEDIR, _FAKE_SNAPSHOT['name']) +_FAKE_VOL_PATH = os.path.join(_FAKE_VOL_BASEDIR, _FAKE_VOLUME_NAME) +_FAKE_SNAP_PATH = os.path.join(_FAKE_VOL_BASEDIR, _FAKE_SNAPSHOT_NAME) _FAKE_MOUNTS_TABLE = [['tmpfs /dev/shm\n'], ['fuse ' + _FAKE_MNT_POINT + '\n']] @@ -66,6 +65,30 @@ class ScalityDriverTestCase(test.TestCase): self.drv = driver.ScalityDriver(configuration=self.cfg) self.drv.db = mock.Mock() + self.context = context.get_admin_context() + + def _simple_volume(self, **kwargs): + updates = {'display_name': _FAKE_VOLUME_NAME, + 'id': _FAKE_VOLUME_ID, + 'provider_location': 'fake_share'} + updates.update(kwargs) + + return fake_volume.fake_volume_obj(self.context, **updates) + + def _simple_snapshot(self, **kwargs): + updates = {'id': _FAKE_SNAPSHOT_ID, + 'display_name': _FAKE_SNAPSHOT_NAME, + 'status': 'available', + 'provider_location': None, + 'volume_size': 1} + + updates.update(kwargs) + snapshot = fake_snapshot.fake_snapshot_obj(self.context, **updates) + volume = self._simple_volume() + snapshot.volume = volume + + return snapshot + @mock.patch.object(driver.urllib.request, 'urlopen') @mock.patch('os.access') def test_check_for_setup_error(self, mock_os_access, mock_urlopen): @@ -206,27 +229,28 @@ class ScalityDriverTestCase(test.TestCase): def test_initialize_connection(self, mock_qemu_img_info): info = imageutils.QemuImgInfo() info.file_format = 'raw' - info.image = _FAKE_VOLUME['name'] + info.image = _FAKE_VOLUME_NAME mock_qemu_img_info.return_value = info + volume = self._simple_volume() with mock.patch.object(self.drv, 'get_active_image_from_info') as \ mock_get_active_image_from_info: - mock_get_active_image_from_info.return_value = _FAKE_VOLUME['name'] - conn_info = self.drv.initialize_connection(_FAKE_VOLUME, None) + mock_get_active_image_from_info.return_value = _FAKE_VOLUME_NAME + conn_info = self.drv.initialize_connection(volume, None) expected_conn_info = { 'driver_volume_type': driver.ScalityDriver.driver_volume_type, 'mount_point_base': _FAKE_MNT_POINT, 'data': { - 'export': _FAKE_VOLUME['provider_location'], - 'name': _FAKE_VOLUME['name'], - 'sofs_path': 'cinder/volumes/00/' + _FAKE_VOLUME['name'], + 'export': volume.provider_location, + 'name': volume.name, + 'sofs_path': 'cinder/volumes/00/' + volume.name, 'format': 'raw' } } self.assertEqual(expected_conn_info, conn_info) - mock_get_active_image_from_info.assert_called_once_with(_FAKE_VOLUME) + mock_get_active_image_from_info.assert_called_once_with(volume) mock_qemu_img_info.assert_called_once_with(_FAKE_VOL_PATH) @mock.patch("cinder.image.image_utils.resize_image") @@ -236,7 +260,7 @@ class ScalityDriverTestCase(test.TestCase): info.file_format = 'raw' mock_qemu_img_info.return_value = info - self.drv.extend_volume(_FAKE_VOLUME, 2) + self.drv.extend_volume(self._simple_volume(), 2) mock_qemu_img_info.assert_called_once_with(_FAKE_VOL_PATH) @@ -249,7 +273,7 @@ class ScalityDriverTestCase(test.TestCase): mock_qemu_img_info.return_value = info self.assertRaises(exception.InvalidVolume, - self.drv.extend_volume, _FAKE_VOLUME, 2) + self.drv.extend_volume, self._simple_volume(), 2) @mock.patch("cinder.image.image_utils.resize_image") @mock.patch("cinder.image.image_utils.convert_image") @@ -260,8 +284,8 @@ class ScalityDriverTestCase(test.TestCase): mock.patch.object(self.drv, '_set_rw_permissions_for_all') as \ mock_set_rw_permissions: mock_read_info_file.side_effect = IOError(errno.ENOENT, '') - self.drv._copy_volume_from_snapshot(_FAKE_SNAPSHOT, - _FAKE_VOLUME, 1) + self.drv._copy_volume_from_snapshot(self._simple_snapshot(), + self._simple_volume(), 1) mock_read_info_file.assert_called_once_with("%s.info" % _FAKE_VOL_PATH) mock_convert_image.assert_called_once_with(_FAKE_SNAP_PATH, @@ -276,10 +300,11 @@ class ScalityDriverTestCase(test.TestCase): def test_copy_volume_from_snapshot(self, mock_qemu_img_info, mock_convert_image, mock_resize_image): - new_volume = {'name': 'volume-3fa63b02-1fe5-11e5-b492-abf97a8fb23b', - 'id': '3fa63b02-1fe5-11e5-b492-abf97a8fb23b', - 'provider_location': 'fake_share'} - new_vol_path = os.path.join(_FAKE_VOL_BASEDIR, new_volume['name']) + new_volume = self._simple_volume( + display_name='volume-3fa63b02-1fe5-11e5-b492-abf97a8fb23b', + id='3fa63b02-1fe5-11e5-b492-abf97a8fb23b', + provider_location='fake_share') + new_vol_path = os.path.join(_FAKE_VOL_BASEDIR, new_volume.name) info = imageutils.QemuImgInfo() info.file_format = 'raw' @@ -290,7 +315,7 @@ class ScalityDriverTestCase(test.TestCase): mock_read_info_file, \ mock.patch.object(self.drv, '_set_rw_permissions_for_all') as \ mock_set_rw_permissions: - self.drv._copy_volume_from_snapshot(_FAKE_SNAPSHOT, + self.drv._copy_volume_from_snapshot(self._simple_snapshot(), new_volume, 1) mock_read_info_file.assert_called_once_with("%s.info" % _FAKE_VOL_PATH) @@ -311,9 +336,9 @@ class ScalityDriverTestCase(test.TestCase): info.file_format = 'raw' mock_qemu_img_info.return_value = info - backup = {'volume_id': _FAKE_VOLUME['id']} + backup = {'volume_id': _FAKE_VOLUME_ID} mock_backup_service = mock.MagicMock() - self.drv.db.volume_get.return_value = _FAKE_VOLUME + self.drv.db.volume_get.return_value = self._simple_volume() self.drv.backup_volume(context, backup, mock_backup_service) @@ -330,7 +355,7 @@ class ScalityDriverTestCase(test.TestCase): info.file_format = 'qcow2' mock_qemu_img_info.return_value = info - self.drv.db.volume_get.return_value = _FAKE_VOLUME + self.drv.db.volume_get.return_value = self._simple_volume() self.assertRaises(exception.InvalidVolume, self.drv.backup_volume, context, _FAKE_BACKUP, mock.MagicMock()) @@ -345,8 +370,8 @@ class ScalityDriverTestCase(test.TestCase): info.backing_file = 'fake.img' mock_qemu_img_info.return_value = info - backup = {'volume_id': _FAKE_VOLUME['id']} - self.drv.db.volume_get.return_value = _FAKE_VOLUME + backup = {'volume_id': _FAKE_VOLUME_ID} + self.drv.db.volume_get.return_value = self._simple_volume() self.assertRaises(exception.InvalidVolume, self.drv.backup_volume, context, backup, mock.MagicMock()) @@ -358,10 +383,10 @@ class ScalityDriverTestCase(test.TestCase): def test_restore_bakup(self, mock_open, mock_temporary_chown): mock_backup_service = mock.MagicMock() - self.drv.restore_backup(context, _FAKE_BACKUP, _FAKE_VOLUME, + self.drv.restore_backup(context, _FAKE_BACKUP, self._simple_volume(), mock_backup_service) mock_temporary_chown.assert_called_once_with(_FAKE_VOL_PATH) mock_open.assert_called_once_with(_FAKE_VOL_PATH, 'wb') mock_backup_service.restore.assert_called_once_with( - _FAKE_BACKUP, _FAKE_VOLUME['id'], mock_open().__enter__()) + _FAKE_BACKUP, _FAKE_VOLUME_ID, mock_open().__enter__()) diff --git a/cinder/tests/unit/test_smbfs.py b/cinder/tests/unit/test_smbfs.py index 3a6bb47d30a..a4a73139249 100644 --- a/cinder/tests/unit/test_smbfs.py +++ b/cinder/tests/unit/test_smbfs.py @@ -21,11 +21,10 @@ import mock from oslo_utils import fileutils from cinder import context -from cinder import db from cinder import exception from cinder.image import image_utils -from cinder import objects from cinder import test +from cinder.tests.unit import fake_snapshot from cinder.tests.unit import fake_volume from cinder.volume.drivers import remotefs from cinder.volume.drivers import smbfs @@ -39,7 +38,7 @@ def requires_allocation_data_update(expected_size): inst._smbfs_driver, 'update_disk_allocation_data') as fake_update: func(inst, *args, **kwargs) - fake_update.assert_called_once_with(inst._FAKE_VOLUME, + fake_update.assert_called_once_with(inst.volume, expected_size) return inner return wrapper @@ -55,18 +54,10 @@ class SmbFsTestCase(test.TestCase): _FAKE_TOTAL_SIZE = '2048' _FAKE_TOTAL_AVAILABLE = '1024' _FAKE_TOTAL_ALLOCATED = 1024 - _FAKE_VOLUME = {'id': '4f711859-4928-4cb7-801a-a50c37ceaccc', - 'size': 1, - 'provider_location': _FAKE_SHARE, - 'name': _FAKE_VOLUME_NAME, - 'status': 'available'} _FAKE_MNT_POINT = os.path.join(_FAKE_MNT_BASE, _FAKE_SHARE_HASH) _FAKE_VOLUME_PATH = os.path.join(_FAKE_MNT_POINT, _FAKE_VOLUME_NAME) - _FAKE_SNAPSHOT_ID = '5g811859-4928-4cb7-801a-a50c37ceacba' - _FAKE_SNAPSHOT = {'id': _FAKE_SNAPSHOT_ID, - 'volume': _FAKE_VOLUME, - 'status': 'available', - 'volume_size': 1} + _FAKE_VOLUME_SIZE = 1 + _FAKE_SNAPSHOT_ID = '50811859-4928-4cb7-801a-a50c37ceacba' _FAKE_SNAPSHOT_PATH = ( _FAKE_VOLUME_PATH + '-snapshot' + _FAKE_SNAPSHOT_ID) _FAKE_SHARE_OPTS = '-o username=Administrator,password=12345' @@ -93,6 +84,22 @@ class SmbFsTestCase(test.TestCase): self._smbfs_driver.base = self._FAKE_MNT_BASE self._smbfs_driver._alloc_info_file_path = ( self._FAKE_ALLOCATION_DATA_PATH) + self.context = context.get_admin_context() + + self.volume = fake_volume.fake_volume_obj( + self.context, + id='4f711859-4928-4cb7-801a-a50c37ceaccc', + size=self._FAKE_VOLUME_SIZE, + provider_location=self._FAKE_SHARE, + display_name=self._FAKE_VOLUME_NAME, + status='available') + + self.snapshot = fake_snapshot.fake_snapshot_obj( + self.context, + id=self._FAKE_SNAPSHOT_ID, + status='available', + volume_size=1) + self.snapshot.volume = self.volume self.addCleanup(mock.patch.stopall) @@ -137,24 +144,24 @@ class SmbFsTestCase(test.TestCase): fake_alloc_data = self._get_fake_allocation_data() if volume_exists: fake_alloc_data[self._FAKE_SHARE_HASH][ - self._FAKE_VOLUME_NAME] = self._FAKE_VOLUME['size'] + self._FAKE_VOLUME_NAME] = self.volume.size self._smbfs_driver._allocation_data = fake_alloc_data - self._smbfs_driver.update_disk_allocation_data(self._FAKE_VOLUME, + self._smbfs_driver.update_disk_allocation_data(self.volume, virtual_size_gb) vol_allocated_size = fake_alloc_data[self._FAKE_SHARE_HASH].get( self._FAKE_VOLUME_NAME, None) if not virtual_size_gb: expected_total_allocated = (self._FAKE_TOTAL_ALLOCATED - - self._FAKE_VOLUME['size']) + self.volume.size) self.assertIsNone(vol_allocated_size) else: expected_total_allocated = (self._FAKE_TOTAL_ALLOCATED + virtual_size_gb - - self._FAKE_VOLUME['size']) + self.volume.size) self.assertEqual(virtual_size_gb, vol_allocated_size) update_func.assert_called_once_with() @@ -168,11 +175,11 @@ class SmbFsTestCase(test.TestCase): def test_update_allocation_data_volume_extended(self): self._test_update_allocation_data( - virtual_size_gb=self._FAKE_VOLUME['size'] + 1) + virtual_size_gb=self.volume.size + 1) def test_update_allocation_data_volume_created(self): self._test_update_allocation_data( - virtual_size_gb=self._FAKE_VOLUME['size']) + virtual_size_gb=self.volume.size) @requires_allocation_data_update(expected_size=None) def test_delete_volume(self): @@ -191,7 +198,7 @@ class SmbFsTestCase(test.TestCase): return_value=fake_vol_info) with mock.patch('os.path.exists', lambda x: True): - drv.delete_volume(self._FAKE_VOLUME) + drv.delete_volume(self.volume) fake_ensure_mounted.assert_called_once_with(self._FAKE_SHARE) drv._delete.assert_any_call( @@ -266,12 +273,12 @@ class SmbFsTestCase(test.TestCase): if volume_exists: self.assertRaises(exception.InvalidVolume, self._smbfs_driver._do_create_volume, - self._FAKE_VOLUME) + self.volume) return - self._smbfs_driver._do_create_volume(self._FAKE_VOLUME) + self._smbfs_driver._do_create_volume(self.volume) expected_create_args = [mock.sentinel.vol_path, - self._FAKE_VOLUME['size']] + self.volume.size] if volume_format in [self._smbfs_driver._DISK_FORMAT_VHDX, self._smbfs_driver._DISK_FORMAT_VHD]: expected_create_args.append(volume_format) @@ -320,14 +327,14 @@ class SmbFsTestCase(test.TestCase): if not mounted_shares: self.assertRaises(exception.SmbfsNoSharesMounted, self._smbfs_driver._find_share, - self._FAKE_VOLUME['size']) + self.volume.size) elif not eligible_shares: self.assertRaises(exception.SmbfsNoSuitableShareFound, self._smbfs_driver._find_share, - self._FAKE_VOLUME['size']) + self.volume.size) else: ret_value = self._smbfs_driver._find_share( - self._FAKE_VOLUME['size']) + self.volume.size) # The eligible share with the minimum allocated space # will be selected self.assertEqual('fake_share3', ret_value) @@ -404,12 +411,12 @@ class SmbFsTestCase(test.TestCase): expected_vol_path if volume_exists else None) mock_get_volume_format.return_value = volume_format - ret_val = drv.local_path(self._FAKE_VOLUME) + ret_val = drv.local_path(self.volume) if volume_exists: self.assertFalse(mock_get_volume_format.called) else: - mock_get_volume_format.assert_called_once_with(self._FAKE_VOLUME) + mock_get_volume_format.assert_called_once_with(self.volume) self.assertEqual(expected_vol_path, ret_val) def test_get_existing_volume_path(self): @@ -422,7 +429,7 @@ class SmbFsTestCase(test.TestCase): def test_get_local_volume_path_template(self, mock_get_local_dir): mock_get_local_dir.return_value = self._FAKE_MNT_POINT ret_val = self._smbfs_driver._get_local_volume_path_template( - self._FAKE_VOLUME) + self.volume) self.assertEqual(self._FAKE_VOLUME_PATH, ret_val) @mock.patch('os.path.exists') @@ -457,7 +464,7 @@ class SmbFsTestCase(test.TestCase): mock_qemu_img_info.return_value.file_format = volume_format mock_get_format_spec.return_value = volume_format - ret_val = self._smbfs_driver.get_volume_format(self._FAKE_VOLUME, + ret_val = self._smbfs_driver.get_volume_format(self.volume, qemu_format) if volume_exists: @@ -465,7 +472,7 @@ class SmbFsTestCase(test.TestCase): self._FAKE_VOLUME_NAME) self.assertFalse(mock_get_format_spec.called) else: - mock_get_format_spec.assert_called_once_with(self._FAKE_VOLUME) + mock_get_format_spec.assert_called_once_with(self.volume) self.assertFalse(mock_qemu_img_info.called) return ret_val @@ -506,7 +513,7 @@ class SmbFsTestCase(test.TestCase): 'data': fake_data, 'mount_point_base': self._FAKE_MNT_BASE} ret_val = self._smbfs_driver.initialize_connection( - self._FAKE_VOLUME, None) + self.volume, None) self.assertEqual(expected, ret_val) @@ -529,9 +536,9 @@ class SmbFsTestCase(test.TestCase): if extend_failed: self.assertRaises(exception.ExtendVolumeError, drv.extend_volume, - self._FAKE_VOLUME, mock.sentinel.new_size) + self.volume, mock.sentinel.new_size) else: - drv.extend_volume(self._FAKE_VOLUME, mock.sentinel.new_size) + drv.extend_volume(self.volume, mock.sentinel.new_size) if image_format in (drv._DISK_FORMAT_VHDX, drv._DISK_FORMAT_VHD_LEGACY): @@ -576,14 +583,14 @@ class SmbFsTestCase(test.TestCase): if has_snapshots: self.assertRaises(exception.InvalidVolume, self._smbfs_driver._check_extend_volume_support, - self._FAKE_VOLUME, 2) + self.volume, 2) elif not is_eligible: self.assertRaises(exception.ExtendVolumeError, self._smbfs_driver._check_extend_volume_support, - self._FAKE_VOLUME, 2) + self.volume, 2) else: self._smbfs_driver._check_extend_volume_support( - self._FAKE_VOLUME, 2) + self.volume, 2) self._smbfs_driver._is_share_eligible.assert_called_once_with( self._FAKE_SHARE, 1) @@ -596,35 +603,35 @@ class SmbFsTestCase(test.TestCase): def test_check_extend_volume_uneligible_share(self): self._test_check_extend_support(is_eligible=False) - @requires_allocation_data_update(expected_size=_FAKE_VOLUME['size']) + @requires_allocation_data_update(expected_size=_FAKE_VOLUME_SIZE) @mock.patch.object(remotefs.RemoteFSSnapDriver, 'create_volume') def test_create_volume_base(self, mock_create_volume): - self._smbfs_driver.create_volume(self._FAKE_VOLUME) - mock_create_volume.assert_called_once_with(self._FAKE_VOLUME) + self._smbfs_driver.create_volume(self.volume) + mock_create_volume.assert_called_once_with(self.volume) - @requires_allocation_data_update(expected_size=_FAKE_VOLUME['size']) + @requires_allocation_data_update(expected_size=_FAKE_VOLUME_SIZE) @mock.patch.object(smbfs.SmbfsDriver, '_create_volume_from_snapshot') def test_create_volume_from_snapshot(self, mock_create_volume): - self._smbfs_driver.create_volume_from_snapshot(self._FAKE_VOLUME, - self._FAKE_SNAPSHOT) - mock_create_volume.assert_called_once_with(self._FAKE_VOLUME, - self._FAKE_SNAPSHOT) + self._smbfs_driver.create_volume_from_snapshot(self.volume, + self.snapshot) + mock_create_volume.assert_called_once_with(self.volume, + self.snapshot) - @requires_allocation_data_update(expected_size=_FAKE_VOLUME['size']) + @requires_allocation_data_update(expected_size=_FAKE_VOLUME_SIZE) @mock.patch.object(smbfs.SmbfsDriver, '_create_cloned_volume') def test_create_cloned_volume(self, mock_create_volume): - self._smbfs_driver.create_cloned_volume(self._FAKE_VOLUME, + self._smbfs_driver.create_cloned_volume(self.volume, mock.sentinel.src_vol) - mock_create_volume.assert_called_once_with(self._FAKE_VOLUME, + mock_create_volume.assert_called_once_with(self.volume, mock.sentinel.src_vol) - def test_create_volume_from_in_use_snapshot(self): - fake_snapshot = {'status': 'in-use'} + def test_create_volume_from_unavailable_snapshot(self): + self.snapshot.status = 'error' self.assertRaises( exception.InvalidSnapshot, self._smbfs_driver.create_volume_from_snapshot, - self._FAKE_VOLUME, fake_snapshot) + self.volume, self.snapshot) def test_copy_volume_from_snapshot(self): drv = self._smbfs_driver @@ -651,10 +658,10 @@ class SmbFsTestCase(test.TestCase): with mock.patch.object(image_utils, 'convert_image') as ( fake_convert_image): drv._copy_volume_from_snapshot( - self._FAKE_SNAPSHOT, self._FAKE_VOLUME, - self._FAKE_VOLUME['size']) + self.snapshot, self.volume, + self.volume.size) drv._extend_volume.assert_called_once_with( - self._FAKE_VOLUME, self._FAKE_VOLUME['size']) + self.volume, self.volume.size) fake_convert_image.assert_called_once_with( self._FAKE_VOLUME_PATH, self._FAKE_VOLUME_PATH[:-1], 'raw') @@ -668,7 +675,7 @@ class SmbFsTestCase(test.TestCase): def _test_copy_image_to_volume(self, wrong_size_after_fetch=False): drv = self._smbfs_driver - vol_size_bytes = self._FAKE_VOLUME['size'] << 30 + vol_size_bytes = self.volume.size << 30 fake_img_info = mock.MagicMock() @@ -696,12 +703,12 @@ class SmbFsTestCase(test.TestCase): self.assertRaises( exception.ImageUnacceptable, drv.copy_image_to_volume, - mock.sentinel.context, self._FAKE_VOLUME, + mock.sentinel.context, self.volume, mock.sentinel.image_service, mock.sentinel.image_id) else: drv.copy_image_to_volume( - mock.sentinel.context, self._FAKE_VOLUME, + mock.sentinel.context, self.volume, mock.sentinel.image_service, mock.sentinel.image_id) fake_fetch.assert_called_once_with( @@ -711,8 +718,8 @@ class SmbFsTestCase(test.TestCase): mock.sentinel.block_size) drv._do_extend_volume.assert_called_once_with( self._FAKE_VOLUME_PATH, - self._FAKE_VOLUME['size'], - self._FAKE_VOLUME['name']) + self.volume.size, + self.volume.name) def test_copy_image_to_volume(self): self._test_copy_image_to_volume() @@ -740,43 +747,34 @@ class SmbFsTestCase(test.TestCase): self._FAKE_TOTAL_ALLOCATED) self.assertEqual(expected, ret_val) - @ddt.data([True, False, False], - [False, False, False], - [True, True, True], - [False, True, True], - [False, False, True], - [True, False, True]) + @ddt.data([False, False], + [True, True], + [False, True]) @ddt.unpack - def test_get_volume_format_spec(self, volume_versioned_object, + def test_get_volume_format_spec(self, volume_meta_contains_fmt, volume_type_contains_fmt): + self._smbfs_driver.configuration = copy.copy(self._FAKE_SMBFS_CONFIG) + fake_vol_meta_fmt = 'vhd' fake_vol_type_fmt = 'vhdx' volume_metadata = {} volume_type_extra_specs = {} - fake_vol_dict = fake_volume.fake_db_volume() - del fake_vol_dict['name'] - if volume_meta_contains_fmt: volume_metadata['volume_format'] = fake_vol_meta_fmt elif volume_type_contains_fmt: - volume_type_extra_specs['smbfs:volume_format'] = fake_vol_type_fmt + volume_type_extra_specs['volume_format'] = fake_vol_type_fmt - ctxt = context.get_admin_context() - volume_type = db.volume_type_create( - ctxt, {'extra_specs': volume_type_extra_specs, - 'name': 'fake_vol_type'}) - fake_vol_dict.update(metadata=volume_metadata, - volume_type_id=volume_type.id) - # We want to get a 'real' SqlA model object, not just a dict. - volume = db.volume_create(ctxt, fake_vol_dict) - volume = db.volume_get(ctxt, volume.id) - - if volume_versioned_object: - volume = objects.Volume._from_db_object(ctxt, objects.Volume(), - volume) + volume_type = fake_volume.fake_volume_type_obj(self.context) + volume = fake_volume.fake_volume_obj(self.context) + # Optional arguments are not set in _from_db_object, + # so have to set explicitly here + volume.volume_type = volume_type + volume.metadata = volume_metadata + # Same for extra_specs and VolumeType + volume_type.extra_specs = volume_type_extra_specs resulted_fmt = self._smbfs_driver._get_volume_format_spec(volume) @@ -785,6 +783,6 @@ class SmbFsTestCase(test.TestCase): elif volume_type_contains_fmt: expected_fmt = fake_vol_type_fmt else: - expected_fmt = None + expected_fmt = self._FAKE_SMBFS_CONFIG.smbfs_default_volume_format self.assertEqual(expected_fmt, resulted_fmt) diff --git a/cinder/tests/unit/test_vzstorage.py b/cinder/tests/unit/test_vzstorage.py index cb05922eebe..29ca1e57789 100644 --- a/cinder/tests/unit/test_vzstorage.py +++ b/cinder/tests/unit/test_vzstorage.py @@ -21,9 +21,12 @@ import mock from os_brick.remotefs import remotefs from oslo_utils import units +from cinder import context from cinder import exception from cinder.image import image_utils from cinder import test +from cinder.tests.unit import fake_snapshot +from cinder.tests.unit import fake_volume from cinder.volume.drivers import vzstorage @@ -37,24 +40,16 @@ class VZStorageTestCase(test.TestCase): _FAKE_MNT_POINT = os.path.join(_FAKE_MNT_BASE, 'fake_hash') _FAKE_VOLUME_NAME = 'volume-4f711859-4928-4cb7-801a-a50c37ceaccc' _FAKE_VOLUME_PATH = os.path.join(_FAKE_MNT_POINT, _FAKE_VOLUME_NAME) - _FAKE_VOLUME = {'id': '4f711859-4928-4cb7-801a-a50c37ceaccc', - 'size': 1, - 'provider_location': _FAKE_SHARE, - 'name': _FAKE_VOLUME_NAME, - 'status': 'available'} - _FAKE_SNAPSHOT_ID = '5g811859-4928-4cb7-801a-a50c37ceacba' + _FAKE_SNAPSHOT_ID = '50811859-4928-4cb7-801a-a50c37ceacba' _FAKE_SNAPSHOT_PATH = ( _FAKE_VOLUME_PATH + '-snapshot' + _FAKE_SNAPSHOT_ID) - _FAKE_SNAPSHOT = {'id': _FAKE_SNAPSHOT_ID, - 'volume': _FAKE_VOLUME, - 'status': 'available', - 'volume_size': 1} _FAKE_VZ_CONFIG = mock.MagicMock() _FAKE_VZ_CONFIG.vzstorage_shares_config = '/fake/config/path' _FAKE_VZ_CONFIG.vzstorage_sparsed_volumes = False _FAKE_VZ_CONFIG.vzstorage_used_ratio = 0.7 _FAKE_VZ_CONFIG.vzstorage_mount_point_base = _FAKE_MNT_BASE + _FAKE_VZ_CONFIG.vzstorage_default_volume_format = 'raw' _FAKE_VZ_CONFIG.nas_secure_file_operations = 'auto' _FAKE_VZ_CONFIG.nas_secure_file_permissions = 'auto' @@ -72,6 +67,26 @@ class VZStorageTestCase(test.TestCase): self._vz_driver._execute = mock.Mock() self._vz_driver.base = self._FAKE_MNT_BASE + self.context = context.get_admin_context() + vol_type = fake_volume.fake_volume_type_obj(self.context) + vol_type.extra_specs = {} + _FAKE_VOLUME = {'id': '4f711859-4928-4cb7-801a-a50c37ceaccc', + 'size': 1, + 'provider_location': self._FAKE_SHARE, + 'name': self._FAKE_VOLUME_NAME, + 'status': 'available'} + self.vol = fake_volume.fake_volume_obj(self.context, + volume_type_id=vol_type.id, + **_FAKE_VOLUME) + self.vol.volume_type = vol_type + + _FAKE_SNAPSHOT = {'id': self._FAKE_SNAPSHOT_ID, + 'status': 'available', + 'volume_size': 1} + self.snap = fake_snapshot.fake_snapshot_obj(self.context, + **_FAKE_SNAPSHOT) + self.snap.volume = self.vol + def _path_exists(self, path): if path.startswith(self._FAKE_VZ_CONFIG.vzstorage_shares_config): return True @@ -135,9 +150,13 @@ class VZStorageTestCase(test.TestCase): file_format = 'raw' info = mock.Mock() info.file_format = file_format + snap_info = """{"volume_format": "raw", + "active": "%s"}""" % self.vol.id with mock.patch.object(drv, '_qemu_img_info', return_value=info): - ret = drv.initialize_connection(self._FAKE_VOLUME, None) - name = drv.get_active_image_from_info(self._FAKE_VOLUME) + with mock.patch.object(drv, '_read_file', + return_value=snap_info): + ret = drv.initialize_connection(self.vol, None) + name = drv.get_active_image_from_info(self.vol) expected = {'driver_volume_type': 'vzstorage', 'data': {'export': self._FAKE_SHARE, 'format': file_format, @@ -197,9 +216,13 @@ class VZStorageTestCase(test.TestCase): drv._check_extend_volume_support = mock.Mock(return_value=True) drv._is_file_size_equal = mock.Mock(return_value=True) + snap_info = """{"volume_format": "raw", + "active": "%s"}""" % self.vol.id with mock.patch.object(drv, 'local_path', return_value=self._FAKE_VOLUME_PATH): - drv.extend_volume(self._FAKE_VOLUME, 10) + with mock.patch.object(drv, '_read_file', + return_value=snap_info): + drv.extend_volume(self.vol, 10) mock_resize_image.assert_called_once_with(self._FAKE_VOLUME_PATH, 10) @@ -218,13 +241,13 @@ class VZStorageTestCase(test.TestCase): if has_snapshots: self.assertRaises(exception.InvalidVolume, drv._check_extend_volume_support, - self._FAKE_VOLUME, 2) + self.vol, 2) elif not is_eligible: self.assertRaises(exception.ExtendVolumeError, drv._check_extend_volume_support, - self._FAKE_VOLUME, 2) + self.vol, 2) else: - drv._check_extend_volume_support(self._FAKE_VOLUME, 2) + drv._check_extend_volume_support(self.vol, 2) drv._is_share_eligible.assert_called_once_with(self._FAKE_SHARE, 1) def test_check_extend_support(self): @@ -258,10 +281,10 @@ class VZStorageTestCase(test.TestCase): drv._extend_volume = mock.Mock() drv._copy_volume_from_snapshot( - self._FAKE_SNAPSHOT, self._FAKE_VOLUME, - self._FAKE_VOLUME['size']) + self.snap, self.vol, + self.vol['size']) drv._extend_volume.assert_called_once_with( - self._FAKE_VOLUME, self._FAKE_VOLUME['size']) + self.vol, self.vol['size']) mock_convert_image.assert_called_once_with( self._FAKE_VOLUME_PATH, self._FAKE_VOLUME_PATH[:-1], 'raw') @@ -281,7 +304,7 @@ class VZStorageTestCase(test.TestCase): return_value=fake_vol_info) with mock.patch('os.path.exists', lambda x: True): - drv.delete_volume(self._FAKE_VOLUME) + drv.delete_volume(self.vol) fake_ensure_mounted.assert_called_once_with(self._FAKE_SHARE) drv._delete.assert_any_call( diff --git a/cinder/tests/unit/windows/test_smbfs.py b/cinder/tests/unit/windows/test_smbfs.py index bdf42175994..e194335de26 100644 --- a/cinder/tests/unit/windows/test_smbfs.py +++ b/cinder/tests/unit/windows/test_smbfs.py @@ -17,9 +17,12 @@ import os import mock from oslo_utils import units +from cinder import context from cinder import exception from cinder.image import image_utils from cinder import test +from cinder.tests.unit import fake_snapshot +from cinder.tests.unit import fake_volume from cinder.volume.drivers.windows import smbfs @@ -35,12 +38,6 @@ class WindowsSmbFsTestCase(test.TestCase): _FAKE_TOTAL_SIZE = '2048' _FAKE_TOTAL_AVAILABLE = '1024' _FAKE_TOTAL_ALLOCATED = 1024 - _FAKE_VOLUME = {'id': 'e8d76af4-cbb9-4b70-8e9e-5a133f1a1a66', - 'size': 1, - 'provider_location': _FAKE_SHARE} - _FAKE_SNAPSHOT = {'id': '35a23942-7625-4683-ad84-144b76e87a80', - 'volume': _FAKE_VOLUME, - 'volume_size': _FAKE_VOLUME['size']} _FAKE_SHARE_OPTS = '-o username=Administrator,password=12345' _FAKE_VOLUME_PATH = os.path.join(_FAKE_MNT_POINT, _FAKE_VOLUME_NAME + '.vhdx') @@ -56,6 +53,25 @@ class WindowsSmbFsTestCase(test.TestCase): self._smbfs_driver.local_path = mock.Mock( return_value=self._FAKE_VOLUME_PATH) + def _simple_volume(self, **kwargs): + updates = {'id': 'e8d76af4-cbb9-4b70-8e9e-5a133f1a1a66', + 'size': 1, + 'provider_location': self._FAKE_SHARE} + updates.update(kwargs) + ctxt = context.get_admin_context() + return fake_volume.fake_volume_obj(ctxt, **updates) + + def _simple_snapshot(self, **kwargs): + volume = self._simple_volume() + ctxt = context.get_admin_context() + updates = {'id': '35a23942-7625-4683-ad84-144b76e87a80', + 'volume_size': volume.size, + 'volume_id': volume.id} + updates.update(kwargs) + snapshot = fake_snapshot.fake_snapshot_obj(ctxt, **updates) + snapshot.volume = volume + return snapshot + def _test_create_volume(self, volume_exists=False, volume_format='vhdx'): self._smbfs_driver.create_dynamic_vhd = mock.MagicMock() fake_create = self._smbfs_driver._vhdutils.create_dynamic_vhd @@ -63,15 +79,16 @@ class WindowsSmbFsTestCase(test.TestCase): return_value=volume_format) with mock.patch('os.path.exists', new=lambda x: volume_exists): + volume = self._simple_volume() if volume_exists or volume_format not in ('vhd', 'vhdx'): self.assertRaises(exception.InvalidVolume, self._smbfs_driver._do_create_volume, - self._FAKE_VOLUME) + volume) else: fake_vol_path = self._FAKE_VOLUME_PATH - self._smbfs_driver._do_create_volume(self._FAKE_VOLUME) + self._smbfs_driver._do_create_volume(volume) fake_create.assert_called_once_with( - fake_vol_path, self._FAKE_VOLUME['size'] << 30) + fake_vol_path, volume.size << 30) def test_create_volume(self): self._test_create_volume() @@ -120,7 +137,7 @@ class WindowsSmbFsTestCase(test.TestCase): self._smbfs_driver._vhdutils.create_differencing_vhd) self._smbfs_driver._do_create_snapshot( - self._FAKE_SNAPSHOT, + self._simple_snapshot(), os.path.basename(self._FAKE_VOLUME_PATH), self._FAKE_SNAPSHOT_PATH) @@ -156,8 +173,9 @@ class WindowsSmbFsTestCase(test.TestCase): with mock.patch.object(image_utils, 'upload_volume') as ( fake_upload_volume): + volume = self._simple_volume() drv.copy_volume_to_image( - mock.sentinel.context, self._FAKE_VOLUME, + mock.sentinel.context, volume, mock.sentinel.image_service, fake_image_meta) expected_conversion = ( @@ -165,7 +183,7 @@ class WindowsSmbFsTestCase(test.TestCase): if expected_conversion: fake_temp_image_name = '%s.temp_image.%s.%s' % ( - self._FAKE_VOLUME['id'], + volume.id, fake_image_meta['id'], drv._DISK_FORMAT_VHD) fake_temp_image_path = os.path.join( @@ -209,8 +227,9 @@ class WindowsSmbFsTestCase(test.TestCase): with mock.patch.object(image_utils, 'fetch_to_volume_format') as fake_fetch: + volume = self._simple_volume() drv.copy_image_to_volume( - mock.sentinel.context, self._FAKE_VOLUME, + mock.sentinel.context, volume, mock.sentinel.image_service, mock.sentinel.image_id) fake_fetch.assert_called_once_with( @@ -221,13 +240,14 @@ class WindowsSmbFsTestCase(test.TestCase): mock.sentinel.block_size) drv._vhdutils.resize_vhd.assert_called_once_with( self._FAKE_VOLUME_PATH, - self._FAKE_VOLUME['size'] * units.Gi, + volume.size * units.Gi, is_file_max_size=False) def test_copy_volume_from_snapshot(self): drv = self._smbfs_driver + snapshot = self._simple_snapshot() fake_volume_info = { - self._FAKE_SNAPSHOT['id']: 'fake_snapshot_file_name'} + snapshot.id: 'fake_snapshot_file_name'} fake_img_info = mock.MagicMock() fake_img_info.backing_file = self._FAKE_VOLUME_NAME + '.vhdx' @@ -242,9 +262,9 @@ class WindowsSmbFsTestCase(test.TestCase): drv.local_path = mock.Mock( return_value=mock.sentinel.new_volume_path) - drv._copy_volume_from_snapshot( - self._FAKE_SNAPSHOT, self._FAKE_VOLUME, - self._FAKE_VOLUME['size']) + volume = self._simple_volume() + drv._copy_volume_from_snapshot(snapshot, + volume, volume.size) drv._delete.assert_called_once_with(mock.sentinel.new_volume_path) drv._vhdutils.convert_vhd.assert_called_once_with( @@ -252,7 +272,7 @@ class WindowsSmbFsTestCase(test.TestCase): mock.sentinel.new_volume_path) drv._vhdutils.resize_vhd.assert_called_once_with( mock.sentinel.new_volume_path, - self._FAKE_VOLUME['size'] * units.Gi, + volume.size * units.Gi, is_file_max_size=False) def test_rebase_img(self): diff --git a/cinder/volume/drivers/glusterfs.py b/cinder/volume/drivers/glusterfs.py index 341598cbfe5..e8b96170c2b 100644 --- a/cinder/volume/drivers/glusterfs.py +++ b/cinder/volume/drivers/glusterfs.py @@ -146,7 +146,7 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver, pass def _local_volume_dir(self, volume): - hashed = self._get_hash_str(volume['provider_location']) + hashed = self._get_hash_str(volume.provider_location) path = '%s/%s' % (self.configuration.glusterfs_mount_point_base, hashed) return path @@ -185,13 +185,13 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver, self._ensure_shares_mounted() - volume['provider_location'] = self._find_share(volume['size']) + volume.provider_location = self._find_share(volume.size) - LOG.info(_LI('casted to %s'), volume['provider_location']) + LOG.info(_LI('casted to %s'), volume.provider_location) self._do_create_volume(volume) - return {'provider_location': volume['provider_location']} + return {'provider_location': volume.provider_location} def _copy_volume_from_snapshot(self, snapshot, volume, volume_size): """Copy data from snapshot to destination volume. @@ -202,20 +202,20 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver, LOG.debug("snapshot: %(snap)s, volume: %(vol)s, " "volume_size: %(size)s", - {'snap': snapshot['id'], - 'vol': volume['id'], + {'snap': snapshot.id, + 'vol': volume.id, 'size': volume_size}) - info_path = self._local_path_volume_info(snapshot['volume']) + info_path = self._local_path_volume_info(snapshot.volume) snap_info = self._read_info_file(info_path) - vol_path = self._local_volume_dir(snapshot['volume']) - forward_file = snap_info[snapshot['id']] + vol_path = self._local_volume_dir(snapshot.volume) + forward_file = snap_info[snapshot.id] forward_path = os.path.join(vol_path, forward_file) # Find the file which backs this file, which represents the point # when this snapshot was created. img_info = self._qemu_img_info(forward_path, - snapshot['volume']['name']) + snapshot.volume.name) path_to_snap_img = os.path.join(vol_path, img_info.backing_file) path_to_new_vol = self._local_path_volume(volume) @@ -237,13 +237,13 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver, def delete_volume(self, volume): """Deletes a logical volume.""" - if not volume['provider_location']: + if not volume.provider_location: LOG.warning(_LW('Volume %s does not have ' 'provider_location specified, ' - 'skipping'), volume['name']) + 'skipping'), volume.name) return - self._ensure_share_mounted(volume['provider_location']) + self._ensure_share_mounted(volume.provider_location) mounted_path = self._active_volume_path(volume) @@ -264,7 +264,7 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver, def ensure_export(self, ctx, volume): """Synchronously recreates an export for a logical volume.""" - self._ensure_share_mounted(volume['provider_location']) + self._ensure_share_mounted(volume.provider_location) def create_export(self, ctx, volume, connector): """Exports the volume.""" @@ -285,16 +285,16 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver, # Find active qcow2 file active_file = self.get_active_image_from_info(volume) path = '%s/%s/%s' % (self.configuration.glusterfs_mount_point_base, - self._get_hash_str(volume['provider_location']), + self._get_hash_str(volume.provider_location), active_file) - data = {'export': volume['provider_location'], + data = {'export': volume.provider_location, 'name': active_file} - if volume['provider_location'] in self.shares: - data['options'] = self.shares[volume['provider_location']] + if volume.provider_location in self.shares: + data['options'] = self.shares[volume.provider_location] # Test file for raw vs. qcow2 format - info = self._qemu_img_info(path, volume['name']) + info = self._qemu_img_info(path, volume.name) data['format'] = info.file_format if data['format'] not in ['raw', 'qcow2']: msg = _('%s must be a valid raw or qcow2 image.') % path @@ -314,7 +314,7 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver, def extend_volume(self, volume, size_gb): volume_path = self._active_volume_path(volume) - info = self._qemu_img_info(volume_path, volume['name']) + info = self._qemu_img_info(volume_path, volume.name) backing_fmt = info.file_format if backing_fmt not in ['raw', 'qcow2']: @@ -331,7 +331,7 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver, """ volume_path = self.local_path(volume) - volume_size = volume['size'] + volume_size = volume.size LOG.debug("creating new volume at %s", volume_path) @@ -452,7 +452,7 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver, active_file_path = self._active_volume_path(volume) - info = self._qemu_img_info(active_file_path, volume['name']) + info = self._qemu_img_info(active_file_path, volume.name) if info.backing_file is not None: LOG.error(_LE('No snapshots found in database, but %(path)s has ' diff --git a/cinder/volume/drivers/nfs.py b/cinder/volume/drivers/nfs.py index c1ac655e151..6617c31d7da 100644 --- a/cinder/volume/drivers/nfs.py +++ b/cinder/volume/drivers/nfs.py @@ -300,13 +300,13 @@ class NfsDriver(driver.ExtendVD, remotefs.RemoteFSDriver): def extend_volume(self, volume, new_size): """Extend an existing volume to the new size.""" - LOG.info(_LI('Extending volume %s.'), volume['id']) - extend_by = int(new_size) - volume['size'] - if not self._is_share_eligible(volume['provider_location'], + LOG.info(_LI('Extending volume %s.'), volume.id) + extend_by = int(new_size) - volume.size + if not self._is_share_eligible(volume.provider_location, extend_by): raise exception.ExtendVolumeError(reason='Insufficient space to' ' extend volume %s to %sG' - % (volume['id'], new_size)) + % (volume.id, new_size)) path = self.local_path(volume) LOG.info(_LI('Resizing file to %sG...'), new_size) image_utils.resize_image(path, new_size, @@ -396,8 +396,8 @@ class NfsDriver(driver.ExtendVD, remotefs.RemoteFSDriver): # NFS snapshots are introduced. name_id = None if original_volume_status == 'available': - current_name = CONF.volume_name_template % new_volume['id'] - original_volume_name = CONF.volume_name_template % volume['id'] + current_name = CONF.volume_name_template % new_volume.id + original_volume_name = CONF.volume_name_template % volume.id current_path = self.local_path(new_volume) # Replace the volume name with the original volume name original_path = current_path.replace(current_name, @@ -406,16 +406,16 @@ class NfsDriver(driver.ExtendVD, remotefs.RemoteFSDriver): os.rename(current_path, original_path) except OSError: LOG.error(_LE('Unable to rename the logical volume ' - 'for volume: %s'), volume['id']) + 'for volume: %s'), volume.id) # If the rename fails, _name_id should be set to the new # volume id and provider_location should be set to the # one from the new volume as well. - name_id = new_volume['_name_id'] or new_volume['id'] + name_id = new_volume._name_id or new_volume.id else: # The back-end will not be renamed. - name_id = new_volume['_name_id'] or new_volume['id'] + name_id = new_volume._name_id or new_volume.id return {'_name_id': name_id, - 'provider_location': new_volume['provider_location']} + 'provider_location': new_volume.provider_location} def _update_volume_stats(self): """Retrieve stats info from volume group.""" diff --git a/cinder/volume/drivers/quobyte.py b/cinder/volume/drivers/quobyte.py index d6d8c825efd..23554a5acd2 100644 --- a/cinder/volume/drivers/quobyte.py +++ b/cinder/volume/drivers/quobyte.py @@ -185,20 +185,20 @@ class QuobyteDriver(remotefs_drv.RemoteFSSnapDriver): """ LOG.debug("snapshot: %(snap)s, volume: %(vol)s, ", - {'snap': snapshot['id'], - 'vol': volume['id'], + {'snap': snapshot.id, + 'vol': volume.id, 'size': volume_size}) - info_path = self._local_path_volume_info(snapshot['volume']) + info_path = self._local_path_volume_info(snapshot.volume) snap_info = self._read_info_file(info_path) - vol_path = self._local_volume_dir(snapshot['volume']) - forward_file = snap_info[snapshot['id']] + vol_path = self._local_volume_dir(snapshot.volume) + forward_file = snap_info[snapshot.id] forward_path = os.path.join(vol_path, forward_file) # Find the file which backs this file, which represents the point # when this snapshot was created. img_info = self._qemu_img_info(forward_path, - snapshot['volume']['name']) + snapshot.volume.name) path_to_snap_img = os.path.join(vol_path, img_info.backing_file) path_to_new_vol = self._local_path_volume(volume) @@ -221,12 +221,12 @@ class QuobyteDriver(remotefs_drv.RemoteFSSnapDriver): def delete_volume(self, volume): """Deletes a logical volume.""" - if not volume['provider_location']: + if not volume.provider_location: LOG.warning(_LW('Volume %s does not have provider_location ' - 'specified, skipping'), volume['name']) + 'specified, skipping'), volume.name) return - self._ensure_share_mounted(volume['provider_location']) + self._ensure_share_mounted(volume.provider_location) volume_dir = self._local_volume_dir(volume) mounted_path = os.path.join(volume_dir, @@ -261,16 +261,16 @@ class QuobyteDriver(remotefs_drv.RemoteFSSnapDriver): # Find active qcow2 file active_file = self.get_active_image_from_info(volume) path = '%s/%s/%s' % (self.configuration.quobyte_mount_point_base, - self._get_hash_str(volume['provider_location']), + self._get_hash_str(volume.provider_location), active_file) - data = {'export': volume['provider_location'], + data = {'export': volume.provider_location, 'name': active_file} - if volume['provider_location'] in self.shares: - data['options'] = self.shares[volume['provider_location']] + if volume.provider_location in self.shares: + data['options'] = self.shares[volume.provider_location] # Test file for raw vs. qcow2 format - info = self._qemu_img_info(path, volume['name']) + info = self._qemu_img_info(path, volume.name) data['format'] = info.file_format if data['format'] not in ['raw', 'qcow2']: msg = _('%s must be a valid raw or qcow2 image.') % path @@ -299,7 +299,7 @@ class QuobyteDriver(remotefs_drv.RemoteFSSnapDriver): ' driver when no snapshots exist.') raise exception.InvalidVolume(msg) - info = self._qemu_img_info(volume_path, volume['name']) + info = self._qemu_img_info(volume_path, volume.name) backing_fmt = info.file_format if backing_fmt not in ['raw', 'qcow2']: @@ -315,7 +315,7 @@ class QuobyteDriver(remotefs_drv.RemoteFSSnapDriver): :param volume: volume reference """ volume_path = self.local_path(volume) - volume_size = volume['size'] + volume_size = volume.size if self.configuration.quobyte_qcow2_volumes: self._create_qcow2_file(volume_path, volume_size) diff --git a/cinder/volume/drivers/remotefs.py b/cinder/volume/drivers/remotefs.py index c2863d0cbb4..31221c07d32 100644 --- a/cinder/volume/drivers/remotefs.py +++ b/cinder/volume/drivers/remotefs.py @@ -14,6 +14,7 @@ # License for the specific language governing permissions and limitations # under the License. +import collections import hashlib import inspect import json @@ -118,9 +119,9 @@ def locked_volume_id_operation(f, external=False): call_args = inspect.getcallargs(f, inst, *args, **kwargs) if call_args.get('volume'): - volume_id = call_args['volume']['id'] + volume_id = call_args['volume'].id elif call_args.get('snapshot'): - volume_id = call_args['snapshot']['volume']['id'] + volume_id = call_args['snapshot'].volume.id else: err_msg = _('The decorated method must accept either a volume or ' 'a snapshot object') @@ -163,10 +164,10 @@ class RemoteFSDriver(driver.LocalVD, driver.TransferVD, driver.BaseVD): :param volume: volume reference :param connector: connector reference """ - data = {'export': volume['provider_location'], - 'name': volume['name']} - if volume['provider_location'] in self.shares: - data['options'] = self.shares[volume['provider_location']] + data = {'export': volume.provider_location, + 'name': volume.name} + if volume.provider_location in self.shares: + data['options'] = self.shares[volume.provider_location] return { 'driver_volume_type': self.driver_volume_type, 'data': data, @@ -232,13 +233,13 @@ class RemoteFSDriver(driver.LocalVD, driver.TransferVD, driver.BaseVD): """ self._ensure_shares_mounted() - volume['provider_location'] = self._find_share(volume['size']) + volume.provider_location = self._find_share(volume.size) - LOG.info(_LI('casted to %s'), volume['provider_location']) + LOG.info(_LI('casted to %s'), volume.provider_location) self._do_create_volume(volume) - return {'provider_location': volume['provider_location']} + return {'provider_location': volume.provider_location} def _do_create_volume(self, volume): """Create a volume on given remote share. @@ -246,7 +247,7 @@ class RemoteFSDriver(driver.LocalVD, driver.TransferVD, driver.BaseVD): :param volume: volume reference """ volume_path = self.local_path(volume) - volume_size = volume['size'] + volume_size = volume.size if getattr(self.configuration, self.driver_prefix + '_sparsed_volumes'): @@ -280,13 +281,13 @@ class RemoteFSDriver(driver.LocalVD, driver.TransferVD, driver.BaseVD): :param volume: volume reference """ - if not volume['provider_location']: + if not volume.provider_location: LOG.warning(_LW('Volume %s does not have ' 'provider_location specified, ' - 'skipping'), volume['name']) + 'skipping'), volume.name) return - self._ensure_share_mounted(volume['provider_location']) + self._ensure_share_mounted(volume.provider_location) mounted_path = self.local_path(volume) @@ -294,7 +295,7 @@ class RemoteFSDriver(driver.LocalVD, driver.TransferVD, driver.BaseVD): def ensure_export(self, ctx, volume): """Synchronously recreates an export for a logical volume.""" - self._ensure_share_mounted(volume['provider_location']) + self._ensure_share_mounted(volume.provider_location) def create_export(self, ctx, volume, connector): """Exports the volume. @@ -387,9 +388,9 @@ class RemoteFSDriver(driver.LocalVD, driver.TransferVD, driver.BaseVD): :param volume: volume reference """ - remotefs_share = volume['provider_location'] + remotefs_share = volume.provider_location return os.path.join(self._get_mount_point_for_share(remotefs_share), - volume['name']) + volume.name) def copy_image_to_volume(self, context, volume, image_service, image_id): """Fetch the image from image_service and write it to the volume.""" @@ -400,7 +401,7 @@ class RemoteFSDriver(driver.LocalVD, driver.TransferVD, driver.BaseVD): image_id, self.local_path(volume), self.configuration.volume_dd_blocksize, - size=volume['size'], + size=volume.size, run_as_root=run_as_root) # NOTE (leseb): Set the virtual size of the image @@ -410,16 +411,16 @@ class RemoteFSDriver(driver.LocalVD, driver.TransferVD, driver.BaseVD): # thus the initial 'size' parameter is not honored # this sets the size to the one asked in the first place by the user # and then verify the final virtual size - image_utils.resize_image(self.local_path(volume), volume['size'], + image_utils.resize_image(self.local_path(volume), volume.size, run_as_root=run_as_root) data = image_utils.qemu_img_info(self.local_path(volume), run_as_root=run_as_root) virt_size = data.virtual_size / units.Gi - if virt_size != volume['size']: + if virt_size != volume.size: raise exception.ImageUnacceptable( image_id=image_id, - reason=(_("Expected volume size was %d") % volume['size']) + reason=(_("Expected volume size was %d") % volume.size) + (_(" but size is now %d") % virt_size)) def copy_volume_to_image(self, context, volume, image_service, image_meta): @@ -647,20 +648,20 @@ class RemoteFSSnapDriver(RemoteFSDriver, driver.SnapshotVD): self._nova = compute.API() def _local_volume_dir(self, volume): - share = volume['provider_location'] + share = volume.provider_location local_dir = self._get_mount_point_for_share(share) return local_dir def _local_path_volume(self, volume): path_to_disk = os.path.join( self._local_volume_dir(volume), - volume['name']) + volume.name) return path_to_disk def _get_new_snap_path(self, snapshot): - vol_path = self.local_path(snapshot['volume']) - snap_path = '%s.%s' % (vol_path, snapshot['id']) + vol_path = self.local_path(snapshot.volume) + snap_path = '%s.%s' % (vol_path, snapshot.id) return snap_path def _local_path_volume_info(self, volume): @@ -757,7 +758,7 @@ class RemoteFSSnapDriver(RemoteFSDriver, driver.SnapshotVD): output = [] - info = self._qemu_img_info(path, volume['name']) + info = self._qemu_img_info(path, volume.name) new_info = {} new_info['filename'] = os.path.basename(path) new_info['backing-filename'] = info.backing_file @@ -767,7 +768,7 @@ class RemoteFSSnapDriver(RemoteFSDriver, driver.SnapshotVD): while new_info['backing-filename']: filename = new_info['backing-filename'] path = os.path.join(self._local_volume_dir(volume), filename) - info = self._qemu_img_info(path, volume['name']) + info = self._qemu_img_info(path, volume.name) backing_filename = info.backing_file new_info = {} new_info['filename'] = filename @@ -846,13 +847,13 @@ class RemoteFSSnapDriver(RemoteFSDriver, driver.SnapshotVD): active_file = self.get_active_image_from_info(volume) active_file_path = os.path.join(self._local_volume_dir(volume), active_file) - info = self._qemu_img_info(active_file_path, volume['name']) + info = self._qemu_img_info(active_file_path, volume.name) backing_file = info.backing_file root_file_fmt = info.file_format tmp_params = { - 'prefix': '%s.temp_image.%s' % (volume['id'], image_meta['id']), + 'prefix': '%s.temp_image.%s' % (volume.id, image_meta['id']), 'suffix': '.img' } with image_utils.temporary_file(**tmp_params) as temp_path: @@ -886,52 +887,64 @@ class RemoteFSSnapDriver(RemoteFSDriver, driver.SnapshotVD): def _create_cloned_volume(self, volume, src_vref): LOG.info(_LI('Cloning volume %(src)s to volume %(dst)s'), - {'src': src_vref['id'], - 'dst': volume['id']}) + {'src': src_vref.id, + 'dst': volume.id}) - if src_vref['status'] != 'available': + if src_vref.status != 'available': msg = _("Volume status must be 'available'.") raise exception.InvalidVolume(msg) - volume_name = CONF.volume_name_template % volume['id'] + volume_name = CONF.volume_name_template % volume.id + + # Create fake volume and snapshot objects + 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) + + volume_info = Volume(provider_location=src_vref.provider_location, + size=src_vref.size, + id=volume.id, + name=volume_name, + status=src_vref.status, + volume_type=src_vref.volume_type, + metadata=src_vref.metadata) + + 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) - volume_info = {'provider_location': src_vref['provider_location'], - 'size': src_vref['size'], - 'id': volume['id'], - 'name': volume_name, - 'status': src_vref['status']} - temp_snapshot = {'volume_name': volume_name, - 'size': src_vref['size'], - '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} self._create_snapshot(temp_snapshot) try: self._copy_volume_from_snapshot(temp_snapshot, volume_info, - volume['size']) + volume.size) finally: self._delete_snapshot(temp_snapshot) - return {'provider_location': src_vref['provider_location']} + return {'provider_location': src_vref.provider_location} def _delete_stale_snapshot(self, snapshot): - info_path = self._local_path_volume_info(snapshot['volume']) + 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 = self.get_active_image_from_info(snapshot['volume']) + snapshot_file = snap_info[snapshot.id] + active_file = self.get_active_image_from_info(snapshot.volume) snapshot_path = os.path.join( - self._local_volume_dir(snapshot['volume']), snapshot_file) + self._local_volume_dir(snapshot.volume), snapshot_file) if (snapshot_file == active_file): return - LOG.info(_LI('Deleting stale snapshot: %s'), snapshot['id']) + LOG.info(_LI('Deleting stale snapshot: %s'), snapshot.id) self._delete(snapshot_path) - del(snap_info[snapshot['id']]) + del(snap_info[snapshot.id]) self._write_info_file(info_path, snap_info) def _delete_snapshot(self, snapshot): @@ -949,39 +962,39 @@ class RemoteFSSnapDriver(RemoteFSDriver, driver.SnapshotVD): """ - LOG.debug('Deleting snapshot %s:', snapshot['id']) + LOG.debug('Deleting snapshot %s:', snapshot.id) - volume_status = snapshot['volume']['status'] + volume_status = snapshot.volume.status if volume_status not in ['available', 'in-use']: msg = _('Volume status must be "available" or "in-use".') raise exception.InvalidVolume(msg) - vol_path = self._local_volume_dir(snapshot['volume']) + vol_path = self._local_volume_dir(snapshot.volume) self._ensure_share_writable(vol_path) # Determine the true snapshot file for this snapshot # based on the .info file - info_path = self._local_path_volume_info(snapshot['volume']) + info_path = self._local_path_volume_info(snapshot.volume) snap_info = self._read_info_file(info_path, empty_if_missing=True) - if snapshot['id'] not in snap_info: + if snapshot.id not in snap_info: # If snapshot info file is present, but snapshot record does not # exist, do not attempt to delete. # (This happens, for example, if snapshot_create failed due to lack # of permission to write to the share.) LOG.info(_LI('Snapshot record for %s is not present, allowing ' - 'snapshot_delete to proceed.'), snapshot['id']) + 'snapshot_delete to proceed.'), snapshot.id) return - snapshot_file = snap_info[snapshot['id']] + snapshot_file = snap_info[snapshot.id] LOG.debug('snapshot_file for this snap is: %s', snapshot_file) snapshot_path = os.path.join( - self._local_volume_dir(snapshot['volume']), + self._local_volume_dir(snapshot.volume), snapshot_file) snapshot_path_img_info = self._qemu_img_info( snapshot_path, - snapshot['volume']['name']) + snapshot.volume.name) base_file = snapshot_path_img_info.backing_file if base_file is None: @@ -996,15 +1009,15 @@ class RemoteFSSnapDriver(RemoteFSDriver, driver.SnapshotVD): base_path = os.path.join(vol_path, base_file) base_file_img_info = self._qemu_img_info(base_path, - snapshot['volume']['name']) + snapshot.volume.name) # Find what file has this as its backing file - active_file = self.get_active_image_from_info(snapshot['volume']) + active_file = self.get_active_image_from_info(snapshot.volume) active_file_path = os.path.join(vol_path, active_file) if volume_status == 'in-use': # Online delete - context = snapshot['context'] + context = snapshot._context new_base_file = base_file_img_info.backing_file @@ -1048,7 +1061,7 @@ class RemoteFSSnapDriver(RemoteFSDriver, driver.SnapshotVD): # used here) | | ptr update) | backing_chain = self._get_backing_chain_for_path( - snapshot['volume'], active_file_path) + snapshot.volume, active_file_path) # This file is guaranteed to exist since we aren't operating on # the active file. higher_file = next((os.path.basename(f['filename']) @@ -1077,7 +1090,7 @@ class RemoteFSSnapDriver(RemoteFSDriver, driver.SnapshotVD): self._rebase_img(higher_file_path, base_file, base_file_fmt) # Remove snapshot_file from info - del(snap_info[snapshot['id']]) + del(snap_info[snapshot.id]) self._write_info_file(info_path, snap_info) def _create_volume_from_snapshot(self, volume, snapshot): @@ -1086,21 +1099,21 @@ class RemoteFSSnapDriver(RemoteFSDriver, driver.SnapshotVD): Snapshot must not be the active snapshot. (offline) """ - if snapshot['status'] != 'available': + if snapshot.status != 'available': msg = _('Snapshot status must be "available" to clone.') raise exception.InvalidSnapshot(msg) self._ensure_shares_mounted() - volume['provider_location'] = self._find_share(volume['size']) + volume.provider_location = self._find_share(volume.size) self._do_create_volume(volume) self._copy_volume_from_snapshot(snapshot, volume, - volume['size']) + volume.size) - return {'provider_location': volume['provider_location']} + return {'provider_location': volume.provider_location} def _copy_volume_from_snapshot(self, snapshot, volume, volume_size): raise NotImplementedError() @@ -1116,7 +1129,7 @@ class RemoteFSSnapDriver(RemoteFSDriver, driver.SnapshotVD): """ backing_path_full_path = os.path.join( - self._local_volume_dir(snapshot['volume']), + self._local_volume_dir(snapshot.volume), backing_filename) command = ['qemu-img', 'create', '-f', 'qcow2', '-o', @@ -1124,7 +1137,7 @@ class RemoteFSSnapDriver(RemoteFSDriver, driver.SnapshotVD): self._execute(*command, run_as_root=self._execute_as_root) info = self._qemu_img_info(backing_path_full_path, - snapshot['volume']['name']) + snapshot.volume.name) backing_fmt = info.file_format command = ['qemu-img', 'rebase', '-u', @@ -1239,16 +1252,16 @@ class RemoteFSSnapDriver(RemoteFSDriver, driver.SnapshotVD): info file: { 'active': 'volume-1234' } (* changed!) """ - status = snapshot['volume']['status'] + status = snapshot.volume.status if status not in ['available', 'in-use']: msg = _('Volume status must be "available" or "in-use"' ' for snapshot. (is %s)') % status raise exception.InvalidVolume(msg) - info_path = self._local_path_volume_info(snapshot['volume']) + info_path = self._local_path_volume_info(snapshot.volume) snap_info = self._read_info_file(info_path, empty_if_missing=True) backing_filename = self.get_active_image_from_info( - snapshot['volume']) + snapshot.volume) new_snap_path = self._get_new_snap_path(snapshot) if status == 'in-use': @@ -1261,13 +1274,13 @@ class RemoteFSSnapDriver(RemoteFSDriver, driver.SnapshotVD): new_snap_path) snap_info['active'] = os.path.basename(new_snap_path) - snap_info[snapshot['id']] = os.path.basename(new_snap_path) + snap_info[snapshot.id] = os.path.basename(new_snap_path) self._write_info_file(info_path, snap_info) def _create_snapshot_online(self, snapshot, backing_filename, new_snap_path): # Perform online snapshot via Nova - context = snapshot['context'] + context = snapshot._context self._do_create_snapshot(snapshot, backing_filename, @@ -1276,13 +1289,13 @@ class RemoteFSSnapDriver(RemoteFSDriver, driver.SnapshotVD): connection_info = { 'type': 'qcow2', 'new_file': os.path.basename(new_snap_path), - 'snapshot_id': snapshot['id'] + 'snapshot_id': snapshot.id } try: result = self._nova.create_volume_snapshot( context, - snapshot['volume_id'], + snapshot.volume_id, connection_info) LOG.debug('nova call result: %s', result) except Exception: @@ -1296,7 +1309,7 @@ class RemoteFSSnapDriver(RemoteFSDriver, driver.SnapshotVD): increment = 1 timeout = 600 while True: - s = db.snapshot_get(context, snapshot['id']) + s = db.snapshot_get(context, snapshot.id) LOG.debug('Status of snapshot %(id)s is now %(status)s', {'id': snapshot['id'], @@ -1320,7 +1333,7 @@ class RemoteFSSnapDriver(RemoteFSDriver, driver.SnapshotVD): msg = _('Snapshot %(id)s has been asked to be deleted while ' 'waiting for it to become available. Perhaps a ' 'concurrent request was made.') % {'id': - snapshot['id']} + snapshot.id} raise exception.RemoteFSConcurrentRequest(msg) if 10 < seconds_elapsed <= 20: @@ -1332,13 +1345,13 @@ class RemoteFSSnapDriver(RemoteFSDriver, driver.SnapshotVD): if seconds_elapsed > timeout: msg = _('Timed out while waiting for Nova update ' - 'for creation of snapshot %s.') % snapshot['id'] + 'for creation of snapshot %s.') % snapshot.id raise exception.RemoteFSException(msg) def _delete_snapshot_online(self, context, snapshot, info): # Update info over the course of this method # active file never changes - info_path = self._local_path_volume_info(snapshot['volume']) + info_path = self._local_path_volume_info(snapshot.volume) snap_info = self._read_info_file(info_path) if info['active_file'] == info['snapshot_file']: @@ -1357,9 +1370,9 @@ class RemoteFSSnapDriver(RemoteFSDriver, driver.SnapshotVD): delete_info = {'file_to_merge': new_base, 'merge_target_file': None, # current 'type': 'qcow2', - 'volume_id': snapshot['volume']['id']} + 'volume_id': snapshot.volume.id} - del(snap_info[snapshot['id']]) + del(snap_info[snapshot.id]) else: # blockCommit snapshot into base # info['base'] <= snapshot_file @@ -1369,14 +1382,14 @@ class RemoteFSSnapDriver(RemoteFSDriver, driver.SnapshotVD): delete_info = {'file_to_merge': info['snapshot_file'], 'merge_target_file': info['base_file'], 'type': 'qcow2', - 'volume_id': snapshot['volume']['id']} + 'volume_id': snapshot.volume.id} - del(snap_info[snapshot['id']]) + del(snap_info[snapshot.id]) try: self._nova.delete_volume_snapshot( context, - snapshot['id'], + snapshot.id, delete_info) except Exception: LOG.exception(_LE('Call to Nova delete snapshot failed')) @@ -1389,7 +1402,7 @@ class RemoteFSSnapDriver(RemoteFSDriver, driver.SnapshotVD): increment = 1 timeout = 7200 while True: - s = db.snapshot_get(context, snapshot['id']) + s = db.snapshot_get(context, snapshot.id) if s['status'] == fields.SnapshotStatus.DELETING: if s['progress'] == '90%': @@ -1397,12 +1410,12 @@ class RemoteFSSnapDriver(RemoteFSDriver, driver.SnapshotVD): break else: LOG.debug('status of snapshot %s is still "deleting"... ' - 'waiting', snapshot['id']) + 'waiting', snapshot.id) time.sleep(increment) seconds_elapsed += increment else: msg = _('Unable to delete snapshot %(id)s, ' - 'status: %(status)s.') % {'id': snapshot['id'], + 'status: %(status)s.') % {'id': snapshot.id, 'status': s['status']} raise exception.RemoteFSException(msg) @@ -1416,7 +1429,7 @@ class RemoteFSSnapDriver(RemoteFSDriver, driver.SnapshotVD): if seconds_elapsed > timeout: msg = _('Timed out while waiting for Nova update ' 'for deletion of snapshot %(id)s.') %\ - {'id': snapshot['id']} + {'id': snapshot.id} raise exception.RemoteFSException(msg) # Write info file updated above @@ -1424,7 +1437,7 @@ class RemoteFSSnapDriver(RemoteFSDriver, driver.SnapshotVD): # Delete stale file path_to_delete = os.path.join( - self._local_volume_dir(snapshot['volume']), file_to_delete) + self._local_volume_dir(snapshot.volume), file_to_delete) self._execute('rm', '-f', path_to_delete, run_as_root=True) @locked_volume_id_operation diff --git a/cinder/volume/drivers/scality.py b/cinder/volume/drivers/scality.py index 64e4c93483f..63247556cea 100644 --- a/cinder/volume/drivers/scality.py +++ b/cinder/volume/drivers/scality.py @@ -186,14 +186,14 @@ class ScalityDriver(remotefs_drv.RemoteFSSnapDriver): active_file = self.get_active_image_from_info(volume) path = '%s/%s' % (self._get_mount_point_for_share(), active_file) sofs_rel_path = os.path.join(self.sofs_rel_volume_dir, "00", - volume['name']) + volume.name) - data = {'export': volume['provider_location'], + data = {'export': volume.provider_location, 'name': active_file, 'sofs_path': sofs_rel_path} # Test file for raw vs. qcow2 format - info = self._qemu_img_info(path, volume['name']) + info = self._qemu_img_info(path, volume.name) data['format'] = info.file_format if data['format'] not in ['raw', 'qcow2']: msg = _('%s must be a valid raw or qcow2 image.') % path @@ -213,7 +213,7 @@ class ScalityDriver(remotefs_drv.RemoteFSSnapDriver): def extend_volume(self, volume, size_gb): volume_path = self.local_path(volume) - info = self._qemu_img_info(volume_path, volume['name']) + info = self._qemu_img_info(volume_path, volume.name) backing_fmt = info.file_format if backing_fmt not in ['raw', 'qcow2']: @@ -230,7 +230,7 @@ class ScalityDriver(remotefs_drv.RemoteFSSnapDriver): qcow2. """ - info_path = self._local_path_volume_info(snapshot['volume']) + info_path = self._local_path_volume_info(snapshot.volume) # For BC compat' with version < 2 of this driver try: @@ -241,15 +241,15 @@ class ScalityDriver(remotefs_drv.RemoteFSSnapDriver): else: path_to_snap_img = self.local_path(snapshot) else: - vol_path = self._local_volume_dir(snapshot['volume']) + vol_path = self._local_volume_dir(snapshot.volume) - forward_file = snap_info[snapshot['id']] + forward_file = snap_info[snapshot.id] forward_path = os.path.join(vol_path, forward_file) # Find the file which backs this file, which represents the point # when this snapshot was created. img_info = self._qemu_img_info(forward_path, - snapshot['volume']['name']) + snapshot.volume.name) path_to_snap_img = os.path.join(vol_path, img_info.backing_file) @@ -270,7 +270,7 @@ class ScalityDriver(remotefs_drv.RemoteFSSnapDriver): """Create a new backup from an existing volume.""" volume = self.db.volume_get(context, backup['volume_id']) volume_local_path = self.local_path(volume) - LOG.info(_LI('Begin backup of volume %s.'), volume['name']) + LOG.info(_LI('Begin backup of volume %s.'), volume.name) qemu_img_info = image_utils.qemu_img_info(volume_local_path) if qemu_img_info.file_format != 'raw': @@ -290,8 +290,8 @@ class ScalityDriver(remotefs_drv.RemoteFSSnapDriver): def restore_backup(self, context, backup, volume, backup_service): """Restore an existing backup to a new or existing volume.""" LOG.info(_LI('Restoring backup %(backup)s to volume %(volume)s.'), - {'backup': backup['id'], 'volume': volume['name']}) + {'backup': backup['id'], 'volume': volume.name}) volume_local_path = self.local_path(volume) with utils.temporary_chown(volume_local_path): with open(volume_local_path, 'wb') as volume_file: - backup_service.restore(backup, volume['id'], volume_file) + backup_service.restore(backup, volume.id, volume_file) diff --git a/cinder/volume/drivers/smbfs.py b/cinder/volume/drivers/smbfs.py index 29bc6ca2e1d..2c5a021abb8 100644 --- a/cinder/volume/drivers/smbfs.py +++ b/cinder/volume/drivers/smbfs.py @@ -90,7 +90,7 @@ def update_allocation_data(delete=False): if delete: allocated_size_gb = None else: - allocated_size_gb = requested_size or volume['size'] + allocated_size_gb = requested_size or volume.size inst.update_disk_allocation_data(volume, allocated_size_gb) return ret_val @@ -149,11 +149,11 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): active_file = self.get_active_image_from_info(volume) fmt = self.get_volume_format(volume) - data = {'export': volume['provider_location'], + data = {'export': volume.provider_location, 'format': fmt, 'name': active_file} - if volume['provider_location'] in self.shares: - data['options'] = self.shares[volume['provider_location']] + if volume.provider_location in self.shares: + data['options'] = self.shares[volume.provider_location] return { 'driver_volume_type': self.driver_volume_type, 'data': data, @@ -206,8 +206,8 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): self._allocation_data = json.load(f) def update_disk_allocation_data(self, volume, virtual_size_gb=None): - volume_name = volume['name'] - smbfs_share = volume['provider_location'] + volume_name = volume.name + smbfs_share = volume.provider_location if smbfs_share: share_hash = self._get_hash_str(smbfs_share) else: @@ -257,7 +257,7 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): def _get_local_volume_path_template(self, volume): local_dir = self._local_volume_dir(volume) - local_path_template = os.path.join(local_dir, volume['name']) + local_path_template = os.path.join(local_dir, volume.name) return local_path_template def _lookup_local_volume_path(self, volume_path_template): @@ -271,9 +271,9 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): return '%s%s' % (self.local_path(volume), '.info') def _get_new_snap_path(self, snapshot): - vol_path = self.local_path(snapshot['volume']) + vol_path = self.local_path(snapshot.volume) snap_path, ext = os.path.splitext(vol_path) - snap_path += '.' + snapshot['id'] + ext + snap_path += '.' + snapshot.id + ext return snap_path def get_volume_format(self, volume, qemu_format=False): @@ -285,7 +285,7 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): if ext in self._SUPPORTED_IMAGE_FORMATS: volume_format = ext else: - info = self._qemu_img_info(volume_path, volume['name']) + info = self._qemu_img_info(volume_path, volume.name) volume_format = info.file_format else: volume_format = ( @@ -303,12 +303,12 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): @update_allocation_data(delete=True) def delete_volume(self, volume): """Deletes a logical volume.""" - if not volume['provider_location']: + if not volume.provider_location: LOG.warning(_LW('Volume %s does not have provider_location ' - 'specified, skipping.'), volume['name']) + 'specified, skipping.'), volume.name) return - self._ensure_share_mounted(volume['provider_location']) + self._ensure_share_mounted(volume.provider_location) volume_dir = self._local_volume_dir(volume) mounted_path = os.path.join(volume_dir, self.get_active_image_from_info(volume)) @@ -343,7 +343,7 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): """ volume_format = self.get_volume_format(volume) volume_path = self.local_path(volume) - volume_size = volume['size'] + volume_size = volume.size LOG.debug("Creating new volume at %s.", volume_path) @@ -470,7 +470,7 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): snapshot, backing_filename, new_snap_path) def _check_snapshot_support(self, snapshot): - volume_format = self.get_volume_format(snapshot['volume']) + volume_format = self.get_volume_format(snapshot.volume) # qemu-img does not yet support differencing vhd/vhdx if volume_format in (self._DISK_FORMAT_VHD, self._DISK_FORMAT_VHDX): err_msg = _("Snapshots are not supported for this volume " @@ -480,7 +480,7 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): @remotefs_drv.locked_volume_id_operation @update_allocation_data() def extend_volume(self, volume, size_gb): - LOG.info(_LI('Extending volume %s.'), volume['id']) + LOG.info(_LI('Extending volume %s.'), volume.id) self._extend_volume(volume, size_gb) def _extend_volume(self, volume, size_gb): @@ -489,7 +489,7 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): self._check_extend_volume_support(volume, size_gb) LOG.info(_LI('Resizing file to %sG...'), size_gb) - self._do_extend_volume(volume_path, size_gb, volume['name']) + self._do_extend_volume(volume_path, size_gb, volume.name) def _do_extend_volume(self, volume_path, size_gb, volume_name): info = self._qemu_img_info(volume_path, volume_name) @@ -523,12 +523,12 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): 'driver when no snapshots exist.') raise exception.InvalidVolume(msg) - extend_by = int(size_gb) - volume['size'] - if not self._is_share_eligible(volume['provider_location'], + extend_by = int(size_gb) - volume.size + if not self._is_share_eligible(volume.provider_location, extend_by): raise exception.ExtendVolumeError(reason='Insufficient space to ' 'extend volume %s to %sG.' - % (volume['id'], size_gb)) + % (volume.id, size_gb)) @remotefs_drv.locked_volume_id_operation @update_allocation_data() @@ -544,22 +544,22 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): LOG.debug("Snapshot: %(snap)s, volume: %(vol)s, " "volume_size: %(size)s", - {'snap': snapshot['id'], - 'vol': volume['id'], + {'snap': snapshot.id, + 'vol': volume.id, 'size': volume_size}) - info_path = self._local_path_volume_info(snapshot['volume']) + info_path = self._local_path_volume_info(snapshot.volume) snap_info = self._read_info_file(info_path) - vol_dir = self._local_volume_dir(snapshot['volume']) + vol_dir = self._local_volume_dir(snapshot.volume) out_format = self.get_volume_format(volume, qemu_format=True) - forward_file = snap_info[snapshot['id']] + forward_file = snap_info[snapshot.id] forward_path = os.path.join(vol_dir, forward_file) # Find the file which backs this file, which represents the point # when this snapshot was created. img_info = self._qemu_img_info(forward_path, - snapshot['volume']['name']) + snapshot.volume.name) path_to_snap_img = os.path.join(vol_dir, img_info.backing_file) LOG.debug("Will copy from snapshot at %s", path_to_snap_img) @@ -581,15 +581,15 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): self.configuration.volume_dd_blocksize) self._do_extend_volume(self.local_path(volume), - volume['size'], - volume['name']) + volume.size, + volume.name) data = image_utils.qemu_img_info(self.local_path(volume)) virt_size = data.virtual_size / units.Gi - if virt_size != volume['size']: + if virt_size != volume.size: raise exception.ImageUnacceptable( image_id=image_id, - reason=(_("Expected volume size was %d") % volume['size']) + reason=(_("Expected volume size was %d") % volume.size) + (_(" but size is now %d.") % virt_size)) @remotefs_drv.locked_volume_id_operation @@ -639,38 +639,15 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver): return flags.strip(',') def _get_volume_format_spec(self, volume): - # This method needs to be able to parse metadata/volume type - # specs for volume SQLAlchemy objects and versioned objects, - # as the transition to versioned objects is not complete and the - # driver may receive either of them. - # - # TODO(lpetrut): once the transition to oslo.versionedobjects is - # complete, we can skip some of those checks. - volume_metadata_specs = {} - volume_type_specs = {} + vol_type = volume.volume_type + extra_specs = {} + if vol_type and vol_type.extra_specs: + extra_specs = vol_type.extra_specs - if volume.get('metadata') and isinstance(volume.metadata, dict): - volume_metadata_specs.update(volume.metadata) - elif volume.get('volume_metadata'): - volume_metadata_specs.update( - {spec.key: spec.value for spec in volume.volume_metadata}) + extra_specs.update(volume.metadata or {}) - vol_type = volume.get('volume_type') - if vol_type: - specs = vol_type.get('extra_specs') or {} - if isinstance(specs, dict): - volume_type_specs.update(specs) - else: - volume_type_specs.update( - {spec.key: spec.value for spec in specs}) - - # In this case, we want the volume metadata specs to take - # precedence over the volume type specs. - for specs in [volume_metadata_specs, volume_type_specs]: - for key, val in specs.items(): - if 'volume_format' in key: - return val - return None + return (extra_specs.get('volume_format') or + self.configuration.smbfs_default_volume_format) def _is_file_size_equal(self, path, size): """Checks if file size at path is equal to size.""" diff --git a/cinder/volume/drivers/vzstorage.py b/cinder/volume/drivers/vzstorage.py index aaa304e0fed..365341c49df 100644 --- a/cinder/volume/drivers/vzstorage.py +++ b/cinder/volume/drivers/vzstorage.py @@ -110,10 +110,10 @@ class VZStorageDriver(remotefs_drv.RemoteFSSnapDriver): active_file = self.get_active_image_from_info(volume) active_file_path = os.path.join(self._local_volume_dir(volume), active_file) - info = self._qemu_img_info(active_file_path, volume['name']) + info = self._qemu_img_info(active_file_path, volume.name) fmt = info.file_format - data = {'export': volume['provider_location'], + data = {'export': volume.provider_location, 'format': fmt, 'name': active_file, } @@ -228,7 +228,7 @@ class VZStorageDriver(remotefs_drv.RemoteFSSnapDriver): @remotefs_drv.locked_volume_id_operation def extend_volume(self, volume, size_gb): - LOG.info(_LI('Extending volume %s.'), volume['id']) + LOG.info(_LI('Extending volume %s.'), volume.id) self._extend_volume(volume, size_gb) def _extend_volume(self, volume, size_gb): @@ -257,12 +257,12 @@ class VZStorageDriver(remotefs_drv.RemoteFSSnapDriver): 'driver when no snapshots exist.') raise exception.InvalidVolume(msg) - extend_by = int(size_gb) - volume['size'] - if not self._is_share_eligible(volume['provider_location'], + extend_by = int(size_gb) - volume.size + if not self._is_share_eligible(volume.provider_location, extend_by): raise exception.ExtendVolumeError(reason='Insufficient space to ' 'extend volume %s to %sG.' - % (volume['id'], size_gb)) + % (volume.id, size_gb)) def _is_file_size_equal(self, path, size): """Checks if file size at path is equal to size.""" @@ -279,23 +279,23 @@ class VZStorageDriver(remotefs_drv.RemoteFSSnapDriver): LOG.debug("_copy_volume_from_snapshot: snapshot: %(snap)s, " "volume: %(vol)s, volume_size: %(size)s.", - {'snap': snapshot['id'], - 'vol': volume['id'], + {'snap': snapshot.id, + 'vol': volume.id, 'size': volume_size, }) - info_path = self._local_path_volume_info(snapshot['volume']) + info_path = self._local_path_volume_info(snapshot.volume) snap_info = self._read_info_file(info_path) - vol_dir = self._local_volume_dir(snapshot['volume']) + vol_dir = self._local_volume_dir(snapshot.volume) out_format = "raw" - forward_file = snap_info[snapshot['id']] + forward_file = snap_info[snapshot.id] forward_path = os.path.join(vol_dir, forward_file) # Find the file which backs this file, which represents the point # when this snapshot was created. img_info = self._qemu_img_info(forward_path, - snapshot['volume']['name']) + snapshot.volume.name) path_to_snap_img = os.path.join(vol_dir, img_info.backing_file) LOG.debug("_copy_volume_from_snapshot: will copy " @@ -309,13 +309,13 @@ class VZStorageDriver(remotefs_drv.RemoteFSSnapDriver): @remotefs_drv.locked_volume_id_operation def delete_volume(self, volume): """Deletes a logical volume.""" - if not volume['provider_location']: + if not volume.provider_location: msg = (_('Volume %s does not have provider_location ' - 'specified, skipping.') % volume['name']) + 'specified, skipping.') % volume.name) LOG.error(msg) raise exception.VzStorageException(msg) - self._ensure_share_mounted(volume['provider_location']) + self._ensure_share_mounted(volume.provider_location) volume_dir = self._local_volume_dir(volume) mounted_path = os.path.join(volume_dir, self.get_active_image_from_info(volume))