ScaleIO Driver - adding cache and refactoring testsdriverfixes/newton

Changing static lists to a simple cache.
Refactoring some of the unit tests to simplify maintenance.

Related-Bug: #1699573

Change-Id: Idff127801da9e286a6b634594e5577eeb9782571
(cherry picked from commit aa8b87a83cc5a8cfcaa3080c9a6080d8289716a4)
(cherry picked from commit 0e9b173381fce28fded543095360dca5198c4810)
(cherry picked from commit 034feb6d749ace24fe709c9bf1aa8e9eb87a0cff)

5 files changed, 348 insertions, 25 deletions

diff --git a/cinder/tests/unit/volume/drivers/emc/scaleio/__init__.py b/cinder/tests/unit/volume/drivers/emc/scaleio/__init__.py
index 33e31bbd6..31b0b2265 100644
--- a/cinder/tests/unit/volume/drivers/emc/scaleio/__init__.py
+++ b/cinder/tests/unit/volume/drivers/emc/scaleio/__init__.py
@@ -105,6 +105,8 @@ class TestScaleIODriver(test.TestCase):
     PROT_DOMAIN_ID = six.text_type('1')
     PROT_DOMAIN_NAME = 'PD1'
 
+    STORAGE_POOLS = ['{}:{}'.format(PROT_DOMAIN_NAME, STORAGE_POOL_NAME)]
+
     def setUp(self):
         """Setup a test case environment.
 
diff --git a/cinder/tests/unit/volume/drivers/emc/scaleio/test_create_volume.py b/cinder/tests/unit/volume/drivers/emc/scaleio/test_create_volume.py
index e60d64f38..6886d498b 100644
--- a/cinder/tests/unit/volume/drivers/emc/scaleio/test_create_volume.py
+++ b/cinder/tests/unit/volume/drivers/emc/scaleio/test_create_volume.py
@@ -47,6 +47,12 @@ class TestCreateVolume(scaleio.TestScaleIODriver):
                     self.PROT_DOMAIN_ID,
                     self.STORAGE_POOL_NAME
                 ): '"{}"'.format(self.STORAGE_POOL_ID),
+                'instances/ProtectionDomain::{}'.format(
+                    self.PROT_DOMAIN_ID
+                ): {'id': self.PROT_DOMAIN_ID},
+                'instances/StoragePool::{}'.format(
+                    self.STORAGE_POOL_ID
+                ): {'id': self.STORAGE_POOL_ID},
             },
             self.RESPONSE_MODE.Invalid: {
                 'types/Domain/instances/getByName::' +
diff --git a/cinder/tests/unit/volume/drivers/emc/scaleio/test_misc.py b/cinder/tests/unit/volume/drivers/emc/scaleio/test_misc.py
index 569d282fd..f21362354 100644
--- a/cinder/tests/unit/volume/drivers/emc/scaleio/test_misc.py
+++ b/cinder/tests/unit/volume/drivers/emc/scaleio/test_misc.py
@@ -15,7 +15,6 @@
 
 import ddt
 import mock
-from six.moves import urllib
 
 from cinder import context
 from cinder import exception
@@ -27,9 +26,9 @@ from cinder.tests.unit.volume.drivers.emc.scaleio import mocks
 
 @ddt.ddt
 class TestMisc(scaleio.TestScaleIODriver):
-    DOMAIN_NAME = 'PD1'
-    POOL_NAME = 'SP1'
-    STORAGE_POOLS = ['{}:{}'.format(DOMAIN_NAME, POOL_NAME)]
+
+    DOMAIN_ID = '1'
+    POOL_ID = '1'
 
     def setUp(self):
         """Set up the test case environment.
@@ -37,8 +36,6 @@ class TestMisc(scaleio.TestScaleIODriver):
         Defines the mock HTTPS responses for the REST API calls.
         """
         super(TestMisc, self).setUp()
-        self.domain_name_enc = urllib.parse.quote(self.DOMAIN_NAME)
-        self.pool_name_enc = urllib.parse.quote(self.POOL_NAME)
         self.ctx = context.RequestContext('fake', 'fake', auth_token=True)
 
         self.volume = fake_volume.fake_volume_obj(
@@ -50,17 +47,15 @@ class TestMisc(scaleio.TestScaleIODriver):
 
         self.HTTPS_MOCK_RESPONSES = {
             self.RESPONSE_MODE.Valid: {
-                'types/Domain/instances/getByName::' +
-                self.domain_name_enc: '"{}"'.format(self.DOMAIN_NAME).encode(
-                    'ascii',
-                    'ignore'
-                ),
+                'types/Domain/instances/getByName::{}'.format(
+                    self.PROT_DOMAIN_NAME
+                ): '"{}"'.format(self.PROT_DOMAIN_ID),
                 'types/Pool/instances/getByName::{},{}'.format(
-                    self.DOMAIN_NAME,
-                    self.POOL_NAME
-                ): '"{}"'.format(self.POOL_NAME).encode('ascii', 'ignore'),
+                    self.PROT_DOMAIN_ID,
+                    self.STORAGE_POOL_NAME
+                ): '"{}"'.format(self.STORAGE_POOL_ID),
                 'types/StoragePool/instances/action/querySelectedStatistics': {
-                    '"{}"'.format(self.POOL_NAME): {
+                    '"{}"'.format(self.STORAGE_POOL_NAME): {
                         'capacityAvailableForVolumeAllocationInKb': 5000000,
                         'capacityLimitInKb': 16000000,
                         'spareCapacityInKb': 6000000,
@@ -75,10 +70,26 @@ class TestMisc(scaleio.TestScaleIODriver):
                 'instances/Volume::{}/action/setVolumeName'.format(
                     self.new_volume['provider_id']):
                         self.volume['provider_id'],
+                'version': '"{}"'.format('2.0.1'),
+                'instances/StoragePool::{}'.format(
+                    self.STORAGE_POOL_ID
+                ): {
+                    'name': self.STORAGE_POOL_NAME,
+                    'id': self.STORAGE_POOL_ID,
+                    'protectionDomainId': self.PROT_DOMAIN_ID,
+                    'zeroPaddingEnabled': 'true',
+                },
+                'instances/ProtectionDomain::{}'.format(
+                    self.PROT_DOMAIN_ID
+                ): {
+                    'name': self.PROT_DOMAIN_NAME,
+                    'id': self.PROT_DOMAIN_ID
+                },
+
             },
             self.RESPONSE_MODE.BadStatus: {
                 'types/Domain/instances/getByName::' +
-                self.domain_name_enc: self.BAD_STATUS_RESPONSE,
+                self.PROT_DOMAIN_NAME: self.BAD_STATUS_RESPONSE,
                 'instances/Volume::{}/action/setVolumeName'.format(
                     self.volume['provider_id']): mocks.MockHTTPSResponse(
                     {
@@ -89,7 +100,7 @@ class TestMisc(scaleio.TestScaleIODriver):
             },
             self.RESPONSE_MODE.Invalid: {
                 'types/Domain/instances/getByName::' +
-                self.domain_name_enc: None,
+                self.PROT_DOMAIN_NAME: None,
                 'instances/Volume::{}/action/setVolumeName'.format(
                     self.volume['provider_id']): mocks.MockHTTPSResponse(
                     {
@@ -101,12 +112,18 @@ class TestMisc(scaleio.TestScaleIODriver):
         }
 
     def test_valid_configuration(self):
+        self.driver.storage_pools = self.STORAGE_POOLS
         self.driver.check_for_setup_error()
 
     def test_both_storage_pool(self):
-        """Both storage name and ID provided."""
-        self.driver.storage_pool_id = "test_pool_id"
-        self.driver.storage_pool_name = "test_pool_name"
+        """Both storage name and ID provided.
+
+        INVALID
+        """
+        self.driver.configuration.sio_storage_pool_id = self.STORAGE_POOL_ID
+        self.driver.configuration.sio_storage_pool_name = (
+            self.STORAGE_POOL_NAME
+        )
         self.assertRaises(exception.InvalidInput,
                           self.driver.check_for_setup_error)
 
@@ -118,8 +135,14 @@ class TestMisc(scaleio.TestScaleIODriver):
                           self.driver.check_for_setup_error)
 
     def test_both_domain(self):
-        self.driver.protection_domain_name = "test_domain_name"
-        self.driver.protection_domain_id = "test_domain_id"
+        """Both domain and ID are provided
+
+        INVALID
+        """
+        self.driver.configuration.sio_protection_domain_name = (
+            self.PROT_DOMAIN_NAME)
+        self.driver.configuration.sio_protection_domain_id = (
+            self.PROT_DOMAIN_ID)
         self.assertRaises(exception.InvalidInput,
                           self.driver.check_for_setup_error)
 
diff --git a/cinder/volume/drivers/emc/scaleio.py b/cinder/volume/drivers/emc/scaleio.py
index b3e18aea7..16d3cbf1f 100644
--- a/cinder/volume/drivers/emc/scaleio.py
+++ b/cinder/volume/drivers/emc/scaleio.py
@@ -1,4 +1,4 @@
-# Copyright (c) 2013 - 2015 EMC Corporation.
+# Copyright (c) 2017 Dell Inc. or its subsidiaries.
 # All Rights Reserved.
 #
 #    Licensed under the Apache License, Version 2.0 (the "License"); you may
@@ -27,6 +27,7 @@ from oslo_log import log as logging
 from oslo_utils import units
 import requests
 import six
+from six.moves import http_client
 from six.moves import urllib
 
 from cinder import context
@@ -34,12 +35,15 @@ from cinder import exception
 from cinder.i18n import _, _LI, _LW, _LE
 from cinder.image import image_utils
 from cinder import interface
+
 from cinder import utils
 from cinder.volume import driver
+from cinder.volume.drivers.emc import simplecache
 from cinder.volume.drivers.san import san
 from cinder.volume import qos_specs
 from cinder.volume import volume_types
 
+
 CONF = cfg.CONF
 
 LOG = logging.getLogger(__name__)
@@ -115,10 +119,15 @@ SIO_MAX_OVERSUBSCRIPTION_RATIO = 10.0
 
 @interface.volumedriver
 class ScaleIODriver(driver.VolumeDriver):
-    """EMC ScaleIO Driver."""
+    """Cinder ScaleIO Driver
 
-    VERSION = "2.0"
+    ScaleIO Driver version history:
+        2.0.1: Added support for SIO 1.3x in addition to 2.0.x
+        2.0.2: Added consistency group support to generic volume groups
+        2.0.3: Added cache for storage pool and protection domains info
+    """
 
+    VERSION = "2.0.3"
     # ThirdPartySystems wiki
     CI_WIKI_NAME = "EMC_ScaleIO_CI"
 
@@ -128,6 +137,12 @@ class ScaleIODriver(driver.VolumeDriver):
     def __init__(self, *args, **kwargs):
         super(ScaleIODriver, self).__init__(*args, **kwargs)
 
+        # simple caches for PD and SP properties
+        self.spCache = simplecache.SimpleCache("Storage Pool",
+                                               age_minutes=5)
+        self.pdCache = simplecache.SimpleCache("Protection Domain",
+                                               age_minutes=5)
+
         self.configuration.append_config_values(san.san_opts)
         self.configuration.append_config_values(scaleio_opts)
         self.server_ip = self.configuration.san_ip
@@ -256,6 +271,35 @@ class ScaleIODriver(driver.VolumeDriver):
                     'ratio': self.configuration.max_over_subscription_ratio})
             raise exception.InvalidInput(reason=msg)
 
+        # validate the storage pools and check if zero padding is enabled
+        for pool in self.storage_pools:
+            try:
+                pd, sp = pool.split(':')
+            except (ValueError, IndexError):
+                msg = (_("Invalid storage pool name. The correct format is: "
+                         "protection_domain:storage_pool. "
+                         "Value supplied was: %(pool)s") %
+                       {'pool': pool})
+                raise exception.InvalidInput(reason=msg)
+
+            try:
+                properties = self._get_storage_pool_properties(pd, sp)
+                padded = properties['zeroPaddingEnabled']
+            except Exception:
+                msg = (_("Unable to retrieve properties for pool, %(pool)s") %
+                       {'pool': pool})
+                raise exception.InvalidInput(reason=msg)
+
+            if not padded:
+                LOG.warning(_LW(
+                    "Zero padding is disabled for pool, %(pool)s."
+                    "This could lead to existing data being "
+                    "accessible on new thick provisioned volumes. "
+                    "Consult the ScaleIO product documentation "
+                    "for information on how to enable zero padding "
+                    "and prevent this from occurring."),
+                    {'pool': pool})
+
     def _find_storage_pool_id_from_storage_type(self, storage_type):
         # Default to what was configured in configuration file if not defined.
         return storage_type.get(STORAGE_POOL_ID,
@@ -1222,6 +1266,132 @@ class ScaleIODriver(driver.VolumeDriver):
         self._manage_existing_check_legal_response(r, existing_ref)
         return response
 
+    def _get_protection_domain_id(self, domain_name):
+        """"Get the id of the protection domain"""
+
+        response = self._get_protection_domain_properties(domain_name)
+        if response is None:
+            return None
+
+        return response['id']
+
+    def _get_protection_domain_properties(self, domain_name):
+        """Get the props of the configured protection domain"""
+        if not domain_name:
+            msg = _LE("Error getting domain id from None name.")
+            LOG.error(msg)
+            raise exception.VolumeBackendAPIException(data=msg)
+
+        cached_val = self.pdCache.get_value(domain_name)
+        if cached_val is not None:
+            return cached_val
+
+        encoded_domain_name = urllib.parse.quote(domain_name, '')
+        req_vars = {'server_ip': self.server_ip,
+                    'server_port': self.server_port,
+                    'encoded_domain_name': encoded_domain_name}
+        request = ("https://%(server_ip)s:%(server_port)s"
+                   "/api/types/Domain/instances/getByName::"
+                   "%(encoded_domain_name)s") % req_vars
+
+        r, domain_id = self._execute_scaleio_get_request(request)
+
+        if not domain_id:
+            msg = (_("Domain with name %s wasn't found.")
+                   % domain_name)
+            LOG.error(msg)
+            raise exception.VolumeBackendAPIException(data=msg)
+        if r.status_code != http_client.OK and "errorCode" in domain_id:
+            msg = (_LE("Error getting domain id from name %(name)s: %(id)s.")
+                   % {'name': domain_name,
+                      'id': domain_id['message']})
+            LOG.error(msg)
+            raise exception.VolumeBackendAPIException(data=msg)
+
+        LOG.info(_LI("Domain id is %(domain_id)s."),
+                 {'domain_id': domain_id})
+
+        req_vars = {'server_ip': self.server_ip,
+                    'server_port': self.server_port,
+                    'domain_id': domain_id}
+        request = ("https://%(server_ip)s:%(server_port)s"
+                   "/api/instances/ProtectionDomain::%(domain_id)s") % req_vars
+        r, response = self._execute_scaleio_get_request(request)
+
+        if r.status_code != http_client.OK:
+            msg = (_("Error getting domain properties from id %(domain_id)s: "
+                     "%(err_msg)s.")
+                   % {'domain_id': domain_id,
+                      'err_msg': response})
+            LOG.error(msg)
+            raise exception.VolumeBackendAPIException(data=msg)
+
+        self.pdCache.update(domain_name, response)
+        return response
+
+    def _get_storage_pool_properties(self, domain_name, pool_name):
+        """Get the props of the configured storage pool"""
+        if not domain_name or not pool_name:
+            msg = (_("Unable to query the storage pool id for "
+                     "Pool %(pool_name)s and Domain %(domain_name)s.")
+                   % {'pool_name': pool_name,
+                      'domain_name': domain_name})
+            LOG.error(msg)
+            raise exception.VolumeBackendAPIException(data=msg)
+
+        fullname = "{}:{}".format(domain_name, pool_name)
+
+        cached_val = self.spCache.get_value(fullname)
+        if cached_val is not None:
+            return cached_val
+
+        domain_id = self._get_protection_domain_id(domain_name)
+        encoded_pool_name = urllib.parse.quote(pool_name, '')
+        req_vars = {'server_ip': self.server_ip,
+                    'server_port': self.server_port,
+                    'domain_id': domain_id,
+                    'encoded_pool_name': encoded_pool_name}
+        request = ("https://%(server_ip)s:%(server_port)s"
+                   "/api/types/Pool/instances/getByName::"
+                   "%(domain_id)s,%(encoded_pool_name)s") % req_vars
+        LOG.debug("ScaleIO get pool id by name request: %s.", request)
+        r, pool_id = self._execute_scaleio_get_request(request)
+
+        if not pool_id:
+            msg = (_("Pool with name %(pool_name)s wasn't found in "
+                     "domain %(domain_id)s.")
+                   % {'pool_name': pool_name,
+                      'domain_id': domain_id})
+            LOG.error(msg)
+            raise exception.VolumeBackendAPIException(data=msg)
+        if r.status_code != http_client.OK and "errorCode" in pool_id:
+            msg = (_("Error getting pool id from name %(pool_name)s: "
+                     "%(err_msg)s.")
+                   % {'pool_name': pool_name,
+                      'err_msg': pool_id['message']})
+            LOG.error(msg)
+            raise exception.VolumeBackendAPIException(data=msg)
+
+        LOG.info(_LI("Pool id is %(pool_id)s."), {'pool_id': pool_id})
+
+        req_vars = {'server_ip': self.server_ip,
+                    'server_port': self.server_port,
+                    'pool_id': pool_id}
+        request = ("https://%(server_ip)s:%(server_port)s"
+                   "/api/instances/StoragePool::%(pool_id)s") % req_vars
+        r, response = self._execute_scaleio_get_request(request)
+
+        if r.status_code != http_client.OK:
+            msg = (_("Error getting pool properties from id %(pool_id)s: "
+                     "%(err_msg)s.")
+                   % {'pool_id': pool_id,
+                      'err_msg': response})
+            LOG.error(msg)
+            raise exception.VolumeBackendAPIException(data=msg)
+
+        self.spCache.update(fullname, response)
+        return response
+
     def manage_existing(self, volume, existing_ref):
         """Manage an existing ScaleIO volume.
 
diff --git a/cinder/volume/drivers/emc/simplecache.py b/cinder/volume/drivers/emc/simplecache.py
new file mode 100644
index 000000000..317235bca
--- /dev/null
+++ b/cinder/volume/drivers/emc/simplecache.py
@@ -0,0 +1,122 @@
+# 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.
+"""
+SimpleCache utility class for Dell EMC ScaleIO Driver.
+"""
+
+import datetime
+
+from oslo_log import log as logging
+from oslo_utils import timeutils
+
+
+LOG = logging.getLogger(__name__)
+
+
+class SimpleCache(object):
+
+    def __init__(self, name, age_minutes=30):
+        self.cache = {}
+        self.name = name
+        self.age_minutes = age_minutes
+
+    def __contains__(self, key):
+        """Checks if a key exists in cache
+
+        :param key: Key for the item being checked.
+        :return: True if item exists, otherwise False
+        """
+        return key in self.cache
+
+    def _remove(self, key):
+        """Removes item from the cache
+
+        :param key: Key for the item being removed.
+        :return:
+        """
+        if self.__class__(key):
+            del self.cache[key]
+
+    def _validate(self, key):
+        """Validate if an item exists and has not expired.
+
+        :param key: Key for the item being requested.
+        :return: The value of the related key, or None.
+        """
+        if key not in self:
+            return None
+        # make sure the cache has not expired
+        entry = self.cache[key]['value']
+        now = timeutils.utcnow()
+        age = now - self.cache[key]['date']
+        if age > datetime.timedelta(minutes=self.age_minutes):
+            # if has expired, remove from cache
+            LOG.debug("Removing item '%(item)s' from cache '%(name)s' "
+                      "due to age",
+                      {'item': key,
+                       'name': self.name})
+            self._remove(key)
+            return None
+
+        return entry
+
+    def purge(self, key):
+        """Purge an item from the cache, regardless of age
+
+        :param key: Key for the item being removed.
+        :return:
+        """
+        self._remove(key)
+
+    def purge_all(self):
+        """Purge all items from the cache, regardless of age
+
+        :return:
+        """
+        self.cache = {}
+
+    def set_cache_period(self, age_minutes):
+        """Define the period of time to cache values for
+
+        :param age_minutes: Number of minutes to cache items for.
+        :return:
+        """
+        self.age_minutes = age_minutes
+
+    def update(self, key, value):
+        """Update/Store an item in the cache
+
+        :param key: Key for the item being added.
+        :param value: Value to store
+        :return:
+        """
+        LOG.debug("Updating item '%(item)s' in cache '%(name)s'",
+                  {'item': key,
+                   'name': self.name})
+        self.cache[key] = {'date': timeutils.utcnow(),
+                           'value': value}
+
+    def get_value(self, key):
+        """Returns an item from the cache
+
+        :param key: Key for the item being requested.
+        :return: Value of item or None if doesn't exist or expired
+        """
+        value = self._validate(key)
+        if value is None:
+            LOG.debug("Item '%(item)s' is not in cache '%(name)s' ",
+                      {'item': key,
+                       'name': self.name})
+        return value