Support iSER driver within the ISCSITarget flow

Currently the iSER driver is supported over TGT only,
and there are a couple of iSER classes that inherits
from iSCSI classes, but most of their functionality is
the same as the iSCSI classes. This code duplication caused
instability in the iSER driver code, when new features or
changes are added to the iSCSI driver flow.

Main changes:
  1. Added a new parameter to volume/driver.py in order to
     set the iSCSI protocol type to 'iscsi' or 'iser', with default
     to 'iscsi'.
  2. Configured TGT VOLUME_CONF and VOLUME_CONF_WITH_CHAP_AUTH
     with the new iSCSI protocol parameter.
  3. Added support for RDMA (using iSER) to cinder-rtstool.
  4. Set "driver_volume_type" to "iscsi" or "iser" value, according
     to the new parameter value.
  5. Added unit tests for the new iSER flow.
  6. Added deprecation alert to ISERTgtAdm.

DocImpact

Implements: blueprint support-iscsi-driver

Change-Id: Ie2c021e7fd37e7221f99b3311e4792c958045431

cinder/cmd/rtstool.py

@@ -33,7 +33,8 @@ class RtstoolImportError(RtstoolError):
     pass
 
 
-def create(backing_device, name, userid, password, initiator_iqns=None):
+def create(backing_device, name, userid, password, iser_enabled,
+           initiator_iqns=None):
     try:
         rtsroot = rtslib.root.RTSRoot()
     except rtslib.utils.RTSLibError:
@@ -68,12 +69,20 @@ def create(backing_device, name, userid, password, initiator_iqns=None):
     tpg_new.enable = 1
 
     try:
-        rtslib.NetworkPortal(tpg_new, '0.0.0.0', 3260, mode='any')
+        portal = rtslib.NetworkPortal(tpg_new, '0.0.0.0', 3260, mode='any')
     except rtslib.utils.RTSLibError:
         print(_('Error creating NetworkPortal: ensure port 3260 '
                 'is not in use by another service.'))
         raise
 
+    try:
+        if iser_enabled == 'True':
+            portal._set_iser(1)
+    except rtslib.utils.RTSLibError:
+        print(_('Error enabling iSER for NetworkPortal: please ensure that '
+                'RDMA is supported on your iSCSI port.'))
+        raise
+
     try:
         rtslib.NetworkPortal(tpg_new, '::0', 3260, mode='any')
     except rtslib.utils.RTSLibError:
@@ -154,7 +163,7 @@ def verify_rtslib():
 def usage():
     print("Usage:")
     print(sys.argv[0] +
-          " create [device] [name] [userid] [password]" +
+          " create [device] [name] [userid] [password] [iser_enabled]" +
           " <initiator_iqn,iqn2,iqn3,...>")
     print(sys.argv[0] +
           " add-initiator [target_iqn] [userid] [password] [initiator_iqn]")
@@ -174,22 +183,24 @@ def main(argv=None):
         usage()
 
     if argv[1] == 'create':
-        if len(argv) < 6:
+        if len(argv) < 7:
             usage()
 
-        if len(argv) > 7:
+        if len(argv) > 8:
             usage()
 
         backing_device = argv[2]
         name = argv[3]
         userid = argv[4]
         password = argv[5]
+        iser_enabled = argv[6]
         initiator_iqns = None
 
-        if len(argv) > 6:
+        if len(argv) > 7:
-            initiator_iqns = argv[6]
+            initiator_iqns = argv[7]
 
-        create(backing_device, name, userid, password, initiator_iqns)
+        create(backing_device, name, userid, password, iser_enabled,
+               initiator_iqns)
 
     elif argv[1] == 'add-initiator':
         if len(argv) < 6:

cinder/tests/targets/test_iser_driver.py

@@ -10,12 +10,19 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import mock
+
+from cinder.tests.targets import test_lio_driver as test_lio
 from cinder.tests.targets import test_tgt_driver as test_tgt
 from cinder import utils
 from cinder.volume.targets import iser
+from cinder.volume.targets import lio
+from cinder.volume.targets import tgt
 
 
 class TestIserAdmDriver(test_tgt.TestTgtAdmDriver):
+    """Unit tests for the deprecated ISERTgtAdm flow
+    """
 
     def setUp(self):
         super(TestIserAdmDriver, self).setUp()
@@ -23,3 +30,72 @@ class TestIserAdmDriver(test_tgt.TestTgtAdmDriver):
         self.configuration.iser_target_prefix = 'iqn.2010-10.org.openstack:'
         self.target = iser.ISERTgtAdm(root_helper=utils.get_root_helper(),
                                       configuration=self.configuration)
+
+    @mock.patch.object(iser.ISERTgtAdm, '_get_iscsi_properties')
+    def test_initialize_connection(self, mock_get_iscsi):
+
+        connector = {'initiator': 'fake_init'}
+
+        # Test the normal case
+        mock_get_iscsi.return_value = {}
+        expected_return = {'driver_volume_type': 'iser',
+                           'data': {}}
+        self.assertEqual(expected_return,
+                         self.target.initialize_connection(self.testvol_1,
+                                                           connector))
+
+    def test_iscsi_protocol(self):
+        self.assertEqual(self.target.iscsi_protocol, 'iser')
+
+
+class TestIserTgtDriver(test_tgt.TestTgtAdmDriver):
+    """Unit tests for the iSER TGT flow
+    """
+
+    def setUp(self):
+        super(TestIserTgtDriver, self).setUp()
+        self.configuration.iscsi_protocol = 'iser'
+        self.target = tgt.TgtAdm(root_helper=utils.get_root_helper(),
+                                 configuration=self.configuration)
+
+    def test_iscsi_protocol(self):
+        self.assertEqual(self.target.iscsi_protocol, 'iser')
+
+    @mock.patch.object(tgt.TgtAdm, '_get_iscsi_properties')
+    def test_initialize_connection(self, mock_get_iscsi):
+
+        connector = {'initiator': 'fake_init'}
+
+        mock_get_iscsi.return_value = {}
+        expected_return = {'driver_volume_type': 'iser',
+                           'data': {}}
+        self.assertEqual(expected_return,
+                         self.target.initialize_connection(self.testvol_1,
+                                                           connector))
+
+
+class TestIserLioAdmDriver(test_lio.TestLioAdmDriver):
+    """Unit tests for the iSER LIO flow
+    """
+    def setUp(self):
+        super(TestIserLioAdmDriver, self).setUp()
+        self.configuration.iscsi_protocol = 'iser'
+        with mock.patch.object(lio.LioAdm, '_verify_rtstool'):
+            self.target = lio.LioAdm(root_helper=utils.get_root_helper(),
+                                     configuration=self.configuration)
+        self.target.db = mock.MagicMock(
+            volume_get=lambda x, y: {'provider_auth': 'IncomingUser foo bar'})
+
+    def test_iscsi_protocol(self):
+        self.assertEqual(self.target.iscsi_protocol, 'iser')
+
+    @mock.patch.object(utils, 'execute')
+    @mock.patch.object(lio.LioAdm, '_get_iscsi_properties')
+    def test_initialize_connection(self, mock_get_iscsi, mock_execute):
+
+        connector = {'initiator': 'fake_init'}
+
+        mock_get_iscsi.return_value = {}
+        ret = self.target.initialize_connection(self.testvol_1, connector)
+        driver_volume_type = ret['driver_volume_type']
+        self.assertEqual(driver_volume_type, 'iser')

cinder/tests/targets/test_lio_driver.py

@@ -115,3 +115,6 @@ class TestLioAdmDriver(test_tgt.TestTgtAdmDriver):
                           self.target.terminate_connection,
                           self.testvol_1,
                           connector)
+
+    def test_iscsi_protocol(self):
+        self.assertEqual(self.target.iscsi_protocol, 'iscsi')

cinder/tests/targets/test_tgt_driver.py

@@ -64,7 +64,7 @@ class TestTgtAdmDriver(test.TestCase):
              'target_iqn': 'iqn.2010-10.org.openstack:volume-%s' %
                            self.fake_id_2,
              'target_lun': 0,
-             'target_portal': '10.10.7.1:3260',
+             'target_portal': '10.9.8.7:3260',
              'volume_id': self.fake_id_2}
 
         self.fake_iscsi_scan =\
@@ -110,6 +110,11 @@ class TestTgtAdmDriver(test.TestCase):
     def fake_safe_get(self, value):
         if value == 'volumes_dir':
             return self.fake_volumes_dir
+        elif value == 'iscsi_protocol':
+            return self.configuration.iscsi_protocol
+
+    def test_iscsi_protocol(self):
+        self.assertEqual(self.target.iscsi_protocol, 'iscsi')
 
     def test_get_target(self):
 

cinder/tests/test_cmd.py

@@ -700,7 +700,8 @@ class TestCinderRtstoolCmd(test.TestCase):
                           mock.sentinel.backing_device,
                           mock.sentinel.name,
                           mock.sentinel.userid,
-                          mock.sentinel.password)
+                          mock.sentinel.password,
+                          mock.sentinel.iser_enabled)
 
     def _test_create_rtsllib_error_network_portal(self, ip):
         with contextlib.nested(
@@ -728,12 +729,14 @@ class TestCinderRtstoolCmd(test.TestCase):
                                   mock.sentinel.backing_device,
                                   mock.sentinel.name,
                                   mock.sentinel.userid,
-                                  mock.sentinel.password)
+                                  mock.sentinel.password,
+                                  mock.sentinel.iser_enabled)
             else:
                 cinder_rtstool.create(mock.sentinel.backing_device,
                                       mock.sentinel.name,
                                       mock.sentinel.userid,
-                                      mock.sentinel.password)
+                                      mock.sentinel.password,
+                                      mock.sentinel.iser_enabled)
 
             rts_root.assert_called_once_with()
             block_storage_object.assert_called_once_with(
@@ -788,7 +791,8 @@ class TestCinderRtstoolCmd(test.TestCase):
             cinder_rtstool.create(mock.sentinel.backing_device,
                                   mock.sentinel.name,
                                   mock.sentinel.userid,
-                                  mock.sentinel.password)
+                                  mock.sentinel.password,
+                                  mock.sentinel.iser_enabled)
 
             rts_root.assert_called_once_with()
             block_storage_object.assert_called_once_with(
@@ -961,7 +965,8 @@ class TestCinderRtstoolCmd(test.TestCase):
                         mock.sentinel.name,
                         mock.sentinel.userid,
                         mock.sentinel.password,
-                        mock.sentinel.initiator_iqns]
+                        mock.sentinel.initiator_iqns,
+                        mock.sentinel.iser_enabled]
 
             rc = cinder_rtstool.main()
 
@@ -969,7 +974,8 @@ class TestCinderRtstoolCmd(test.TestCase):
                                            mock.sentinel.name,
                                            mock.sentinel.userid,
                                            mock.sentinel.password,
-                                           mock.sentinel.initiator_iqns)
+                                           mock.sentinel.initiator_iqns,
+                                           mock.sentinel.iser_enabled)
             self.assertEqual(0, rc)
 
     def test_main_add_initiator(self):

cinder/volume/driver.py

@@ -117,6 +117,13 @@ volume_opts = [
                     'perform write-back(on) or write-through(off). '
                     'This parameter is valid if iscsi_helper is set '
                     'to tgtadm or iseradm.'),
+    cfg.StrOpt('iscsi_protocol',
+               default='iscsi',
+               help='Determines the iSCSI protocol for new iSCSI volumes, '
+                    'created with tgtadm or lioadm target helpers. In '
+                    'order to enable RDMA, this parameter should be set '
+                    'with the value "iser". The supported iSCSI protocol '
+                    'values are "iscsi" and "iser".'),
     cfg.StrOpt('driver_client_cert_key',
                default=None,
                help='The path to the client certificate key for verification, '

cinder/volume/drivers/lvm.py

@@ -611,10 +611,11 @@ class LVMISERDriver(LVMVolumeDriver):
     def __init__(self, *args, **kwargs):
         super(LVMISERDriver, self).__init__(*args, **kwargs)
 
-        LOG.warning(_LW('LVMISCSIDriver is deprecated, you should '
+        LOG.warning(_LW('LVMISERDriver is deprecated, you should '
                         'now just use LVMVolumeDriver and specify '
                         'target_helper for the target driver you '
-                        'wish to use.'))
+                        'wish to use. In order to enable iser, please '
+                        'set iscsi_protocol with the value iser.'))
 
         LOG.debug('Attempting to initialize LVM driver with the '
                   'following target_driver: '

cinder/volume/targets/iscsi.py

@@ -34,6 +34,8 @@ class ISCSITarget(driver.Target):
         super(ISCSITarget, self).__init__(*args, **kwargs)
         self.iscsi_target_prefix = \
             self.configuration.safe_get('iscsi_target_prefix')
+        self.iscsi_protocol = \
+            self.configuration.safe_get('iscsi_protocol')
         self.protocol = 'iSCSI'
 
     def _get_iscsi_properties(self, volume):
@@ -177,7 +179,7 @@ class ISCSITarget(driver.Target):
 
         iscsi_properties = self._get_iscsi_properties(volume)
         return {
-            'driver_volume_type': 'iscsi',
+            'driver_volume_type': self.iscsi_protocol,
             'data': iscsi_properties
         }
 

cinder/volume/targets/iser.py

@@ -11,6 +11,7 @@
 #    under the License.
 
 
+from cinder.i18n import _LW
 from cinder.openstack.common import log as logging
 from cinder.volume.targets.tgt import TgtAdm
 
@@ -21,25 +22,18 @@ LOG = logging.getLogger(__name__)
 class ISERTgtAdm(TgtAdm):
     VERSION = '0.2'
 
-    VOLUME_CONF = """
-                <target %s>
-                    driver iser
-                    backing-store %s
-                    write_cache %s
-                </target>
-                  """
-    VOLUME_CONF_WITH_CHAP_AUTH = """
-                                <target %s>
-                                    driver iser
-                                    backing-store %s
-                                    %s
-                                    write_cache %s
-                                </target>
-                                 """
-
     def __init__(self, *args, **kwargs):
         super(ISERTgtAdm, self).__init__(*args, **kwargs)
+
+        LOG.warning(_LW('ISERTgtAdm is deprecated, you should '
+                        'now just use LVMVolumeDriver and specify '
+                        'target_helper for the target driver you '
+                        'wish to use. In order to enable iser, please '
+                        'set iscsi_protocol=iser with lioadm or tgtadm '
+                        'target helpers.'))
+
         self.volumes_dir = self.configuration.safe_get('volumes_dir')
+        self.iscsi_protocol = 'iser'
         self.protocol = 'iSER'
 
         # backwards compatibility mess
@@ -63,7 +57,7 @@ class ISERTgtAdm(TgtAdm):
                 'data': {
                     'target_discovered': True,
                     'target_iqn':
-                    'iqn.2010-10.org.iser.openstack:volume-00000001',
+                    'iqn.2010-10.org.openstack:volume-00000001',
                     'target_portal': '127.0.0.0.1:3260',
                     'volume_id': 1,
                 }

cinder/volume/targets/lio.py

@@ -109,7 +109,8 @@ class LioAdm(TgtAdm):
                             path,
                             name,
                             chap_auth_userid,
-                            chap_auth_password]
+                            chap_auth_password,
+                            self.iscsi_protocol == 'iser']
             utils.execute(*command_args, run_as_root=True)
         except putils.ProcessExecutionError as e:
             LOG.error(_LE("Failed to create iscsi target for volume "
@@ -179,7 +180,7 @@ class LioAdm(TgtAdm):
         iscsi_properties['target_lun'] = 0
 
         return {
-            'driver_volume_type': 'iscsi',
+            'driver_volume_type': self.iscsi_protocol,
             'data': iscsi_properties
         }
 

cinder/volume/targets/tgt.py

@@ -41,14 +41,14 @@ class TgtAdm(iscsi.ISCSITarget):
     VOLUME_CONF = """
                 <target %s>
                     backing-store %s
-                    driver iscsi
+                    driver %s
                     write-cache %s
                 </target>
                   """
     VOLUME_CONF_WITH_CHAP_AUTH = """
                                 <target %s>
                                     backing-store %s
-                                    driver iscsi
+                                    driver %s
                                     %s
                                     write-cache %s
                                 </target>
@@ -188,12 +188,14 @@ class TgtAdm(iscsi.ISCSITarget):
 
         vol_id = name.split(':')[1]
         write_cache = kwargs.get('iscsi_write_cache', 'on')
+        driver = self.iscsi_protocol
+
         if chap_auth is None:
-            volume_conf = self.VOLUME_CONF % (name, path, write_cache)
+            volume_conf = self.VOLUME_CONF % (name, path, driver, write_cache)
         else:
             chap_str = re.sub('^IncomingUser ', 'incominguser ', chap_auth)
-            volume_conf = self.VOLUME_CONF_WITH_CHAP_AUTH % (name,
+            volume_conf = self.VOLUME_CONF_WITH_CHAP_AUTH % (name, path,
-                                                             path, chap_str,
+                                                             driver, chap_str,
                                                              write_cache)
         LOG.debug('Creating iscsi_target for: %s', vol_id)
         volumes_dir = self.volumes_dir
@@ -349,7 +351,7 @@ class TgtAdm(iscsi.ISCSITarget):
     def initialize_connection(self, volume, connector):
         iscsi_properties = self._get_iscsi_properties(volume)
         return {
-            'driver_volume_type': 'iscsi',
+            'driver_volume_type': self.iscsi_protocol,
             'data': iscsi_properties
         }