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
This commit is contained in:
Eric Young 2017-09-11 09:46:11 -04:00 committed by Eric Young
parent f645cdab36
commit aa8b87a83c
6 changed files with 329 additions and 103 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
cinder/tests/unit/volume/drivers/dell_emc/scaleio/__init__.py
View File

@ -109,6 +109,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.
@ -135,13 +137,14 @@ class TestScaleIODriver(test.TestCase):
group=conf.SHARED_CONF_GROUP)
self.override_config('san_password', override='pass',
group=conf.SHARED_CONF_GROUP)
self.override_config('sio_storage_pool_id', override='test_pool',
self.override_config('sio_storage_pool_id',
override=self.STORAGE_POOL_ID,
group=conf.SHARED_CONF_GROUP)
self.override_config('sio_protection_domain_id',
override='test_domain',
override=self.PROT_DOMAIN_ID,
group=conf.SHARED_CONF_GROUP)
self.override_config('sio_storage_pools',
override='test_domain:test_pool',
override='PD1:SP1',
group=conf.SHARED_CONF_GROUP)
self.override_config('max_over_subscription_ratio',
override=5.0, group=conf.SHARED_CONF_GROUP)

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

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

22
cinder/tests/unit/volume/drivers/dell_emc/scaleio/test_get_manageable.py
View File

@ -117,17 +117,21 @@ class ScaleIOManageableCase(scaleio.TestScaleIODriver):
self.HTTPS_MOCK_RESPONSES = {
self.RESPONSE_MODE.Valid: {
'instances/StoragePool::test_pool/relationships/Volume':
scaleio_objects,
'instances/StoragePool::{}/relationships/Volume'.format(
self.STORAGE_POOL_ID
): scaleio_objects,
'types/Pool/instances/getByName::{},{}'.format(
"test_domain",
"test_pool"
): '"{}"'.format("test_pool").encode('ascii', 'ignore'),
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},
'types/Domain/instances/getByName::' +
"test_domain": '"{}"'.format("test_domain").encode(
'ascii',
'ignore'
),
self.PROT_DOMAIN_NAME: '"{}"'.format(self.PROT_DOMAIN_ID),
},
}

85
cinder/tests/unit/volume/drivers/dell_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
@ -28,9 +27,9 @@ from cinder.volume import configuration
@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.
@ -38,8 +37,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(
@ -51,17 +48,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,
@ -77,20 +72,23 @@ class TestMisc(scaleio.TestScaleIODriver):
self.volume['provider_id'],
'version': '"{}"'.format('2.0.1'),
'instances/StoragePool::{}'.format(
"test_pool"
self.STORAGE_POOL_ID
): {
'name': 'test_pool',
'protectionDomainId': 'test_domain',
'name': self.STORAGE_POOL_NAME,
'id': self.STORAGE_POOL_ID,
'protectionDomainId': self.PROT_DOMAIN_ID,
'zeroPaddingEnabled': 'true',
},
'instances/ProtectionDomain::{}'.format(
"test_domain"
self.PROT_DOMAIN_ID
): {
'name': 'test_domain',
'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(
{
@ -101,7 +99,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(
{
@ -120,8 +118,10 @@ class TestMisc(scaleio.TestScaleIODriver):
INVALID
"""
self.driver.configuration.sio_storage_pool_id = "test_pool_id"
self.driver.configuration.sio_storage_pool_name = "test_pool_name"
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)
@ -140,9 +140,9 @@ class TestMisc(scaleio.TestScaleIODriver):
INVALID
"""
self.driver.configuration.sio_protection_domain_name = (
"test_domain_name")
self.PROT_DOMAIN_NAME)
self.driver.configuration.sio_protection_domain_id = (
"test_domain_id")
self.PROT_DOMAIN_ID)
self.assertRaises(exception.InvalidInput,
self.driver.check_for_setup_error)
@ -185,17 +185,29 @@ class TestMisc(scaleio.TestScaleIODriver):
"""
self.HTTPS_MOCK_RESPONSES = {
self.RESPONSE_MODE.ValidVariant: {
'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),
'instances/ProtectionDomain::{}'.format(
self.PROT_DOMAIN_ID
): {
'name': self.PROT_DOMAIN_NAME,
'id': self.PROT_DOMAIN_ID
},
'instances/StoragePool::{}'.format(
self.STORAGE_POOL_ID
): {
'name': self.STORAGE_POOL_NAME,
'id': self.STORAGE_POOL_ID,
'protectionDomainId': self.PROT_DOMAIN_ID,
'zeroPaddingEnabled': 'true',
},
'types/StoragePool/instances/action/querySelectedStatistics': {
'"{}"'.format(self.POOL_NAME): {
'"{}"'.format(self.STORAGE_POOL_NAME): {
'capacityAvailableForVolumeAllocationInKb': 5000000,
'capacityLimitInKb': 16000000,
'spareCapacityInKb': 6000000,
@ -210,9 +222,6 @@ class TestMisc(scaleio.TestScaleIODriver):
self.new_volume['provider_id']):
self.volume['provider_id'],
'version': '"{}"'.format('2.0.1'),
'instances/StoragePool::{}'.format(
self.STORAGE_POOL_NAME
): '"{}"'.format(self.STORAGE_POOL_ID),
}
}

188
cinder/volume/drivers/dell_emc/scaleio/driver.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
@ -21,12 +21,13 @@ import binascii
from distutils import version
import json
import math
import re
from os_brick.initiator import connector
from oslo_config import cfg
from oslo_log import log as logging
from oslo_log import versionutils
from oslo_utils import units
import re
import requests
import six
from six.moves import http_client
@ -38,16 +39,17 @@ from cinder.i18n import _
from cinder.image import image_utils
from cinder import interface
from cinder import objects
from cinder import utils
from cinder.objects import fields
from cinder import utils
from cinder.volume import configuration
from cinder.volume import driver
from cinder.volume.drivers.dell_emc.scaleio import simplecache
from cinder.volume.drivers.san import san
from cinder.volume import qos_specs
from cinder.volume import utils as volume_utils
from cinder.volume import volume_types
CONF = cfg.CONF
LOG = logging.getLogger(__name__)
@ -130,13 +132,15 @@ SIO_MAX_OVERSUBSCRIPTION_RATIO = 10.0
@interface.volumedriver
class ScaleIODriver(driver.VolumeDriver):
"""Dell EMC ScaleIO Driver."""
"""Cinder ScaleIO Driver
VERSION = "2.0.2"
# Major changes
# 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
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"
@ -146,6 +150,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
@ -210,10 +220,6 @@ class ScaleIODriver(driver.VolumeDriver):
'bandwidthLimit': None,
}
# simple cache for domain and sp ids
self.cache_pd = {}
self.cache_sp = {}
def check_for_setup_error(self):
# make sure both domain name and id are not specified
if (self.configuration.sio_protection_domain_name
@ -280,6 +286,34 @@ class ScaleIODriver(driver.VolumeDriver):
"sio_storage_pools."))
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("Zero padding is disabled for 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)
def _build_storage_pool_list(self):
"""Build storage pool list
@ -1178,40 +1212,11 @@ class ScaleIODriver(driver.VolumeDriver):
def _get_protection_domain_id(self, domain_name):
""""Get the id of the protection domain"""
if not domain_name:
msg = (_("Error getting domain id from None name."))
LOG.error(msg)
raise exception.VolumeBackendAPIException(data=msg)
response = self._get_protection_domain_properties(domain_name)
if response is None:
return None
# do we already have the id?
if domain_name in self.cache_pd:
return self.cache_pd[domain_name]
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 = (_("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)
# add it to our cache
self.cache_pd[domain_name] = domain_id
return domain_id
return response['id']
def _get_storage_pool_name(self, pool_id):
"""Get the protection domain:storage pool name
@ -1265,8 +1270,61 @@ class ScaleIODriver(driver.VolumeDriver):
return domain_name
def _get_storage_pool_id(self, domain_name, pool_name):
"""Get the id of the configured storage pool"""
def _get_protection_domain_properties(self, domain_name):
"""Get the props of the configured protection domain"""
if not domain_name:
msg = _("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 = (_("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("Domain id is %s.", 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.")
@ -1276,9 +1334,10 @@ class ScaleIODriver(driver.VolumeDriver):
raise exception.VolumeBackendAPIException(data=msg)
fullname = "{}:{}".format(domain_name, pool_name)
if fullname in self.cache_sp:
return self.cache_sp[fullname]
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, '')
@ -1309,9 +1368,32 @@ class ScaleIODriver(driver.VolumeDriver):
LOG.info("Pool id is %s.", pool_id)
# add it to ou cache
self.cache_sp[fullname] = pool_id
return 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 _get_storage_pool_id(self, domain_name, pool_name):
"""Get the id of the configured storage pool"""
response = self._get_storage_pool_properties(domain_name, pool_name)
if response is None:
return None
return response['id']
def _get_all_scaleio_volumes(self):
"""Gets list of all SIO volumes in PD and SP"""

122
cinder/volume/drivers/dell_emc/scaleio/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