From 97b15e9f0c4ddf419ca597e1b6769c29053bc06f Mon Sep 17 00:00:00 2001 From: Mike Wiebe Date: Wed, 18 Sep 2019 07:37:50 -0400 Subject: [PATCH] [stable-2.9] Stabilize nxos initiated copy for nxos_file_copy plugin (#62355) * Retry spawn connection on failure * Add debug logs * Additional debug logs * Close session before respawn attempt * More debug info and increase loops * Remove debug info and reset error dict on reconnect * Add epdb debuger * Add epdb debuger * Add epdb debuger * Wait before sending password and close pexpect session * Fix comment typo * Scrub error logs * Scrub error logs * Add more specific initial connect pattern * Fix shippable errors * Dont make remote_scp_server_password a hard requirement * Add saftey check --- lib/ansible/plugins/action/nxos_file_copy.py | 56 ++++++++++++++----- .../tests/cli/input_validation.yaml | 2 +- 2 files changed, 44 insertions(+), 14 deletions(-) diff --git a/lib/ansible/plugins/action/nxos_file_copy.py b/lib/ansible/plugins/action/nxos_file_copy.py index 0942aee5968..2d96ae6d6ea 100644 --- a/lib/ansible/plugins/action/nxos_file_copy.py +++ b/lib/ansible/plugins/action/nxos_file_copy.py @@ -21,6 +21,7 @@ import copy import hashlib import os import re +import time from ansible.errors import AnsibleError from ansible.module_utils._text import to_text, to_bytes @@ -98,14 +99,12 @@ class ActionModule(ActionBase): raise AnsibleError('Playbook parameter required when is True') if playvals['remote_scp_server'] or \ - playvals['remote_scp_server_user'] or \ - playvals['remote_scp_server_password']: + playvals['remote_scp_server_user']: if None in (playvals['remote_scp_server'], - playvals['remote_scp_server_user'], - playvals['remote_scp_server_password']): - params = ', , ,remote_scp_server_password>' - raise AnsibleError('Playbook parameters {0} must all be set together'.format(params)) + playvals['remote_scp_server_user']): + params = ', ' + raise AnsibleError('Playbook parameters {0} must be set together'.format(params)) return playvals @@ -285,8 +284,8 @@ class ActionModule(ActionBase): # 14) - Copy completed without issues # 15) - nxos_router_prompt# # 16) - pexpect timeout - possible_outcomes = ['yes', - '(?i)Password', + possible_outcomes = [r'sure you want to continue connecting \(yes/no\)\? ', + '(?i)Password: ', 'file existing with this name', 'timed out', '(?i)No space.*#', @@ -341,27 +340,41 @@ class ActionModule(ActionBase): # Spawn pexpect connection to NX-OS device. nxos_session = pexpect.spawn('ssh ' + nxos_username + '@' + nxos_hostname + ' -p' + str(port)) # There might be multiple user_response_required prompts or intermittent timeouts - # spawning the expect session so loop up to 5 times during the spwan process. - for connect_attempt in range(6): + # spawning the expect session so loop up to 24 times during the spawn process. + max_attempts = 24 + for connect_attempt in range(max_attempts): outcome = process_outcomes(nxos_session) if outcome['user_response_required']: nxos_session.sendline('yes') continue if outcome['password_prompt_detected']: + time.sleep(3) nxos_session.sendline(nxos_password) continue if outcome['final_prompt_detected']: break if outcome['error'] or outcome['expect_timeout']: + # Error encountered, try to spawn expect session n more times up to max_attempts - 1 + if connect_attempt < max_attempts: + outcome['error'] = False + outcome['expect_timeout'] = False + nxos_session.close() + nxos_session = pexpect.spawn('ssh ' + nxos_username + '@' + nxos_hostname + ' -p' + str(port)) + continue self.results['failed'] = True + outcome['error_data'] = re.sub(nxos_password, '', outcome['error_data']) self.results['error_data'] = 'Failed to spawn expect session! ' + outcome['error_data'] + nxos_session.close() return else: # The before property will contain all text up to the expected string pattern. # The after string will contain the text that was matched by the expected pattern. msg = 'After {0} attempts, failed to spawn pexpect session to {1}' msg += 'BEFORE: {2}, AFTER: {3}' - raise AnsibleError(msg.format(connect_attempt, nxos_hostname, nxos_session.before, nxos_session.before)) + error_msg = msg.format(connect_attempt, nxos_hostname, nxos_session.before, nxos_session.after) + re.sub(nxos_password, '', error_msg) + nxos_session.close() + raise AnsibleError(error_msg) # Create local file directory under NX-OS filesystem if # local_file_directory playbook parameter is set. @@ -375,6 +388,7 @@ class ActionModule(ActionBase): if outcome['error'] or outcome['expect_timeout']: self.results['mkdir_cmd'] = mkdir_cmd self.results['failed'] = True + outcome['error_data'] = re.sub(nxos_password, '', outcome['error_data']) self.results['error_data'] = outcome['error_data'] return local_dir_root += each + '/' @@ -389,7 +403,12 @@ class ActionModule(ActionBase): nxos_session.sendline('yes') continue if outcome['password_prompt_detected']: - nxos_session.sendline(self.playvals['remote_scp_server_password']) + if self.playvals.get('remote_scp_server_password'): + nxos_session.sendline(self.playvals['remote_scp_server_password']) + else: + err_msg = 'Remote scp server {0} requires a password.'.format(rserver) + err_msg += ' Set the playbook parameter or configure nxos device for passwordless scp' + raise AnsibleError(err_msg) continue if outcome['existing_file_with_same_name']: nxos_session.sendline('y') @@ -399,14 +418,25 @@ class ActionModule(ActionBase): break if outcome['error'] or outcome['expect_timeout']: self.results['failed'] = True + outcome['error_data'] = re.sub(nxos_password, '', outcome['error_data']) + if self.playvals.get('remote_scp_server_password'): + outcome['error_data'] = re.sub(self.playvals['remote_scp_server_password'], '', outcome['error_data']) self.results['error_data'] = outcome['error_data'] + nxos_session.close() return else: # The before property will contain all text up to the expected string pattern. # The after string will contain the text that was matched by the expected pattern. msg = 'After {0} attempts, failed to copy file to {1}' msg += 'BEFORE: {2}, AFTER: {3}, CMD: {4}' - raise AnsibleError(msg.format(copy_attempt, nxos_hostname, nxos_session.before, nxos_session.before, copy_cmd)) + error_msg = msg.format(copy_attempt, nxos_hostname, nxos_session.before, nxos_session.before, copy_cmd) + re.sub(nxos_password, '', error_msg) + if self.playvals.get('remote_scp_server_password'): + re.sub(self.playvals['remote_scp_server_password'], '', error_msg) + nxos_session.close() + raise AnsibleError(error_msg) + + nxos_session.close() def file_pull(self): local_file = self.playvals['local_file'] diff --git a/test/integration/targets/nxos_file_copy/tests/cli/input_validation.yaml b/test/integration/targets/nxos_file_copy/tests/cli/input_validation.yaml index 606633f0b19..df4d4eadc82 100644 --- a/test/integration/targets/nxos_file_copy/tests/cli/input_validation.yaml +++ b/test/integration/targets/nxos_file_copy/tests/cli/input_validation.yaml @@ -60,6 +60,6 @@ - assert: that: - - result is search('Playbook parameters , , ,remote_scp_server_password> must all be set together') + - result is search('Playbook parameters , must be set together') - debug: msg="END nxos_file_copy input_validation test"