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

diff --git a/cinder/brick/iscsi/iscsi.py b/cinder/brick/iscsi/iscsi.py
index 49534a3f5203965aba8cdb4b60968fae0c7aadc0..103014e9855c4d1c6fef5d8ef9402efa866d6dfc 100644 (file)
--- 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 65f20e6e424a702fff4211bb5e5280d02cda9154..f7b62bce821b7f3314f4c5020639691af04ca6bb 100644 (file)
--- 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 ff3d5de4126b1c442f63d7a5bfe7a502a044606b..b31e7edf4981de7cb73752b186d9f7249c04d9e4 100644 (file)
--- 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 1233211bd0cf485d1fe06d46d9bf9be3dedbb52f..76bfcf0181e7d4bdb5ee5787b0e7603fda1c6606 100644 (file)
--- 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 e89e9dd8988526716748959d18e436e76f71a59e..905543dd628a6dd35f705716d8015b171c7c968a 100644 (file)
--- 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 932bfdbafb2220be324c978936d7ffa22f29d893..5b2dcae6a56ae921457f5f83e57fd598abcac8ba 100644 (file)
--- 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