From 5693012f7ac57ce68f03a91eee37cbcc99dfc506 Mon Sep 17 00:00:00 2001 From: Jia Min Date: Thu, 6 Apr 2017 02:17:02 -0700 Subject: [PATCH] ds8k: should verify REST version separately for fb and ckd volume, they require different versions of REST API, because APIs supporting their features are added one by one and not at the same time, so driver should separate them when verifying REST version, otherwise it will make user need to upgrade REST unnecessarily. Change-Id: Ib42db3f06705212fe3b8f4cfcb625d8a5fb270b7 backport: Ocata Closes-Bug: #1680379 --- .../volume/drivers/ibm/test_ds8k_proxy.py | 57 ++++++++++++-- .../drivers/ibm/ibm_storage/ds8k_helper.py | 77 ++++++++++++------- 2 files changed, 97 insertions(+), 37 deletions(-) diff --git a/cinder/tests/unit/volume/drivers/ibm/test_ds8k_proxy.py b/cinder/tests/unit/volume/drivers/ibm/test_ds8k_proxy.py index 579193f43e5..3827707d4be 100644 --- a/cinder/tests/unit/volume/drivers/ibm/test_ds8k_proxy.py +++ b/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): - """test the min version of REST version for 7.x.""" + def test_verify_rest_version_for_5_7_fb(self, mock_get_version): + """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): - """test the min version of REST version for 8.x.""" + def test_verify_rest_version_for_5_8_fb(self, mock_get_version): + """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.""" diff --git a/cinder/volume/drivers/ibm/ibm_storage/ds8k_helper.py b/cinder/volume/drivers/ibm/ibm_storage/ds8k_helper.py index b1441a74644..ad2e39fa3a5 100644 --- a/cinder/volume/drivers/ibm/ibm_storage/ds8k_helper.py +++ b/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, " - "if you want to use this version of driver, " - "please upgrade the CCL, and make sure the REST " - "version is not lower than %s.") - % VALID_REST_VERSION_5_8_MIN)) - else: - if (('5.7' in self.backend['rest_version'] and - dist_version.LooseVersion(self.backend['rest_version']) < - dist_version.LooseVersion(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(VALID_REST_VERSION_5_8_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 + 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)) + 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)): + 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})) 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')