openstack
/
manila
Code Issues Proposed changes

Fix sshpool.remove 

collections.deque does not have a 'pop'
method, and the sshpool.remove method
currently leaks SSH connections that
it creates.

This bugfix was ported from cinder [1]

[1] https://review.openstack.org/#/c/285687/

Change-Id: I2dc9ffc13f11884b3069e6b4a453c933edf16d89
Co-Authored-By: Surya Ghatty <ghatty@us.ibm.com>
Closes-Bug: #1463557
This commit is contained in:
Goutham Pacha Ravi 2019-02-18 14:31:18 -08:00
parent 3017c3239e
commit 1af8026e73
2 changed files with 37 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
manila/tests/test_utils.py
View File

@ -325,6 +325,40 @@ class SSHPoolTestCase(test.TestCase):
self.assertNotEqual(first_id, third_id)
paramiko.SSHClient.assert_called_once_with()
@mock.patch('six.moves.builtins.open')
@mock.patch('paramiko.SSHClient')
@mock.patch('os.path.isfile', return_value=True)
def test_sshpool_remove(self, mock_isfile, mock_sshclient, mock_open):
ssh_to_remove = mock.Mock()
mock_sshclient.side_effect = [mock.Mock(), ssh_to_remove, mock.Mock()]
sshpool = utils.SSHPool("127.0.0.1", 22, 10, "test", password="test",
min_size=3, max_size=3)
self.assertIn(ssh_to_remove, list(sshpool.free_items))
sshpool.remove(ssh_to_remove)
self.assertNotIn(ssh_to_remove, list(sshpool.free_items))
@mock.patch('six.moves.builtins.open')
@mock.patch('paramiko.SSHClient')
@mock.patch('os.path.isfile', return_value=True)
def test_sshpool_remove_object_not_in_pool(self, mock_isfile,
mock_sshclient, mock_open):
# create an SSH Client that is not a part of sshpool.
ssh_to_remove = mock.Mock()
mock_sshclient.side_effect = [mock.Mock(), mock.Mock()]
sshpool = utils.SSHPool("127.0.0.1", 22, 10, "test", password="test",
min_size=2, max_size=2)
listBefore = list(sshpool.free_items)
self.assertNotIn(ssh_to_remove, listBefore)
sshpool.remove(ssh_to_remove)
self.assertEqual(listBefore, list(sshpool.free_items))
@ddt.ddt
class CidrToNetmaskTestCase(test.TestCase):

7
manila/utils.py
View File

@ -178,11 +178,10 @@ class SSHPool(pools.Pool):
def remove(self, ssh):
"""Close an ssh client and remove it from free_items."""
ssh.close()
ssh = None
if ssh in self.free_items:
self.free_items.pop(ssh)
if self.current_size > 0:
self.current_size -= 1
self.free_items.remove(ssh)
if self.current_size > 0:
self.current_size -= 1
def check_ssh_injection(cmd_list):