From 09f2bea49b853c327762d122d6d9e18f62aed31f Mon Sep 17 00:00:00 2001 From: John Griffith Date: Thu, 26 Sep 2013 15:23:01 -0600 Subject: [PATCH] Remove need for CONF acces in brick iscsi At some point we'd like brick to be a standalone lib, and as such we don't want to have a requirement for CONF files and having duplicate conf entries across projects. The better approach would be to let the projects decide what they want to use, and how they want defaults to be set and then pass those settings in via __init__ or when calling the methods that need them. Partial-Bug: #1230066 Change-Id: Ib108f1abb2948cb896bdad58070daa7a725a205d --- cinder/brick/iscsi/iscsi.py | 89 ++++++++++----------------- cinder/tests/test_iscsi.py | 13 ++-- cinder/volume/driver.py | 40 +++++++++++- cinder/volume/drivers/block_device.py | 3 +- cinder/volume/drivers/lvm.py | 7 +-- etc/cinder/cinder.conf.sample | 41 ++++++------ 6 files changed, 101 insertions(+), 92 deletions(-) diff --git a/cinder/brick/iscsi/iscsi.py b/cinder/brick/iscsi/iscsi.py index 49534a3f5..103014e98 100644 --- a/cinder/brick/iscsi/iscsi.py +++ b/cinder/brick/iscsi/iscsi.py @@ -26,8 +26,6 @@ import re import stat import time -from oslo.config import cfg - from cinder.brick import exception from cinder.brick import executor from cinder.openstack.common import fileutils @@ -38,35 +36,6 @@ from cinder.openstack.common import processutils as putils LOG = logging.getLogger(__name__) -iscsi_helper_opt = [cfg.StrOpt('iscsi_helper', - default='tgtadm', - help='iscsi target user-land tool to use'), - cfg.StrOpt('volumes_dir', - default='$state_path/volumes', - help='Volume configuration file storage ' - 'directory'), - cfg.StrOpt('iet_conf', - default='/etc/iet/ietd.conf', - help='IET configuration file'), - cfg.StrOpt('lio_initiator_iqns', - default='', - help=('Comma-separatd list of initiator IQNs ' - 'allowed to connect to the ' - 'iSCSI target. (From Nova compute nodes.)' - ) - ), - cfg.StrOpt('iscsi_iotype', - default='fileio', - help=('Sets the behavior of the iSCSI target ' - 'to either perform blockio or fileio ' - 'optionally, auto can be set and Cinder ' - 'will autodetect type of backing device') - ) - ] - -CONF = cfg.CONF -CONF.register_opts(iscsi_helper_opt) - class TargetAdmin(executor.Executor): """iSCSI target administration. @@ -114,9 +83,14 @@ class TargetAdmin(executor.Executor): class TgtAdm(TargetAdmin): """iSCSI target administration using tgtadm.""" - def __init__(self, root_helper, execute=putils.execute): + def __init__(self, root_helper, volumes_dir, + target_prefix='iqn.2010-10.org.openstack:', + execute=putils.execute): super(TgtAdm, self).__init__('tgtadm', root_helper, execute) + self.iscsi_target_prefix = target_prefix + self.volumes_dir = volumes_dir + def _get_target(self, iqn): (out, err) = self._execute('tgt-admin', '--show', run_as_root=True) lines = out.split('\n') @@ -178,7 +152,7 @@ class TgtAdm(TargetAdmin): # Note(jdg) tid and lun aren't used by TgtAdm but remain for # compatibility - fileutils.ensure_tree(CONF.volumes_dir) + fileutils.ensure_tree(self.volumes_dir) vol_id = name.split(':')[1] if chap_auth is None: @@ -196,7 +170,7 @@ class TgtAdm(TargetAdmin): """ % (name, path, chap_auth) LOG.info(_('Creating iscsi_target for: %s') % vol_id) - volumes_dir = CONF.volumes_dir + volumes_dir = self.volumes_dir volume_path = os.path.join(volumes_dir, vol_id) f = open(volume_path, 'w+') @@ -238,7 +212,7 @@ class TgtAdm(TargetAdmin): os.unlink(volume_path) raise exception.ISCSITargetCreateFailed(volume_id=vol_id) - iqn = '%s%s' % (CONF.iscsi_target_prefix, vol_id) + iqn = '%s%s' % (self.iscsi_target_prefix, vol_id) tid = self._get_target(iqn) if tid is None: LOG.error(_("Failed to create iscsi target for volume " @@ -274,9 +248,9 @@ class TgtAdm(TargetAdmin): def remove_iscsi_target(self, tid, lun, vol_id, vol_name, **kwargs): LOG.info(_('Removing iscsi_target for: %s') % vol_id) vol_uuid_file = vol_name - volume_path = os.path.join(CONF.volumes_dir, vol_uuid_file) + volume_path = os.path.join(self.volumes_dir, vol_uuid_file) if os.path.isfile(volume_path): - iqn = '%s%s' % (CONF.iscsi_target_prefix, + iqn = '%s%s' % (self.iscsi_target_prefix, vol_uuid_file) else: raise exception.ISCSITargetRemoveFailed(volume_id=vol_id) @@ -309,18 +283,21 @@ class TgtAdm(TargetAdmin): class IetAdm(TargetAdmin): """iSCSI target administration using ietadm.""" - def __init__(self, root_helper, execute=putils.execute): + def __init__(self, root_helper, iet_conf='/etc/iet/ietd.conf', + iscsi_iotype='fileio', execute=putils.execute): super(IetAdm, self).__init__('ietadm', root_helper, execute) + self.iet_conf = iet_conf + self.iscsi_iotype = iscsi_iotype def _is_block(self, path): mode = os.stat(path).st_mode return stat.S_ISBLK(mode) def _iotype(self, path): - if CONF.iscsi_iotype == 'auto': + if self.iscsi_iotype == 'auto': return 'blockio' if self._is_block(path) else 'fileio' else: - return CONF.iscsi_iotype + return self.iscsi_iotype @contextlib.contextmanager def temporary_chown(self, path, owner_uid=None): @@ -356,7 +333,7 @@ class IetAdm(TargetAdmin): (type, username, password) = chap_auth.split() self._new_auth(tid, type, username, password, **kwargs) - conf_file = CONF.iet_conf + conf_file = self.iet_conf if os.path.exists(conf_file): try: volume_conf = """ @@ -382,7 +359,7 @@ class IetAdm(TargetAdmin): self._delete_logicalunit(tid, lun, **kwargs) self._delete_target(tid, **kwargs) vol_uuid_file = vol_name - conf_file = CONF.iet_conf + conf_file = self.iet_conf if os.path.exists(conf_file): with self.temporary_chown(conf_file): try: @@ -458,9 +435,16 @@ class FakeIscsiHelper(object): class LioAdm(TargetAdmin): """iSCSI target administration for LIO using python-rtslib.""" - def __init__(self, root_helper, execute=putils.execute): + def __init__(self, root_helper, lio_initiator_iqns='', + iscsi_target_prefix='iqn.2010-10.org.openstack:', + execute=putils.execute): super(LioAdm, self).__init__('rtstool', root_helper, execute) + self.iscsi_target_prefix = iscsi_target_prefix + self.lio_initiator_iqns = lio_initiator_iqns + self._verify_rtstool() + + def _verify_rtstool(self): try: self._execute('rtstool', 'verify') except (OSError, putils.ProcessExecutionError): @@ -494,8 +478,8 @@ class LioAdm(TargetAdmin): (chap_auth_userid, chap_auth_password) = chap_auth.split(' ')[1:] extra_args = [] - if CONF.lio_initiator_iqns: - extra_args.append(CONF.lio_initiator_iqns) + if self.lio_initiator_iqns: + extra_args.append(self.lio_initiator_iqns) try: command_args = ['rtstool', @@ -514,7 +498,7 @@ class LioAdm(TargetAdmin): raise exception.ISCSITargetCreateFailed(volume_id=vol_id) - iqn = '%s%s' % (CONF.iscsi_target_prefix, vol_id) + iqn = '%s%s' % (self.iscsi_target_prefix, vol_id) tid = self._get_target(iqn) if tid is None: LOG.error(_("Failed to create iscsi target for volume " @@ -526,7 +510,7 @@ class LioAdm(TargetAdmin): def remove_iscsi_target(self, tid, lun, vol_id, vol_name, **kwargs): LOG.info(_('Removing iscsi_target: %s') % vol_id) vol_uuid_name = vol_name - iqn = '%s%s' % (CONF.iscsi_target_prefix, vol_uuid_name) + iqn = '%s%s' % (self.iscsi_target_prefix, vol_uuid_name) try: self._execute('rtstool', @@ -566,14 +550,3 @@ class LioAdm(TargetAdmin): LOG.error(_("Failed to add initiator iqn %s to target") % connector['initiator']) raise exception.ISCSITargetAttachFailed(volume_id=volume['id']) - - -def get_target_admin(root_helper): - if CONF.iscsi_helper == 'tgtadm': - return TgtAdm(root_helper) - elif CONF.iscsi_helper == 'fake': - return FakeIscsiHelper() - elif CONF.iscsi_helper == 'lioadm': - return LioAdm(root_helper) - else: - return IetAdm(root_helper) diff --git a/cinder/tests/test_iscsi.py b/cinder/tests/test_iscsi.py index 65f20e6e4..f7b62bce8 100644 --- a/cinder/tests/test_iscsi.py +++ b/cinder/tests/test_iscsi.py @@ -21,6 +21,7 @@ import tempfile from cinder.brick.iscsi import iscsi from cinder import test +from cinder.volume import driver from cinder.volume import utils as volume_utils @@ -41,15 +42,19 @@ class TargetAdminTestCase(object): self.stubs.Set(os, 'unlink', lambda _: '') self.stubs.Set(iscsi.TgtAdm, '_get_target', self.fake_get_target) self.stubs.Set(iscsi.LioAdm, '_get_target', self.fake_get_target) - self.stubs.Set(iscsi.LioAdm, '__init__', self.fake_init) + self.stubs.Set(iscsi.LioAdm, + '_verify_rtstool', + self.fake_verify_rtstool) + self.driver = driver.ISCSIDriver() self.stubs.Set(iscsi.TgtAdm, '_verify_backing_lun', self.fake_verify_backing_lun) + self.driver = driver.ISCSIDriver() def fake_verify_backing_lun(obj, iqn, tid): return True - def fake_init(obj, root_helper): - return + def fake_verify_rtstool(obj): + pass def fake_get_target(obj, iqn): return 1 @@ -85,7 +90,7 @@ class TargetAdminTestCase(object): self.verify_cmds(cmds) def run_commands(self): - tgtadm = iscsi.get_target_admin(None) + tgtadm = self.driver.get_target_admin() tgtadm.set_execute(self.fake_execute) tgtadm.create_iscsi_target(self.target_name, self.tid, self.lun, self.path) diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py index ff3d5de41..b31e7edf4 100644 --- a/cinder/volume/driver.py +++ b/cinder/volume/driver.py @@ -26,6 +26,7 @@ import time from oslo.config import cfg from cinder.brick.initiator import connector as initiator +from cinder.brick.iscsi import iscsi from cinder import exception from cinder.image import image_utils from cinder.openstack.common import excutils @@ -91,7 +92,28 @@ volume_opts = [ 'none, zero, shred)'), cfg.IntOpt('volume_clear_size', default=0, - help='Size in MiB to wipe at start of old volumes. 0 => all'), ] + help='Size in MiB to wipe at start of old volumes. 0 => all'), + cfg.StrOpt('iscsi_helper', + default='tgtadm', + help='iscsi target user-land tool to use'), + cfg.StrOpt('volumes_dir', + default='$state_path/volumes', + help='Volume configuration file storage ' + 'directory'), + cfg.StrOpt('iet_conf', + default='/etc/iet/ietd.conf', + help='IET configuration file'), + cfg.StrOpt('lio_initiator_iqns', + default='', + help=('Comma-separated list of initiator IQNs ' + 'allowed to connect to the ' + 'iSCSI target. (From Nova compute nodes.)')), + cfg.StrOpt('iscsi_iotype', + default='fileio', + help=('Sets the behavior of the iSCSI target ' + 'to either perform blockio or fileio ' + 'optionally, auto can be set and Cinder ' + 'will autodetect type of backing device'))] CONF = cfg.CONF @@ -665,6 +687,22 @@ class ISCSIDriver(VolumeDriver): def accept_transfer(self, context, volume, new_user, new_project): pass + def get_target_admin(self): + root_helper = utils.get_root_helper() + + if CONF.iscsi_helper == 'tgtadm': + return iscsi.TgtAdm(root_helper, + CONF.volumes_dir, + CONF.iscsi_target_prefix) + elif CONF.iscsi_helper == 'fake': + return iscsi.FakeIscsiHelper() + elif CONF.iscsi_helper == 'lioadm': + return iscsi.LioAdm(root_helper, + CONF.lio_initiator_iqns, + CONF.iscsi_target_prefix) + else: + return iscsi.IetAdm(root_helper, CONF.iet_conf, CONF.iscsi_iotype) + class FakeISCSIDriver(ISCSIDriver): """Logs calls instead of executing.""" diff --git a/cinder/volume/drivers/block_device.py b/cinder/volume/drivers/block_device.py index 1233211bd..76bfcf018 100644 --- a/cinder/volume/drivers/block_device.py +++ b/cinder/volume/drivers/block_device.py @@ -44,8 +44,7 @@ class BlockDeviceDriver(driver.ISCSIDriver): VERSION = '1.0.0' def __init__(self, *args, **kwargs): - root_helper = 'sudo cinder-rootwrap %s' % CONF.rootwrap_config - self.tgtadm = iscsi.get_target_admin(root_helper) + self.tgtadm = self.get_target_admin() super(BlockDeviceDriver, self).__init__(*args, **kwargs) self.configuration.append_config_values(volume_opts) diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py index e89e9dd89..905543dd6 100644 --- a/cinder/volume/drivers/lvm.py +++ b/cinder/volume/drivers/lvm.py @@ -83,7 +83,7 @@ class LVMVolumeDriver(driver.VolumeDriver): def check_for_setup_error(self): """Verify that requirements are in place to use LVM driver.""" if self.vg is None: - root_helper = 'sudo cinder-rootwrap %s' % CONF.rootwrap_config + root_helper = utils.get_root_helper() try: self.vg = lvm.LVM(self.configuration.volume_group, root_helper, @@ -401,8 +401,7 @@ class LVMISCSIDriver(LVMVolumeDriver, driver.ISCSIDriver): """ def __init__(self, *args, **kwargs): - root_helper = 'sudo cinder-rootwrap %s' % CONF.rootwrap_config - self.tgtadm = iscsi.get_target_admin(root_helper) + self.tgtadm = self.get_target_admin() super(LVMISCSIDriver, self).__init__(*args, **kwargs) self.backend_name =\ self.configuration.safe_get('volume_backend_name') or 'LVM_iSCSI' @@ -749,7 +748,7 @@ class LVMISERDriver(LVMISCSIDriver, driver.ISERDriver): """ def __init__(self, *args, **kwargs): - root_helper = 'sudo cinder-rootwrap %s' % CONF.rootwrap_config + root_helper = utils.get_root_helper() self.tgtadm = iser.get_target_admin(root_helper) LVMVolumeDriver.__init__(self, *args, **kwargs) self.backend_name =\ diff --git a/etc/cinder/cinder.conf.sample b/etc/cinder/cinder.conf.sample index 932bfdbaf..5b2dcae6a 100644 --- a/etc/cinder/cinder.conf.sample +++ b/etc/cinder/cinder.conf.sample @@ -234,29 +234,6 @@ #backup_driver=cinder.backup.drivers.swift -# -# Options defined in cinder.brick.iscsi.iscsi -# - -# iscsi target user-land tool to use (string value) -#iscsi_helper=tgtadm - -# Volume configuration file storage directory (string value) -#volumes_dir=$state_path/volumes - -# IET configuration file (string value) -#iet_conf=/etc/iet/ietd.conf - -# Comma-separatd list of initiator IQNs allowed to connect to -# the iSCSI target. (From Nova compute nodes.) (string value) -#lio_initiator_iqns= - -# Sets the behavior of the iSCSI target to either perform -# blockio or fileio optionally, auto can be set and Cinder -# will autodetect type of backing device (string value) -#iscsi_iotype=fileio - - # # Options defined in cinder.brick.iser.iser # @@ -1098,6 +1075,24 @@ # (integer value) #volume_clear_size=0 +# iscsi target user-land tool to use (string value) +#iscsi_helper=tgtadm + +# Volume configuration file storage directory (string value) +#volumes_dir=$state_path/volumes + +# IET configuration file (string value) +#iet_conf=/etc/iet/ietd.conf + +# Comma-separated list of initiator IQNs allowed to connect to +# the iSCSI target. (From Nova compute nodes.) (string value) +#lio_initiator_iqns= + +# Sets the behavior of the iSCSI target to either perform +# blockio or fileio optionally, auto can be set and Cinder +# will autodetect type of backing device (string value) +#iscsi_iotype=fileio + # # Options defined in cinder.volume.drivers.block_device -- 2.45.2