Skip to content

Commit

Permalink
Fix config registration in cinder volume drivers.
Browse files Browse the repository at this point in the history
The config documentation relies on options being registered
on a modules import.  Our need to move the drivers to using
self.configuration for multi-backend support means that options
wouldn't be loaded until object initialization which breaks
documentation.

This patch puts a dummy CONF init/load back in the drivers. While putting
this change together I came across a number of drivers still using FLAGS,
and even worse a number of drivers using a mixture of FLAGS and CONF and
self.configuraiton.  So most of those are cleaned up here as well.

Note there are two drivers that were not updated at all here:
  1. windows.py
  2. zadara.py

The zadara folks have indicated that they're in the process of updating and
releasing a new version of their driver so I left that as is.

The windows driver needs a bit of work to switch over.

Fixes bug: 1179159

Change-Id: I90165299bf080da17741d027e36e361540da0ff8
  • Loading branch information
j-griffith committed Jun 5, 2013
1 parent c2b8f20 commit 0b553dc
Show file tree
Hide file tree
Showing 25 changed files with 167 additions and 138 deletions.
2 changes: 2 additions & 0 deletions cinder/tests/test_coraid.py
Expand Up @@ -120,6 +120,8 @@ def setUp(self):
configuration.coraid_user = fake_esm_username
configuration.coraid_group = fake_esm_group
configuration.coraid_password = fake_esm_password
configuration.volume_name_template = "volume-%s"
configuration.snapshot_name_template = "snapshot-%s"

self.drv = CoraidDriver(configuration=configuration)
self.drv.do_setup({})
Expand Down
16 changes: 8 additions & 8 deletions cinder/tests/test_glusterfs.py
Expand Up @@ -87,7 +87,7 @@ def stub_out_not_replaying(self, obj, attr_name):

def test_local_path(self):
"""local_path common use case."""
glusterfs.FLAGS.glusterfs_mount_point_base = self.TEST_MNT_POINT_BASE
glusterfs.CONF.glusterfs_mount_point_base = self.TEST_MNT_POINT_BASE
drv = self._driver

volume = DumbVolume()
Expand Down Expand Up @@ -188,7 +188,7 @@ def test_get_mount_point_for_share(self):
"""_get_mount_point_for_share should calculate correct value."""
drv = self._driver

glusterfs.FLAGS.glusterfs_mount_point_base = self.TEST_MNT_POINT_BASE
glusterfs.CONF.glusterfs_mount_point_base = self.TEST_MNT_POINT_BASE

self.assertEqual('/mnt/test/ab03ab34eaca46a5fb81878f7e9b91fc',
drv._get_mount_point_for_share(
Expand All @@ -206,7 +206,7 @@ def test_get_available_capacity_with_df(self):
(df_total_size, df_avail)
df_output = df_head + df_data

setattr(glusterfs.FLAGS, 'glusterfs_disk_util', 'df')
setattr(glusterfs.CONF, 'glusterfs_disk_util', 'df')

mox.StubOutWithMock(drv, '_get_mount_point_for_share')
drv._get_mount_point_for_share(self.TEST_EXPORT1).\
Expand All @@ -225,7 +225,7 @@ def test_get_available_capacity_with_df(self):

mox.VerifyAll()

delattr(glusterfs.FLAGS, 'glusterfs_disk_util')
delattr(glusterfs.CONF, 'glusterfs_disk_util')

def test_get_available_capacity_with_du(self):
"""_get_available_capacity should calculate correct value."""
Expand Down Expand Up @@ -368,7 +368,7 @@ def test_setup_should_throw_error_if_shares_config_not_configured(self):
"""do_setup should throw error if shares config is not configured."""
drv = self._driver

glusterfs.FLAGS.glusterfs_shares_config = self.TEST_SHARES_CONFIG_FILE
glusterfs.CONF.glusterfs_shares_config = self.TEST_SHARES_CONFIG_FILE

self.assertRaises(exception.GlusterfsException,
drv.do_setup, IsA(context.RequestContext))
Expand All @@ -378,7 +378,7 @@ def test_setup_should_throw_exception_if_client_is_not_installed(self):
mox = self._mox
drv = self._driver

glusterfs.FLAGS.glusterfs_shares_config = self.TEST_SHARES_CONFIG_FILE
glusterfs.CONF.glusterfs_shares_config = self.TEST_SHARES_CONFIG_FILE

mox.StubOutWithMock(os.path, 'exists')
os.path.exists(self.TEST_SHARES_CONFIG_FILE).AndReturn(True)
Expand Down Expand Up @@ -457,7 +457,7 @@ def test_create_sparsed_volume(self):
drv = self._driver
volume = self._simple_volume()

setattr(glusterfs.FLAGS, 'glusterfs_sparsed_volumes', True)
setattr(glusterfs.CONF, 'glusterfs_sparsed_volumes', True)

mox.StubOutWithMock(drv, '_create_sparsed_file')
mox.StubOutWithMock(drv, '_set_rw_permissions_for_all')
Expand All @@ -471,7 +471,7 @@ def test_create_sparsed_volume(self):

mox.VerifyAll()

delattr(glusterfs.FLAGS, 'glusterfs_sparsed_volumes')
delattr(glusterfs.CONF, 'glusterfs_sparsed_volumes')

def test_create_nonsparsed_volume(self):
mox = self._mox
Expand Down
10 changes: 5 additions & 5 deletions cinder/tests/test_scality.py
Expand Up @@ -77,9 +77,9 @@ def _remove_fake_config(self):
raise e

def _configure_driver(self):
scality.FLAGS.scality_sofs_config = self.TEST_CONFIG
scality.FLAGS.scality_sofs_mount_point = self.TEST_MOUNT
scality.FLAGS.scality_sofs_volume_dir = self.TEST_VOLDIR
scality.CONF.scality_sofs_config = self.TEST_CONFIG
scality.CONF.scality_sofs_mount_point = self.TEST_MOUNT
scality.CONF.scality_sofs_volume_dir = self.TEST_VOLDIR

def _execute_wrapper(self, cmd, *args, **kwargs):
try:
Expand Down Expand Up @@ -116,13 +116,13 @@ def tearDown(self):

def test_setup_no_config(self):
"""Missing SOFS configuration shall raise an error."""
scality.FLAGS.scality_sofs_config = None
scality.CONF.scality_sofs_config = None
self.assertRaises(exception.VolumeBackendAPIException,
self._driver.do_setup, None)

def test_setup_missing_config(self):
"""Non-existent SOFS configuration file shall raise an error."""
scality.FLAGS.scality_sofs_config = 'nonexistent.conf'
scality.CONF.scality_sofs_config = 'nonexistent.conf'
self.assertRaises(exception.VolumeBackendAPIException,
self._driver.do_setup, None)

Expand Down
15 changes: 8 additions & 7 deletions cinder/tests/test_windows.py
Expand Up @@ -20,13 +20,14 @@
"""
import sys

import cinder.flags
from oslo.config import cfg

from cinder.tests.windows import basetestcase
from cinder.tests.windows import db_fakes
from cinder.tests.windows import windowsutils
from cinder.volume.drivers import windows

FLAGS = cinder.flags.FLAGS
CONF = cfg.CONF


class TestWindowsDriver(basetestcase.BaseTestCase):
Expand Down Expand Up @@ -88,19 +89,19 @@ def tearDown(self):
self._wutils.delete_snapshot(self._snapshot_data['name'])
if (self._connector_data and
self._wutils.initiator_id_exists(
"%s%s" % (FLAGS.iscsi_target_prefix,
"%s%s" % (CONF.iscsi_target_prefix,
self._volume_data['name']),
self._connector_data['initiator'])):
target_name = "%s%s" % (FLAGS.iscsi_target_prefix,
target_name = "%s%s" % (CONF.iscsi_target_prefix,
self._volume_data['name'])
initiator_name = self._connector_data['initiator']
self._wutils.delete_initiator_id(target_name, initiator_name)
if (self._volume_data and
self._wutils.export_exists("%s%s" %
(FLAGS.iscsi_target_prefix,
(CONF.iscsi_target_prefix,
self._volume_data['name']))):
self._wutils.delete_export(
"%s%s" % (FLAGS.iscsi_target_prefix,
"%s%s" % (CONF.iscsi_target_prefix,
self._volume_data['name']))

finally:
Expand Down Expand Up @@ -182,7 +183,7 @@ def test_create_export(self):
volume_name = self._volume_data['name']
self.assertEquals(
retval,
{'provider_location': "%s%s" % (FLAGS.iscsi_target_prefix,
{'provider_location': "%s%s" % (CONF.iscsi_target_prefix,
volume_name)})

def test_initialize_connection(self):
Expand Down
10 changes: 5 additions & 5 deletions cinder/tests/test_xenapi_sm.py
Expand Up @@ -54,7 +54,7 @@ def get_configured_driver(server='ignore_server', path='ignore_path'):
class DriverTestCase(test.TestCase):

def assert_flag(self, flagname):
self.assertTrue(hasattr(driver.FLAGS, flagname))
self.assertTrue(hasattr(driver.CONF, flagname))

def test_config_options(self):
self.assert_flag('xenapi_connection_url')
Expand Down Expand Up @@ -210,10 +210,10 @@ def _setup_mock_driver(self, server, serverpath, sr_base_path="_srbp"):
drv.nfs_ops = ops
drv.db = db

mock.StubOutWithMock(driver, 'FLAGS')
driver.FLAGS.xenapi_nfs_server = server
driver.FLAGS.xenapi_nfs_serverpath = serverpath
driver.FLAGS.xenapi_sr_base_path = sr_base_path
mock.StubOutWithMock(driver, 'CONF')
driver.CONF.xenapi_nfs_server = server
driver.CONF.xenapi_nfs_serverpath = serverpath
driver.CONF.xenapi_sr_base_path = sr_base_path

return mock, drv

Expand Down
27 changes: 12 additions & 15 deletions cinder/volume/drivers/coraid.py
Expand Up @@ -22,23 +22,18 @@
"""

import cookielib
import os
import time
import urllib2

from oslo.config import cfg

from cinder import context
from cinder import exception
from cinder import flags
from cinder.openstack.common import jsonutils
from cinder.openstack.common import log as logging
from cinder.volume import driver
from cinder.volume import volume_types

LOG = logging.getLogger(__name__)

FLAGS = flags.FLAGS
coraid_opts = [
cfg.StrOpt('coraid_esm_address',
default='',
Expand All @@ -57,7 +52,9 @@
default='coraid_repository',
help='Volume Type key name to store ESM Repository Name'),
]
FLAGS.register_opts(coraid_opts)

CONF = cfg.CONF
CONF.register_opts(coraid_opts)


class CoraidException(Exception):
Expand Down Expand Up @@ -325,11 +322,11 @@ def delete_volume(self, volume):

def create_snapshot(self, snapshot):
"""Create a Snapshot."""
volume_name = (self.configuration.volume_name_template
% snapshot['volume_id'])
snapshot_name = (self.configuration.snapshot_name_template
% snapshot['id'])
try:
volume_name = (FLAGS.volume_name_template
% snapshot['volume_id'])
snapshot_name = (FLAGS.snapshot_name_template
% snapshot['id'])
self.esm.create_snapshot(volume_name, snapshot_name)
except Exception, e:
msg = _('Failed to Create Snapshot %(snapname)s')
Expand All @@ -339,9 +336,9 @@ def create_snapshot(self, snapshot):

def delete_snapshot(self, snapshot):
"""Delete a Snapshot."""
snapshot_name = (self.configuration.snapshot_name_template
% snapshot['id'])
try:
snapshot_name = (FLAGS.snapshot_name_template
% snapshot['id'])
self.esm.delete_snapshot(snapshot_name)
except Exception:
msg = _('Failed to Delete Snapshot %(snapname)s')
Expand All @@ -351,10 +348,10 @@ def delete_snapshot(self, snapshot):

def create_volume_from_snapshot(self, volume, snapshot):
"""Create a Volume from a Snapshot."""
snapshot_name = (self.configuration.snapshot_name_template
% snapshot['id'])
repository = self._get_repository(volume['volume_type'])
try:
snapshot_name = (FLAGS.snapshot_name_template
% snapshot['id'])
repository = self._get_repository(volume['volume_type'])
self.esm.create_volume_from_snapshot(snapshot_name,
volume['name'],
repository)
Expand Down
5 changes: 2 additions & 3 deletions cinder/volume/drivers/emc/emc_smis_common.py
Expand Up @@ -29,12 +29,11 @@
from xml.dom.minidom import parseString

from cinder import exception
from cinder import flags
from cinder.openstack.common import log as logging

LOG = logging.getLogger(__name__)

FLAGS = flags.FLAGS
CONF = cfg.CONF

try:
import pywbem
Expand Down Expand Up @@ -62,7 +61,7 @@ def __init__(self, prtcl, configuration=None):
default=CINDER_EMC_CONFIG_FILE,
help='use this file for cinder emc plugin '
'config data')
FLAGS.register_opt(opt)
CONF.register_opt(opt)
self.protocol = prtcl
self.configuration = configuration
self.configuration.append_config_values([opt])
Expand Down
10 changes: 2 additions & 8 deletions cinder/volume/drivers/emc/emc_smis_iscsi.py
Expand Up @@ -20,20 +20,14 @@
"""

import os
import time

from cinder import exception
from cinder import flags
from cinder.openstack.common import log as logging
from cinder import utils
from cinder.volume import driver
from cinder.volume.drivers.emc import emc_smis_common

LOG = logging.getLogger(__name__)

FLAGS = flags.FLAGS


class EMCSMISISCSIDriver(driver.ISCSIDriver):
"""EMC ISCSI Drivers for VMAX and VNX using SMI-S."""
Expand All @@ -42,8 +36,8 @@ def __init__(self, *args, **kwargs):

super(EMCSMISISCSIDriver, self).__init__(*args, **kwargs)
self.common = emc_smis_common.EMCSMISCommon(
'iSCSI',
configuration=self.configuration)
'iSCSI',
configuration=self.configuration)

def check_for_setup_error(self):
pass
Expand Down
5 changes: 2 additions & 3 deletions cinder/volume/drivers/glusterfs.py
Expand Up @@ -21,7 +21,6 @@
from oslo.config import cfg

from cinder import exception
from cinder import flags
from cinder.openstack.common import log as logging
from cinder.volume.drivers import nfs

Expand All @@ -44,8 +43,8 @@
'In such case volume creation takes a lot of time.'))]
VERSION = '1.0'

FLAGS = flags.FLAGS
FLAGS.register_opts(volume_opts)
CONF = cfg.CONF
CONF.register_opts(volume_opts)


class GlusterfsDriver(nfs.RemoteFsDriver):
Expand Down
4 changes: 4 additions & 0 deletions cinder/volume/drivers/huawei/huawei_iscsi.py
Expand Up @@ -48,6 +48,10 @@
READBUFFERSIZE = 8192


CONF = cfg.CONF
CONF.register_opts(huawei_opt)


class SSHConn(utils.SSHPool):
"""Define a new class inherited to SSHPool.
Expand Down

0 comments on commit 0b553dc

Please sign in to comment.