ScaleIO: Fixing support for SIO 1.3x

A few changes necessary to make driver compatible with
ScaleIO version 1.3x
- refactored SIO REST calls to go through common code
- obtain REST API version number dynamically
- allow configuration to override what API version to use
- Make API calls based upon API version
- Add unit tests to validate calls to retrieve version

This commit adds a new configuration option, called:
   sio_server_api_version
   - This can be used to override the value
     retrieved from the ScaleIO server at runtime
   - This is helpful for manual testing, but should
     not be needed in a production environment

DocImpact

Change-Id: I741bc5384fa3a5afcb80f52f4b39b4123e2b8e11
Closes-Bug: #1644904

cinder/tests/unit/volume/drivers/dell_emc/scaleio/mocks.py

@@ -46,6 +46,8 @@ class ScaleIODriver(driver.ScaleIODriver):
                                    override='test_domain:test_pool')
         configuration.set_override('max_over_subscription_ratio',
                                    override=5.0)
+        configuration.set_override('sio_server_api_version',
+                                   override='2.0.0')
         if 'san_thin_provision' in kwargs:
             configuration.set_override(
                 'san_thin_provision',

cinder/tests/unit/volume/drivers/dell_emc/scaleio/test_versions.py

@@ -0,0 +1,93 @@
+# Copyright (C) 2017 Dell Inc. or its subsidiaries.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import ddt
+
+from cinder import exception
+from cinder.tests.unit.volume.drivers.dell_emc import scaleio
+
+
+@ddt.ddt
+class TestMultipleVersions(scaleio.TestScaleIODriver):
+
+    version = '1.2.3.4'
+    good_versions = ['1.2.3.4',
+                     '101.102.103.104.105.106.107',
+                     '1.0'
+                     ]
+    bad_versions = ['bad',
+                    'bad.version.number',
+                    '1.0b',
+                    '.6'
+                    ]
+
+    # Test cases for ``ScaleIODriver._get_server_api_version()``
+    def setUp(self):
+        """Setup a test case environment."""
+        super(TestMultipleVersions, self).setUp()
+
+        self.HTTPS_MOCK_RESPONSES = {
+            self.RESPONSE_MODE.Valid: {
+                'version': '"{}"'.format(self.version),
+            },
+            self.RESPONSE_MODE.Invalid: {
+                'version': None,
+            },
+            self.RESPONSE_MODE.BadStatus: {
+                'version': self.BAD_STATUS_RESPONSE,
+            },
+        }
+
+    def test_version_api_fails(self):
+        """version api returns a non-200 response."""
+        self.set_https_response_mode(self.RESPONSE_MODE.Invalid)
+        self.assertRaises(exception.VolumeBackendAPIException,
+                          self.test_version)
+
+    def test_version(self):
+        """Valid version request."""
+        self.driver._get_server_api_version(False)
+
+    def test_version_badstatus_response(self):
+        """Version api returns a bad response."""
+        self.set_https_response_mode(self.RESPONSE_MODE.BadStatus)
+        self.assertRaises(exception.VolumeBackendAPIException,
+                          self.test_version)
+
+    def setup_response(self):
+        self.HTTPS_MOCK_RESPONSES = {
+            self.RESPONSE_MODE.Valid: {
+                'version': '"{}"'.format(self.version),
+            },
+        }
+
+    def test_version_badversions(self):
+        """Version api returns an invalid version number."""
+        for vers in self.bad_versions:
+            self.version = vers
+            self.setup_response()
+            self.assertRaises(exception.VolumeBackendAPIException,
+                              self.test_version)
+
+    def test_version_goodversions(self):
+        """Version api returns a valid version number."""
+        for vers in self.good_versions:
+            self.version = vers
+            self.setup_response()
+            self.driver._get_server_api_version(False)
+            self.assertEqual(
+                self.driver._get_server_api_version(False),
+                vers
+            )

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

@@ -18,13 +18,14 @@ Driver for Dell EMC ScaleIO based on ScaleIO remote CLI.
 
 import base64
 import binascii
+from distutils import version
 import json
 import math
-
 from os_brick.initiator import connector
 from oslo_config import cfg
 from oslo_log import log as logging
 from oslo_utils import units
+import re
 import requests
 import six
 from six.moves import urllib
@@ -69,6 +70,8 @@ scaleio_opts = [
                help='Storage Pool name.'),
     cfg.StrOpt('sio_storage_pool_id',
                help='Storage Pool ID.'),
+    cfg.StrOpt('sio_server_api_version',
+               help='ScaleIO API version.'),
     cfg.FloatOpt('sio_max_over_subscription_ratio',
                  # This option exists to provide a default value for the
                  # ScaleIO driver which is different than the global default.
@@ -111,7 +114,9 @@ SIO_MAX_OVERSUBSCRIPTION_RATIO = 10.0
 class ScaleIODriver(driver.VolumeDriver):
     """Dell EMC ScaleIO Driver."""
 
-    VERSION = "2.0"
+    VERSION = "2.0.1"
+    # Major changes
+    # 2.0.1: Added support for SIO 1.3x in addition to 2.0.x
 
     # ThirdPartySystems wiki
     CI_WIKI_NAME = "EMC_ScaleIO_CI"
@@ -129,6 +134,7 @@ class ScaleIODriver(driver.VolumeDriver):
         self.server_username = self.configuration.san_login
         self.server_password = self.configuration.san_password
         self.server_token = None
+        self.server_api_version = self.configuration.sio_server_api_version
         self.verify_server_certificate = (
             self.configuration.sio_verify_server_certificate)
         self.server_certificate_path = None
@@ -299,6 +305,14 @@ class ScaleIODriver(driver.VolumeDriver):
                          "of OpenStack. Please use QoS specs.")
         return qos_limit if qos_limit is not None else extraspecs_limit
 
+    @staticmethod
+    def _version_greater_than(ver1, ver2):
+        return version.LooseVersion(ver1) > version.LooseVersion(ver2)
+
+    @staticmethod
+    def _version_greater_than_or_equal(ver1, ver2):
+        return version.LooseVersion(ver1) >= version.LooseVersion(ver2)
+
     @staticmethod
     def _id_to_base64(id):
         # Base64 encode the id to get a volume name less than 32 characters due
@@ -345,8 +359,6 @@ class ScaleIODriver(driver.VolumeDriver):
                   'domain_id': protection_domain_id,
                   'domain_name': protection_domain_name})
 
-        verify_cert = self._get_verify_cert()
-
         if storage_pool_name:
             self.storage_pool_name = storage_pool_name
             self.storage_pool_id = None
@@ -377,15 +389,9 @@ class ScaleIODriver(driver.VolumeDriver):
                        "%(encoded_domain_name)s") % req_vars
             LOG.info("ScaleIO get domain id by name request: %s.",
                      request)
-            r = requests.get(
-                request,
-                auth=(
-                    self.server_username,
-                    self.server_token),
-                verify=verify_cert)
-            r = self._check_response(r, request)
 
-            domain_id = r.json()
+            r, domain_id = self._execute_scaleio_get_request(request)
+
             if not domain_id:
                 msg = (_("Domain with name %s wasn't found.")
                        % self.protection_domain_name)
@@ -411,13 +417,8 @@ class ScaleIODriver(driver.VolumeDriver):
                        "/api/types/Pool/instances/getByName::"
                        "%(domain_id)s,%(encoded_domain_name)s") % req_vars
             LOG.info("ScaleIO get pool id by name request: %s.", request)
-            r = requests.get(
+            r, pool_id = self._execute_scaleio_get_request(request)
-                request,
+
-                auth=(
-                    self.server_username,
-                    self.server_token),
-                verify=verify_cert)
-            pool_id = r.json()
             if not pool_id:
                 msg = (_("Pool with name %(pool_name)s wasn't found in "
                          "domain %(domain_id)s.")
@@ -449,19 +450,12 @@ class ScaleIODriver(driver.VolumeDriver):
                   'storagePoolId': pool_id}
 
         LOG.info("Params for add volume request: %s.", params)
-        r = requests.post(
+        req_vars = {'server_ip': self.server_ip,
-            "https://" +
+                    'server_port': self.server_port}
-            self.server_ip +
+        request = ("https://%(server_ip)s:%(server_port)s"
-            ":" +
+                   "/api/types/Volume/instances") % req_vars
-            self.server_port +
+        r, response = self._execute_scaleio_post_request(params, request)
-            "/api/types/Volume/instances",
+
-            data=json.dumps(params),
-            headers=self._get_headers(),
-            auth=(
-                self.server_username,
-                self.server_token),
-            verify=verify_cert)
-        response = r.json()
         LOG.info("Add volume response: %s", response)
 
         if r.status_code != OK_STATUS_CODE and "errorCode" in response:
@@ -524,7 +518,11 @@ class ScaleIODriver(driver.VolumeDriver):
                 self.server_token),
             verify=self._get_verify_cert())
         r = self._check_response(r, request, False, params)
-        response = r.json()
+        response = None
+        try:
+            response = r.json()
+        except ValueError:
+            response = None
         return r, response
 
     def _check_response(self, response, request, is_get_request=True,
@@ -562,6 +560,31 @@ class ScaleIODriver(driver.VolumeDriver):
             return res
         return response
 
+    def _get_server_api_version(self, fromcache=True):
+        if self.server_api_version is None or fromcache is False:
+            request = (
+                "https://" + self.server_ip +
+                ":" + self.server_port + "/api/version")
+            r, unused = self._execute_scaleio_get_request(request)
+
+            if r.status_code == OK_STATUS_CODE:
+                self.server_api_version = r.text.replace('\"', '')
+                LOG.info("REST API Version: %(api_version)s",
+                         {'api_version': self.server_api_version})
+            else:
+                msg = (_("Error calling version api "
+                         "status code: %d") % r.status_code)
+                raise exception.VolumeBackendAPIException(data=msg)
+
+            # make sure the response was valid
+            pattern = re.compile("^\d+(\.\d+)*$")
+            if not pattern.match(self.server_api_version):
+                msg = (_("Error calling version api "
+                         "response: %s") % r.text)
+                raise exception.VolumeBackendAPIException(data=msg)
+
+        return self.server_api_version
+
     def create_volume_from_snapshot(self, volume, snapshot):
         """Creates a volume from a snapshot."""
         # We interchange 'volume' and 'snapshot' because in ScaleIO
@@ -666,8 +689,6 @@ class ScaleIODriver(driver.VolumeDriver):
         self._delete_volume(volume_id)
 
     def _delete_volume(self, vol_id):
-        verify_cert = self._get_verify_cert()
-
         req_vars = {'server_ip': self.server_ip,
                     'server_port': self.server_port,
                     'vol_id': six.text_type(vol_id)}
@@ -684,34 +705,16 @@ class ScaleIODriver(driver.VolumeDriver):
             LOG.info("Trying to unmap volume from all sdcs"
                      " before deletion: %s.",
                      request)
-            r = requests.post(
+            r, unused = self._execute_scaleio_post_request(params, request)
-                request,
-                data=json.dumps(params),
-                headers=self._get_headers(),
-                auth=(
-                    self.server_username,
-                    self.server_token),
-                verify=verify_cert
-            )
-            r = self._check_response(r, request, False, params)
             LOG.debug("Unmap volume response: %s.", r.text)
 
         params = {'removeMode': 'ONLY_ME'}
         request = ("https://%(server_ip)s:%(server_port)s"
                    "/api/instances/Volume::%(vol_id)s"
                    "/action/removeVolume") % req_vars
-        r = requests.post(
+        r, response = self._execute_scaleio_post_request(params, request)
-            request,
-            data=json.dumps(params),
-            headers=self._get_headers(),
-            auth=(self.server_username,
-                  self.server_token),
-            verify=verify_cert
-        )
-        r = self._check_response(r, request, False, params)
 
         if r.status_code != OK_STATUS_CODE:
-            response = r.json()
             error_code = response['errorCode']
             if error_code == VOLUME_NOT_FOUND_ERROR:
                 LOG.warning("Ignoring error in delete volume %s:"
@@ -854,15 +857,7 @@ class ScaleIODriver(driver.VolumeDriver):
             LOG.info("username: %(username)s, verify_cert: %(verify)s.",
                      {'username': self.server_username,
                       'verify': verify_cert})
-            r = requests.get(
+            r, domain_id = self._execute_scaleio_get_request(request)
-                request,
-                auth=(
-                    self.server_username,
-                    self.server_token),
-                verify=verify_cert)
-            r = self._check_response(r, request)
-            LOG.info("Get domain by name response: %s", r.text)
-            domain_id = r.json()
             if not domain_id:
                 msg = (_("Domain with name %s wasn't found.")
                        % self.protection_domain_name)
@@ -887,13 +882,7 @@ class ScaleIODriver(driver.VolumeDriver):
                        "/api/types/Pool/instances/getByName::"
                        "%(domain_id)s,%(encoded_pool_name)s") % req_vars
             LOG.info("ScaleIO get pool id by name request: %s.", request)
-            r = requests.get(
+            r, pool_id = self._execute_scaleio_get_request(request)
-                request,
-                auth=(
-                    self.server_username,
-                    self.server_token),
-                verify=verify_cert)
-            pool_id = r.json()
             if not pool_id:
                 msg = (_("Pool with name %(pool)s wasn't found in domain "
                          "%(domain)s.")
@@ -914,20 +903,22 @@ class ScaleIODriver(driver.VolumeDriver):
             request = ("https://%(server_ip)s:%(server_port)s"
                        "/api/types/StoragePool/instances/action/"
                        "querySelectedStatistics") % req_vars
-            # The 'Km' in thinCapacityAllocatedInKm is a bug in REST API
+            # SIO version 2+ added a property...
-            params = {'ids': [pool_id], 'properties': [
+            if self._version_greater_than_or_equal(
-                "capacityAvailableForVolumeAllocationInKb",
+                    self._get_server_api_version(),
-                "capacityLimitInKb", "spareCapacityInKb",
+                    "2.0.0"):
-                "thickCapacityInUseInKb", "thinCapacityAllocatedInKm"]}
+                # The 'Km' in thinCapacityAllocatedInKm is a bug in REST API
-            r = requests.post(
+                params = {'ids': [pool_id], 'properties': [
-                request,
+                    "capacityAvailableForVolumeAllocationInKb",
-                data=json.dumps(params),
+                    "capacityLimitInKb", "spareCapacityInKb",
-                headers=self._get_headers(),
+                    "thickCapacityInUseInKb", "thinCapacityAllocatedInKm"]}
-                auth=(
+            else:
-                    self.server_username,
+                params = {'ids': [pool_id], 'properties': [
-                    self.server_token),
+                    "capacityAvailableForVolumeAllocationInKb",
-                verify=verify_cert)
+                    "capacityLimitInKb", "spareCapacityInKb",
-            response = r.json()
+                    "thickCapacityInUseInKb"]}
+
+            r, response = self._execute_scaleio_post_request(params, request)
             LOG.info("Query capacity stats response: %s.", response)
             for res in response.values():
                 # Divide by two because ScaleIO creates a copy for each volume
@@ -940,10 +931,16 @@ class ScaleIODriver(driver.VolumeDriver):
                 free_capacity_gb = (
                     res['capacityAvailableForVolumeAllocationInKb'] / units.Mi)
                 # Divide by two because ScaleIO creates a copy for each volume
-                provisioned_capacity = (
+                if self._version_greater_than_or_equal(
-                    ((res['thickCapacityInUseInKb'] +
+                        self._get_server_api_version(),
-                     res['thinCapacityAllocatedInKm']) / 2) / units.Mi)
+                        "2.0.0"):
-                LOG.info("Free capacity of pool %(pool)s is: %(free)s, "
+                    provisioned_capacity = (
+                        ((res['thickCapacityInUseInKb'] +
+                          res['thinCapacityAllocatedInKm']) / 2) / units.Mi)
+                else:
+                    provisioned_capacity = (
+                        (res['thickCapacityInUseInKb'] / 2) / units.Mi)
+                LOG.info("free capacity of pool %(pool)s is: %(free)s, "
                          "total capacity: %(total)s, "
                          "provisioned capacity: %(prov)s",
                          {'pool': pool_name,
@@ -1120,18 +1117,9 @@ class ScaleIODriver(driver.VolumeDriver):
         LOG.info("ScaleIO rename volume request: %s.", request)
 
         params = {'newName': new_name}
-        r = requests.post(
+        r, response = self._execute_scaleio_post_request(params, request)
-            request,
-            data=json.dumps(params),
-            headers=self._get_headers(),
-            auth=(self.server_username,
-                  self.server_token),
-            verify=self._get_verify_cert()
-        )
-        r = self._check_response(r, request, False, params)
 
         if r.status_code != OK_STATUS_CODE:
-            response = r.json()
             error_code = response['errorCode']
             if ((error_code == VOLUME_NOT_FOUND_ERROR or
                  error_code == OLD_VOLUME_NOT_FOUND_ERROR or