Merge "ds8k: should verify REST version separately"

cinder/tests/unit/volume/drivers/ibm/test_ds8k_proxy.py

@@ -580,7 +580,7 @@ FAKE_GET_REST_VERSION_RESPONSE = {
         "api_info":
         [
             {
-                "bundle_version": "5.7.51.1047"
+                "bundle_version": "5.7.51.1068"
             }
         ]
     }
@@ -1111,22 +1111,63 @@ class DS8KProxyTest(test.TestCase):
                           FakeDS8KCommonHelper, self.configuration, None)
 
     @mock.patch.object(helper.DS8KCommonHelper, '_get_version')
-    def test_verify_rest_version_for_5_7(self, mock_get_version):
+    def test_verify_rest_version_for_5_7_fb(self, mock_get_version):
-        """test the min version of REST version for 7.x."""
+        """test the min version of REST for fb volume in 7.x."""
         mock_get_version.return_value = {
-            "bundle_version": "5.7.0.0"
+            "bundle_version": "5.7.50.0"
         }
         self.assertRaises(exception.VolumeDriverException,
                           FakeDS8KCommonHelper, self.configuration, None)
 
     @mock.patch.object(helper.DS8KCommonHelper, '_get_version')
-    def test_verify_rest_version_for_5_8(self, mock_get_version):
+    def test_verify_rest_version_for_5_8_fb(self, mock_get_version):
-        """test the min version of REST version for 8.x."""
+        """test the min version of REST for fb volume in 8.1."""
         mock_get_version.return_value = {
-            "bundle_version": "5.8.0.0"
+            "bundle_version": "5.8.10.0"
+        }
+        FakeDS8KCommonHelper(self.configuration, None)
+
+    @mock.patch.object(helper.DS8KECKDHelper, '_get_version')
+    def test_verify_rest_version_for_5_7_eckd(self, mock_get_version):
+        """test the min version of REST for eckd volume in 7.x."""
+        self.configuration.connection_type = (
+            storage.XIV_CONNECTION_TYPE_FC_ECKD)
+        self.configuration.ds8k_devadd_unitadd_mapping = 'C4-10'
+        self.configuration.ds8k_ssid_prefix = 'FF'
+        self.configuration.san_clustername = TEST_ECKD_POOL_ID
+        mock_get_version.return_value = {
+            "bundle_version": "5.7.50.0"
         }
         self.assertRaises(exception.VolumeDriverException,
-                          FakeDS8KCommonHelper, self.configuration, None)
+                          FakeDS8KECKDHelper, self.configuration, None)
+
+    @mock.patch.object(helper.DS8KECKDHelper, '_get_version')
+    def test_verify_rest_version_for_5_8_eckd_1(self, mock_get_version):
+        """test the min version of REST for eckd volume in 8.1."""
+        self.configuration.connection_type = (
+            storage.XIV_CONNECTION_TYPE_FC_ECKD)
+        self.configuration.ds8k_devadd_unitadd_mapping = 'C4-10'
+        self.configuration.ds8k_ssid_prefix = 'FF'
+        self.configuration.san_clustername = TEST_ECKD_POOL_ID
+        mock_get_version.return_value = {
+            "bundle_version": "5.8.10.0"
+        }
+        self.assertRaises(exception.VolumeDriverException,
+                          FakeDS8KECKDHelper, self.configuration, None)
+
+    @mock.patch.object(helper.DS8KECKDHelper, '_get_version')
+    def test_verify_rest_version_for_5_8_eckd_2(self, mock_get_version):
+        """test the min version of REST for eckd volume in 8.2."""
+        self.configuration.connection_type = (
+            storage.XIV_CONNECTION_TYPE_FC_ECKD)
+        self.configuration.ds8k_devadd_unitadd_mapping = 'C4-10'
+        self.configuration.ds8k_ssid_prefix = 'FF'
+        self.configuration.san_clustername = TEST_ECKD_POOL_ID
+        mock_get_version.return_value = {
+            "bundle_version": "5.8.20.0"
+        }
+        self.assertRaises(exception.VolumeDriverException,
+                          FakeDS8KECKDHelper, self.configuration, None)
 
     def test_verify_pools_with_wrong_type(self):
         """pool should be set according to the connection type."""

cinder/volume/drivers/ibm/ibm_storage/ds8k_helper.py

@@ -37,10 +37,6 @@ LOG = logging.getLogger(__name__)
 
 LSS_VOL_SLOTS = 0x100
 LSS_SLOTS = 0xFF
-# if use new REST API, please update the version below
-VALID_REST_VERSION_5_7_MIN = '5.7.51.1047'
-VALID_REST_VERSION_5_8_MIN = '5.8.20.1018'
-VALID_STORAGE_VERSION = '8.1'
 
 VALID_HOST_TYPES = (
     'auto', 'AMDLinuxRHEL', 'AMDLinuxSuse',
@@ -63,6 +59,10 @@ class DS8KCommonHelper(object):
     """Manage the primary backend, it is common class too."""
 
     OPTIONAL_PARAMS = ['ds8k_host_type', 'lss_range_for_cg']
+    # if use new REST API, please update the version below
+    VALID_REST_VERSION_5_7_MIN = '5.7.51.1047'
+    VALID_REST_VERSION_5_8_MIN = ''
+    INVALID_STORAGE_VERSION = '8.0.1'
 
     def __init__(self, conf, HTTPConnectorObject=None):
         self.conf = conf
@@ -187,32 +187,20 @@ class DS8KCommonHelper(object):
             None if ds8k_host_type == 'auto' else ds8k_host_type)
 
     def _verify_version(self):
-        if self.backend['storage_version'] == '8.0.1':
+        if self.backend['storage_version'] == self.INVALID_STORAGE_VERSION:
             raise exception.VolumeDriverException(
-                data=(_("8.0.1 does not support bulk deletion of volumes, "
+                message=(_("%s does not support bulk deletion of volumes, "
-                        "if you want to use this version of driver, "
+                           "if you want to use this version of driver, "
-                        "please upgrade the CCL, and make sure the REST "
+                           "please upgrade the CCL.")
-                        "version is not lower than %s.")
+                         % self.INVALID_STORAGE_VERSION))
-                      % VALID_REST_VERSION_5_8_MIN))
+        if ('5.7' in self.backend['rest_version'] and
-        else:
+           dist_version.LooseVersion(self.backend['rest_version']) <
-            if (('5.7' in self.backend['rest_version'] and
+           dist_version.LooseVersion(self.VALID_REST_VERSION_5_7_MIN)):
-               dist_version.LooseVersion(self.backend['rest_version']) <
+            raise exception.VolumeDriverException(
-               dist_version.LooseVersion(VALID_REST_VERSION_5_7_MIN)) or
+                message=(_("REST version %(invalid)s is lower than "
-               ('5.8' in self.backend['rest_version'] and
+                           "%(valid)s, please upgrade it in DS8K.")
-               dist_version.LooseVersion(self.backend['rest_version']) <
+                         % {'invalid': self.backend['rest_version'],
-               dist_version.LooseVersion(VALID_REST_VERSION_5_8_MIN))):
+                            'valid': self.VALID_REST_VERSION_5_7_MIN}))
-                raise exception.VolumeDriverException(
-                    data=(_("REST version %(invalid)s is lower than "
-                            "%(valid)s, please upgrade it in DS8K.")
-                          % {'invalid': self.backend['rest_version'],
-                             'valid': (VALID_REST_VERSION_5_7_MIN if '5.7' in
-                                       self.backend['rest_version'] else
-                                       VALID_REST_VERSION_5_8_MIN)}))
-
-        if self._connection_type == storage.XIV_CONNECTION_TYPE_FC_ECKD:
-            if (dist_version.LooseVersion(self.backend['storage_version']) <
-               dist_version.LooseVersion(VALID_STORAGE_VERSION)):
-                self._disable_thin_provision = True
 
     def _verify_pools(self):
         if self._connection_type == storage.XIV_CONNECTION_TYPE_FC:
@@ -905,6 +893,11 @@ class DS8KECKDHelper(DS8KCommonHelper):
 
     OPTIONAL_PARAMS = ['ds8k_host_type', 'port_pairs', 'ds8k_ssid_prefix',
                        'lss_range_for_cg']
+    # if use new REST API, please update the version below
+    VALID_REST_VERSION_5_7_MIN = '5.7.51.1068'
+    VALID_REST_VERSION_5_8_MIN = '5.8.20.1059'
+    MIN_VALID_STORAGE_VERSION = '8.1'
+    INVALID_STORAGE_VERSION = '8.0.1'
 
     @staticmethod
     def _gb2cyl(gb):
@@ -940,6 +933,32 @@ class DS8KECKDHelper(DS8KCommonHelper):
         self._verify_version()
         self._verify_pools()
 
+    def _verify_version(self):
+        if self.backend['storage_version'] == self.INVALID_STORAGE_VERSION:
+            raise exception.VolumeDriverException(
+                message=(_("%s does not support bulk deletion of volumes, "
+                           "if you want to use this version of driver, "
+                           "please upgrade the CCL.")
+                         % self.INVALID_STORAGE_VERSION))
+        # DS8K supports ECKD ESE volume from 8.1
+        if (dist_version.LooseVersion(self.backend['storage_version']) <
+           dist_version.LooseVersion(self.MIN_VALID_STORAGE_VERSION)):
+            self._disable_thin_provision = True
+
+        if (('5.7' in self.backend['rest_version'] and
+           dist_version.LooseVersion(self.backend['rest_version']) <
+           dist_version.LooseVersion(self.VALID_REST_VERSION_5_7_MIN)) or
+           ('5.8' in self.backend['rest_version'] and
+           dist_version.LooseVersion(self.backend['rest_version']) <
+           dist_version.LooseVersion(self.VALID_REST_VERSION_5_8_MIN))):
+            raise exception.VolumeDriverException(
+                message=(_("REST version %(invalid)s is lower than "
+                           "%(valid)s, please upgrade it in DS8K.")
+                         % {'invalid': self.backend['rest_version'],
+                            'valid': (self.VALID_REST_VERSION_5_7_MIN if '5.7'
+                                      in self.backend['rest_version'] else
+                                      self.VALID_REST_VERSION_5_8_MIN)}))
+
     @proxy.logger
     def _check_and_verify_lcus(self):
         map_str = self._get_value('ds8k_devadd_unitadd_mapping')