Merge "VNX: cg and generic group cleanup"

cinder/tests/unit/volume/drivers/dell_emc/vnx/mocked_cinder.yaml

@@ -447,7 +447,14 @@ test_validate_cg_type:
         _uuid: GROUP_ID
       volume_type_ids: ['type1']
 
-
+test_require_consistent_group_snapshot_enabled:
+  group:
+    _type: 'group'
+    _properties:
+      id:
+        _uuid: group_id
+      group_type_id:
+        _uuid: group_type_id
 ###########################################################
 # TestClient
 ###########################################################

cinder/tests/unit/volume/drivers/dell_emc/vnx/res_mock.py

@@ -19,6 +19,7 @@ import six
 from cinder.tests.unit.consistencygroup import fake_cgsnapshot
 from cinder.tests.unit.consistencygroup import fake_consistencygroup
 from cinder.tests.unit import fake_constants
+from cinder.tests.unit import fake_group
 from cinder.tests.unit import fake_snapshot
 from cinder.tests.unit import fake_volume
 from cinder.tests.unit.volume.drivers.dell_emc.vnx import fake_exception as \
@@ -121,6 +122,10 @@ def _fake_cg_snapshot_wrapper(*args, **kwargs):
     return fake_cgsnapshot.fake_cgsnapshot_obj(None, **kwargs)
 
 
+def _fake_group_wrapper(*args, **kwargs):
+    return fake_group.fake_group_obj(None, **kwargs)
+
+
 class EnumBuilder(object):
     def __init__(self, enum_dict):
         enum_dict = enum_dict[SYMBOL_ENUM]
@@ -137,7 +142,8 @@ class CinderResourceMock(DriverResourceMock):
     fake_func_mapping = {'volume': _fake_volume_wrapper,
                          'cg': _fake_cg_wrapper,
                          'snapshot': _fake_snapshot_wrapper,
-                         'cg_snapshot': _fake_cg_snapshot_wrapper}
+                         'cg_snapshot': _fake_cg_snapshot_wrapper,
+                         'group': _fake_group_wrapper}
 
     def __init__(self, yaml_file):
         super(CinderResourceMock, self).__init__(yaml_file)

cinder/tests/unit/volume/drivers/dell_emc/vnx/test_adapter.py

@@ -297,7 +297,6 @@ class TestCommonAdapter(test.TestCase):
         self.assertTrue(stats['fast_support'])
         self.assertTrue(stats['deduplication_support'])
         self.assertTrue(stats['thin_provisioning_support'])
-        self.assertTrue(stats['consistencygroup_support'])
         self.assertTrue(stats['consistent_group_snapshot_enabled'])
 
     @res_mock.patch_common_adapter
@@ -310,8 +309,8 @@ class TestCommonAdapter(test.TestCase):
             'fast_support': True,
             'deduplication_support': True,
             'thin_provisioning_support': True,
-            'consistencygroup_support': True,
             'consistent_group_snapshot_enabled': True,
+            'consistencygroup_support': True
 
         }
         pool_stats = vnx_common.get_pool_stats(stats)
@@ -341,8 +340,8 @@ class TestCommonAdapter(test.TestCase):
             'fast_support': True,
             'deduplication_support': True,
             'thin_provisioning_support': True,
-            'consistencygroup_support': True,
             'consistent_group_snapshot_enabled': True,
+            'consistencygroup_support': True
 
         }
         pool_stats = vnx_common.get_pool_stats(stats)
@@ -360,8 +359,8 @@ class TestCommonAdapter(test.TestCase):
             'fast_support': True,
             'deduplication_support': True,
             'thin_provisioning_support': True,
-            'consistencygroup_support': True,
             'consistent_group_snapshot_enabled': True,
+            'consistencygroup_support': True
 
         }
         vnx_common.reserved_percentage = 15

cinder/tests/unit/volume/drivers/dell_emc/vnx/test_driver.py

@@ -71,11 +71,3 @@ class TestVNXDriver(test.TestCase):
         _driver.terminate_connection('fake_volume', {'host': 'fake_host'})
         _driver.adapter.terminate_connection.assert_called_once_with(
             'fake_volume', {'host': 'fake_host'})
-
-    def test_is_consistent_group_snapshot_enabled(self):
-        _driver = self._get_driver('iscsi')
-        _driver._stats = {'consistent_group_snapshot_enabled': True}
-        self.assertTrue(_driver.is_consistent_group_snapshot_enabled())
-        _driver._stats = {'consistent_group_snapshot_enabled': False}
-        self.assertFalse(_driver.is_consistent_group_snapshot_enabled())
-        self.assertFalse(_driver.is_consistent_group_snapshot_enabled())

cinder/tests/unit/volume/drivers/dell_emc/vnx/test_utils.py

@@ -15,28 +15,22 @@
 
 import mock
 
-from cinder import exception
 from cinder import test
 from cinder.tests.unit.volume.drivers.dell_emc.vnx import fake_exception \
     as storops_ex
 from cinder.tests.unit.volume.drivers.dell_emc.vnx import fake_storops \
     as storops
 from cinder.tests.unit.volume.drivers.dell_emc.vnx import res_mock
-from cinder.tests.unit.volume.drivers.dell_emc.vnx import utils as ut_utils
+from cinder.tests.unit.volume.drivers.dell_emc.vnx import utils
 from cinder.volume.drivers.dell_emc.vnx import common
-from cinder.volume.drivers.dell_emc.vnx import utils
+from cinder.volume.drivers.dell_emc.vnx import utils as vnx_utils
 
 
 class FakeDriver(object):
-    def __init__(self, support):
-        self.support = support
 
-    def is_consistent_group_snapshot_enabled(self):
+    @vnx_utils.require_consistent_group_snapshot_enabled
-        return self.support
+    def fake_group_method(self, context, group_or_snap):
-
+        return True
-    @utils.require_consistent_group_snapshot_enabled
-    def fake_method(self):
-        return 'called'
 
 
 class TestUtils(test.TestCase):
@@ -51,7 +45,7 @@ class TestUtils(test.TestCase):
 
     def test_wait_until(self):
         mock_testmethod = mock.Mock(return_value=True)
-        utils.wait_until(mock_testmethod, interval=0)
+        vnx_utils.wait_until(mock_testmethod, interval=0)
         mock_testmethod.assert_has_calls([mock.call()])
 
     def test_wait_until_with_exception(self):
@@ -59,7 +53,7 @@ class TestUtils(test.TestCase):
             side_effect=storops_ex.VNXAttachSnapError('Unknown error'))
         mock_testmethod.__name__ = 'test_method'
         self.assertRaises(storops_ex.VNXAttachSnapError,
-                          utils.wait_until,
+                          vnx_utils.wait_until,
                           mock_testmethod,
                           timeout=1,
                           interval=0,
@@ -70,9 +64,9 @@ class TestUtils(test.TestCase):
 
     def test_wait_until_with_params(self):
         mock_testmethod = mock.Mock(return_value=True)
-        utils.wait_until(mock_testmethod,
+        vnx_utils.wait_until(mock_testmethod,
-                         param1=1,
+                             param1=1,
-                         param2='test')
+                             param2='test')
         mock_testmethod.assert_has_calls(
             [mock.call(param1=1, param2='test')])
         mock_testmethod.assert_has_calls([mock.call(param1=1, param2='test')])
@@ -81,7 +75,7 @@ class TestUtils(test.TestCase):
     def test_retype_need_migration_when_host_changed(self, driver_in):
         volume = driver_in['volume']
         another_host = driver_in['host']
-        re = utils.retype_need_migration(
+        re = vnx_utils.retype_need_migration(
             volume, None, None, another_host)
         self.assertTrue(re)
 
@@ -89,7 +83,7 @@ class TestUtils(test.TestCase):
     def test_retype_need_migration_for_smp_volume(self, driver_in):
         volume = driver_in['volume']
         host = driver_in['host']
-        re = utils.retype_need_migration(
+        re = vnx_utils.retype_need_migration(
             volume, None, None, host)
         self.assertTrue(re)
 
@@ -100,7 +94,7 @@ class TestUtils(test.TestCase):
         host = driver_in['host']
         old_spec = common.ExtraSpecs({'provisioning:type': 'thin'})
         new_spec = common.ExtraSpecs({'provisioning:type': 'deduplicated'})
-        re = utils.retype_need_migration(
+        re = vnx_utils.retype_need_migration(
             volume, old_spec.provision, new_spec.provision, host)
         self.assertTrue(re)
 
@@ -111,7 +105,7 @@ class TestUtils(test.TestCase):
         host = driver_in['host']
         old_spec = common.ExtraSpecs({'provisioning:type': 'thick'})
         new_spec = common.ExtraSpecs({'provisioning:type': 'compressed'})
-        re = utils.retype_need_migration(
+        re = vnx_utils.retype_need_migration(
             volume, old_spec.provision, new_spec.provision, host)
         self.assertFalse(re)
 
@@ -122,49 +116,41 @@ class TestUtils(test.TestCase):
         old_spec = common.ExtraSpecs({'storagetype:tiering': 'auto'})
         new_spec = common.ExtraSpecs(
             {'storagetype:tiering': 'starthighthenauto'})
-        re = utils.retype_need_migration(
+        re = vnx_utils.retype_need_migration(
             volume, old_spec.provision, new_spec.provision, host)
         self.assertFalse(re)
 
     def test_retype_need_change_tier(self):
-        re = utils.retype_need_change_tier(
+        re = vnx_utils.retype_need_change_tier(
             storops.VNXTieringEnum.AUTO, storops.VNXTieringEnum.HIGH_AUTO)
         self.assertTrue(re)
 
     def test_retype_need_turn_on_compression(self):
-        re = utils.retype_need_turn_on_compression(
+        re = vnx_utils.retype_need_turn_on_compression(
             storops.VNXProvisionEnum.THIN,
             storops.VNXProvisionEnum.COMPRESSED)
         self.assertTrue(re)
-        re = utils.retype_need_turn_on_compression(
+        re = vnx_utils.retype_need_turn_on_compression(
             storops.VNXProvisionEnum.THICK,
             storops.VNXProvisionEnum.COMPRESSED)
         self.assertTrue(re)
 
     def test_retype_not_need_turn_on_compression(self):
-        re = utils.retype_need_turn_on_compression(
+        re = vnx_utils.retype_need_turn_on_compression(
             storops.VNXProvisionEnum.DEDUPED,
             storops.VNXProvisionEnum.COMPRESSED)
         self.assertFalse(re)
-        re = utils.retype_need_turn_on_compression(
+        re = vnx_utils.retype_need_turn_on_compression(
             storops.VNXProvisionEnum.DEDUPED,
             storops.VNXProvisionEnum.COMPRESSED)
         self.assertFalse(re)
 
-    @ut_utils.patch_extra_specs({'provisioning:type': 'compressed'})
-    @res_mock.mock_driver_input
-    def test_validate_cg_type(self, mocked_input):
-        cg = mocked_input['cg']
-        self.assertRaises(exception.InvalidInput,
-                          utils.validate_cg_type,
-                          cg)
-
     @res_mock.mock_driver_input
     def test_get_base_lun_name(self, mocked):
         volume = mocked['volume']
         self.assertEqual(
             'test',
-            utils.get_base_lun_name(volume))
+            vnx_utils.get_base_lun_name(volume))
 
     def test_convert_to_tgt_list_and_itor_tgt_map(self):
         zone_mapping = {
@@ -179,20 +165,16 @@ class TestUtils(test.TestCase):
         }
 
         tgt_wwns, itor_tgt_map = (
-            utils.convert_to_tgt_list_and_itor_tgt_map(zone_mapping))
+            vnx_utils.convert_to_tgt_list_and_itor_tgt_map(zone_mapping))
-        self.assertEqual(set(['wwnt_1', 'wwnt_2', 'wwnt_3']), set(tgt_wwns))
+        self.assertEqual({'wwnt_1', 'wwnt_2', 'wwnt_3'}, set(tgt_wwns))
         self.assertEqual({'wwn1_1': ['wwnt_1', 'wwnt_2'],
                           'wwn2_1': ['wwnt_1', 'wwnt_3'],
                           'wwn2_2': ['wwnt_1', 'wwnt_3']},
                          itor_tgt_map)
 
-    def test_cg_snapshot_is_not_enabled(self):
+    @utils.patch_group_specs('<is> True')
-        def do():
+    @res_mock.mock_driver_input
-            driver = FakeDriver(False)
+    def test_require_consistent_group_snapshot_enabled(self, input):
-            driver.fake_method()
+        driver = FakeDriver()
-        self.assertRaises(NotImplementedError, do)
+        is_called = driver.fake_group_method('context', input['group'])
-
+        self.assertTrue(is_called)
-    def test_cg_snapshot_is_enabled(self):
-        driver = FakeDriver(True)
-        ret = driver.fake_method()
-        self.assertEqual('called', ret)

cinder/tests/unit/volume/drivers/dell_emc/vnx/utils.py

@@ -44,6 +44,12 @@ def patch_extra_specs(specs):
         return_value=specs)
 
 
+def patch_group_specs(specs):
+    return _build_patch_decorator(
+        'cinder.volume.group_types.get_group_type_specs',
+        return_value=specs)
+
+
 def patch_extra_specs_validate(return_value=None, side_effect=None):
     return _build_patch_decorator(
         'cinder.volume.drivers.dell_emc.vnx.common.ExtraSpecs.validate',

cinder/volume/drivers/dell_emc/vnx/adapter.py

@@ -237,7 +237,7 @@ class CommonAdapter(object):
                   'provision': provision,
                   'tier': tier})
 
-        cg_id = volume.group_id or volume.consistencygroup_id
+        cg_id = volume.group_id
         lun = self.client.create_lun(
             pool, volume_name, volume_size,
             provision, tier, cg_id,
@@ -461,7 +461,6 @@ class CommonAdapter(object):
 
     def create_consistencygroup(self, context, group):
         cg_name = group.id
-        utils.validate_cg_type(group)
         model_update = {'status': fields.ConsistencyGroupStatus.AVAILABLE}
         self.client.create_consistency_group(cg_name=cg_name)
         return model_update

cinder/volume/drivers/dell_emc/vnx/driver.py

@@ -250,52 +250,14 @@ class VNXDriver(driver.ManageableVD,
         """Return size of volume to be managed by manage_existing."""
         return self.adapter.manage_existing_get_size(volume, existing_ref)
 
-    def create_consistencygroup(self, context, group):
-        """Creates a consistencygroup."""
-        return self.adapter.create_consistencygroup(context, group)
-
-    def delete_consistencygroup(self, context, group, volumes):
-        """Deletes a consistency group."""
-        return self.adapter.delete_consistencygroup(
-            context, group, volumes)
-
-    def create_cgsnapshot(self, context, cgsnapshot, snapshots):
-        """Creates a cgsnapshot."""
-        return self.adapter.create_cgsnapshot(
-            context, cgsnapshot, snapshots)
-
-    def delete_cgsnapshot(self, context, cgsnapshot, snapshots):
-        """Deletes a cgsnapshot."""
-        return self.adapter.delete_cgsnapshot(
-            context, cgsnapshot, snapshots)
-
     def get_pool(self, volume):
         """Returns the pool name of a volume."""
         return self.adapter.get_pool_name(volume)
 
-    def update_consistencygroup(self, context, group,
-                                add_volumes,
-                                remove_volumes):
-        """Updates LUNs in consistency group."""
-        return self.adapter.update_consistencygroup(context, group,
-                                                    add_volumes,
-                                                    remove_volumes)
-
     def unmanage(self, volume):
         """Unmanages a volume."""
         return self.adapter.unmanage(volume)
 
-    def create_consistencygroup_from_src(self, context, group, volumes,
-                                         cgsnapshot=None, snapshots=None,
-                                         source_cg=None, source_vols=None):
-        """Creates a consistency group from source."""
-        if cgsnapshot:
-            return self.adapter.create_cg_from_cgsnapshot(
-                context, group, volumes, cgsnapshot, snapshots)
-        elif source_cg:
-            return self.adapter.create_cloned_cg(
-                context, group, volumes, source_cg, source_vols)
-
     def update_migrated_volume(self, context, volume, new_volume,
                                original_volume_status=None):
         """Returns model update for migrated volume."""
@@ -372,6 +334,3 @@ class VNXDriver(driver.ManageableVD,
         """Deletes a group_snapshot."""
         return self.adapter.delete_group_snapshot(
             context, group_snapshot, snapshots)
-
-    def is_consistent_group_snapshot_enabled(self):
-        return self._stats.get('consistent_group_snapshot_enabled')

cinder/volume/drivers/dell_emc/vnx/utils.py

@@ -21,14 +21,13 @@ from oslo_service import loopingcall
 from oslo_utils import excutils
 from oslo_utils import importutils
 
-storops = importutils.try_import('storops')
-
-from cinder import exception
 from cinder.i18n import _
 from cinder.volume.drivers.dell_emc.vnx import common
 from cinder.volume.drivers.san.san import san_opts
 from cinder.volume import utils as vol_utils
-from cinder.volume import volume_types
+
+storops = importutils.try_import('storops')
+
 
 LOG = logging.getLogger(__name__)
 
@@ -259,21 +258,6 @@ def get_migration_rate(volume):
             return storops.VNXMigrationRate.HIGH
 
 
-def validate_cg_type(group):
-    if not group.get('volume_type_ids'):
-        return
-    for type_id in group.get('volume_type_ids'):
-        if type_id:
-            specs = volume_types.get_volume_type_extra_specs(type_id)
-            extra_specs = common.ExtraSpecs(specs)
-            if extra_specs.provision == storops.VNXProvisionEnum.COMPRESSED:
-                msg = _("Failed to create consistency group %s "
-                        "because VNX consistency group cannot "
-                        "accept compressed LUNs as members."
-                        ) % group['id']
-                raise exception.InvalidInput(reason=msg)
-
-
 def update_res_without_poll(res):
     with res.with_no_poll():
         res.update()
@@ -356,7 +340,7 @@ def is_volume_smp(volume):
 def require_consistent_group_snapshot_enabled(func):
     @six.wraps(func)
     def inner(self, *args, **kwargs):
-        if not self.is_consistent_group_snapshot_enabled():
+        if not vol_utils.is_group_a_cg_snapshot_type(args[1]):
             raise NotImplementedError
         return func(self, *args, **kwargs)
     return inner