Fix the eqlx driver to retry on ssh timeout

When the ssh session is timing out, the driver
should make attempts to retry based on the value
in eqlx_cli_max_retries. Instead it was raising
the exception and bailing out on a single attempt.

Fixed the driver to raise a different exception
so the ssh sessions can be retried.

Added unit tests to ensure the max retries
happen

Also fixed the actual attempts made in the message

Change-Id: I9bda46f9ef63eec436a381da3c96c09ce77c31ef
Closes-Bug: #1412940

diff --git a/cinder/tests/test_eqlx.py b/cinder/tests/test_eqlx.py
index 56b66d0c0db23a2c055987fdbda372277adaa78b..c63935db2ec9b21e65123c9b4b638fb10193a106 100644 (file)
--- a/cinder/tests/test_eqlx.py
+++ b/cinder/tests/test_eqlx.py
@@ -13,8 +13,10 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import random
 import time
 
+import mock
 import mox
 from oslo_concurrency import processutils
 import paramiko
@@ -22,11 +24,12 @@ import paramiko
 from cinder import context
 from cinder import exception
 from cinder.openstack.common import log as logging
+from cinder import ssh_utils
 from cinder import test
+from cinder import utils
 from cinder.volume import configuration as conf
 from cinder.volume.drivers import eqlx
 
-
 LOG = logging.getLogger(__name__)
 
 
@@ -42,6 +45,10 @@ class DellEQLSanISCSIDriverTestCase(test.TestCase):
         self.configuration.san_password = "bar"
         self.configuration.san_ssh_port = 16022
         self.configuration.san_thin_provision = True
+        self.configuration.san_private_key = 'foo'
+        self.configuration.ssh_min_pool_conn = 1
+        self.configuration.ssh_max_pool_conn = 5
+        self.configuration.ssh_conn_timeout = 30
         self.configuration.eqlx_pool = 'non-default'
         self.configuration.eqlx_use_chap = True
         self.configuration.eqlx_group_name = 'group-0'
@@ -317,6 +324,55 @@ class DellEQLSanISCSIDriverTestCase(test.TestCase):
         self.assertRaises(processutils.ProcessExecutionError,
                           self.driver._ssh_execute, ssh, cmd)
 
+    def test_ensure_retries(self):
+        num_attempts = random.randint(1, 5)
+        self.driver.configuration.eqlx_cli_max_retries = num_attempts
+
+        self.mock_object(self.driver, '_ssh_execute',
+                         mock.Mock(side_effect=exception.
+                                   VolumeBackendAPIException("some error")))
+        # mocks for calls in _run_ssh
+        self.mock_object(utils, 'check_ssh_injection')
+        self.mock_object(ssh_utils, 'SSHPool')
+
+        sshpool = ssh_utils.SSHPool("127.0.0.1", 22, 10,
+                                    "test",
+                                    password="test",
+                                    min_size=1,
+                                    max_size=1)
+        self.mock_object(sshpool.item(), 'close')
+        self.driver.sshpool = mock.Mock(return_value=sshpool)
+        # now call the execute
+        self.assertRaises(exception.VolumeBackendAPIException,
+                          self.driver._eql_execute, "fake command")
+        self.assertEqual(num_attempts + 1,
+                         self.driver._ssh_execute.call_count)
+
+    def test_ensure_retries_on_channel_timeout(self):
+
+        num_attempts = random.randint(1, 5)
+        self.driver.configuration.eqlx_cli_max_retries = num_attempts
+
+        # mocks for calls and objects in _run_ssh
+        self.mock_object(utils, 'check_ssh_injection')
+        self.mock_object(ssh_utils, 'SSHPool')
+
+        sshpool = ssh_utils.SSHPool("127.0.0.1", 22, 10,
+                                    "test",
+                                    password="test",
+                                    min_size=1,
+                                    max_size=1)
+        self.mock_object(sshpool.item(), 'close')
+        self.driver.sshpool = mock.Mock(return_value=sshpool)
+        # mocks for _ssh_execute and _get_output
+        self.mock_object(self.driver, '_get_output',
+                         mock.Mock(side_effect=exception.
+                                   VolumeBackendAPIException("some error")))
+        # now call the execute
+        self.assertRaises(exception.VolumeBackendAPIException,
+                          self.driver._eql_execute, "fake command")
+        self.assertEqual(num_attempts + 1, self.driver._get_output.call_count)
+
     def test_with_timeout(self):
         @eqlx.with_timeout
         def no_timeout(cmd, *args, **kwargs):

diff --git a/cinder/volume/drivers/eqlx.py b/cinder/volume/drivers/eqlx.py
index 9b9fff01eadb37f11693cd034e8d443edccc64ad..d761806e9fee92d8557b09cf68e5d73ee8870c64 100644 (file)
--- a/cinder/volume/drivers/eqlx.py
+++ b/cinder/volume/drivers/eqlx.py
@@ -144,7 +144,7 @@ class DellEQLSanISCSIDriver(SanISCSIDriver):
                 # has closed the connection.
                 msg = _("The EQL array has closed the connection.")
                 LOG.error(msg)
-                raise processutils.ProcessExecutionError(description=msg)
+                raise exception.VolumeBackendAPIException(data=msg)
             out += ret
 
         LOG.debug("CLI output\n%s", out)
@@ -211,23 +211,25 @@ class DellEQLSanISCSIDriver(SanISCSIDriver):
                 max_size=max_size)
         try:
             total_attempts = attempts
-            with self.sshpool.item() as ssh:
-                while attempts > 0:
-                    attempts -= 1
-                    try:
-                        LOG.info(_LI('EQL-driver: executing "%s".'), command)
-                        return self._ssh_execute(
-                            ssh, command,
-                            timeout=self.configuration.eqlx_cli_timeout)
-                    except processutils.ProcessExecutionError:
-                        raise
-                    except Exception as e:
-                        LOG.exception(e)
-                        greenthread.sleep(random.randint(20, 500) / 100.0)
-                msg = (_("SSH Command failed after '%(total_attempts)r' "
-                         "attempts : '%(command)s'") %
-                       {'total_attempts': total_attempts, 'command': command})
-                raise exception.VolumeBackendAPIException(data=msg)
+            ssh = self.sshpool.item()
+            while attempts > 0:
+                attempts -= 1
+                try:
+                    LOG.info(_LI('EQL-driver: executing "%s".'), command)
+                    return self._ssh_execute(
+                        ssh, command,
+                        timeout=self.configuration.eqlx_cli_timeout)
+                except processutils.ProcessExecutionError:
+                    raise
+                except Exception as e:
+                    LOG.exception(e)
+                    greenthread.sleep(random.randint(20, 500) / 100.0)
+            msg = (_("SSH Command failed after '%(total_attempts)r' "
+                     "attempts : '%(command)s'") %
+                   {'total_attempts': total_attempts - attempts,
+                    'command': command})
+            ssh.close()
+            raise exception.VolumeBackendAPIException(data=msg)
 
         except Exception:
             with excutils.save_and_reraise_exception():