From a95fef70b407a400c27195bd1cfbf09858511e9f Mon Sep 17 00:00:00 2001 From: "Erlon R. Cruz" Date: Fri, 10 Jul 2015 15:36:22 -0300 Subject: [PATCH] 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 --- cinder/exception.py | 5 ++ .../tests/unit/test_hitachi_hnas_backend.py | 32 +++++++++++-- cinder/tests/unit/test_hitachi_hnas_iscsi.py | 11 ++++- cinder/volume/drivers/hitachi/hnas_backend.py | 46 +++++++++++++------ cinder/volume/drivers/hitachi/hnas_iscsi.py | 24 +++++++--- 5 files changed, 94 insertions(+), 24 deletions(-) diff --git a/cinder/exception.py b/cinder/exception.py index 2e312d8c4..efaf1b3f0 100644 --- 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 26e9b9735..c4b0f7700 100644 --- 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 59dbed844..46c2af3d7 100644 --- 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 4260c199c..79e1efeab 100644 --- 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 0f81831c4..a21dbeb7b 100644 --- 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. -- 2.45.2