diff --git a/changelogs/fragments/61570-netcli-put-get.yaml b/changelogs/fragments/61570-netcli-put-get.yaml new file mode 100644 index 00000000000..8d835cf8766 --- /dev/null +++ b/changelogs/fragments/61570-netcli-put-get.yaml @@ -0,0 +1,2 @@ +bugfixes: +- "fixed issues when using net_get & net_put before the persistent connection has been started" diff --git a/lib/ansible/plugins/action/net_get.py b/lib/ansible/plugins/action/net_get.py index 9ba8c2aaff3..a33f3c46353 100644 --- a/lib/ansible/plugins/action/net_get.py +++ b/lib/ansible/plugins/action/net_get.py @@ -25,7 +25,7 @@ import hashlib from ansible.errors import AnsibleError from ansible.module_utils._text import to_text, to_bytes -from ansible.module_utils.connection import Connection +from ansible.module_utils.connection import Connection, ConnectionError from ansible.plugins.action import ActionBase from ansible.module_utils.six.moves.urllib.parse import urlsplit from ansible.utils.display import Display @@ -66,33 +66,31 @@ class ActionModule(ActionBase): if proto is None: proto = 'scp' - sock_timeout = play_context.timeout - if socket_path is None: socket_path = self._connection.socket_path conn = Connection(socket_path) + sock_timeout = conn.get_option('persistent_command_timeout') try: changed = self._handle_existing_file(conn, src, dest, proto, sock_timeout) if changed is False: - result['changed'] = False + result['changed'] = changed result['destination'] = dest return result except Exception as exc: - result['msg'] = ('Warning: exception %s idempotency check failed. Check ' - 'dest' % exc) + result['msg'] = ('Warning: %s idempotency check failed. Check dest' % exc) try: - out = conn.get_file( + conn.get_file( source=src, destination=dest, proto=proto, timeout=sock_timeout ) except Exception as exc: result['failed'] = True - result['msg'] = ('Exception received : %s' % exc) + result['msg'] = 'Exception received: %s' % exc - result['changed'] = True + result['changed'] = changed result['destination'] = dest return result @@ -117,27 +115,37 @@ class ActionModule(ActionBase): return filename def _handle_existing_file(self, conn, source, dest, proto, timeout): + """ + Determines whether the source and destination file match. + + :return: False if source and dest both exist and have matching sha1 sums, True otherwise. + """ if not os.path.exists(dest): return True + cwd = self._loader.get_basedir() filename = str(uuid.uuid4()) tmp_dest_file = os.path.join(cwd, filename) try: - out = conn.get_file( + conn.get_file( source=source, destination=tmp_dest_file, proto=proto, timeout=timeout ) - except Exception as exc: - os.remove(tmp_dest_file) - raise Exception(exc) + except ConnectionError as exc: + error = to_text(exc) + if error.endswith("No such file or directory"): + if os.path.exists(tmp_dest_file): + os.remove(tmp_dest_file) + return True try: with open(tmp_dest_file, 'r') as f: new_content = f.read() with open(dest, 'r') as f: old_content = f.read() - except (IOError, OSError) as ioexc: - raise IOError(ioexc) + except (IOError, OSError): + os.remove(tmp_dest_file) + raise sha1 = hashlib.sha1() old_content_b = to_bytes(old_content, errors='surrogate_or_strict') @@ -151,8 +159,7 @@ class ActionModule(ActionBase): os.remove(tmp_dest_file) if checksum_old == checksum_new: return False - else: - return True + return True def _get_working_path(self): cwd = self._loader.get_basedir() diff --git a/lib/ansible/plugins/action/net_put.py b/lib/ansible/plugins/action/net_put.py index bf6dd52d292..dd4b850e68d 100644 --- a/lib/ansible/plugins/action/net_put.py +++ b/lib/ansible/plugins/action/net_put.py @@ -19,15 +19,12 @@ __metaclass__ = type import copy import os -import time import uuid import hashlib -import sys -import re from ansible.errors import AnsibleError from ansible.module_utils._text import to_text, to_bytes -from ansible.module_utils.connection import Connection +from ansible.module_utils.connection import Connection, ConnectionError from ansible.plugins.action import ActionBase from ansible.module_utils.six.moves.urllib.parse import urlsplit from ansible.utils.display import Display @@ -38,7 +35,6 @@ display = Display() class ActionModule(ActionBase): def run(self, tmp=None, task_vars=None): - changed = True socket_path = None play_context = copy.deepcopy(self._play_context) play_context.network_os = self._get_network_os(task_vars) @@ -52,7 +48,7 @@ class ActionModule(ActionBase): return result try: - src = self._task.args.get('src') + src = self._task.args['src'] except KeyError as exc: return {'failed': True, 'msg': 'missing required argument: %s' % exc} @@ -106,15 +102,14 @@ class ActionModule(ActionBase): try: changed = self._handle_existing_file(conn, output_file, dest, proto, sock_timeout) if changed is False: - result['changed'] = False + result['changed'] = changed result['destination'] = dest return result except Exception as exc: - result['msg'] = ('Warning: Exc %s idempotency check failed. Check' - 'dest' % exc) + result['msg'] = ('Warning: %s idempotency check failed. Check dest' % exc) try: - out = conn.copy_file( + conn.copy_file( source=output_file, destination=dest, proto=proto, timeout=sock_timeout ) @@ -126,7 +121,7 @@ class ActionModule(ActionBase): result['msg'] = 'Warning: iosxr scp server pre close issue. Please check dest' else: result['failed'] = True - result['msg'] = ('Exception received : %s' % exc) + result['msg'] = 'Exception received: %s' % exc if mode == 'text': # Cleanup tmp file expanded wih ansible vars @@ -137,35 +132,34 @@ class ActionModule(ActionBase): return result def _handle_existing_file(self, conn, source, dest, proto, timeout): + """ + Determines whether the source and destination file match. + + :return: False if source and dest both exist and have matching sha1 sums, True otherwise. + """ cwd = self._loader.get_basedir() filename = str(uuid.uuid4()) - source_file = os.path.join(cwd, filename) + tmp_source_file = os.path.join(cwd, filename) try: - out = conn.get_file( - source=dest, destination=source_file, + conn.get_file( + source=dest, destination=tmp_source_file, proto=proto, timeout=timeout ) - except Exception as exc: - pattern = to_text(exc) - not_found_exc = "No such file or directory" - if re.search(not_found_exc, pattern, re.I): - if os.path.exists(source_file): - os.remove(source_file) + except ConnectionError as exc: + error = to_text(exc) + if error.endswith("No such file or directory"): + if os.path.exists(tmp_source_file): + os.remove(tmp_source_file) return True - else: - try: - os.remove(source_file) - except OSError as osex: - raise Exception(osex) try: with open(source, 'r') as f: new_content = f.read() - with open(source_file, 'r') as f: + with open(tmp_source_file, 'r') as f: old_content = f.read() - except (IOError, OSError) as ioexc: - os.remove(source_file) - raise IOError(ioexc) + except (IOError, OSError): + os.remove(tmp_source_file) + raise sha1 = hashlib.sha1() old_content_b = to_bytes(old_content, errors='surrogate_or_strict') @@ -176,11 +170,10 @@ class ActionModule(ActionBase): new_content_b = to_bytes(new_content, errors='surrogate_or_strict') sha1.update(new_content_b) checksum_new = sha1.digest() - os.remove(source_file) + os.remove(tmp_source_file) if checksum_old == checksum_new: return False - else: - return True + return True def _get_binary_src_file(self, src): working_path = self._get_working_path() diff --git a/lib/ansible/plugins/cliconf/__init__.py b/lib/ansible/plugins/cliconf/__init__.py index 97f1feda931..b4dd9e57853 100644 --- a/lib/ansible/plugins/cliconf/__init__.py +++ b/lib/ansible/plugins/cliconf/__init__.py @@ -365,8 +365,12 @@ class CliconfBase(AnsiblePlugin): if proto == 'scp': if not HAS_SCP: raise AnsibleError("Required library scp is not installed. Please install it using `pip install scp`") - with SCPClient(ssh.get_transport(), socket_timeout=timeout) as scp: - scp.get(source, destination) + try: + with SCPClient(ssh.get_transport(), socket_timeout=timeout) as scp: + scp.get(source, destination) + except EOFError: + # This appears to be benign. + pass elif proto == 'sftp': with ssh.open_sftp() as sftp: sftp.get(source, destination) diff --git a/lib/ansible/plugins/connection/network_cli.py b/lib/ansible/plugins/connection/network_cli.py index 6611d91be51..d5937f3a7e9 100644 --- a/lib/ansible/plugins/connection/network_cli.py +++ b/lib/ansible/plugins/connection/network_cli.py @@ -318,7 +318,7 @@ class Connection(NetworkConnectionBase): self._terminal = None self.cliconf = None - self.paramiko_conn = None + self._paramiko_conn = None if self._play_context.verbosity > 3: logging.getLogger('paramiko').setLevel(logging.DEBUG) @@ -341,6 +341,13 @@ class Connection(NetworkConnectionBase): ) self.queue_message('log', 'network_os is set to %s' % self._network_os) + @property + def paramiko_conn(self): + if self._paramiko_conn is None: + self._paramiko_conn = connection_loader.get('paramiko', self._play_context, '/dev/null') + self._paramiko_conn.set_options(direct={'look_for_keys': not bool(self._play_context.password and not self._play_context.private_key_file)}) + return self._paramiko_conn + def _get_log_channel(self): name = "p=%s u=%s | " % (os.getpid(), getpass.getuser()) name += "paramiko [%s]" % self._play_context.remote_addr @@ -405,9 +412,7 @@ class Connection(NetworkConnectionBase): Connects to the remote device and starts the terminal ''' if not self.connected: - self.paramiko_conn = connection_loader.get('paramiko', self._play_context, '/dev/null') self.paramiko_conn._set_log_channel(self._get_log_channel()) - self.paramiko_conn.set_options(direct={'look_for_keys': not bool(self._play_context.password and not self._play_context.private_key_file)}) self.paramiko_conn.force_persistence = self.force_persistence command_timeout = self.get_option('persistent_command_timeout') @@ -474,7 +479,7 @@ class Connection(NetworkConnectionBase): self.queue_message('debug', "cli session is now closed") self.paramiko_conn.close() - self.paramiko_conn = None + self._paramiko_conn = None self.queue_message('debug', "ssh connection has been closed successfully") super(Connection, self).close() diff --git a/test/units/plugins/connection/test_network_cli.py b/test/units/plugins/connection/test_network_cli.py index a625449a72e..2268721bf77 100644 --- a/test/units/plugins/connection/test_network_cli.py +++ b/test/units/plugins/connection/test_network_cli.py @@ -77,13 +77,13 @@ class TestConnectionClass(unittest.TestCase): terminal = MagicMock(supports_multiplexing=False) conn._terminal = terminal conn._ssh_shell = MagicMock() - conn.paramiko_conn = MagicMock() + conn._paramiko_conn = MagicMock() conn._connected = True conn.close() self.assertTrue(terminal.on_close_shell.called) self.assertIsNone(conn._ssh_shell) - self.assertIsNone(conn.paramiko_conn) + self.assertIsNone(conn._paramiko_conn) @patch("ansible.plugins.connection.paramiko_ssh.Connection._connect") def test_network_cli_exec_command(self, mocked_super):