From 7d3c4a88823846cbcea7c61de38658a6d63d4265 Mon Sep 17 00:00:00 2001 From: Nathaniel Case Date: Wed, 14 Aug 2019 16:58:03 -0400 Subject: [PATCH] Delay persistent connection until needed (#59153) * Delay calling connect() until absolutely necessary * Implement transport_test to enable wait_for_connection * plugin might be connected already for some reason? * ensure_connect for httpapi There's some become shenanigans still needing to be ironed out * Fix tests for network_cli --- .../scripts/ansible_connection_cli_stub.py | 5 +--- lib/ansible/plugins/connection/httpapi.py | 3 ++- lib/ansible/plugins/connection/network_cli.py | 27 +++++++++++++++---- .../plugins/connection/test_network_cli.py | 7 ++--- 4 files changed, 29 insertions(+), 13 deletions(-) diff --git a/lib/ansible/cli/scripts/ansible_connection_cli_stub.py b/lib/ansible/cli/scripts/ansible_connection_cli_stub.py index 2b4855410ec..7351457e562 100755 --- a/lib/ansible/cli/scripts/ansible_connection_cli_stub.py +++ b/lib/ansible/cli/scripts/ansible_connection_cli_stub.py @@ -101,12 +101,9 @@ class ConnectionProcess(object): ansible_playbook_pid=self._ansible_playbook_pid) self.connection.set_options(var_options=variables) - self.connection._connect() - self.connection._socket_path = self.socket_path self.srv.register(self.connection) messages.extend([('vvvv', msg) for msg in sys.stdout.getvalue().splitlines()]) - messages.append(('vvvv', 'connection to remote device started successfully')) self.sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) self.sock.bind(self.socket_path) @@ -123,7 +120,7 @@ class ConnectionProcess(object): def run(self): try: - while self.connection.connected: + while True: signal.signal(signal.SIGALRM, self.connect_timeout) signal.signal(signal.SIGTERM, self.handler) signal.alarm(self.connection.get_option('persistent_connect_timeout')) diff --git a/lib/ansible/plugins/connection/httpapi.py b/lib/ansible/plugins/connection/httpapi.py index 254cbd624a0..f47163c2902 100644 --- a/lib/ansible/plugins/connection/httpapi.py +++ b/lib/ansible/plugins/connection/httpapi.py @@ -174,7 +174,7 @@ from ansible.module_utils.six.moves.urllib.error import HTTPError, URLError from ansible.module_utils.urls import open_url from ansible.playbook.play_context import PlayContext from ansible.plugins.loader import httpapi_loader -from ansible.plugins.connection import NetworkConnectionBase +from ansible.plugins.connection import NetworkConnectionBase, ensure_connect class Connection(NetworkConnectionBase): @@ -250,6 +250,7 @@ class Connection(NetworkConnectionBase): super(Connection, self).close() + @ensure_connect def send(self, path, data, **kwargs): ''' Sends the command to the device over api diff --git a/lib/ansible/plugins/connection/network_cli.py b/lib/ansible/plugins/connection/network_cli.py index 94db7a9d25c..32947664193 100644 --- a/lib/ansible/plugins/connection/network_cli.py +++ b/lib/ansible/plugins/connection/network_cli.py @@ -209,7 +209,7 @@ from ansible.module_utils.six.moves import cPickle from ansible.module_utils.network.common.utils import to_list from ansible.module_utils._text import to_bytes, to_text from ansible.playbook.play_context import PlayContext -from ansible.plugins.connection import NetworkConnectionBase +from ansible.plugins.connection import NetworkConnectionBase, ensure_connect from ansible.plugins.loader import cliconf_loader, terminal_loader, connection_loader @@ -326,8 +326,8 @@ class Connection(NetworkConnectionBase): self.paramiko_conn.force_persistence = self.force_persistence ssh = self.paramiko_conn._connect() - host = self.get_option('host') self.queue_message('vvvv', 'ssh connection done, setting terminal') + self._connected = True self._ssh_shell = ssh.ssh.invoke_shell() self._ssh_shell.settimeout(self.get_option('persistent_command_timeout')) @@ -338,8 +338,11 @@ class Connection(NetworkConnectionBase): self.queue_message('vvvv', 'loaded terminal plugin for network_os %s' % self._network_os) - self.receive(prompts=self._terminal.terminal_initial_prompt, answer=self._terminal.terminal_initial_answer, - newline=self._terminal.terminal_inital_prompt_newline) + self.receive( + prompts=self._terminal.terminal_initial_prompt, + answer=self._terminal.terminal_initial_answer, + newline=self._terminal.terminal_inital_prompt_newline + ) self.queue_message('vvvv', 'firing event: on_open_shell()') self._terminal.on_open_shell() @@ -350,7 +353,6 @@ class Connection(NetworkConnectionBase): self._terminal.on_become(passwd=auth_pass) self.queue_message('vvvv', 'ssh connection has completed successfully') - self._connected = True return self @@ -450,6 +452,7 @@ class Connection(NetworkConnectionBase): else: command_prompt_matched = True + @ensure_connect def send(self, command, prompt=None, answer=None, newline=True, sendonly=False, prompt_retry_check=False, check_all=False): ''' Sends the command to the device in the opened shell @@ -587,3 +590,17 @@ class Connection(NetworkConnectionBase): def _validate_timeout_value(self, timeout, timer_name): if timeout < 0: raise AnsibleConnectionFailure("'%s' timer value '%s' is invalid, value should be greater than or equal to zero." % (timer_name, timeout)) + + def transport_test(self, connect_timeout): + """This method enables wait_for_connection to work. + + As it is used by wait_for_connection, it is called by that module's action plugin, + which is on the controller process, which means that nothing done on this instance + should impact the actual persistent connection... this check is for informational + purposes only and should be properly cleaned up. + """ + + # Force a fresh connect if for some reason we have connected before. + self.close() + self._connect() + self.close() diff --git a/test/units/plugins/connection/test_network_cli.py b/test/units/plugins/connection/test_network_cli.py index 103ee99a76d..6dc8cd292c8 100644 --- a/test/units/plugins/connection/test_network_cli.py +++ b/test/units/plugins/connection/test_network_cli.py @@ -111,7 +111,8 @@ class TestConnectionClass(unittest.TestCase): self.assertEqual(out, b'command response') mock_send.assert_called_with(command=b'command') - def test_network_cli_send(self): + @patch("ansible.plugins.connection.network_cli.Connection._connect") + def test_network_cli_send(self, mocked_connect): pc = PlayContext() pc.network_os = 'ios' conn = connection_loader.get('network_cli', pc, '/dev/null') @@ -131,7 +132,7 @@ class TestConnectionClass(unittest.TestCase): """ mock__shell.recv.side_effect = [response, None] - output = conn.send(b'command', None, None, None) + output = conn.send(b'command') mock__shell.sendall.assert_called_with(b'command\r') self.assertEqual(to_text(conn._command_response), 'command response') @@ -140,5 +141,5 @@ class TestConnectionClass(unittest.TestCase): mock__shell.recv.side_effect = [b"ERROR: error message device#"] with self.assertRaises(AnsibleConnectionFailure) as exc: - conn.send(b'command', None, None, None) + conn.send(b'command') self.assertEqual(str(exc.exception), 'ERROR: error message device#')