diff --git a/lib/ansible/plugins/connection/ssh.py b/lib/ansible/plugins/connection/ssh.py index 1339af74492..b526905ff36 100644 --- a/lib/ansible/plugins/connection/ssh.py +++ b/lib/ansible/plugins/connection/ssh.py @@ -491,8 +491,7 @@ class Connection(ConnectionBase): return b''.join(output), remainder - @_ssh_retry - def _run(self, cmd, in_data, sudoable=True, checkrc=True): + def _bare_run(self, cmd, in_data, sudoable=True, checkrc=True): ''' Starts the command and communicates with it until it ends. ''' @@ -767,6 +766,13 @@ class Connection(ConnectionBase): return (p.returncode, b_stdout, b_stderr) + @_ssh_retry + def _run(self, cmd, in_data, sudoable=True, checkrc=True): + """Wrapper around _bare_run that retries the connection + """ + return self._bare_run(cmd, in_data, sudoable, checkrc) + + @_ssh_retry def _file_transport_command(self, in_path, out_path, sftp_action): # scp and sftp require square brackets for IPv6 addresses, but # accept them for hostnames and IPv4 addresses too. @@ -807,14 +813,14 @@ class Connection(ConnectionBase): cmd = self._build_command('sftp', to_bytes(host)) in_data = u"{0} {1} {2}\n".format(sftp_action, shlex_quote(in_path), shlex_quote(out_path)) in_data = to_bytes(in_data, nonstring='passthru') - (returncode, stdout, stderr) = self._run(cmd, in_data, checkrc=False) + (returncode, stdout, stderr) = self._bare_run(cmd, in_data, checkrc=False) elif method == 'scp': if sftp_action == 'get': cmd = self._build_command('scp', u'{0}:{1}'.format(host, shlex_quote(in_path)), out_path) else: cmd = self._build_command('scp', in_path, u'{0}:{1}'.format(host, shlex_quote(out_path))) in_data = None - (returncode, stdout, stderr) = self._run(cmd, in_data, checkrc=False) + (returncode, stdout, stderr) = self._bare_run(cmd, in_data, checkrc=False) elif method == 'piped': if sftp_action == 'get': # we pass sudoable=False to disable pty allocation, which diff --git a/test/units/plugins/connection/test_ssh.py b/test/units/plugins/connection/test_ssh.py index ef7632d579c..2ca4d93a8e6 100644 --- a/test/units/plugins/connection/test_ssh.py +++ b/test/units/plugins/connection/test_ssh.py @@ -198,11 +198,11 @@ class TestConnectionBaseClass(unittest.TestCase): new_stdin = StringIO() conn = ssh.Connection(pc, new_stdin) conn._build_command = MagicMock() - conn._run = MagicMock() + conn._bare_run = MagicMock() mock_ospe.return_value = True conn._build_command.return_value = 'some command to run' - conn._run.return_value = (0, '', '') + conn._bare_run.return_value = (0, '', '') conn.host = "some_host" C.ANSIBLE_SSH_RETRIES = 9 @@ -212,41 +212,41 @@ class TestConnectionBaseClass(unittest.TestCase): C.DEFAULT_SCP_IF_SSH = 'smart' expected_in_data = b' '.join((b'put', to_bytes(shlex_quote('/path/to/in/file')), to_bytes(shlex_quote('/path/to/dest/file')))) + b'\n' conn.put_file('/path/to/in/file', '/path/to/dest/file') - conn._run.assert_called_with('some command to run', expected_in_data, checkrc=False) + conn._bare_run.assert_called_with('some command to run', expected_in_data, checkrc=False) # Test when SFTP doesn't work but SCP does - conn._run.side_effect = [(1, 'stdout', 'some errors'), (0, '', '')] + conn._bare_run.side_effect = [(1, 'stdout', 'some errors'), (0, '', '')] conn.put_file('/path/to/in/file', '/path/to/dest/file') - conn._run.assert_called_with('some command to run', None, checkrc=False) - conn._run.side_effect = None + conn._bare_run.assert_called_with('some command to run', None, checkrc=False) + conn._bare_run.side_effect = None # test with C.DEFAULT_SCP_IF_SSH enabled C.DEFAULT_SCP_IF_SSH = True conn.put_file('/path/to/in/file', '/path/to/dest/file') - conn._run.assert_called_with('some command to run', None, checkrc=False) + conn._bare_run.assert_called_with('some command to run', None, checkrc=False) conn.put_file(u'/path/to/in/file/with/unicode-fö〩', u'/path/to/dest/file/with/unicode-fö〩') - conn._run.assert_called_with('some command to run', None, checkrc=False) + conn._bare_run.assert_called_with('some command to run', None, checkrc=False) # test with C.DEFAULT_SCP_IF_SSH disabled C.DEFAULT_SCP_IF_SSH = False expected_in_data = b' '.join((b'put', to_bytes(shlex_quote('/path/to/in/file')), to_bytes(shlex_quote('/path/to/dest/file')))) + b'\n' conn.put_file('/path/to/in/file', '/path/to/dest/file') - conn._run.assert_called_with('some command to run', expected_in_data, checkrc=False) + conn._bare_run.assert_called_with('some command to run', expected_in_data, checkrc=False) expected_in_data = b' '.join((b'put', to_bytes(shlex_quote('/path/to/in/file/with/unicode-fö〩')), to_bytes(shlex_quote('/path/to/dest/file/with/unicode-fö〩')))) + b'\n' conn.put_file(u'/path/to/in/file/with/unicode-fö〩', u'/path/to/dest/file/with/unicode-fö〩') - conn._run.assert_called_with('some command to run', expected_in_data, checkrc=False) + conn._bare_run.assert_called_with('some command to run', expected_in_data, checkrc=False) # test that a non-zero rc raises an error - conn._run.return_value = (1, 'stdout', 'some errors') + conn._bare_run.return_value = (1, 'stdout', 'some errors') self.assertRaises(AnsibleError, conn.put_file, '/path/to/bad/file', '/remote/path/to/file') # test that a not-found path raises an error mock_ospe.return_value = False - conn._run.return_value = (0, 'stdout', '') + conn._bare_run.return_value = (0, 'stdout', '') self.assertRaises(AnsibleFileNotFound, conn.put_file, '/path/to/bad/file', '/remote/path/to/file') @patch('time.sleep') @@ -255,10 +255,10 @@ class TestConnectionBaseClass(unittest.TestCase): new_stdin = StringIO() conn = ssh.Connection(pc, new_stdin) conn._build_command = MagicMock() - conn._run = MagicMock() + conn._bare_run = MagicMock() conn._build_command.return_value = 'some command to run' - conn._run.return_value = (0, '', '') + conn._bare_run.return_value = (0, '', '') conn.host = "some_host" C.ANSIBLE_SSH_RETRIES = 9 @@ -268,36 +268,36 @@ class TestConnectionBaseClass(unittest.TestCase): C.DEFAULT_SCP_IF_SSH = 'smart' expected_in_data = b' '.join((b'get', to_bytes(shlex_quote('/path/to/in/file')), to_bytes(shlex_quote('/path/to/dest/file')))) + b'\n' conn.fetch_file('/path/to/in/file', '/path/to/dest/file') - conn._run.assert_called_with('some command to run', expected_in_data, checkrc=False) + conn._bare_run.assert_called_with('some command to run', expected_in_data, checkrc=False) # Test when SFTP doesn't work but SCP does - conn._run.side_effect = [(1, 'stdout', 'some errors'), (0, '', '')] + conn._bare_run.side_effect = [(1, 'stdout', 'some errors'), (0, '', '')] conn.fetch_file('/path/to/in/file', '/path/to/dest/file') - conn._run.assert_called_with('some command to run', None, checkrc=False) - conn._run.side_effect = None + conn._bare_run.assert_called_with('some command to run', None, checkrc=False) + conn._bare_run.side_effect = None # test with C.DEFAULT_SCP_IF_SSH enabled C.DEFAULT_SCP_IF_SSH = True conn.fetch_file('/path/to/in/file', '/path/to/dest/file') - conn._run.assert_called_with('some command to run', None, checkrc=False) + conn._bare_run.assert_called_with('some command to run', None, checkrc=False) conn.fetch_file(u'/path/to/in/file/with/unicode-fö〩', u'/path/to/dest/file/with/unicode-fö〩') - conn._run.assert_called_with('some command to run', None, checkrc=False) + conn._bare_run.assert_called_with('some command to run', None, checkrc=False) # test with C.DEFAULT_SCP_IF_SSH disabled C.DEFAULT_SCP_IF_SSH = False expected_in_data = b' '.join((b'get', to_bytes(shlex_quote('/path/to/in/file')), to_bytes(shlex_quote('/path/to/dest/file')))) + b'\n' conn.fetch_file('/path/to/in/file', '/path/to/dest/file') - conn._run.assert_called_with('some command to run', expected_in_data, checkrc=False) + conn._bare_run.assert_called_with('some command to run', expected_in_data, checkrc=False) expected_in_data = b' '.join((b'get', to_bytes(shlex_quote('/path/to/in/file/with/unicode-fö〩')), to_bytes(shlex_quote('/path/to/dest/file/with/unicode-fö〩')))) + b'\n' conn.fetch_file(u'/path/to/in/file/with/unicode-fö〩', u'/path/to/dest/file/with/unicode-fö〩') - conn._run.assert_called_with('some command to run', expected_in_data, checkrc=False) + conn._bare_run.assert_called_with('some command to run', expected_in_data, checkrc=False) # test that a non-zero rc raises an error - conn._run.return_value = (1, 'stdout', 'some errors') + conn._bare_run.return_value = (1, 'stdout', 'some errors') self.assertRaises(AnsibleError, conn.fetch_file, '/path/to/bad/file', '/remote/path/to/file')