From c33f84a8df58d45a31afdeeb453627f6eac2d748 Mon Sep 17 00:00:00 2001
From: Liu Qing <liuqing@chinac.com>
Date: Sat, 27 May 2017 17:39:19 +0800
Subject: [PATCH] Fix SSHPool current_size not work correctly

SSHPool will create a new SSHClient if the client it got from the pool
is not active. If the create failed, then the current_size should minus
one, as this connection will never be put back to the pool.

Closes-Bug: 1693984

Change-Id: Ic5fa53d6426a18138009fdf7de04e3ed6120231b
Signed-off-by: Liu Qing <liuqing@chinac.com>
---
 cinder/ssh_utils.py                 | 10 +++++++++-
 cinder/tests/unit/test_ssh_utils.py | 23 +++++++++++++++++++++++
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/cinder/ssh_utils.py b/cinder/ssh_utils.py
index d6e8889085a..c2fad969799 100644
--- a/cinder/ssh_utils.py
+++ b/cinder/ssh_utils.py
@@ -23,6 +23,7 @@ import os
 from eventlet import pools
 from oslo_config import cfg
 from oslo_log import log as logging
+from oslo_utils import excutils
 import paramiko
 import six
 
@@ -166,7 +167,14 @@ class SSHPool(pools.Pool):
                 return conn
             else:
                 conn.close()
-        return self.create()
+        try:
+            new_conn = self.create()
+        except Exception:
+            LOG.error("Create new item in SSHPool failed.")
+            with excutils.save_and_reraise_exception():
+                if conn:
+                    self.current_size -= 1
+        return new_conn
 
     def remove(self, ssh):
         """Close an ssh client and remove it from free_items."""
diff --git a/cinder/tests/unit/test_ssh_utils.py b/cinder/tests/unit/test_ssh_utils.py
index 88ade3b2ac8..393139b0152 100644
--- a/cinder/tests/unit/test_ssh_utils.py
+++ b/cinder/tests/unit/test_ssh_utils.py
@@ -324,3 +324,26 @@ class SSHPoolTestCase(test.TestCase):
         with sshpool.item() as ssh:
             self.assertIsInstance(ssh.get_policy(),
                                   paramiko.AutoAddPolicy)
+
+    @mock.patch('paramiko.SSHClient')
+    @mock.patch('six.moves.builtins.open')
+    @mock.patch('os.path.isfile', return_value=False)
+    def test_ssh_timeout(self, mock_isfile, mock_open, mock_sshclient):
+        sshpool = ssh_utils.SSHPool("127.0.0.1", 22, 10,
+                                    "test",
+                                    password="test",
+                                    min_size=1,
+                                    max_size=1)
+        self.assertEqual(1, sshpool.current_size)
+        conn = sshpool.get()
+        conn.connect = mock.MagicMock()
+        # create failed due to time out
+        conn.connect.side_effect = paramiko.SSHException("time out")
+        mock_transport = mock.MagicMock()
+        conn.get_transport.return_value = mock_transport
+        # connection is down
+        mock_transport.is_active.return_value = False
+        sshpool.put(conn)
+        self.assertRaises(paramiko.SSHException,
+                          sshpool.get)
+        self.assertEqual(0, sshpool.current_size)