From 2a77c63071abcf2e2088b72f1a81b3ffc69be7c7 Mon Sep 17 00:00:00 2001 From: Andrey Kurilin Date: Mon, 10 Jan 2022 15:38:11 +0200 Subject: [PATCH] Close stringio objects at sshutils Closes-Bug: #1956956 Change-Id: I94f597d99951459b12f0f0211ec73f2ae7fa908d --- CHANGELOG.rst | 10 ++++++++++ rally/utils/sshutils.py | 30 +++++++++++++++++------------ tests/unit/utils/test_sshutils.py | 32 +++++++++++++++++-------------- 3 files changed, 46 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index ef30cb158e..9905016eac 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -17,6 +17,16 @@ Changelog .. Release notes for existing releases are MUTABLE! If there is something that was missed or can be improved, feel free to change it! +[unreleased] +------------ + +Fixed +~~~~~ + +* rally.utils.sshutils.SSH.execute leaves fifo files. + + `Launchpad-bug #1956956 `_ + [3.3.0] - 2021-06-16 -------------------- diff --git a/rally/utils/sshutils.py b/rally/utils/sshutils.py index 7b287eb6d9..80e0542e6e 100644 --- a/rally/utils/sshutils.py +++ b/rally/utils/sshutils.py @@ -24,7 +24,7 @@ Execute command and get output: status, stdout, stderr = ssh.execute("ps ax") if status: raise Exception("Command failed with non-zero status.") - print stdout.splitlines() + print(stdout.splitlines()) Execute command with huge output: @@ -157,12 +157,18 @@ class SSH(object): client = self._get_client() + should_close_stdin = False if isinstance(stdin, str): stdin = io.StringIO(stdin) + should_close_stdin = True - return self._run(client, cmd, stdin=stdin, stdout=stdout, - stderr=stderr, raise_on_error=raise_on_error, - timeout=timeout) + try: + return self._run(client, cmd, stdin=stdin, stdout=stdout, + stderr=stderr, raise_on_error=raise_on_error, + timeout=timeout) + finally: + if should_close_stdin: + stdin.close() def _run(self, client, cmd, stdin=None, stdout=None, stderr=None, raise_on_error=True, timeout=3600): @@ -246,15 +252,15 @@ class SSH(object): :returns: tuple (exit_status, stdout, stderr) """ - stdout = io.StringIO() - stderr = io.StringIO() + with io.StringIO() as stdout: + with io.StringIO() as stderr: - exit_status, data = self.run(cmd, stderr=stderr, stdout=stdout, - stdin=stdin, timeout=timeout, - raise_on_error=False) - stdout.seek(0) - stderr.seek(0) - return (exit_status, stdout.read(), stderr.read()) + exit_status, data = self.run(cmd, stderr=stderr, stdout=stdout, + stdin=stdin, timeout=timeout, + raise_on_error=False) + stdout.seek(0) + stderr.seek(0) + return exit_status, stdout.read(), stderr.read() def wait(self, timeout=120, interval=1): """Wait for the host will be available via ssh.""" diff --git a/tests/unit/utils/test_sshutils.py b/tests/unit/utils/test_sshutils.py index 555432041e..b3289bef1d 100644 --- a/tests/unit/utils/test_sshutils.py +++ b/tests/unit/utils/test_sshutils.py @@ -128,22 +128,26 @@ class SSHTestCase(test.TestCase): m_client.close.assert_called_once_with() self.assertFalse(self.ssh._client) - @mock.patch("rally.utils.sshutils.io.StringIO") - def test_execute(self, mock_string_io): - mock_string_io.side_effect = stdio = [mock.Mock(), mock.Mock()] - stdio[0].read.return_value = "stdout fake data" - stdio[1].read.return_value = "stderr fake data" - with mock.patch.object(self.ssh, "run") as mock_run: - mock_run.return_value = (0, None) - status, stdout, stderr = self.ssh.execute("cmd", - stdin="fake_stdin", - timeout=43) + def test_execute(self): + stdout_txt = "stdout fake data" + stdout_io = io.StringIO(stdout_txt) + stderr_txt = "stderr fake data" + stderr_io = io.StringIO(stderr_txt) + + with mock.patch("rally.utils.sshutils.io.StringIO") as mock_string_io: + mock_string_io.side_effect = [stdout_io, stderr_io] + with mock.patch.object(self.ssh, "run") as mock_run: + mock_run.return_value = (0, None) + status, stdout, stderr = self.ssh.execute("cmd", + stdin="fake_stdin", + timeout=43) + mock_run.assert_called_once_with( - "cmd", stdin="fake_stdin", stdout=stdio[0], - stderr=stdio[1], timeout=43, raise_on_error=False) + "cmd", stdin="fake_stdin", stdout=stdout_io, stderr=stderr_io, + timeout=43, raise_on_error=False) self.assertEqual(0, status) - self.assertEqual("stdout fake data", stdout) - self.assertEqual("stderr fake data", stderr) + self.assertEqual(stdout_txt, stdout) + self.assertEqual(stderr_txt, stderr) @mock.patch("rally.utils.sshutils.time") def test_wait_timeout(self, mock_time):