Solaris OpenStack (Juno) iSCSI Multipathing proposed changes

README

What Changed:
- Don't use an iscsi:// suri in zone config anymore, for iscsi attached volumes we do the attach ourselves and give it the dev:/dev/dsk/<name> path for the storage device.
- Fixup some issues in how we determine what path to use in Cinder, it wasn't accounting for the LUN when looking for paths.
- Added in step to snag additional discovery addresses when doing the iSCSI setup to help with single portal limitation in Juno
- Take that fixed up code and use it in Nova to do the previously mentioned attach/detach
- Fixup some issues with the solaris changes done in the PureISCSIDriver (unsure if maybe I started from an older one though)

Known issues:
- Cinder can't seem to create a volume from an image...? Maybe thats just with the iscsi drivers?
- We never remove discovery addresses, maybe not so bad... but will at some point probably require manual cleanup.
- We only give a single portal to start discovery on, if that one is down (and we haven't accumulated the others yet..) the volume attachment will fail. This is fixed in liberty with drivers returning all portals. As a temporary bandaid we will add any discovered addresses as discovery addresses too so that the system will have more of them pile up and give a better chance to survive a path going down...

cinder.diff

diff --git a/brick/initiator/connector.py b/brick/initiator/connector.py
index d45a3c0..306c48d 100644
--- a/brick/initiator/connector.py
+++ b/brick/initiator/connector.py
@@ -193,7 +193,10 @@ class ISCSIConnector(InitiatorConnector):

     def set_execute(self, execute):
         super(ISCSIConnector, self).set_execute(execute)
-        self._linuxscsi.set_execute(execute)
+        if sys.platform == 'sunos5':
+            self._solarisiscsi.execute = execute
+        else:
+            self._linuxscsi.set_execute(execute)

     @synchronized('connect_volume')
     def connect_volume(self, connection_properties):
diff --git a/brick/initiator/solarisiscsi.py b/brick/initiator/solarisiscsi.py
index 8badc71..0a1da09 100644
--- a/brick/initiator/solarisiscsi.py
+++ b/brick/initiator/solarisiscsi.py
@@ -43,11 +43,17 @@ class SolarisiSCSI(object):
         (out, _err) = self.execute('/usr/sbin/iscsiadm', 'list',
                                    'target', '-S',
                                    connection_properties['target_iqn'])
-
-        for line in [l.strip() for l in out.splitlines()]:
-            if line.startswith("OS Device Name:"):
-                dev_path = line.split()[-1]
-                return dev_path
+        lines = out.splitlines()
+        current_lun = None
+        for raw_line in lines:
+            line = raw_line.strip()
+            if line.startswith("LUN:"):
+                current_lun = int(line.split()[-1])
+                continue
+            if (line.startswith("OS Device Name:") and
+                    current_lun == connection_properties['target_lun']):
+                path = line.split()[-1]
+                return path
         else:
             LOG.error(_("No device is found for the target %s.") %
                       connection_properties['target_iqn'])
@@ -62,11 +68,16 @@ class SolarisiSCSI(object):
         initiator_name_line = out.splitlines()[0]
         return initiator_name_line.rsplit(' ', 1)[1]

+    def _add_discovery_address(self, address):
+        self.execute('/usr/sbin/iscsiadm', 'add', 'discovery-address',
+                         address)
+
     def _connect_to_iscsi_portal(self, connection_properties):
         # TODO(Strony): handle the CHAP authentication
         target_ip = connection_properties['target_portal'].split(":")[0]
-        self.execute('/usr/sbin/iscsiadm', 'add', 'discovery-address',
-                     target_ip)
+
+        self._add_discovery_address(target_ip)
+
         self.execute('/usr/sbin/iscsiadm', 'modify', 'discovery',
                      '--sendtargets', 'enable')
         (out, _err) = self.execute('/usr/sbin/iscsiadm', 'list',
@@ -81,11 +92,21 @@ class SolarisiSCSI(object):
             raise

         target_iqn = connection_properties['target_iqn']
+        discovered_target_iqn = False
         for line in [l.strip() for l in lines]:
             if line.startswith("Target name:") and \
                     line.split()[-1] == target_iqn:
-                return
-        else:
+                discovered_target_iqn = True
+            if line.startswith("Target address:"):
+                # Grab any addresses we haven't seen yet and set them as
+                # discovery addresses.
+                # The line will be in this format:
+                #    Target address:   192.168.81.18:3260, 1
+                addr = line.split()[2].rstrip(',')
+                if addr != target_ip:
+                    self._add_discovery_address(addr)
+
+        if not discovered_target_iqn:
             LOG.error(_("No active session is found for the target %s.") %
                       target_iqn)
             raise
@@ -100,7 +121,6 @@ class SolarisiSCSI(object):
         """
         device_info = {'type': 'block'}

-        # TODO(Strony): support the iSCSI multipath on Solaris.
         self._connect_to_iscsi_portal(connection_properties)

         host_device = self._get_device_path(connection_properties)
diff --git a/volume/drivers/pure.py b/volume/drivers/pure.py
index 3729eb3..ef2b7d5 100644
--- a/volume/drivers/pure.py
+++ b/volume/drivers/pure.py
@@ -21,6 +21,7 @@ This driver requires Purity version 3.4.0 or later.
 import cookielib
 import json
 import urllib2
+import ssl
 import sys

 from oslo.config import cfg
@@ -177,13 +178,18 @@ class PureISCSIDriver(san.SanISCSIDriver):
     def _get_target_iscsi_port(self):
         """Return dictionary describing iSCSI-enabled port on target array."""
         try:
-          if sys.platform == 'sunos5':
-            (out, _err) = self.solaris_execute('/usr/sbin/iscsiadm',
-                                               'list', 'discovery-address',
-                                               '-v', port["portal"])
-          else:
-            self._run_iscsiadm_bare(["-m", "discovery", "-t", "sendtargets",
-                                     "-p", self._iscsi_port["portal"]])
+            if sys.platform == 'sunos5':
+                # (patrickeast) Because of the way that iscsiadm works
+                # on this platform we can't use discovery to check if
+                # the portal is 'alive'. Instead we just ping and pick
+                # the first one that is OK.
+                self.solaris_execute('/usr/sbin/ping',
+                                     self._iscsi_port["portal"].split(':')[0],
+                                     '5')
+            else:
+                self._run_iscsiadm_bare(["-m", "discovery", "-t",
+                                         "sendtargets", "-p",
+                                         self._iscsi_port["portal"]])
         except processutils.ProcessExecutionError as err:
             LOG.warn(_("iSCSI discovery of port {0[name]} at {0[portal]} "
                        "failed with error: {1}").format(self._iscsi_port,
@@ -198,13 +204,19 @@ class PureISCSIDriver(san.SanISCSIDriver):
         for port in iscsi_ports:
             try:
                 if sys.platform == 'sunos5':
-                  (out, _err) = self.solaris_execute('/usr/sbin/iscsiadm',
-                                                     'list', 'discovery-address',
-                                                     '-v', port["portal"])
+                    # (patrickeast) Because of the way that iscsiadm works
+                    # on this platform we can't use discovery to check if
+                    # the portal is 'alive'. Instead we just ping and pick
+                    # the first one that is OK.
+                    self.solaris_execute(
+                        '/usr/sbin/ping',
+                        port["portal"].split(':')[0],
+                        '5'
+                    )
                 else:
-                  self._run_iscsiadm_bare(["-m", "discovery",
-                                           "-t", "sendtargets",
-                                           "-p", port["portal"]])
+                    self._run_iscsiadm_bare(["-m", "discovery",
+                                            "-t", "sendtargets",
+                                            "-p", port["portal"]])
             except processutils.ProcessExecutionError as err:
                 LOG.debug(("iSCSI discovery of port {0[name]} at {0[portal]} "
                            "failed with error: {1}").format(port, err.stderr))
@@ -295,7 +307,11 @@ class FlashArray(object):

     def __init__(self, target, api_token):
         cookie_handler = urllib2.HTTPCookieProcessor(cookielib.CookieJar())
-        self._opener = urllib2.build_opener(cookie_handler)
+        ctx = ssl.create_default_context()
+        ctx.check_hostname = False
+        ctx.verify_mode = ssl.CERT_NONE
+        https_handler = urllib2.HTTPSHandler(context=ctx)
+        self._opener = urllib2.build_opener(https_handler, cookie_handler)
         self._target = target
         self._rest_version = self._choose_rest_version()
         self._root_url = "https://{0}/api/{1}/".format(target,

nova.diff

diff --git a/virt/solariszones/driver.py b/virt/solariszones/driver.py
index e57eb7a..5dbdffd 100644
--- a/virt/solariszones/driver.py
+++ b/virt/solariszones/driver.py
@@ -22,6 +22,7 @@ Driver for Solaris Zones (nee Containers):
 import glob
 import os
 import platform
+import re
 import shutil
 import tempfile
 import uuid
@@ -54,6 +55,7 @@ from nova import objects
 from nova.objects import flavor as flavor_obj
 from nova.openstack.common import fileutils
 from nova.openstack.common import jsonutils
+from nova.openstack.common import lockutils
 from nova.openstack.common import log as logging
 from nova.openstack.common import loopingcall
 from nova.openstack.common import processutils
@@ -62,6 +64,7 @@ from nova import utils
 from nova.virt import driver
 from nova.virt import event as virtevent
 from nova.virt import images
+from nova.virt.solariszones import solarisiscsi
 from nova.virt.solariszones import sysconfig
 from nova import volume

@@ -126,6 +129,10 @@ VNC_CONSOLE_BASE_FMRI = 'svc:/application/openstack/nova/zone-vnc-console'
 VNC_SERVER_PATH = '/usr/bin/vncserver'
 XTERM_PATH = '/usr/bin/xterm'

+synchronized = lockutils.synchronized_with_prefix('brick-')
+
+BLOCK_DEVICE_NAME_REGEX = re.compile(r'^/dev/r*dsk/(.*)[sp]\d+$')
+

 def lookup_resource_property(zone, resource, prop, filter=None):
     """Lookup specified property from specified Solaris Zone resource."""
@@ -304,6 +311,7 @@ class SolarisZonesDriver(driver.ComputeDriver):
         self._uname = os.uname()
         self._validated_archives = list()
         self._volume_api = volume.API()
+        self._iscsi = solarisiscsi.SolarisiSCSI()

     @property
     def rad_connection(self):
@@ -416,16 +424,6 @@ class SolarisZonesDriver(driver.ComputeDriver):
                 wwpns.append(wwpn)
         return wwpns

-    def _get_iscsi_initiator(self):
-        """ Return the iSCSI initiator node name IQN for this host """
-        out, err = utils.execute('/usr/sbin/iscsiadm', 'list',
-                                 'initiator-node')
-        # Sample first line of command output:
-        # Initiator node name: iqn.1986-03.com.sun:01:e00000000000.4f757217
-        initiator_name_line = out.splitlines()[0]
-        initiator_iqn = initiator_name_line.rsplit(' ', 1)[1]
-        return initiator_iqn
-
     def _get_zone_auto_install_state(self, zone_name):
         """Returns the SMF state of the auto-installer service,
            or None if auto-installer service is non-existent
@@ -764,6 +762,24 @@ class SolarisZonesDriver(driver.ComputeDriver):
             self._install_engine.doc.volatile.delete_children(
                 class_type=UnifiedArchive)

+    @synchronized('connect_volume')
+    def _attach_iscsi_volume(self, connection_info):
+        """Attach an iSCSI volume and return the device path."""
+        try:
+            device_info = self._iscsi.connect_volume(
+                connection_info['data'], 3)
+            return device_info['path']
+        except:
+            self._iscsi.disconnect_iscsi()
+            raise
+
+    @synchronized('connect_volume')
+    def _detach_iscsi_volume(self, connection_info):
+        """Detach an iSCSI volume and return the device path."""
+        path = self._iscsi._get_device_path(connection_info['data'])
+        self._iscsi.disconnect_iscsi()
+        return path
+
     def _suri_from_volume_info(self, connection_info):
         """Returns a suri(5) formatted string based on connection_info
         Currently supports local ZFS volume and iSCSI driver types.
@@ -774,17 +790,27 @@ class SolarisZonesDriver(driver.ComputeDriver):
         if driver_type == 'local':
             suri = 'dev:/dev/zvol/dsk/%s' % connection_info['volume_path']
         elif driver_type == 'iscsi':
-            data = connection_info['data']
-            # suri(5) format:
-            #       iscsi://<host>[:<port>]/target.<IQN>,lun.<LUN>
-            # Sample iSCSI connection data values:
-            # target_portal: 192.168.1.244:3260
-            # target_iqn: iqn.2010-10.org.openstack:volume-a89c.....
-            # target_lun: 1
-            suri = 'iscsi://%s/target.%s,lun.%d' % (data['target_portal'],
-                                                    data['target_iqn'],
-                                                    data['target_lun'])
-            # TODO(npower): need to handle CHAP authentication also
+            if ('device_path' in connection_info and
+                    connection_info['device_path']):
+
+                # Find the base name for the device, it might be /dev/rdsk
+                # in which case we need to switch to the /dev/dsk path or
+                # things fail.. We also want to use the base device and
+                # trim off any slice or partition numbers.
+                path = connection_info['device_path']
+                name = BLOCK_DEVICE_NAME_REGEX.sub(r'\1', path)
+                suri = 'dev:/dev/dsk/' + name
+            else:
+                data = connection_info['data']
+                # suri(5) format:
+                #       iscsi://<host>[:<port>]/target.<IQN>,lun.<LUN>
+                # Sample iSCSI connection data values:
+                # target_portal: 192.168.1.244:3260
+                # target_iqn: iqn.2010-10.org.openstack:volume-a89c.....
+                # target_lun: 1
+                suri = 'iscsi://%s/target.%s,lun.%d' % (data['target_portal'],
+                                                        data['target_iqn'],
+                                                        data['target_lun'])
         elif driver_type == 'fibre_channel':
             data = connection_info['data']
             target_wwn = data['target_wwn']
@@ -894,8 +920,8 @@ class SolarisZonesDriver(driver.ComputeDriver):
         # local to this compute node. If it is, then don't use it for
         # Solaris branded zones in order to avoid a known ZFS deadlock issue
         # when using a zpool within another zpool on the same system.
+        driver_type = connection_info['driver_volume_type']
         if brand == ZONE_BRAND_SOLARIS:
-            driver_type = connection_info['driver_volume_type']
             if driver_type == 'local':
                 msg = _("Detected 'local' zvol driver volume type "
                         "from volume service, which should not be "
@@ -922,6 +948,7 @@ class SolarisZonesDriver(driver.ComputeDriver):
                             "as a boot device for 'solaris' branded "
                             "zones.")
                     raise exception.InvalidVolume(reason=msg)
+
             # Assuming that fibre_channel is non-local
             elif driver_type != 'fibre_channel':
                 # Some other connection type that we don't understand
@@ -930,6 +957,12 @@ class SolarisZonesDriver(driver.ComputeDriver):
                         "as a boot device for zones." % driver_type)
                 raise exception.InvalidVolume(reason=msg)

+        if driver_type == 'iscsi':
+            # Connect the volume now on the local system and insert
+            # the path into the connection info for use later.
+            connection_info['device_path'] = \
+                self._attach_iscsi_volume(connection_info)
+
         # Volume looks OK to use. Notify Cinder of the attachment.
         self._volume_api.attach(context, volume_id, instance_uuid,
                                 mountpoint)
@@ -1567,6 +1600,10 @@ class SolarisZonesDriver(driver.ComputeDriver):
                 self._uninstall(instance)
             if self._get_state(zone) == power_state.NOSTATE:
                 self._delete_config(instance)
+            if block_device_info:
+                mapping = driver.block_device_info_get_mapping(block_device_info)
+                if 'connection_info' in mapping:
+                    self._detach_iscsi_volume(jsonutils.loads(mapping['connection_info']))
         except Exception as reason:
             LOG.warning(_("Unable to destroy instance '%s' via zonemgr(3RAD): "
                           "%s") % (name, reason))
@@ -1845,6 +1882,11 @@ class SolarisZonesDriver(driver.ComputeDriver):
             # Only Solaris Kernel zones are currently supported.
             raise NotImplementedError()

+        driver_type = connection_info['driver_volume_type']
+        if driver_type == 'iscsi':
+            connection_info['device_path'] = \
+                self._attach_iscsi_volume(connection_info)
+
         suri = self._suri_from_volume_info(connection_info)

         with ZoneConfig(zone) as zc:
@@ -1864,6 +1906,11 @@ class SolarisZonesDriver(driver.ComputeDriver):
             # Only Solaris Kernel zones are currently supported.
             raise NotImplementedError()

+        driver_type = connection_info['driver_volume_type']
+        if driver_type == 'iscsi':
+            connection_info['device_path'] = \
+                self._detach_iscsi_volume(connection_info)
+
         suri = self._suri_from_volume_info(connection_info)

         # Check if the specific property value exists before attempting removal
@@ -2840,7 +2887,7 @@ class SolarisZonesDriver(driver.ComputeDriver):
         connector = {'ip': self.get_host_ip_addr(),
                      'host': CONF.host}
         if not self._initiator:
-            self._initiator = self._get_iscsi_initiator()
+            self._initiator = self._iscsi.get_initiator()

         if self._initiator:
             connector['initiator'] = self._initiator
diff --git a/virt/solariszones/solarisiscsi.py b/virt/solariszones/solarisiscsi.py
new file mode 100644
index 0000000..cfc195d
--- /dev/null
+++ b/virt/solariszones/solarisiscsi.py
@@ -0,0 +1,140 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+"""Generic Solaris iSCSI utilities."""
+
+import os
+import time
+
+from nova import exception
+from nova.i18n import _
+from nova.openstack.common import log as logging
+from nova.openstack.common import processutils as putils
+
+LOG = logging.getLogger(__name__)
+
+
+class SolarisiSCSI(object):
+    def __init__(self, *args, **kwargs):
+        self.execute = putils.execute
+
+    def disconnect_iscsi(self):
+        """Disable the iSCSI discovery method to detach the volume
+        from instance_name.
+        """
+        self.execute('/usr/sbin/iscsiadm', 'modify', 'discovery',
+                     '--sendtargets', 'disable')
+
+    def _get_device_path(self, connection_properties):
+        """Get the device path from the target info."""
+        (out, _err) = self.execute('/usr/sbin/iscsiadm', 'list',
+                                   'target', '-S',
+                                   connection_properties['target_iqn'])
+        lines = out.splitlines()
+        current_lun = None
+        for raw_line in lines:
+            line = raw_line.strip()
+            if line.startswith("LUN:"):
+                current_lun = int(line.split()[-1])
+                continue
+            if (line.startswith("OS Device Name:") and
+                    current_lun == connection_properties['target_lun']):
+                path = line.split()[-1]
+                return path
+        else:
+            LOG.error(_("No device is found for the target %s.") %
+                      connection_properties['target_iqn'])
+            raise
+
+    def get_initiator(self):
+        """Return the iSCSI initiator node name IQN"""
+        out, err = self.execute('/usr/sbin/iscsiadm', 'list', 'initiator-node')
+
+        # Sample first line of command output:
+        # Initiator node name: iqn.1986-03.com.sun:01:e00000000000.4f757217
+        initiator_name_line = out.splitlines()[0]
+        return initiator_name_line.rsplit(' ', 1)[1]
+
+    def _add_discovery_address(self, address):
+        self.execute('/usr/sbin/iscsiadm', 'add', 'discovery-address',
+                         address)
+
+    def _connect_to_iscsi_portal(self, connection_properties):
+        # TODO(Strony): handle the CHAP authentication
+        target_ip = connection_properties['target_portal'].split(":")[0]
+
+        self._add_discovery_address(target_ip)
+
+        self.execute('/usr/sbin/iscsiadm', 'modify', 'discovery',
+                     '--sendtargets', 'enable')
+        (out, _err) = self.execute('/usr/sbin/iscsiadm', 'list',
+                                   'discovery-address', '-v',
+                                   target_ip)
+
+        lines = out.splitlines()
+        if not lines[0].strip().startswith('Discovery Address: ') or \
+                lines[1].strip().startswith('Unable to get targets.'):
+            msg = _("No iSCSI target is found.")
+            LOG.error(msg)
+            raise
+
+        target_iqn = connection_properties['target_iqn']
+        discovered_target_iqn = False
+        for line in [l.strip() for l in lines]:
+            if line.startswith("Target name:") and \
+                    line.split()[-1] == target_iqn:
+                discovered_target_iqn = True
+            if line.startswith("Target address:"):
+                # Grab any addresses we haven't seen yet and set them as
+                # discovery addresses.
+                # The line will be in this format:
+                #    Target address:   192.168.81.18:3260, 1
+                addr = line.split()[2].rstrip(',')
+                if addr != target_ip:
+                    self._add_discovery_address(addr)
+
+        if not discovered_target_iqn:
+            LOG.error(_("No active session is found for the target %s.") %
+                      target_iqn)
+            raise
+
+    def connect_volume(self, connection_properties, scan_tries):
+        """Attach the volume to instance_name.
+
+        connection_properties for iSCSI must include:
+        target_portal - ip and optional port
+        target_iqn - iSCSI Qualified Name
+        target_lun - LUN id of the volume
+        """
+        device_info = {'type': 'block'}
+
+        self._connect_to_iscsi_portal(connection_properties)
+
+        host_device = self._get_device_path(connection_properties)
+
+        # check if it is a valid device path.
+        for i in range(1, scan_tries):
+            if os.path.exists(host_device):
+                break
+            else:
+                time.sleep(i ** 2)
+        else:
+            msg = _("Volume device not found at %s") % host_device
+            LOG.exception(msg)
+            raise exception.NovaException(msg)
+
+        device_info['path'] = host_device
+        return device_info