openstack
/
cinder
Code Issues Proposed changes

ScaleIO Driver - adding cache and refactoring tests 

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 aa8b87a83c)
(cherry picked from commit 0e9b173381)
(cherry picked from commit 034feb6d74)
This commit is contained in:
Eric Young 2017-09-11 09:46:11 -04:00 committed by Sofia Enriquez
parent da9c174c91
commit 442832947f
5 changed files with 348 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
cinder/tests/unit/volume/drivers/emc/scaleio/__init__.py
View File

@ -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.

6
cinder/tests/unit/volume/drivers/emc/scaleio/test_create_volume.py
View File

@ -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::' +

67
cinder/tests/unit/volume/drivers/emc/scaleio/test_misc.py
View File

@ -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)

176
cinder/volume/drivers/emc/scaleio.py
View File

@ -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.

122
cinder/volume/drivers/emc/simplecache.py Normal file
View File

@ -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