Fix concurrent attaches on HNAS iSCSI driver

It might happen that multiple cinder nodes/backends are trying to add
volumes to the same target. In this situation, the driver can get an
error trying to add a volume to a hlun just taken by other instance.
This patch fix this avoiding concurrency using mutual exclusion and adds
retries to the failed attempts to cover scenarios where multiple cinder
hosts are used.

Closes-bug: #1475007
Change-Id: Ie2d3b286eecbf0299ac0c0e32d3e098bb5d11e4f

diff --git a/cinder/exception.py b/cinder/exception.py
index 2e312d8c4e59e0a24cae04bf86a299f9981d07ba..efaf1b3f0bb8bd589d854badf1c2a7feceebfea8 100644 (file)
--- a/cinder/exception.py
+++ b/cinder/exception.py
@@ -985,3 +985,8 @@ class MetadataAbsent(CinderException):
 class NotSupportedOperation(Invalid):
     message = _("Operation not supported: %(operation)s.")
     code = 405
+
+
+# Hitachi HNAS drivers
+class HNASConnError(CinderException):
+    message = _("%(message)s")

diff --git a/cinder/tests/unit/test_hitachi_hnas_backend.py b/cinder/tests/unit/test_hitachi_hnas_backend.py
index 26e9b973530b66f4b175dd322f103f2be6b5aecc..c4b0f77000b846bece2dba1a8ea7d9664557b69e 100644 (file)
--- a/cinder/tests/unit/test_hitachi_hnas_backend.py
+++ b/cinder/tests/unit/test_hitachi_hnas_backend.py
@@ -13,11 +13,13 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 #
+import time
 
 import mock
-from oslo_concurrency import processutils
+from oslo_concurrency import processutils as putils
 from oslo_config import cfg
 
+from cinder import exception
 from cinder import test
 from cinder import utils
 from cinder.volume.drivers.hitachi import hnas_backend
@@ -193,6 +195,8 @@ HNAS_RESULT20 = "Target does not exist."
 
 HNAS_RESULT21 = "Target created successfully."
 
+HNAS_RESULT22 = "Failed to establish SSC connection"
+
 HNAS_CMDS = {
     ('ssh', '0.0.0.0', 'supervisor', 'supervisor', 'evsfs', 'list'):
         ["%s" % HNAS_RESULT1, ""],
@@ -285,20 +289,42 @@ class HDSHNASBendTest(test.TestCase):
     @mock.patch('os.path.isfile', return_value=True)
     @mock.patch('paramiko.RSAKey.from_private_key_file')
     @mock.patch('paramiko.SSHClient')
-    @mock.patch.object(processutils, 'ssh_execute',
+    @mock.patch.object(putils, 'ssh_execute',
                        return_value=(HNAS_RESULT5, ''))
-    def test_run_cmd(self, m_ssh_exec, m_ssh_cli, m_pvt_key, m_file, m_open):
+    @mock.patch.object(utils, 'execute')
+    @mock.patch.object(time, 'sleep')
+    def test_run_cmd(self, m_sleep, m_utl, m_ssh, m_ssh_cli,
+                     m_pvt_key, m_file, m_open):
         save_hkey_file = CONF.ssh_hosts_key_file
         save_spath = CONF.state_path
         CONF.ssh_hosts_key_file = '/var/lib/cinder/ssh_known_hosts'
         CONF.state_path = '/var/lib/cinder'
 
+        # Test main flow
         out, err = self.hnas_bend.run_cmd('ssh', '0.0.0.0',
                                           'supervisor', 'supervisor',
                                           'df', '-a')
         self.assertIn('fs01-husvm', out)
         self.assertIn('WFS-2,128 DSBs', out)
 
+        # Test exception throwing when not using SSH
+        m_utl.side_effect = putils.ProcessExecutionError(stdout='',
+                                                         stderr=HNAS_RESULT22,
+                                                         exit_code=255)
+        self.hnas_bend.drv_configs['ssh_enabled'] = 'False'
+        self.assertRaises(exception.HNASConnError, self.hnas_bend.run_cmd,
+                          'ssh', '0.0.0.0', 'supervisor', 'supervisor',
+                          'df', '-a')
+
+        # Test exception throwing when using SSH
+        m_ssh.side_effect = putils.ProcessExecutionError(stdout='',
+                                                         stderr=HNAS_RESULT22,
+                                                         exit_code=255)
+        self.hnas_bend.drv_configs['ssh_enabled'] = 'True'
+        self.assertRaises(exception.HNASConnError, self.hnas_bend.run_cmd,
+                          'ssh', '0.0.0.0', 'supervisor', 'supervisor',
+                          'df', '-a')
+
         CONF.state_path = save_spath
         CONF.ssh_hosts_key_file = save_hkey_file
 

diff --git a/cinder/tests/unit/test_hitachi_hnas_iscsi.py b/cinder/tests/unit/test_hitachi_hnas_iscsi.py
index 59dbed844c9773871c6c19271c1b4b5c7c7d2f0b..46c2af3d76a809669c16ab6c95561138350476c4 100644 (file)
--- a/cinder/tests/unit/test_hitachi_hnas_iscsi.py
+++ b/cinder/tests/unit/test_hitachi_hnas_iscsi.py
@@ -20,8 +20,10 @@ Self test for Hitachi Unified Storage (HUS-HNAS) platform.
 
 import os
 import tempfile
+import time
 
 import mock
+from oslo_concurrency import processutils as putils
 import six
 
 from cinder import exception
@@ -382,14 +384,21 @@ class HNASiSCSIDriverTest(test.TestCase):
         self.backend.deleteVolumebyProvider(svol['provider_location'])
         self.backend.deleteVolumebyProvider(vol['provider_location'])
 
+    @mock.patch.object(time, 'sleep')
     @mock.patch.object(iscsi.HDSISCSIDriver, '_update_vol_location')
-    def test_initialize_connection(self, m_update_vol_location):
+    def test_initialize_connection(self, m_update_vol_location, m_sleep):
         connector = {}
         connector['initiator'] = 'iqn.1993-08.org.debian:01:11f90746eb2'
         connector['host'] = 'dut_1.lab.hds.com'
         vol = self._create_volume()
         conn = self.driver.initialize_connection(vol, connector)
         self.assertTrue('3260' in conn['data']['target_portal'])
+
+        self.backend.add_iscsi_conn = mock.MagicMock()
+        self.backend.add_iscsi_conn.side_effect = putils.ProcessExecutionError
+        self.assertRaises(exception.ISCSITargetAttachFailed,
+                          self.driver.initialize_connection, vol, connector)
+
         # cleanup
         self.backend.deleteVolumebyProvider(vol['provider_location'])
 

diff --git a/cinder/volume/drivers/hitachi/hnas_backend.py b/cinder/volume/drivers/hitachi/hnas_backend.py
index 4260c199cc959e85a8f821039d5f0e16143d6bc0..79e1efeab001729747ffc29e2731208410e9b24f 100644 (file)
--- a/cinder/volume/drivers/hitachi/hnas_backend.py
+++ b/cinder/volume/drivers/hitachi/hnas_backend.py
@@ -20,16 +20,18 @@ Hitachi Unified Storage (HUS-HNAS) platform. Backend operations.
 
 import re
 
-from oslo_concurrency import processutils
+from oslo_concurrency import processutils as putils
 from oslo_log import log as logging
 from oslo_utils import units
 import six
 
-from cinder.i18n import _LE, _LW, _LI
+from cinder.i18n import _, _LW, _LI
+from cinder import exception
 from cinder import ssh_utils
 from cinder import utils
 
 LOG = logging.getLogger("cinder.volume.driver")
+HNAS_SSC_RETRIES = 5
 
 
 class HnasBackend(object):
@@ -38,6 +40,7 @@ class HnasBackend(object):
         self.drv_configs = drv_configs
         self.sshpool = None
 
+    @utils.retry(exceptions=exception.HNASConnError, retries=HNAS_SSC_RETRIES)
     def run_cmd(self, cmd, ip0, user, pw, *args, **kwargs):
         """Run a command on SMU or using SSH
 
@@ -53,10 +56,20 @@ class HnasBackend(object):
         if self.drv_configs['ssh_enabled'] != 'True':
             # Direct connection via ssc
             args = (cmd, '-u', user, '-p', pw, ip0) + args
-            out, err = utils.execute(*args, **kwargs)
-            LOG.debug("command %(cmd)s result: out = %(out)s - err = "
-                      "%(err)s", {'cmd': cmd, 'out': out, 'err': err})
-            return out, err
+
+            try:
+                out, err = utils.execute(*args, **kwargs)
+                LOG.debug("command %(cmd)s result: out = %(out)s - err = "
+                          "%(err)s", {'cmd': cmd, 'out': out, 'err': err})
+                return out, err
+            except putils.ProcessExecutionError as e:
+                if 'Failed to establish SSC connection' in e.stderr:
+                    LOG.debug("SSC connection error!")
+                    msg = _("Failed to establish SSC connection.")
+                    raise exception.HNASConnError(msg)
+                else:
+                    raise putils.ProcessExecutionError
+
         else:
             if self.drv_configs['cluster_admin_ip0'] is None:
                 # Connect to SMU through SSH and run ssc locally
@@ -84,15 +97,21 @@ class HnasBackend(object):
                                                  privatekey=privatekey)
 
             with self.sshpool.item() as ssh:
+
                 try:
-                    out, err = processutils.ssh_execute(ssh, command,
-                                                        check_exit_code=True)
-                    LOG.debug("command %(cmd)s result: out = %(out)s - err = "
-                              "%(err)s", {'cmd': cmd, 'out': out, 'err': err})
+                    out, err = putils.ssh_execute(ssh, command,
+                                                  check_exit_code=True)
+                    LOG.debug("command %(cmd)s result: out = "
+                              "%(out)s - err = %(err)s",
+                              {'cmd': cmd, 'out': out, 'err': err})
                     return out, err
-                except processutils.ProcessExecutionError:
-                    LOG.error(_LE("Error running SSH command."))
-                    raise
+                except putils.ProcessExecutionError as e:
+                    if 'Failed to establish SSC connection' in e.stderr:
+                        LOG.debug("SSC connection error!")
+                        msg = _("Failed to establish SSC connection.")
+                        raise exception.HNASConnError(msg)
+                    else:
+                        raise putils.ProcessExecutionError
 
     def get_version(self, cmd, ver, ip0, user, pw):
         """Gets version information from the storage unit
@@ -446,6 +465,7 @@ class HnasBackend(object):
         LOG.debug('extend_vol: %s', out)
         return out
 
+    @utils.retry(putils.ProcessExecutionError, retries=HNAS_SSC_RETRIES)
     def add_iscsi_conn(self, cmd, ip0, user, pw, lun, hdp,
                        port, iqn, initiator):
         """Setup the lun on on the specified target port

diff --git a/cinder/volume/drivers/hitachi/hnas_iscsi.py b/cinder/volume/drivers/hitachi/hnas_iscsi.py
index 0f81831c4b0ff0ac12f1bb24b1c8dfaa4532b549..a21dbeb7b2da176bd8e7b2d4eabf8cd966dbde08 100644 (file)
--- a/cinder/volume/drivers/hitachi/hnas_iscsi.py
+++ b/cinder/volume/drivers/hitachi/hnas_iscsi.py
@@ -20,6 +20,7 @@ iSCSI Cinder Volume driver for Hitachi Unified Storage (HUS-HNAS) platform.
 import os
 from xml.etree import ElementTree as ETree
 
+from oslo_concurrency import processutils
 from oslo_config import cfg
 from oslo_log import log as logging
 from oslo_utils import excutils
@@ -27,13 +28,14 @@ from oslo_utils import units
 
 from cinder import exception
 from cinder.i18n import _, _LE, _LI, _LW
+from cinder import utils as cinder_utils
 from cinder.volume import driver
 from cinder.volume.drivers.hitachi import hnas_backend
 from cinder.volume import utils
 from cinder.volume import volume_types
 
 
-HDS_HNAS_ISCSI_VERSION = '3.0.1'
+HDS_HNAS_ISCSI_VERSION = '3.1.0'
 
 LOG = logging.getLogger(__name__)
 
@@ -550,6 +552,7 @@ class HDSISCSIDriver(driver.ISCSIDriver):
                             self.config['password'],
                             hdp, lun)
 
+    @cinder_utils.synchronized('volume_mapping')
     def initialize_connection(self, volume, connector):
         """Map the created volume to connector['initiator'].
 
@@ -572,12 +575,18 @@ class HDSISCSIDriver(driver.ISCSIDriver):
         loc = arid + '.' + lun
         # sps, use target if provided
         iqn = target
-        out = self.bend.add_iscsi_conn(self.config['hnas_cmd'],
-                                       self.config['mgmt_ip0'],
-                                       self.config['username'],
-                                       self.config['password'],
-                                       lun, _hdp, port, iqn,
-                                       connector['initiator'])
+
+        try:
+            out = self.bend.add_iscsi_conn(self.config['hnas_cmd'],
+                                           self.config['mgmt_ip0'],
+                                           self.config['username'],
+                                           self.config['password'],
+                                           lun, _hdp, port, iqn,
+                                           connector['initiator'])
+        except processutils.ProcessExecutionError:
+            msg = _("Error attaching volume %s. "
+                    "Target limit might be reached!") % volume['id']
+            raise exception.ISCSITargetAttachFailed(message=msg)
 
         hnas_portal = ip + ':' + ipp
         # sps need hlun, fulliqn
@@ -604,6 +613,7 @@ class HDSISCSIDriver(driver.ISCSIDriver):
 
         return {'driver_volume_type': 'iscsi', 'data': properties}
 
+    @cinder_utils.synchronized('volume_mapping')
     def terminate_connection(self, volume, connector, **kwargs):
         """Terminate a connection to a volume.