[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

(cherry picked from commit 97b15e9f0c4ddf419ca597e1b6769c29053bc06f)

3 files changed, 45 insertions, 17 deletions

diff --git a/changelogs/fragments/fixnxos_file_copy29.yaml b/changelogs/fragments/fixnxos_file_copy29.yaml
new file mode 100644
index 0000000000..639ae24444
--- /dev/null
+++ b/changelogs/fragments/fixnxos_file_copy29.yaml
@@ -0,0 +1,2 @@
+bugfixes:
+- Stabilize nxos initiated copy for nxos_file_copy plugin (https://github.com/ansible/ansible/pull/62355).
diff --git a/lib/ansible/plugins/action/nxos_file_copy.py b/lib/ansible/plugins/action/nxos_file_copy.py
index 43388fe468..c9cdcaa21d 100644
--- a/lib/ansible/plugins/action/nxos_file_copy.py
+++ b/lib/ansible/plugins/action/nxos_file_copy.py
@@ -21,10 +21,7 @@ import copy
 import hashlib
 import os
 import re
-import sys
 import time
-import traceback
-import uuid
 
 from ansible.errors import AnsibleError
 from ansible.module_utils._text import to_text, to_bytes
@@ -106,14 +103,12 @@ class ActionModule(ActionBase):
                 raise AnsibleError('Playbook parameter <remote_scp_server> required when <file_pull> 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>, <remote_scp_server_user>, ,remote_scp_server_password>'
-                raise AnsibleError('Playbook parameters {0} must all be set together'.format(params))
+                        playvals['remote_scp_server_user']):
+                params = '<remote_scp_server>, <remote_scp_server_user>'
+                raise AnsibleError('Playbook parameters {0} must be set together'.format(params))
 
         return playvals
 
@@ -294,8 +289,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.*#',
@@ -350,27 +345,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.
@@ -384,6 +393,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 + '/'
@@ -398,7 +408,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 <remote_scp_server_password> 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')
@@ -408,14 +423,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 606633f0b1..df4d4eadc8 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>, <remote_scp_server_user>, ,remote_scp_server_password> must all be set together')
+      - result is search('Playbook parameters <remote_scp_server>, <remote_scp_server_user> must be set together')
 
 - debug: msg="END nxos_file_copy input_validation test"