EMC VMAX - Fix for last volume in VMAX3 storage group

This patch fixes bug in the deletion the last volume in a
storage group for the VMAX3. If a masking view did not exist
we tried to get an instance of it which threw an exception.
Now we handle both scenarios correctly.

Closes-Bug: #1512796

Change-Id: Ideb2aaffe4fbc1ae2137d569d7b137ac0edef573

diff --git a/cinder/tests/unit/test_emc_vmax.py b/cinder/tests/unit/test_emc_vmax.py
index 1c688e65fe74b05c4b7b0109313f5ea851193ea8..259e6d84c264ba869f887b8a3380daa46e750970 100644 (file)
--- a/cinder/tests/unit/test_emc_vmax.py
+++ b/cinder/tests/unit/test_emc_vmax.py
@@ -5603,6 +5603,85 @@ class EMCV3DriverTestCase(test.TestCase):
                 'isV3': True,
                 'portgroupname': 'OS-portgroup-PG'}
 
+    def default_vol(self):
+        vol = EMC_StorageVolume()
+        vol['name'] = self.data.test_volume['name']
+        vol['CreationClassName'] = 'Symm_StorageVolume'
+        vol['ElementName'] = self.data.test_volume['id']
+        vol['DeviceID'] = self.data.test_volume['device_id']
+        vol['Id'] = self.data.test_volume['id']
+        vol['SystemName'] = self.data.storage_system
+        vol['NumberOfBlocks'] = self.data.test_volume['NumberOfBlocks']
+        vol['BlockSize'] = self.data.test_volume['BlockSize']
+        # Added vol to vol.path
+        vol['SystemCreationClassName'] = 'Symm_StorageSystem'
+        vol.path = vol
+        vol.path.classname = vol['CreationClassName']
+        return vol
+
+    def default_storage_group(self):
+        storagegroup = {}
+        storagegroup['CreationClassName'] = (
+            self.data.storagegroup_creationclass)
+        storagegroup['ElementName'] = 'no_masking_view'
+        return storagegroup
+
+    def test_last_vol_in_SG_with_MV(self):
+        conn = self.fake_ecom_connection()
+        controllerConfigService = (
+            self.driver.common.utils.find_controller_configuration_service(
+                conn, self.data.storage_system))
+
+        extraSpecs = self.default_extraspec()
+
+        storageGroupName = self.data.storagegroupname
+        storageGroupInstanceName = (
+            self.driver.common.utils.find_storage_masking_group(
+                conn, controllerConfigService, storageGroupName))
+
+        vol = self.default_vol()
+        self.driver.common.masking._delete_mv_and_sg = mock.Mock()
+        self.assertTrue(self.driver.common.masking._last_vol_in_SG(
+            conn, controllerConfigService, storageGroupInstanceName,
+            storageGroupName, vol, vol['name'], extraSpecs))
+
+    def test_last_vol_in_SG_no_MV(self):
+        conn = self.fake_ecom_connection()
+        controllerConfigService = (
+            self.driver.common.utils.find_controller_configuration_service(
+                conn, self.data.storage_system))
+
+        extraSpecs = self.default_extraspec()
+        self.driver.common.masking.get_masking_view_from_storage_group = (
+            mock.Mock(return_value=None))
+        self.driver.common.masking.utils.get_existing_instance = (
+            mock.Mock(return_value=None))
+        storagegroup = self.default_storage_group()
+
+        vol = self.default_vol()
+        self.assertTrue(self.driver.common.masking._last_vol_in_SG(
+            conn, controllerConfigService, storagegroup,
+            storagegroup['ElementName'], vol, vol['name'], extraSpecs))
+
+    def test_last_vol_in_SG_no_MV_fail(self):
+        self.driver.common.masking.utils.get_existing_instance = (
+            mock.Mock(return_value='value'))
+        conn = self.fake_ecom_connection()
+        controllerConfigService = (
+            self.driver.common.utils.find_controller_configuration_service(
+                conn, self.data.storage_system))
+
+        extraSpecs = self.default_extraspec()
+        vol = self.default_vol()
+        storagegroup = self.default_storage_group()
+        storagegroup['ElementName'] = 'no_masking_view'
+
+        self.assertRaises(exception.VolumeBackendAPIException,
+                          self.driver.common.masking._last_vol_in_SG,
+                          conn, controllerConfigService,
+                          storagegroup, storagegroup['ElementName'], vol,
+                          vol['name'], extraSpecs)
+
     @mock.patch.object(
         emc_vmax_utils.EMCVMAXUtils,
         'find_storageSystem',

diff --git a/cinder/volume/drivers/emc/emc_vmax_fc.py b/cinder/volume/drivers/emc/emc_vmax_fc.py
index 40843c0fb79a4f0ba4ba82baf61face85f1cae6b..4842864396ceae74b8c7831788a85895366a075d 100644 (file)
--- a/cinder/volume/drivers/emc/emc_vmax_fc.py
+++ b/cinder/volume/drivers/emc/emc_vmax_fc.py
@@ -53,6 +53,7 @@ class EMCVMAXFCDriver(driver.FibreChannelDriver):
               https://blueprints.launchpad.net/cinder/+spec/vmax3-extend-volume
               - Incorrect SG selected on an attach (#1515176)
               - Cleanup Zoning (bug #1501938)  NOTE: FC only
+              - Last volume in SG fix
     """
 
     VERSION = "2.3.0"

diff --git a/cinder/volume/drivers/emc/emc_vmax_iscsi.py b/cinder/volume/drivers/emc/emc_vmax_iscsi.py
index 3240754a59fb7a9ca569a0b4cab18943be268134..05eeb97f8efb85c5b33b8b2b037b083816cd25e9 100644 (file)
--- a/cinder/volume/drivers/emc/emc_vmax_iscsi.py
+++ b/cinder/volume/drivers/emc/emc_vmax_iscsi.py
@@ -59,6 +59,7 @@ class EMCVMAXISCSIDriver(driver.ISCSIDriver):
               https://blueprints.launchpad.net/cinder/+spec/vmax3-extend-volume
               - Incorrect SG selected on an attach (#1515176)
               - Cleanup Zoning (bug #1501938)  NOTE: FC only
+              - Last volume in SG fix
     """
 
     VERSION = "2.3.0"

diff --git a/cinder/volume/drivers/emc/emc_vmax_masking.py b/cinder/volume/drivers/emc/emc_vmax_masking.py
index d2bed73f887c6224830f4e55c25f87e572d1ae1a..c3f794f03a8a0502af582931ade8655e220ad8fa 100644 (file)
--- a/cinder/volume/drivers/emc/emc_vmax_masking.py
+++ b/cinder/volume/drivers/emc/emc_vmax_masking.py
@@ -1784,11 +1784,11 @@ class EMCVMAXMasking(object):
         storageSystemInstanceName = self.utils.find_storage_system(
             conn, controllerConfigService)
 
-        numVolInMaskingView = len(volumeInstanceNames)
+        numVolInStorageGroup = len(volumeInstanceNames)
         LOG.debug(
             "There are %(numVol)d volumes in the storage group "
             "%(maskingGroup)s.",
-            {'numVol': numVolInMaskingView,
+            {'numVol': numVolInStorageGroup,
              'maskingGroup': storageGroupInstanceName})
 
         if not isV3:
@@ -1796,22 +1796,57 @@ class EMCVMAXMasking(object):
                 self._get_tiering_info(conn, storageSystemInstanceName,
                                        fastPolicyName))
 
-        if numVolInMaskingView == 1:
+        if numVolInStorageGroup == 1:
             # Last volume in the storage group.
-            LOG.warning(_LW("Only one volume remains in storage group "
-                            "%(sgname)s. Driver will attempt cleanup."),
-                        {'sgname': storageGroupName})
-            mvInstanceName = self.get_masking_view_from_storage_group(
-                conn, storageGroupInstanceName)
-            if mvInstanceName is None:
-                LOG.warning(_LW("Unable to get masking view %(maskingView)s "
-                                "from storage group."),
-                            {'maskingView': mvInstanceName})
-            else:
-                maskingViewInstance = conn.GetInstance(
-                    mvInstanceName, LocalOnly=False)
-                maskingViewName = maskingViewInstance['ElementName']
+            self._last_vol_in_SG(
+                conn, controllerConfigService, storageGroupInstanceName,
+                storageGroupName, volumeInstance, volumeName, extraSpecs)
+        else:
+            # Not the last volume so remove it from storage group in
+            # the masking view.
+            self._multiple_vols_in_SG(
+                conn, controllerConfigService, storageGroupInstanceName,
+                storageGroupName, volumeInstance, volumeName,
+                numVolInStorageGroup, fastPolicyName, extraSpecs)
+
+        return storageGroupInstanceName
+
+    def _last_vol_in_SG(
+            self, conn, controllerConfigService, storageGroupInstanceName,
+            storageGroupName, volumeInstance, volumeName, extraSpecs):
+        """Steps if the volume is the last in a storage group.
+
+        1. Check if the volume is in a masking view.
+        2. If it is in a masking view, delete the masking view,
+           remove the volume from the storage group, and delete
+           the storage group.
+        3. If it is not in a masking view, remove the volume from the
+           storage group and delete the storage group.
+
+        :param conn: the ecom connection
+        :param controllerConfigService: storage system instance name
+        :param storageGroupInstanceName: the SG instance name
+        :param storageGroupName: the Storage group name (String)
+        :param volumeInstance: the volume instance
+        :param volumeName: the volume name
+        :param extraSpecs: the extra specifications
+        """
+        status = False
+        LOG.debug("Only one volume remains in storage group "
+                  "%(sgname)s. Driver will attempt cleanup.",
+                  {'sgname': storageGroupName})
+        mvInstanceName = self.get_masking_view_from_storage_group(
+            conn, storageGroupInstanceName)
+        if mvInstanceName is None:
+            LOG.debug("Unable to get masking view %(maskingView)s "
+                      "from storage group.",
+                      {'maskingView': mvInstanceName})
+        else:
+            maskingViewInstance = conn.GetInstance(
+                mvInstanceName, LocalOnly=False)
+            maskingViewName = maskingViewInstance['ElementName']
 
+        if mvInstanceName:
             maskingViewInstance = conn.GetInstance(
                 mvInstanceName, LocalOnly=False)
             maskingViewName = maskingViewInstance['ElementName']
@@ -1822,41 +1857,73 @@ class EMCVMAXMasking(object):
                 return self._delete_mv_and_sg(
                     conn, controllerConfigService, mvInstanceName,
                     maskingViewName, storageGroupInstanceName,
-                    storageGroupName, volumeInstance, volumeName, extraSpecs)
+                    storageGroupName, volumeInstance, volumeName,
+                    extraSpecs)
             do_delete_mv_and_sg()
+            status = True
         else:
-            # Not the last volume so remove it from storage group in
-            # the masking view.
-            LOG.debug("Start: number of volumes in masking storage group: "
-                      "%(numVol)d", {'numVol': len(volumeInstanceNames)})
-            self.provision.remove_device_from_storage_group(
-                conn, controllerConfigService, storageGroupInstanceName,
-                volumeInstance.path, volumeName, extraSpecs)
+            # Remove the volume from the storage group and delete the SG.
+            self._remove_last_vol_and_delete_sg(
+                conn, controllerConfigService,
+                storageGroupInstanceName,
+                storageGroupName, volumeInstance.path,
+                volumeName, extraSpecs)
+            status = True
+        return status
 
-            LOG.debug(
-                "RemoveMembers for volume %(volumeName)s completed "
-                "successfully.", {'volumeName': volumeName})
+    def _multiple_vols_in_SG(
+            self, conn, controllerConfigService, storageGroupInstanceName,
+            storageGroupName, volumeInstance, volumeName, numVolsInSG,
+            fastPolicyName, extraSpecs):
+        """Necessary steps if the volume is not the last in the SG.
 
-            # Add it back to the default storage group.
-            if isV3:
-                self._return_volume_to_default_storage_group_v3(
-                    conn, controllerConfigService, storageGroupName,
-                    volumeInstance, volumeName, storageSystemInstanceName,
-                    extraSpecs)
-            else:
-                # V2 if FAST POLICY enabled, move the volume to the default SG.
-                if fastPolicyName is not None and isTieringPolicySupported:
-                    self._cleanup_tiering(
-                        conn, controllerConfigService, fastPolicyName,
-                        volumeInstance, volumeName, extraSpecs)
-
-            volumeInstanceNames = self.get_devices_from_storage_group(
-                conn, storageGroupInstanceName)
-            LOG.debug(
-                "End: number of volumes in masking storage group: %(numVol)d.",
-                {'numVol': len(volumeInstanceNames)})
+        1. Remove the volume from the SG.
+        2. Return the volume to default SG if necessary.
 
-        return storageGroupInstanceName
+        :param conn: the ecom connection
+        :param controllerConfigService: storage system instance name
+        :param storageGroupInstanceName: the SG instance name
+        :param storageGroupName: the Storage group name (String)
+        :param volumeInstance: the volume instance
+        :param volumeName: the volume name
+        :param numVolsInSG: the number of volumes in the SG
+        :param fastPolicyName: the FAST policy name
+        :param extraSpecs: the extra specifications
+        """
+        storageSystemInstanceName = self.utils.find_storage_system(
+            conn, controllerConfigService)
+        if not extraSpecs[ISV3]:
+            isTieringPolicySupported, __ = (
+                self._get_tiering_info(conn, storageSystemInstanceName,
+                                       fastPolicyName))
+        LOG.debug("Start: number of volumes in masking storage group: "
+                  "%(numVol)d", {'numVol': numVolsInSG})
+        self.provision.remove_device_from_storage_group(
+            conn, controllerConfigService, storageGroupInstanceName,
+            volumeInstance.path, volumeName, extraSpecs)
+
+        LOG.debug(
+            "RemoveMembers for volume %(volumeName)s completed "
+            "successfully.", {'volumeName': volumeName})
+
+        # Add it back to the default storage group.
+        if extraSpecs[ISV3]:
+            self._return_volume_to_default_storage_group_v3(
+                conn, controllerConfigService, storageGroupName,
+                volumeInstance, volumeName, storageSystemInstanceName,
+                extraSpecs)
+        else:
+            # V2 if FAST POLICY enabled, move the volume to the default SG.
+            if fastPolicyName is not None and isTieringPolicySupported:
+                self._cleanup_tiering(
+                    conn, controllerConfigService, fastPolicyName,
+                    volumeInstance, volumeName, extraSpecs)
+
+        volumeInstanceNames = self.get_devices_from_storage_group(
+            conn, storageGroupInstanceName)
+        LOG.debug(
+            "End: number of volumes in masking storage group: %(numVol)d.",
+            {'numVol': len(volumeInstanceNames)})
 
     def _delete_mv_and_sg(self, conn, controllerConfigService, mvInstanceName,
                           maskingViewName, storageGroupInstanceName,