From 62e1c14edc408e3f0bdd82c6bb264a969ce98fe9 Mon Sep 17 00:00:00 2001 From: Ricardo Carrillo Cruz Date: Wed, 16 May 2018 14:59:01 +0200 Subject: [PATCH] Pull persistent connection parameters via get_option (#39367) * WIP Pull persistent connection parameters via get_option * Fix pep8 * Add use_persistent_connection setting to paramiko_ssh plugin * Add vars section to persistent_command_timeout setting and prevail provider values over config manager * Use persistent_command_timeout on network_cli instead of timeout * Fix unit tests If we don't call loader to get network_cli, then _load_name is never set and we get KeyError. * Pull persistent_command_timeout via config manager for ios connection local * Pull persistent_command_timeout via config manager on connection local --- bin/ansible-connection | 5 ++-- lib/ansible/executor/task_executor.py | 2 +- lib/ansible/plugins/action/eos.py | 5 +++- lib/ansible/plugins/action/ios.py | 5 +++- lib/ansible/plugins/action/iosxr.py | 5 +++- lib/ansible/plugins/action/junos.py | 5 +++- lib/ansible/plugins/action/nxos.py | 5 +++- lib/ansible/plugins/action/vyos.py | 5 +++- lib/ansible/plugins/connection/network_cli.py | 6 ++--- .../plugins/connection/paramiko_ssh.py | 12 ++++++++-- lib/ansible/plugins/connection/persistent.py | 24 +++++++++++++++---- .../plugins/connection/test_network_cli.py | 7 +++--- 12 files changed, 64 insertions(+), 22 deletions(-) diff --git a/bin/ansible-connection b/bin/ansible-connection index 958783181793cc..006bcd77878404 100755 --- a/bin/ansible-connection +++ b/bin/ansible-connection @@ -107,7 +107,7 @@ class ConnectionProcess(object): while self.connection.connected: signal.signal(signal.SIGALRM, self.connect_timeout) signal.signal(signal.SIGTERM, self.handler) - signal.alarm(C.PERSISTENT_CONNECT_TIMEOUT) + signal.alarm(self.connection.get_option('persistent_connect_timeout')) self.exception = None (s, addr) = self.sock.accept() @@ -141,7 +141,8 @@ class ConnectionProcess(object): self.shutdown() def connect_timeout(self, signum, frame): - display.display('persistent connection idle timeout triggered, timeout value is %s secs' % C.PERSISTENT_CONNECT_TIMEOUT, log_only=True) + display.display('persistent connection idle timeout triggered, timeout value is %s secs' + % self.connection.get_option('persistent_connect_timeout'), log_only=True) self.shutdown() def command_timeout(self, signum, frame): diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py index 2b49e2cf85aa7c..6ce6441ebf93fa 100644 --- a/lib/ansible/executor/task_executor.py +++ b/lib/ansible/executor/task_executor.py @@ -810,7 +810,7 @@ def _get_connection(self, variables, templar): self._play_context.set_options_from_plugin(connection) if any(((connection.supports_persistence and C.USE_PERSISTENT_CONNECTIONS), connection.force_persistence)): - self._play_context.timeout = C.PERSISTENT_COMMAND_TIMEOUT + self._play_context.timeout = connection.get_option('persistent_command_timeout') display.vvvv('attempting to start connection', host=self._play_context.remote_addr) display.vvvv('using connection plugin %s' % connection.transport, host=self._play_context.remote_addr) socket_path = self._start_connection() diff --git a/lib/ansible/plugins/action/eos.py b/lib/ansible/plugins/action/eos.py index c7a25cd3d29244..9f396aaf854350 100644 --- a/lib/ansible/plugins/action/eos.py +++ b/lib/ansible/plugins/action/eos.py @@ -66,7 +66,7 @@ def run(self, tmp=None, task_vars=None): pc.remote_user = provider['username'] or self._play_context.connection_user pc.password = provider['password'] or self._play_context.password pc.private_key_file = provider['ssh_keyfile'] or self._play_context.private_key_file - pc.timeout = int(provider['timeout'] or C.PERSISTENT_COMMAND_TIMEOUT) + pc.timeout = int(provider['timeout']) if provider['timeout'] else None pc.become = provider['authorize'] or False if pc.become: pc.become_method = 'enable' @@ -75,6 +75,9 @@ def run(self, tmp=None, task_vars=None): display.vvv('using connection plugin %s (was local)' % pc.connection, pc.remote_addr) connection = self._shared_loader_obj.connection_loader.get('persistent', pc, sys.stdin) + if connection._play_context.timeout is None: + connection._play_context.timeout = connection.get_option('persistent_command_timeout') + socket_path = connection.run() display.vvvv('socket_path: %s' % socket_path, pc.remote_addr) if not socket_path: diff --git a/lib/ansible/plugins/action/ios.py b/lib/ansible/plugins/action/ios.py index 1a501660867c6b..6fbe7dba9fb7dd 100644 --- a/lib/ansible/plugins/action/ios.py +++ b/lib/ansible/plugins/action/ios.py @@ -58,7 +58,7 @@ def run(self, tmp=None, task_vars=None): pc.remote_user = provider['username'] or self._play_context.connection_user pc.password = provider['password'] or self._play_context.password pc.private_key_file = provider['ssh_keyfile'] or self._play_context.private_key_file - pc.timeout = int(provider['timeout'] or C.PERSISTENT_COMMAND_TIMEOUT) + pc.timeout = int(provider['timeout']) if provider['timeout'] else None pc.become = provider['authorize'] or False if pc.become: pc.become_method = 'enable' @@ -67,6 +67,9 @@ def run(self, tmp=None, task_vars=None): display.vvv('using connection plugin %s (was local)' % pc.connection, pc.remote_addr) connection = self._shared_loader_obj.connection_loader.get('persistent', pc, sys.stdin) + if connection._play_context.timeout is None: + connection._play_context.timeout = connection.get_option('persistent_command_timeout') + socket_path = connection.run() display.vvvv('socket_path: %s' % socket_path, pc.remote_addr) if not socket_path: diff --git a/lib/ansible/plugins/action/iosxr.py b/lib/ansible/plugins/action/iosxr.py index 6c16a110c5f9fc..8b9e5eae82d8e5 100644 --- a/lib/ansible/plugins/action/iosxr.py +++ b/lib/ansible/plugins/action/iosxr.py @@ -61,11 +61,14 @@ def run(self, tmp=None, task_vars=None): pc.port = int(provider['port'] or self._play_context.port or 22) pc.remote_user = provider['username'] or self._play_context.connection_user pc.password = provider['password'] or self._play_context.password - pc.timeout = int(provider['timeout'] or C.PERSISTENT_COMMAND_TIMEOUT) + pc.timeout = int(provider['timeout']) if provider['timeout'] else None display.vvv('using connection plugin %s (was local)' % pc.connection, pc.remote_addr) connection = self._shared_loader_obj.connection_loader.get('persistent', pc, sys.stdin) + if connection._play_context.timeout is None: + connection._play_context.timeout = connection.get_option('persistent_command_timeout') + socket_path = connection.run() display.vvvv('socket_path: %s' % socket_path, pc.remote_addr) if not socket_path: diff --git a/lib/ansible/plugins/action/junos.py b/lib/ansible/plugins/action/junos.py index 87b2d6e2342da5..5f3cbafa3fa10b 100644 --- a/lib/ansible/plugins/action/junos.py +++ b/lib/ansible/plugins/action/junos.py @@ -72,11 +72,14 @@ def run(self, tmp=None, task_vars=None): pc.remote_user = provider['username'] or self._play_context.connection_user pc.password = provider['password'] or self._play_context.password pc.private_key_file = provider['ssh_keyfile'] or self._play_context.private_key_file - pc.timeout = int(provider['timeout'] or C.PERSISTENT_COMMAND_TIMEOUT) + pc.timeout = int(provider['timeout']) if provider['timeout'] else None display.vvv('using connection plugin %s (was local)' % pc.connection, pc.remote_addr) connection = self._shared_loader_obj.connection_loader.get('persistent', pc, sys.stdin) + if connection._play_context.timeout is None: + connection._play_context.timeout = connection.get_option('persistent_command_timeout') + socket_path = connection.run() display.vvvv('socket_path: %s' % socket_path, pc.remote_addr) if not socket_path: diff --git a/lib/ansible/plugins/action/nxos.py b/lib/ansible/plugins/action/nxos.py index d98afa256fe137..4b3663f8b321b9 100644 --- a/lib/ansible/plugins/action/nxos.py +++ b/lib/ansible/plugins/action/nxos.py @@ -69,7 +69,7 @@ def run(self, tmp=None, task_vars=None): pc.remote_user = provider['username'] or self._play_context.connection_user pc.password = provider['password'] or self._play_context.password pc.private_key_file = provider['ssh_keyfile'] or self._play_context.private_key_file - pc.timeout = int(provider['timeout'] or C.PERSISTENT_COMMAND_TIMEOUT) + pc.timeout = int(provider['timeout']) if provider['timeout'] else None pc.become = provider['authorize'] or False if pc.become: pc.become_method = 'enable' @@ -78,6 +78,9 @@ def run(self, tmp=None, task_vars=None): display.vvv('using connection plugin %s (was local)' % pc.connection, pc.remote_addr) connection = self._shared_loader_obj.connection_loader.get('persistent', pc, sys.stdin) + if connection._play_context.timeout is None: + connection._play_context.timeout = connection.get_option('persistent_command_timeout') + socket_path = connection.run() display.vvvv('socket_path: %s' % socket_path, pc.remote_addr) if not socket_path: diff --git a/lib/ansible/plugins/action/vyos.py b/lib/ansible/plugins/action/vyos.py index 102c6ecb117f72..383db5ed66a9cc 100644 --- a/lib/ansible/plugins/action/vyos.py +++ b/lib/ansible/plugins/action/vyos.py @@ -58,11 +58,14 @@ def run(self, tmp=None, task_vars=None): pc.remote_user = provider['username'] or self._play_context.connection_user pc.password = provider['password'] or self._play_context.password pc.private_key_file = provider['ssh_keyfile'] or self._play_context.private_key_file - pc.timeout = int(provider['timeout'] or C.PERSISTENT_COMMAND_TIMEOUT) + pc.timeout = int(provider['timeout']) if provider['timeout'] else None display.vvv('using connection plugin %s (was local)' % pc.connection, pc.remote_addr) connection = self._shared_loader_obj.connection_loader.get('persistent', pc, sys.stdin) + if connection._play_context.timeout is None: + connection._play_context.timeout = connection.get_option('persistent_command_timeout') + socket_path = connection.run() display.vvvv('socket_path: %s' % socket_path, pc.remote_addr) if not socket_path: diff --git a/lib/ansible/plugins/connection/network_cli.py b/lib/ansible/plugins/connection/network_cli.py index d5e9ca76b6328d..43208465de74bd 100644 --- a/lib/ansible/plugins/connection/network_cli.py +++ b/lib/ansible/plugins/connection/network_cli.py @@ -151,8 +151,8 @@ close default: 10 ini: - section: persistent_connection - key: persistent_command_timeout + - section: persistent_connection + key: command_timeout env: - name: ANSIBLE_PERSISTENT_COMMAND_TIMEOUT """ @@ -298,7 +298,7 @@ def _connect(self): display.vvvv('ssh connection done, setting terminal', host=self._play_context.remote_addr) self._ssh_shell = ssh.ssh.invoke_shell() - self._ssh_shell.settimeout(self._play_context.timeout) + self._ssh_shell.settimeout(self.get_option('persistent_command_timeout')) network_os = self._play_context.network_os if not network_os: diff --git a/lib/ansible/plugins/connection/paramiko_ssh.py b/lib/ansible/plugins/connection/paramiko_ssh.py index 60f96a7f2b3879..6ff21434c733dd 100644 --- a/lib/ansible/plugins/connection/paramiko_ssh.py +++ b/lib/ansible/plugins/connection/paramiko_ssh.py @@ -114,8 +114,16 @@ version_added: '2.5' - name: ansible_paramiko_host_key_checking version_added: '2.5' + use_persistent_connections: + description: 'Toggles the use of persistence for connections' + type: boolean + default: False + env: + - name: ANSIBLE_USE_PERSISTENT_CONNECTIONS + ini: + - section: defaults + key: use_persistent_connections # TODO: -#C.USE_PERSISTENT_CONNECTIONS: #timeout=self._play_context.timeout, """ @@ -188,7 +196,7 @@ def missing_host_key(self, client, hostname, key): fingerprint = hexlify(key.get_fingerprint()) ktype = key.get_name() - if C.USE_PERSISTENT_CONNECTIONS or self.connection.force_persistence: + if self.connection.get_option('use_persistent_connections') or self.connection.force_persistence: # don't print the prompt string since the user cannot respond # to the question anyway raise AnsibleError(AUTHENTICITY_MSG[1:92] % (hostname, ktype, fingerprint)) diff --git a/lib/ansible/plugins/connection/persistent.py b/lib/ansible/plugins/connection/persistent.py index ac2406f4c1a40f..1eb4d2d6aa0bda 100644 --- a/lib/ansible/plugins/connection/persistent.py +++ b/lib/ansible/plugins/connection/persistent.py @@ -6,12 +6,26 @@ __metaclass__ = type DOCUMENTATION = """ - author: Ansible Core Team - connection: persistent - short_description: Use a persistent unix socket for connection +author: Ansible Core Team +connection: persistent +short_description: Use a persistent unix socket for connection +description: + - This is a helper plugin to allow making other connections persistent. +version_added: "2.3" +options: + persistent_command_timeout: + type: int description: - - This is a helper plugin to allow making other connections persistent. - version_added: "2.3" + - Configures, in seconds, the amount of time to wait for a command to + return from the remote device. If this timer is exceeded before the + command returns, the connection plugin will raise an exception and + close + default: 10 + ini: + - section: persistent_connection + key: command_timeout + env: + - name: ANSIBLE_PERSISTENT_COMMAND_TIMEOUT """ import os import pty diff --git a/test/units/plugins/connection/test_network_cli.py b/test/units/plugins/connection/test_network_cli.py index aae1c3c9889eff..b939e24f1710cf 100644 --- a/test/units/plugins/connection/test_network_cli.py +++ b/test/units/plugins/connection/test_network_cli.py @@ -31,6 +31,7 @@ from ansible.errors import AnsibleConnectionFailure from ansible.playbook.play_context import PlayContext from ansible.plugins.connection import network_cli +from ansible.plugins.loader import connection_loader class TestConnectionClass(unittest.TestCase): @@ -40,7 +41,7 @@ def test_network_cli__connect_error(self, mocked_super): pc = PlayContext() new_stdin = StringIO() - conn = network_cli.Connection(pc, new_stdin) + conn = connection_loader.get('network_cli', pc, '/dev/null') conn.ssh = MagicMock() conn.receive = MagicMock() conn._terminal = MagicMock() @@ -52,7 +53,7 @@ def test_network_cli__invalid_os(self, mocked_super): pc = PlayContext() new_stdin = StringIO() - conn = network_cli.Connection(pc, new_stdin) + conn = connection_loader.get('network_cli', pc, '/dev/null') conn.ssh = MagicMock() conn.receive = MagicMock() conn._terminal = MagicMock() @@ -65,7 +66,7 @@ def test_network_cli__connect(self, mocked_super, mocked_terminal_loader): pc = PlayContext() new_stdin = StringIO() - conn = network_cli.Connection(pc, new_stdin) + conn = connection_loader.get('network_cli', pc, '/dev/null') pc.network_os = 'ios' conn.ssh = MagicMock()