Merge "Make IBM drivers return snapshot object for cg ops"

cinder/tests/unit/test_gpfs.py

@@ -23,8 +23,10 @@ from oslo_utils import units
 
 from cinder import context
 from cinder import exception
+from cinder import objects
 from cinder.objects import fields
 from cinder import test
+from cinder.tests.unit import fake_constants as fake
 from cinder import utils
 from cinder.volume import configuration as conf
 from cinder.volume.drivers.ibm import gpfs
@@ -721,19 +723,6 @@ class GPFSDriverTestCase(test.TestCase):
         self.flags(volume_driver=self.driver_name,
                    gpfs_storage_pool=org_value)
 
-    def test_get_volume_metadata(self):
-        volume = self._fake_volume()
-        volume['volume_metadata'] = [{'key': 'fake_key',
-                                      'value': 'fake_value'}]
-        expected_metadata = {'fake_key': 'fake_value'}
-        v_metadata = self.driver._get_volume_metadata(volume)
-        self.assertEqual(expected_metadata, v_metadata)
-        volume.pop('volume_metadata')
-        volume['metadata'] = {'key': 'value'}
-        expected_metadata = {'key': 'value'}
-        v_metadata = self.driver._get_volume_metadata(volume)
-        self.assertEqual(expected_metadata, v_metadata)
-
     @mock.patch('cinder.utils.execute')
     @mock.patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver.'
                 '_allocate_file_blocks')
@@ -1091,6 +1080,7 @@ class GPFSDriverTestCase(test.TestCase):
         self.assertTrue(self.driver._is_gpfs_parent_file(''))
         self.assertFalse(self.driver._is_gpfs_parent_file(''))
 
+    @mock.patch('cinder.objects.volume.Volume.get_by_id')
     @mock.patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver._gpfs_redirect')
     @mock.patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver.'
                 '_set_rw_permission')
@@ -1103,14 +1093,16 @@ class GPFSDriverTestCase(test.TestCase):
                              mock_local_path,
                              mock_create_gpfs_snap,
                              mock_set_rw_permission,
-                             mock_gpfs_redirect):
+                             mock_gpfs_redirect,
+                             mock_vol_get_by_id):
         org_value = self.driver.configuration.gpfs_mount_point_base
         mock_get_snapshot_path.return_value = "/tmp/fakepath"
         self.flags(volume_driver=self.driver_name,
                    gpfs_mount_point_base=self.volumes_path)
-        snapshot = {}
+
-        snapshot['volume_name'] = 'test'
+        vol = self._fake_volume()
-        self.driver.create_snapshot(snapshot)
+        mock_vol_get_by_id.return_value = vol
+        self.driver.create_snapshot(self._fake_snapshot())
         self.flags(volume_driver=self.driver_name,
                    gpfs_mount_point_base=org_value)
 
@@ -1144,7 +1136,7 @@ class GPFSDriverTestCase(test.TestCase):
         volume = self._fake_volume()
         mock_local_path.return_value = "/tmp/fakepath"
         data = self.driver.initialize_connection(volume, '')
-        self.assertEqual('test', data['data']['name'])
+        self.assertEqual(volume.name, data['data']['name'])
         self.assertEqual("/tmp/fakepath", data['data']['device_path'])
         self.assertEqual('gpfs', data['driver_volume_type'])
 
@@ -1717,65 +1709,49 @@ class GPFSDriverTestCase(test.TestCase):
     def test_create_cgsnapshot(self, mock_create_snap):
         ctxt = self.context
         cgsnap = self._fake_cgsnapshot()
-        self.driver.db = mock.Mock()
-        self.driver.db.snapshot_get_all_for_cgsnapshot = mock.Mock()
         snapshot1 = self._fake_snapshot()
-        snapshots = [snapshot1]
-        self.driver.db.snapshot_get_all_for_cgsnapshot.return_value = snapshots
         model_update, snapshots = self.driver.create_cgsnapshot(ctxt, cgsnap,
-                                                                [])
+                                                                [snapshot1])
         self.driver.create_snapshot.assert_called_once_with(snapshot1)
-        self.assertEqual({'status': cgsnap['status']}, model_update)
+        self.assertEqual({'status': fields.ConsistencyGroupStatus.AVAILABLE},
-        self.assertEqual(fields.SnapshotStatus.AVAILABLE, snapshot1['status'])
+                         model_update)
-        self.driver.db.snapshot_get_all_for_cgsnapshot.\
+        self.assertEqual({'id': snapshot1.id,
-            assert_called_once_with(ctxt, cgsnap['id'])
+                         'status': fields.SnapshotStatus.AVAILABLE},
+                         snapshots[0])
 
     @mock.patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver.create_snapshot')
     def test_create_cgsnapshot_empty(self, mock_create_snap):
         ctxt = self.context
         cgsnap = self._fake_cgsnapshot()
-        self.driver.db = mock.Mock()
-        self.driver.db.snapshot_get_all_for_cgsnapshot = mock.Mock()
-        snapshots = []
-        self.driver.db.snapshot_get_all_for_cgsnapshot.return_value = snapshots
         model_update, snapshots = self.driver.create_cgsnapshot(ctxt, cgsnap,
                                                                 [])
         self.assertFalse(self.driver.create_snapshot.called)
-        self.assertEqual({'status': cgsnap['status']}, model_update)
+        self.assertEqual({'status': fields.ConsistencyGroupStatus.AVAILABLE},
-        self.driver.db.snapshot_get_all_for_cgsnapshot.\
+                         model_update)
-            assert_called_once_with(ctxt, cgsnap['id'])
 
     @mock.patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver.delete_snapshot')
     def test_delete_cgsnapshot(self, mock_delete_snap):
         ctxt = self.context
         cgsnap = self._fake_cgsnapshot()
-        self.driver.db = mock.Mock()
-        self.driver.db.snapshot_get_all_for_cgsnapshot = mock.Mock()
         snapshot1 = self._fake_snapshot()
-        snapshots = [snapshot1]
-        self.driver.db.snapshot_get_all_for_cgsnapshot.return_value = snapshots
         model_update, snapshots = self.driver.delete_cgsnapshot(ctxt, cgsnap,
-                                                                [])
+                                                                [snapshot1])
         self.driver.delete_snapshot.assert_called_once_with(snapshot1)
-        self.assertEqual({'status': cgsnap['status']}, model_update)
+        self.assertEqual({'status': fields.ConsistencyGroupStatus.DELETED},
-        self.assertEqual(fields.SnapshotStatus.DELETED, snapshot1['status'])
+                         model_update)
-        self.driver.db.snapshot_get_all_for_cgsnapshot.\
+        self.assertEqual({'id': snapshot1.id,
-            assert_called_once_with(ctxt, cgsnap['id'])
+                         'status': fields.SnapshotStatus.DELETED},
+                         snapshots[0])
 
     @mock.patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver.delete_snapshot')
     def test_delete_cgsnapshot_empty(self, mock_delete_snap):
         ctxt = self.context
         cgsnap = self._fake_cgsnapshot()
-        self.driver.db = mock.Mock()
-        self.driver.db.snapshot_get_all_for_cgsnapshot = mock.Mock()
-        snapshots = []
-        self.driver.db.snapshot_get_all_for_cgsnapshot.return_value = snapshots
         model_update, snapshots = self.driver.delete_cgsnapshot(ctxt, cgsnap,
                                                                 [])
         self.assertFalse(self.driver.delete_snapshot.called)
-        self.assertEqual({'status': cgsnap['status']}, model_update)
+        self.assertEqual({'status': fields.ConsistencyGroupStatus.DELETED},
-        self.driver.db.snapshot_get_all_for_cgsnapshot.\
+                         model_update)
-            assert_called_once_with(ctxt, cgsnap['id'])
 
     def test_local_path_volume_not_in_cg(self):
         volume = self._fake_volume()
@@ -1799,18 +1775,18 @@ class GPFSDriverTestCase(test.TestCase):
         self.assertEqual(volume_path, ret)
 
     @mock.patch('cinder.context.get_admin_context')
+    @mock.patch('cinder.objects.volume.Volume.get_by_id')
     @mock.patch('cinder.volume.drivers.ibm.gpfs.GPFSDriver.local_path')
-    def test_get_snapshot_path(self, mock_local_path, mock_admin_context):
+    def test_get_snapshot_path(self, mock_local_path, mock_vol_get_by_id,
+                               mock_admin_context):
         volume = self._fake_volume()
-        self.driver.db = mock.Mock()
+        mock_vol_get_by_id.return_value = volume
-        self.driver.db.volume_get = mock.Mock()
-        self.driver.db.volume_get.return_value = volume
         volume_path = self.volumes_path
         mock_local_path.return_value = volume_path
         snapshot = self._fake_snapshot()
         ret = self.driver._get_snapshot_path(snapshot)
         self.assertEqual(
-            os.path.join(os.path.dirname(volume_path), snapshot['name']), ret
+            os.path.join(os.path.dirname(volume_path), snapshot.name), ret
         )
 
     @mock.patch('cinder.utils.execute')
@@ -1823,42 +1799,41 @@ class GPFSDriverTestCase(test.TestCase):
 
     def _fake_volume(self):
         volume = {}
-        volume['id'] = '123456'
+        volume['id'] = fake.VOLUME_ID
-        volume['name'] = 'test'
+        volume['display_name'] = 'test'
+        volume['metadata'] = {'key1': 'val1'}
+        volume['_name_id'] = None
         volume['size'] = 1000
-        volume['consistencygroup_id'] = 'cg-1234'
+        volume['consistencygroup_id'] = fake.CONSISTENCY_GROUP_ID
-        return volume
+
+        return objects.Volume(self.context, **volume)
 
     def _fake_snapshot(self):
         snapshot = {}
-        snapshot['id'] = '12345'
+        snapshot['id'] = fake.SNAPSHOT_ID
-        snapshot['name'] = 'test-snap'
+        snapshot['display_name'] = 'test-snap'
         snapshot['size'] = 1000
-        snapshot['volume_id'] = '123456'
+        snapshot['volume_id'] = fake.VOLUME_ID
         snapshot['status'] = 'available'
-        return snapshot
+        snapshot['snapshot_metadata'] = []
+
+        return objects.Snapshot(context=self.context, **snapshot)
 
     def _fake_volume_in_cg(self):
-        volume = {}
+        volume = self._fake_volume()
-        volume['id'] = '123456'
+        volume.consistencygroup_id = fake.CONSISTENCY_GROUP_ID
-        volume['name'] = 'test'
-        volume['size'] = 1000
-        volume['consistencygroup_id'] = 'fakecg'
         return volume
 
     def _fake_group(self):
         group = {}
         group['name'] = 'test_group'
-        group['id'] = '123456'
+        group['id'] = fake.CONSISTENCY_GROUP_ID
         return group
 
     def _fake_cgsnapshot(self):
-        cgsnap = {}
+        snapshot = self._fake_snapshot()
-        cgsnap['id'] = '123456'
+        snapshot.consistencygroup_id = fake.CONSISTENCY_GROUP_ID
-        cgsnap['name'] = 'testsnap'
+        return snapshot
-        cgsnap['consistencygroup_id'] = '123456'
-        cgsnap['status'] = 'available'
-        return cgsnap
 
     def _fake_qemu_qcow2_image_info(self, path):
         data = FakeQemuImgInfo()
@@ -1894,7 +1869,7 @@ class GPFSDriverTestCase(test.TestCase):
                                                       new_type_ref['id'])
 
         volume = self._fake_volume()
-        volume['host'] = host
+        volume['host'] = 'foo'
 
         return (volume, new_type, diff, host)
 

cinder/tests/unit/test_storwize_svc.py

@@ -34,6 +34,7 @@ import six
 from cinder import context
 from cinder import exception
 from cinder.i18n import _
+from cinder import objects
 from cinder.objects import fields
 from cinder import ssh_utils
 from cinder import test
@@ -4254,6 +4255,8 @@ class StorwizeSVCCommonDriverTestCase(test.TestCase):
 
         cg_snapshot, snapshots = self._create_cgsnapshot_in_db(cg['id'])
 
+        snapshots = objects.SnapshotList.get_all_for_cgsnapshot(
+            self.ctxt, cg_snapshot.id)
         model_update = self.driver.create_cgsnapshot(self.ctxt, cg_snapshot,
                                                      snapshots)
         self.assertEqual('available',

cinder/volume/drivers/ibm/gpfs.py

@@ -511,14 +511,6 @@ class GPFSDriver(driver.ConsistencyGroupVD, driver.ExtendVD,
         if fstype:
             self._mkfs(volume, fstype, fslabel)
 
-    def _get_volume_metadata(self, volume):
-        volume_metadata = {}
-        if 'volume_metadata' in volume:
-            for metadata in volume['volume_metadata']:
-                volume_metadata[metadata['key']] = metadata['value']
-            return volume_metadata
-        return volume['metadata'] if 'metadata' in volume else {}
-
     def create_volume(self, volume):
         """Creates a GPFS volume."""
         # Check if GPFS is mounted
@@ -532,8 +524,7 @@ class GPFSDriver(driver.ConsistencyGroupVD, driver.ExtendVD,
         self._set_rw_permission(volume_path)
         # Set the attributes prior to allocating any blocks so that
         # they are allocated according to the policy
-        v_metadata = self._get_volume_metadata(volume)
+        self._set_volume_attributes(volume, volume_path, volume.metadata)
-        self._set_volume_attributes(volume, volume_path, v_metadata)
 
         if not self.configuration.gpfs_sparse_volumes:
             self._allocate_file_blocks(volume_path, volume_size)
@@ -556,8 +547,7 @@ class GPFSDriver(driver.ConsistencyGroupVD, driver.ExtendVD,
             self._gpfs_full_copy(snapshot_path, volume_path)
 
         self._set_rw_permission(volume_path)
-        v_metadata = self._get_volume_metadata(volume)
+        self._set_volume_attributes(volume, volume_path, volume.metadata)
-        self._set_volume_attributes(volume, volume_path, v_metadata)
 
     def create_volume_from_snapshot(self, volume, snapshot):
         """Creates a GPFS volume from a snapshot."""
@@ -576,8 +566,7 @@ class GPFSDriver(driver.ConsistencyGroupVD, driver.ExtendVD,
         else:
             self._gpfs_full_copy(src, dest)
         self._set_rw_permission(dest)
-        v_metadata = self._get_volume_metadata(volume)
+        self._set_volume_attributes(volume, dest, volume.metadata)
-        self._set_volume_attributes(volume, dest, v_metadata)
 
     def create_cloned_volume(self, volume, src_vref):
         """Create a GPFS volume from another volume."""
@@ -718,7 +707,7 @@ class GPFSDriver(driver.ConsistencyGroupVD, driver.ExtendVD,
         """Creates a GPFS snapshot."""
         snapshot_path = self._get_snapshot_path(snapshot)
         volume_path = os.path.join(os.path.dirname(snapshot_path),
-                                   snapshot['volume_name'])
+                                   snapshot.volume.name)
         self._create_gpfs_snap(src=volume_path, dest=snapshot_path)
         self._set_rw_permission(snapshot_path, modebits='640')
         self._gpfs_redirect(volume_path)
@@ -737,11 +726,9 @@ class GPFSDriver(driver.ConsistencyGroupVD, driver.ExtendVD,
                           check_exit_code=False)
 
     def _get_snapshot_path(self, snapshot):
-        ctxt = context.get_admin_context()
+        snap_parent_vol_path = self.local_path(snapshot.volume)
-        snap_parent_vol = self.db.volume_get(ctxt, snapshot['volume_id'])
-        snap_parent_vol_path = self.local_path(snap_parent_vol)
         snapshot_path = os.path.join(os.path.dirname(snap_parent_vol_path),
-                                     snapshot['name'])
+                                     snapshot.name)
         return snapshot_path
 
     def local_path(self, volume):
@@ -1180,7 +1167,6 @@ class GPFSDriver(driver.ConsistencyGroupVD, driver.ExtendVD,
 
         model_update = {}
         model_update['status'] = group['status']
-        volumes = self.db.volume_get_all_by_group(context, group['id'])
 
         # Unlink and delete the fileset associated with the consistency group.
         # All of the volumes and volume snapshot data will also be deleted.
@@ -1211,29 +1197,47 @@ class GPFSDriver(driver.ConsistencyGroupVD, driver.ExtendVD,
 
     def create_cgsnapshot(self, context, cgsnapshot, snapshots):
         """Create snapshot of a consistency group of GPFS volumes."""
-        snapshots = self.db.snapshot_get_all_for_cgsnapshot(
+        model_update = {'status': fields.ConsistencyGroupStatus.AVAILABLE}
-            context, cgsnapshot['id'])
+        snapshots_model_update = []
+
+        try:
+            for snapshot in snapshots:
+                self.create_snapshot(snapshot)
+        except exception.VolumeBackendAPIException as err:
+            model_update['status'] = (
+                fields.ConsistencyGroupStatus.ERROR_CREATE)
+            LOG.error(_LE("Failed to create the snapshot %(snap)s of "
+                          "CGSnapshot. Exception: %(exception)s."),
+                      {'snap': snapshot.name, 'exception': err})
 
         for snapshot in snapshots:
-            self.create_snapshot(snapshot)
+            snapshots_model_update.append(
-            snapshot['status'] = fields.SnapshotStatus.AVAILABLE
+                {'id': snapshot.id,
+                 'status': model_update['status']})
 
-        model_update = {'status': 'available'}
+        return model_update, snapshots_model_update
-
-        return model_update, snapshots
 
     def delete_cgsnapshot(self, context, cgsnapshot, snapshots):
         """Delete snapshot of a consistency group of GPFS volumes."""
-        snapshots = self.db.snapshot_get_all_for_cgsnapshot(
+        model_update = {'status': fields.ConsistencyGroupStatus.DELETED}
-            context, cgsnapshot['id'])
+        snapshots_model_update = []
+
+        try:
+            for snapshot in snapshots:
+                self.delete_snapshot(snapshot)
+        except exception.VolumeBackendAPIException as err:
+            model_update['status'] = (
+                fields.ConsistencyGroupStatus.ERROR_DELETING)
+            LOG.error(_LE("Failed to delete the snapshot %(snap)s of "
+                          "CGSnapshot. Exception: %(exception)s."),
+                      {'snap': snapshot.name, 'exception': err})
 
         for snapshot in snapshots:
-            self.delete_snapshot(snapshot)
+            snapshots_model_update.append(
-            snapshot['status'] = fields.SnapshotStatus.DELETED
+                {'id': snapshot.id,
+                 'status': model_update['status']})
 
-        model_update = {'status': cgsnapshot['status']}
+        return model_update, snapshots_model_update
-
-        return model_update, snapshots
 
 
 class GPFSNFSDriver(GPFSDriver, nfs.NfsDriver, san.SanDriver):

cinder/volume/drivers/ibm/storwize_svc/storwize_svc_common.py

@@ -3077,7 +3077,7 @@ class StorwizeSVCCommonDriver(san.SanDriver,
     def create_cgsnapshot(self, ctxt, cgsnapshot, snapshots):
         """Creates a cgsnapshot."""
         # Use cgsnapshot id as cg name
-        cg_name = 'cg_snap-' + cgsnapshot['id']
+        cg_name = 'cg_snap-' + cgsnapshot.id
         # Create new cg as cg_snapshot
         self._helpers.create_fc_consistgrp(cg_name)