From 6b37214585808aa06af0b1f303966bb22f47dc43 Mon Sep 17 00:00:00 2001
From: Peter Penchev <openstack-dev@storpool.com>
Date: Mon, 12 Mar 2018 12:00:10 +0200
Subject: [PATCH] Add iSCSI export support to the StorPool driver

Add four new driver options:
- iscsi_cinder_volume: use StorPool iSCSI attachments whenever
  the cinder-volume service needs to attach a volume to the controller,
  e.g. for copying an image to a volume or vice versa
- iscsi_export_to:
  - an empty string to use the StorPool native protocol for exporting volumes
    protocol for exporting volumes)
  - the string "*" to always use iSCSI for exporting volumes
  - an experimental, not fully supported list of IQN patterns to export
    volumes to using iSCSI; this results in a Cinder driver that exports
    different volumes using different storage protocols
- iscsi_portal_group: the name of the iSCSI portal group defined in
  the StorPool configuration to use for these export
- iscsi_learn_initiator_iqns: automatically create StorPool configuration
  records for an initiator when a volume is first exported to it

When exporting volumes via iSCSI, report the storage protocol as "iSCSI" and
disable multiattach (the StorPool CI failures with iSCSI multiattach may need
further investigation).

Change-Id: I9de64306e0e6976268df782053b0651dd1cca96f
---
 .../unit/volume/drivers/test_storpool.py      |  64 +++-
 cinder/volume/drivers/storpool.py             | 360 +++++++++++++++++-
 .../drivers/storpool-volume-driver.rst        |  49 ++-
 3 files changed, 464 insertions(+), 9 deletions(-)

diff --git a/cinder/tests/unit/volume/drivers/test_storpool.py b/cinder/tests/unit/volume/drivers/test_storpool.py
index 65d4ed304..350f8c9bd 100644
--- a/cinder/tests/unit/volume/drivers/test_storpool.py
+++ b/cinder/tests/unit/volume/drivers/test_storpool.py
@@ -32,6 +32,7 @@ fakeStorPool.sptypes = mock.Mock()
 sys.modules['storpool'] = fakeStorPool
 
 
+from cinder.common import constants
 from cinder import exception
 from cinder.tests.unit import test
 from cinder.volume import configuration as conf
@@ -219,7 +220,14 @@ class StorPoolTestCase(test.TestCase):
         self.cfg.volume_backend_name = 'storpool_test'
         self.cfg.storpool_template = None
         self.cfg.storpool_replication = 3
+        self.cfg.iscsi_cinder_volume = False
+        self.cfg.iscsi_export_to = ''
+        self.cfg.iscsi_portal_group = 'test-group'
 
+        self._setup_test_driver()
+
+    def _setup_test_driver(self):
+        """Initialize a StorPool driver as per the current configuration."""
         mock_exec = mock.Mock()
         mock_exec.return_value = ('', '')
 
@@ -228,7 +236,7 @@ class StorPoolTestCase(test.TestCase):
         self.driver.check_for_setup_error()
 
     @ddt.data(
-        (5, TypeError),
+        (5, (TypeError, AttributeError)),
         ({'no-host': None}, KeyError),
         ({'host': 'sbad'}, driver.StorPoolConfigurationInvalid),
         ({'host': 's01'}, None),
@@ -244,7 +252,7 @@ class StorPoolTestCase(test.TestCase):
                               conn)
 
     @ddt.data(
-        (5, TypeError),
+        (5, (TypeError, AttributeError)),
         ({'no-host': None}, KeyError),
         ({'host': 'sbad'}, driver.StorPoolConfigurationInvalid),
     )
@@ -635,3 +643,55 @@ class StorPoolTestCase(test.TestCase):
                          self.driver.get_pool({
                              'volume_type': volume_type
                          }))
+
+    @ddt.data(
+        # The default values
+        ('', False, constants.STORPOOL, 'beleriand', False),
+
+        # Export to all
+        ('*', True, constants.ISCSI, 'beleriand', True),
+        ('*', True, constants.ISCSI, 'beleriand', True),
+
+        # Only export to the controller
+        ('', False, constants.STORPOOL, 'beleriand', False),
+
+        # Some of the not-fully-supported pattern lists
+        ('roh*', False, constants.STORPOOL, 'beleriand', False),
+        ('roh*', False, constants.STORPOOL, 'rohan', True),
+        ('*riand roh*', False, constants.STORPOOL, 'beleriand', True),
+        ('*riand roh*', False, constants.STORPOOL, 'rohan', True),
+    )
+    @ddt.unpack
+    def test_wants_iscsi(self, iscsi_export_to, use_iscsi, storage_protocol,
+                         hostname, expected):
+        """Check the "should this export use iSCSI?" detection."""
+        self.cfg.iscsi_export_to = iscsi_export_to
+        self._setup_test_driver()
+        self.assertEqual(self.driver._use_iscsi, use_iscsi)
+
+        # Make sure the driver reports the correct protocol in the stats
+        self.driver._update_volume_stats()
+        self.assertEqual(self.driver._stats["vendor_name"], "StorPool")
+        self.assertEqual(self.driver._stats["storage_protocol"],
+                         storage_protocol)
+
+        def check(conn, forced, expected):
+            """Pass partially or completely valid connector info."""
+            for initiator in (None, hostname):
+                for host in (None, 'gondor'):
+                    self.assertEqual(
+                        self.driver._connector_wants_iscsi({
+                            "host": host,
+                            "initiator": initiator,
+                            **conn,
+                        }),
+                        expected if initiator is not None and host is not None
+                        else forced)
+
+        # If iscsi_cinder_volume is set and this is the controller, then yes.
+        check({"storpool_wants_iscsi": True}, True, True)
+
+        # If iscsi_cinder_volume is not set or this is not the controller, then
+        # look at the specified expected value.
+        check({"storpool_wants_iscsi": False}, use_iscsi, expected)
+        check({}, use_iscsi, expected)
diff --git a/cinder/volume/drivers/storpool.py b/cinder/volume/drivers/storpool.py
index 418e5750f..abb23865d 100644
--- a/cinder/volume/drivers/storpool.py
+++ b/cinder/volume/drivers/storpool.py
@@ -15,6 +15,7 @@
 
 """StorPool block device driver"""
 
+import fnmatch
 import platform
 
 from oslo_config import cfg
@@ -44,6 +45,31 @@ if storpool:
 
 
 storpool_opts = [
+    cfg.BoolOpt('iscsi_cinder_volume',
+                default=False,
+                help='Let the cinder-volume service use iSCSI instead of '
+                     'the StorPool block device driver for accessing '
+                     'StorPool volumes, e.g. when creating a volume from '
+                     'an image or vice versa.'),
+    cfg.StrOpt('iscsi_export_to',
+               default='',
+               help='Whether to export volumes using iSCSI. '
+                    'An empty string (the default) makes the driver export '
+                    'all volumes using the StorPool native network protocol. '
+                    'The value "*" makes the driver export all volumes using '
+                    'iSCSI. '
+                    'Any other value leads to an experimental not fully '
+                    'supported configuration and is interpreted as '
+                    'a whitespace-separated list of patterns for IQNs for '
+                    'hosts that need volumes to be exported via iSCSI, e.g. '
+                    '"iqn.1991-05.com.microsoft:\\*" for Windows hosts.'),
+    cfg.BoolOpt('iscsi_learn_initiator_iqns',
+                default=True,
+                help='Create a StorPool record for a new initiator as soon as '
+                     'Cinder asks for a volume to be exported to it.'),
+    cfg.StrOpt('iscsi_portal_group',
+               default=None,
+               help='The portal group to export volumes via iSCSI in.'),
     cfg.StrOpt('storpool_template',
                default=None,
                help='The StorPool template for volumes with no type.'),
@@ -105,6 +131,7 @@ class StorPoolDriver(driver.VolumeDriver):
         self._ourId = None
         self._ourIdInt = None
         self._attach = None
+        self._use_iscsi = None
 
     @staticmethod
     def get_driver_options():
@@ -162,10 +189,312 @@ class StorPoolDriver(driver.VolumeDriver):
             raise StorPoolConfigurationInvalid(
                 section=hostname, param='SP_OURID', error=e)
 
+    def _connector_wants_iscsi(self, connector):
+        """Should we do this export via iSCSI?
+
+        Check the configuration to determine whether this connector is
+        expected to provide iSCSI exports as opposed to native StorPool
+        protocol ones.  Match the initiator's IQN against the list of
+        patterns supplied in the "iscsi_export_to" configuration setting.
+        """
+        if connector is None:
+            return False
+        if self._use_iscsi:
+            LOG.debug('  - forcing iSCSI for all exported volumes')
+            return True
+        if connector.get('storpool_wants_iscsi'):
+            LOG.debug('  - forcing iSCSI for the controller')
+            return True
+
+        try:
+            iqn = connector.get('initiator')
+        except Exception:
+            iqn = None
+        try:
+            host = connector.get('host')
+        except Exception:
+            host = None
+        if iqn is None or host is None:
+            LOG.debug('  - this connector certainly does not want iSCSI')
+            return False
+
+        LOG.debug('  - check whether %(host)s (%(iqn)s) wants iSCSI',
+                  {
+                      'host': host,
+                      'iqn': iqn,
+                  })
+
+        export_to = self.configuration.iscsi_export_to
+        if export_to is None:
+            return False
+
+        for pat in export_to.split():
+            LOG.debug('    - matching against %(pat)s', {'pat': pat})
+            if fnmatch.fnmatch(iqn, pat):
+                LOG.debug('      - got it!')
+                return True
+        LOG.debug('    - nope')
+        return False
+
     def validate_connector(self, connector):
+        if self._connector_wants_iscsi(connector):
+            return True
         return self._storpool_client_id(connector) >= 0
 
+    def _get_iscsi_config(self, iqn, volume_id):
+        """Get the StorPool iSCSI config items pertaining to this volume.
+
+        Find the elements of the StorPool iSCSI configuration tree that
+        will be needed to create, ensure, or remove the iSCSI export of
+        the specified volume to the specified initiator.
+        """
+        cfg = self._attach.api().iSCSIConfig()
+
+        pg_name = self.configuration.iscsi_portal_group
+        pg_found = [
+            pg for pg in cfg.iscsi.portalGroups.values() if pg.name == pg_name
+        ]
+        if not pg_found:
+            raise Exception('StorPool Cinder iSCSI configuration error: '
+                            'no portal group "{pg}"'.format(pg=pg_name))
+        pg = pg_found[0]
+
+        # Do we know about this initiator?
+        i_found = [
+            init for init in cfg.iscsi.initiators.values() if init.name == iqn
+        ]
+        if i_found:
+            initiator = i_found[0]
+        else:
+            initiator = None
+
+        # Is this volume already being exported?
+        volname = self._attach.volumeName(volume_id)
+        t_found = [
+            tgt for tgt in cfg.iscsi.targets.values() if tgt.volume == volname
+        ]
+        if t_found:
+            target = t_found[0]
+        else:
+            target = None
+
+        # OK, so is this volume being exported to this initiator?
+        export = None
+        if initiator is not None and target is not None:
+            e_found = [
+                exp for exp in initiator.exports
+                if exp.portalGroup == pg.name and exp.target == target.name
+            ]
+            if e_found:
+                export = e_found[0]
+
+        return {
+            'cfg': cfg,
+            'pg': pg,
+            'initiator': initiator,
+            'target': target,
+            'export': export,
+            'volume_name': volname,
+            'volume_id': volume_id,
+        }
+
+    def _create_iscsi_export(self, volume, connector):
+        """Create (if needed) an iSCSI export for the StorPool volume."""
+        LOG.debug(
+            '_create_iscsi_export() invoked for volume '
+            '"%(vol_name)s" (%(vol_id)s) connector %(connector)s',
+            {
+                'vol_name': volume['display_name'],
+                'vol_id': volume['id'],
+                'connector': connector,
+            }
+        )
+        iqn = connector['initiator']
+        try:
+            cfg = self._get_iscsi_config(iqn, volume['id'])
+        except Exception as exc:
+            LOG.error(
+                'Could not fetch the iSCSI config: %(exc)s', {'exc': exc}
+            )
+            raise
+
+        if cfg['initiator'] is None:
+            if not (self.configuration.iscsi_learn_initiator_iqns or
+                    self.configuration.iscsi_cinder_volume and
+                    connector.get('storpool_wants_iscsi')):
+                raise Exception('The "{iqn}" initiator IQN for the "{host}" '
+                                'host is not defined in the StorPool '
+                                'configuration.'
+                                .format(iqn=iqn, host=connector['host']))
+            else:
+                LOG.info('Creating a StorPool iSCSI initiator '
+                         'for "{host}s" ({iqn}s)',
+                         {'host': connector['host'], 'iqn': iqn})
+                self._attach.api().iSCSIConfigChange({
+                    'commands': [
+                        {
+                            'createInitiator': {
+                                'name': iqn,
+                                'username': '',
+                                'secret': '',
+                            },
+                        },
+                        {
+                            'initiatorAddNetwork': {
+                                'initiator': iqn,
+                                'net': '0.0.0.0/0',
+                            },
+                        },
+                    ]
+                })
+
+        if cfg['target'] is None:
+            LOG.info(
+                'Creating a StorPool iSCSI target '
+                'for the "%(vol_name)s" volume (%(vol_id)s)',
+                {
+                    'vol_name': volume['display_name'],
+                    'vol_id': volume['id'],
+                }
+            )
+            self._attach.api().iSCSIConfigChange({
+                'commands': [
+                    {
+                        'createTarget': {
+                            'volumeName': cfg['volume_name'],
+                        },
+                    },
+                ]
+            })
+            cfg = self._get_iscsi_config(iqn, volume['id'])
+
+        if cfg['export'] is None:
+            LOG.info('Creating a StorPool iSCSI export '
+                     'for the "{vol_name}s" volume ({vol_id}s) '
+                     'to the "{host}s" initiator ({iqn}s) '
+                     'in the "{pg}s" portal group',
+                     {
+                         'vol_name': volume['display_name'],
+                         'vol_id': volume['id'],
+                         'host': connector['host'],
+                         'iqn': iqn,
+                         'pg': cfg['pg'].name
+                     })
+            self._attach.api().iSCSIConfigChange({
+                'commands': [
+                    {
+                        'export': {
+                            'initiator': iqn,
+                            'portalGroup': cfg['pg'].name,
+                            'volumeName': cfg['volume_name'],
+                        },
+                    },
+                ]
+            })
+
+        res = {
+            'driver_volume_type': 'iscsi',
+            'data': {
+                'target_discovered': False,
+                'target_iqn': cfg['target'].name,
+                'target_portal': '{}:3260'.format(
+                    cfg['pg'].networks[0].address
+                ),
+                'target_lun': 0,
+                'volume_id': volume['id'],
+                'discard': True,
+            },
+        }
+        LOG.debug('returning %(res)s', {'res': res})
+        return res
+
+    def _remove_iscsi_export(self, volume, connector):
+        """Remove an iSCSI export for the specified StorPool volume."""
+        LOG.debug(
+            '_remove_iscsi_export() invoked for volume '
+            '"%(vol_name)s" (%(vol_id)s) connector %(conn)s',
+            {
+                'vol_name': volume['display_name'],
+                'vol_id': volume['id'],
+                'conn': connector,
+            }
+        )
+        try:
+            cfg = self._get_iscsi_config(connector['initiator'], volume['id'])
+        except Exception as exc:
+            LOG.error(
+                'Could not fetch the iSCSI config: %(exc)s', {'exc': exc}
+            )
+            raise
+
+        if cfg['export'] is not None:
+            LOG.info('Removing the StorPool iSCSI export '
+                     'for the "%(vol_name)s" volume (%(vol_id)s) '
+                     'to the "%(host)s" initiator (%(iqn)s) '
+                     'in the "%(pg)s" portal group',
+                     {
+                         'vol_name': volume['display_name'],
+                         'vol_id': volume['id'],
+                         'host': connector['host'],
+                         'iqn': connector['initiator'],
+                         'pg': cfg['pg'].name,
+                     })
+            try:
+                self._attach.api().iSCSIConfigChange({
+                    'commands': [
+                        {
+                            'exportDelete': {
+                                'initiator': cfg['initiator'].name,
+                                'portalGroup': cfg['pg'].name,
+                                'volumeName': cfg['volume_name'],
+                            },
+                        },
+                    ]
+                })
+            except spapi.ApiError as e:
+                if e.name not in ('objectExists', 'objectDoesNotExist'):
+                    raise
+                LOG.info('Looks like somebody beat us to it')
+
+        if cfg['target'] is not None:
+            last = True
+            for initiator in cfg['cfg'].iscsi.initiators.values():
+                if initiator.name == cfg['initiator'].name:
+                    continue
+                for exp in initiator.exports:
+                    if exp.target == cfg['target'].name:
+                        last = False
+                        break
+                if not last:
+                    break
+
+            if last:
+                LOG.info(
+                    'Removing the StorPool iSCSI target '
+                    'for the "{vol_name}s" volume ({vol_id}s)',
+                    {
+                        'vol_name': volume['display_name'],
+                        'vol_id': volume['id'],
+                    }
+                )
+                try:
+                    self._attach.api().iSCSIConfigChange({
+                        'commands': [
+                            {
+                                'deleteTarget': {
+                                    'volumeName': cfg['volume_name'],
+                                },
+                            },
+                        ]
+                    })
+                except spapi.ApiError as e:
+                    if e.name not in ('objectDoesNotExist', 'invalidParam'):
+                        raise
+                    LOG.info('Looks like somebody beat us to it')
+
     def initialize_connection(self, volume, connector):
+        if self._connector_wants_iscsi(connector):
+            return self._create_iscsi_export(volume, connector)
         return {'driver_volume_type': 'storpool',
                 'data': {
                     'client_id': self._storpool_client_id(connector),
@@ -174,6 +503,9 @@ class StorPoolDriver(driver.VolumeDriver):
                 }}
 
     def terminate_connection(self, volume, connector, **kwargs):
+        if self._connector_wants_iscsi(connector):
+            LOG.debug('- removing an iSCSI export')
+            self._remove_iscsi_export(volume, connector)
         pass
 
     def create_snapshot(self, snapshot):
@@ -275,11 +607,20 @@ class StorPoolDriver(driver.VolumeDriver):
                     )
 
     def create_export(self, context, volume, connector):
-        pass
+        if self._connector_wants_iscsi(connector):
+            LOG.debug('- creating an iSCSI export')
+            self._create_iscsi_export(volume, connector)
 
     def remove_export(self, context, volume):
         pass
 
+    def _attach_volume(self, context, volume, properties, remote=False):
+        if self.configuration.iscsi_cinder_volume and not remote:
+            LOG.debug('- adding the "storpool_wants_iscsi" flag')
+            properties['storpool_wants_iscsi'] = True
+
+        return super()._attach_volume(context, volume, properties, remote)
+
     def delete_volume(self, volume):
         name = self._attach.volumeName(volume['id'])
         try:
@@ -316,6 +657,17 @@ class StorPoolDriver(driver.VolumeDriver):
             LOG.error("StorPoolDriver API initialization failed: %s", e)
             raise
 
+        export_to = self.configuration.iscsi_export_to
+        export_to_set = export_to is not None and export_to.split()
+        vol_iscsi = self.configuration.iscsi_cinder_volume
+        pg_name = self.configuration.iscsi_portal_group
+        if (export_to_set or vol_iscsi) and pg_name is None:
+            msg = _('The "iscsi_portal_group" option is required if '
+                    'any patterns are listed in "iscsi_export_to"')
+            raise exception.VolumeDriverException(message=msg)
+
+        self._use_iscsi = export_to == "*"
+
     def _update_volume_stats(self):
         try:
             dl = self._attach.api().disksList()
@@ -341,7 +693,7 @@ class StorPoolDriver(driver.VolumeDriver):
             'total_capacity_gb': total / units.Gi,
             'free_capacity_gb': free / units.Gi,
             'reserved_percentage': 0,
-            'multiattach': True,
+            'multiattach': not self._use_iscsi,
             'QoS_support': False,
             'thick_provisioning_support': False,
             'thin_provisioning_support': True,
@@ -359,7 +711,9 @@ class StorPoolDriver(driver.VolumeDriver):
                 'volume_backend_name') or 'storpool',
             'vendor_name': 'StorPool',
             'driver_version': self.VERSION,
-            'storage_protocol': constants.STORPOOL,
+            'storage_protocol': (
+                constants.ISCSI if self._use_iscsi else constants.STORPOOL
+            ),
 
             'clone_across_pools': True,
             'sparse_copy_volume': True,
diff --git a/doc/source/configuration/block-storage/drivers/storpool-volume-driver.rst b/doc/source/configuration/block-storage/drivers/storpool-volume-driver.rst
index d2c5895a9..c891675bc 100644
--- a/doc/source/configuration/block-storage/drivers/storpool-volume-driver.rst
+++ b/doc/source/configuration/block-storage/drivers/storpool-volume-driver.rst
@@ -19,12 +19,15 @@ Prerequisites
 * The controller and all the compute nodes must have access to the StorPool
   API service.
 
-* All nodes where StorPool-backed volumes will be attached must have access to
+* If iSCSI is not being used as a transport (see below), all nodes where
+  StorPool-backed volumes will be attached must have access to
   the StorPool data network and run the ``storpool_block`` service.
 
-* If StorPool-backed Cinder volumes need to be created directly from Glance
-  images, then the node running the ``cinder-volume`` service must also have
-  access to the StorPool data network and run the ``storpool_block`` service.
+* If Glance uses Cinder as its image store, or if StorPool-backed Cinder
+  volumes need to be created directly from Glance images, and iSCSI is not
+  being used as a transport, then the node running the ``cinder-volume``
+  service must also have access to the StorPool data network and run
+  the ``storpool_block`` service.
 
 * All nodes that need to access the StorPool API (the compute nodes and
   the node running the ``cinder-volume`` service) must have the following
@@ -34,6 +37,29 @@ Prerequisites
   * the storpool Python bindings package
   * the storpool.spopenstack Python helper package
 
+Using iSCSI as the transport protocol
+-------------------------------------
+
+The StorPool distributed storage system uses its own, highly optimized and
+tailored for its specifics, network protocol for communication between
+the storage servers and the clients (the OpenStack cluster nodes where
+StorPool-backed volumes will be attached). There are cases when granting
+various nodes access to the StorPool data network or installing and
+running the ``storpool_block`` client service on them may pose difficulties.
+The StorPool servers may also expose the user-created volumes and snapshots
+using the standard iSCSI protocol that only requires TCP routing and
+connectivity between the storage servers and the StorPool clients.
+The StorPool Cinder driver may be configured to export volumes and
+snapshots via iSCSI using the ``iscsi_export_to`` and ``iscsi_portal_group``
+configuration options.
+
+Additionally, even if e.g. the hypervisor nodes running Nova will use
+the StorPool network protocol and run the ``storpool_block`` service
+(so the ``iscsi_export_to`` option has its default empty string value),
+the ``iscsi_cinder_volume`` option configures the StorPool Cinder driver
+so that only the ``cinder-volume`` service will use the iSCSI protocol when
+attaching volumes and snapshots to transfer data to and from Glance images.
+
 Configuring the StorPool volume driver
 --------------------------------------
 
@@ -55,6 +81,21 @@ volume backend definition) and per volume type:
   with the default placement constraints for the StorPool cluster.
   The default value for the chain replication is 3.
 
+- ``iscsi_export_to``: if set to the value ``*``, the StorPool Cinder driver
+  will export volumes and snapshots using the iSCSI protocol instead of
+  the StorPool network protocol. The ``iscsi_portal_group`` option must also
+  be specified.
+
+- ``iscsi_portal_group``: if the ``iscsi_export_to`` option is set to
+  the value ``*`` or the ``iscsi_cinder_volume`` option is turned on,
+  this option specifies the name of the iSCSI portal group that Cinder
+  volumes will be exported to.
+
+- ``iscsi_cinder_volume``: if enabled, even if the ``iscsi_export_to`` option
+  has its default empty value, the ``cinder-volume`` service will use iSCSI
+  to attach the volumes and snapshots for transferring data to and from
+  Glance images.
+
 Using the StorPool volume driver
 --------------------------------
 
-- 
2.35.1