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

@@ -26,8 +26,6 @@
 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 @@
 
 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 @@ def _delete_logicalunit(self, tid, lun, **kwargs):
 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 @@ def create_iscsi_target(self, name, tid, lun, path,
         # 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 @@ def create_iscsi_target(self, name, tid, lun, path,
             """ % (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 @@ def create_iscsi_target(self, name, tid, lun, path,
             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 @@ def create_iscsi_target(self, name, tid, lun, path,
     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 @@ def show_target(self, tid, iqn=None, **kwargs):
 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 @@ def create_iscsi_target(self, name, tid, lun, path,
             (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 @@ def remove_iscsi_target(self, tid, lun, vol_id, vol_name, **kwargs):
         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 @@ def create_iscsi_target(self, *args, **kwargs):
 
 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 @@ def create_iscsi_target(self, name, tid, lun, path,
             (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 @@ def create_iscsi_target(self, name, tid, lun, 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 "
@@ -526,7 +510,7 @@ def create_iscsi_target(self, name, tid, lun, path,
     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 @@ def initialize_connection(self, volume, connector):
             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)

cinder/tests/test_iscsi.py

@@ -21,6 +21,7 @@
 
 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 @@ def setUp(self):
         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 @@ def verify(self):
         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)

cinder/volume/driver.py

@@ -26,6 +26,7 @@
 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 @@
                     '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 @@ def _update_volume_stats(self):
     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."""

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)

cinder/volume/drivers/lvm.py

@@ -83,7 +83,7 @@ def set_execute(self, execute):
     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 =\

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