From eb0e7e198bc558fe3d85c3e1fe1679721297e02f Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Fri, 4 Sep 2015 10:11:25 -0400 Subject: [PATCH 1/2] remove closing connections after every task, this goes against conneciton caching and was not expected behaviuor nor inhertited from v1 --- lib/ansible/executor/task_executor.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py index 1c6346d94a..8c8990a97d 100644 --- a/lib/ansible/executor/task_executor.py +++ b/lib/ansible/executor/task_executor.py @@ -128,13 +128,6 @@ class TaskExecutor: return result except AnsibleError as e: return dict(failed=True, msg=to_unicode(e, nonstring='simplerepr')) - finally: - try: - self._connection.close() - except AttributeError: - pass - except Exception as e: - debug("error closing connection: %s" % to_unicode(e)) def _get_loop_items(self): ''' From c17fbf2f1261576359121193702ea926b492236d Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Fri, 4 Sep 2015 09:30:31 -0400 Subject: [PATCH 2/2] simplify become testing and handling, we had drifted and were doulbe checking prompt, become and become_pass fixed tests to conform to new signature and now tests both with and w/o password now we are more explicit about self.prompt --- lib/ansible/playbook/play_context.py | 13 ++-- lib/ansible/plugins/connections/local.py | 2 +- .../plugins/connections/paramiko_ssh.py | 47 ++++++----- lib/ansible/plugins/connections/ssh.py | 77 +++++++++---------- test/units/playbook/test_play_context.py | 9 ++- 5 files changed, 77 insertions(+), 71 deletions(-) diff --git a/lib/ansible/playbook/play_context.py b/lib/ansible/playbook/play_context.py index 9888de6d84..55d7d99b5a 100644 --- a/lib/ansible/playbook/play_context.py +++ b/lib/ansible/playbook/play_context.py @@ -335,6 +335,7 @@ class PlayContext(Base): prompt = None success_key = None + self.prompt = None if executable is None: executable = C.DEFAULT_EXECUTABLE @@ -366,13 +367,14 @@ class PlayContext(Base): # directly doesn't work, so we shellquote it with pipes.quote() and pass the quoted # string to the user's shell. We loop reading output until we see the randomly-generated # sudo prompt set with the -p option. - prompt = '[sudo via ansible, key=%s] password: ' % randbits # force quick error if password is required but not supplied, should prevent sudo hangs. - if not self.become_pass: - flags += " -n " + if self.become_pass: + prompt = '[sudo via ansible, key=%s] password: ' % randbits + becomecmd = '%s %s -p "%s" -S -u %s %s -c %s' % (exe, flags, prompt, self.become_user, executable, success_cmd) + else: + becomecmd = '%s %s -n -S -u %s %s -c %s' % (exe, flags, self.become_user, executable, success_cmd) - becomecmd = '%s %s -S -p "%s" -u %s %s -c %s' % (exe, flags, prompt, self.become_user, executable, success_cmd) elif self.become_method == 'su': @@ -415,7 +417,8 @@ class PlayContext(Base): else: raise AnsibleError("Privilege escalation method not found: %s" % self.become_method) - self.prompt = prompt + if self.become_pass: + self.prompt = prompt self.success_key = success_key return ('%s -c %s' % (executable, pipes.quote(becomecmd))) diff --git a/lib/ansible/plugins/connections/local.py b/lib/ansible/plugins/connections/local.py index e4eddbd4cb..e307798913 100644 --- a/lib/ansible/plugins/connections/local.py +++ b/lib/ansible/plugins/connections/local.py @@ -70,7 +70,7 @@ class Connection(ConnectionBase): ) self._display.debug("done running command with Popen()") - if self._play_context.prompt and self._play_context.become_pass and sudoable: + if self._play_context.prompt and sudoable: fcntl.fcntl(p.stdout, fcntl.F_SETFL, fcntl.fcntl(p.stdout, fcntl.F_GETFL) | os.O_NONBLOCK) fcntl.fcntl(p.stderr, fcntl.F_SETFL, fcntl.fcntl(p.stderr, fcntl.F_GETFL) | os.O_NONBLOCK) become_output = '' diff --git a/lib/ansible/plugins/connections/paramiko_ssh.py b/lib/ansible/plugins/connections/paramiko_ssh.py index a3491be938..6df6a3ebe0 100644 --- a/lib/ansible/plugins/connections/paramiko_ssh.py +++ b/lib/ansible/plugins/connections/paramiko_ssh.py @@ -224,33 +224,32 @@ class Connection(ConnectionBase): try: chan.exec_command(cmd) if self._play_context.prompt: - if self._play_context.become and self._play_context.become_pass: - passprompt = False - while True: - self._display.debug('Waiting for Privilege Escalation input') - if self.check_become_success(become_output): - break - elif self.check_password_prompt(become_output): - passprompt = True - break + passprompt = False + while True: + self._display.debug('Waiting for Privilege Escalation input') + if self.check_become_success(become_output): + break + elif self.check_password_prompt(become_output): + passprompt = True + break - chunk = chan.recv(bufsize) - self._display.debug("chunk is: %s" % chunk) - if not chunk: - if 'unknown user' in become_output: - raise AnsibleError( 'user %s does not exist' % become_user) - else: - break - #raise AnsibleError('ssh connection closed waiting for password prompt') - become_output += chunk - if passprompt: - if self._play_context.become and self._play_context.become_pass: - chan.sendall(self._play_context.become_pass + '\n') + chunk = chan.recv(bufsize) + self._display.debug("chunk is: %s" % chunk) + if not chunk: + if 'unknown user' in become_output: + raise AnsibleError( 'user %s does not exist' % become_user) else: - raise AnsibleError("A password is reqired but none was supplied") + break + #raise AnsibleError('ssh connection closed waiting for password prompt') + become_output += chunk + if passprompt: + if self._play_context.become and self._play_context.become_pass: + chan.sendall(self._play_context.become_pass + '\n') else: - no_prompt_out += become_output - no_prompt_err += become_output + raise AnsibleError("A password is reqired but none was supplied") + else: + no_prompt_out += become_output + no_prompt_err += become_output except socket.timeout: raise AnsibleError('ssh timed out waiting for privilege escalation.\n' + become_output) diff --git a/lib/ansible/plugins/connections/ssh.py b/lib/ansible/plugins/connections/ssh.py index ee912374a7..80da729ea7 100644 --- a/lib/ansible/plugins/connections/ssh.py +++ b/lib/ansible/plugins/connections/ssh.py @@ -378,54 +378,53 @@ class Connection(ConnectionBase): self._display.debug("Handling privilege escalation password prompt.") - if self._play_context.become and self._play_context.become_pass: - fcntl.fcntl(p.stdout, fcntl.F_SETFL, fcntl.fcntl(p.stdout, fcntl.F_GETFL) | os.O_NONBLOCK) - fcntl.fcntl(p.stderr, fcntl.F_SETFL, fcntl.fcntl(p.stderr, fcntl.F_GETFL) | os.O_NONBLOCK) + fcntl.fcntl(p.stdout, fcntl.F_SETFL, fcntl.fcntl(p.stdout, fcntl.F_GETFL) | os.O_NONBLOCK) + fcntl.fcntl(p.stderr, fcntl.F_SETFL, fcntl.fcntl(p.stderr, fcntl.F_GETFL) | os.O_NONBLOCK) - become_output = '' - become_errput = '' - passprompt = False - while True: - self._display.debug('Waiting for Privilege Escalation input') + become_output = '' + become_errput = '' + passprompt = False + while True: + self._display.debug('Waiting for Privilege Escalation input') - if self.check_become_success(become_output + become_errput): - self._display.debug('Succeded!') - break - elif self.check_password_prompt(become_output) or self.check_password_prompt(become_errput): - self._display.debug('Password prompt!') - passprompt = True - break + if self.check_become_success(become_output + become_errput): + self._display.debug('Succeded!') + break + elif self.check_password_prompt(become_output) or self.check_password_prompt(become_errput): + self._display.debug('Password prompt!') + passprompt = True + break - self._display.debug('Read next chunks') - rfd, wfd, efd = select.select([p.stdout, p.stderr], [], [p.stdout], self._play_context.timeout) - if not rfd: - # timeout. wrap up process communication - stdout, stderr = p.communicate() - raise AnsibleError('Connection error waiting for privilege escalation password prompt: %s' % become_output) + self._display.debug('Read next chunks') + rfd, wfd, efd = select.select([p.stdout, p.stderr], [], [p.stdout], self._play_context.timeout) + if not rfd: + # timeout. wrap up process communication + stdout, stderr = p.communicate() + raise AnsibleError('Connection error waiting for privilege escalation password prompt: %s' % become_output) - elif p.stderr in rfd: - chunk = p.stderr.read() - become_errput += chunk - self._display.debug('stderr chunk is: %s' % chunk) - self.check_incorrect_password(become_errput) + elif p.stderr in rfd: + chunk = p.stderr.read() + become_errput += chunk + self._display.debug('stderr chunk is: %s' % chunk) + self.check_incorrect_password(become_errput) - elif p.stdout in rfd: - chunk = p.stdout.read() - become_output += chunk - self._display.debug('stdout chunk is: %s' % chunk) + elif p.stdout in rfd: + chunk = p.stdout.read() + become_output += chunk + self._display.debug('stdout chunk is: %s' % chunk) - if not chunk: - break - #raise AnsibleError('Connection closed waiting for privilege escalation password prompt: %s ' % become_output) + if not chunk: + break + #raise AnsibleError('Connection closed waiting for privilege escalation password prompt: %s ' % become_output) - if passprompt: - self._display.debug("Sending privilege escalation password.") - stdin.write(self._play_context.become_pass + '\n') - else: - no_prompt_out = become_output - no_prompt_err = become_errput + if passprompt: + self._display.debug("Sending privilege escalation password.") + stdin.write(self._play_context.become_pass + '\n') + else: + no_prompt_out = become_output + no_prompt_err = become_errput (returncode, stdout, stderr) = self._communicate(p, stdin, in_data, sudoable=sudoable) diff --git a/test/units/playbook/test_play_context.py b/test/units/playbook/test_play_context.py index dcfc6df539..c1582a971a 100644 --- a/test/units/playbook/test_play_context.py +++ b/test/units/playbook/test_play_context.py @@ -116,7 +116,7 @@ class TestPlayContext(unittest.TestCase): default_cmd = "/bin/foo" default_exe = "/bin/bash" sudo_exe = C.DEFAULT_SUDO_EXE or 'sudo' - sudo_flags = C.DEFAULT_SUDO_FLAGS + " -n " + sudo_flags = C.DEFAULT_SUDO_FLAGS su_exe = C.DEFAULT_SU_EXE or 'su' su_flags = C.DEFAULT_SU_FLAGS or '' pbrun_exe = 'pbrun' @@ -134,7 +134,12 @@ class TestPlayContext(unittest.TestCase): play_context.become_method = 'sudo' cmd = play_context.make_become_cmd(cmd=default_cmd, executable="/bin/bash") - self.assertEqual(cmd, """%s -c '%s %s -S -p "%s" -u %s %s -c '"'"'echo %s; %s'"'"''""" % (default_exe, sudo_exe, sudo_flags, play_context.prompt, play_context.become_user, default_exe, play_context.success_key, default_cmd)) + self.assertEqual(cmd, """%s -c '%s %s -n -S -u %s %s -c '"'"'echo %s; %s'"'"''""" % (default_exe, sudo_exe, sudo_flags, play_context.become_user, default_exe, play_context.success_key, default_cmd)) + play_context.become_pass = 'testpass' + cmd = play_context.make_become_cmd(cmd=default_cmd, executable=default_exe) + self.assertEqual(cmd, """%s -c '%s %s -p "%s" -S -u %s %s -c '"'"'echo %s; %s'"'"''""" % (default_exe, sudo_exe, sudo_flags, play_context.prompt, play_context.become_user, default_exe, play_context.success_key, default_cmd)) + + play_context.become_pass = None play_context.become_method = 'su' cmd = play_context.make_become_cmd(cmd=default_cmd, executable="/bin/bash")