From 988db558b30114bb6e2ff7e4fb7dfaacb6375ba3 Mon Sep 17 00:00:00 2001 From: Dag Wieers Date: Sun, 23 Dec 2012 01:44:00 +0100 Subject: [PATCH] Add return code and error output to raw module Since we use 'raw' heavily on equipment where 'command' and 'shell' are not (yet) working (and python may need to be installed first using raw) these improvements are necessary in order to write more complex scripts (with return code handling and separated stdout/stderr). This change includes the following changes: - exec_command() now returns the return code of the command - _low_level_exec_command() now returns a dict, including 'rc', 'stdout' and 'stderr' - all users of the above interfaces have been improved to make use of the above changes - all connection plugins have been modified to return rc and stderr - fix the newline problem (stdout and stderr would have excess newlines) In a future commit I intend to add assertions or error handling code to verify the return code in those places where it wasn't done. Since only the output was available, the return code was ignored, even though we expect them to be 0. --- lib/ansible/runner/__init__.py | 17 ++++++++++------- lib/ansible/runner/action_plugins/raw.py | 6 +++--- .../runner/connection_plugins/fireball.py | 2 +- lib/ansible/runner/connection_plugins/local.py | 2 +- .../runner/connection_plugins/paramiko_ssh.py | 2 +- lib/ansible/runner/connection_plugins/ssh.py | 17 ++++++++++++----- library/fireball | 2 +- library/raw | 10 +++++----- 8 files changed, 34 insertions(+), 24 deletions(-) diff --git a/lib/ansible/runner/__init__.py b/lib/ansible/runner/__init__.py index d41cf623ac..002625eda4 100644 --- a/lib/ansible/runner/__init__.py +++ b/lib/ansible/runner/__init__.py @@ -226,7 +226,7 @@ class Runner(object): if tmp.find("tmp") != -1 and C.DEFAULT_KEEP_REMOTE_FILES != '1': cmd = cmd + "; rm -rf %s >/dev/null 2>&1" % tmp res = self._low_level_exec_command(conn, cmd, tmp, sudoable=True) - return ReturnData(conn=conn, result=res) + return ReturnData(conn=conn, result=res['stdout']) # ***************************************************** @@ -442,19 +442,22 @@ class Runner(object): ''' execute a command string over SSH, return the output ''' sudo_user = self.sudo_user - stdin, stdout, stderr = conn.exec_command(cmd, tmp, sudo_user, sudoable=sudoable) + rc, stdin, stdout, stderr = conn.exec_command(cmd, tmp, sudo_user, sudoable=sudoable) if type(stdout) not in [ str, unicode ]: - out = "\n".join(stdout.readlines()) + out = ''.join(stdout.readlines()) else: out = stdout if type(stderr) not in [ str, unicode ]: - err = "\n".join(stderr.readlines()) + err = ''.join(stderr.readlines()) else: err = stderr - return out + err + if rc != None: + return dict(rc=rc, stdout=out, stderr=err ) + else: + return dict(stdout=out, stderr=err ) # ***************************************************** @@ -474,7 +477,7 @@ class Runner(object): cmd = " || ".join(md5s) cmd = "%s; %s || (echo \"${rc} %s\")" % (test, cmd, path) data = self._low_level_exec_command(conn, cmd, tmp, sudoable=False) - data2 = utils.last_non_blank_line(data) + data2 = utils.last_non_blank_line(data['stdout']) try: return data2.split()[0] except IndexError: @@ -502,7 +505,7 @@ class Runner(object): cmd += ' && echo %s' % basetmp result = self._low_level_exec_command(conn, cmd, None, sudoable=False) - rc = utils.last_non_blank_line(result).strip() + '/' + rc = utils.last_non_blank_line(result['stdout']).strip() + '/' return rc diff --git a/lib/ansible/runner/action_plugins/raw.py b/lib/ansible/runner/action_plugins/raw.py index 3e4dca7605..c683ac6431 100644 --- a/lib/ansible/runner/action_plugins/raw.py +++ b/lib/ansible/runner/action_plugins/raw.py @@ -34,7 +34,7 @@ class ActionModule(object): self.runner = runner def run(self, conn, tmp, module_name, module_args, inject): - return ReturnData(conn=conn, result=dict( - stdout=self.runner._low_level_exec_command(conn, module_args.encode('utf-8'), tmp, sudoable=True) - )) + return ReturnData(conn=conn, + result=self.runner._low_level_exec_command(conn, module_args.encode('utf-8'), tmp, sudoable=True) + ) diff --git a/lib/ansible/runner/connection_plugins/fireball.py b/lib/ansible/runner/connection_plugins/fireball.py index 5e88ce285f..c21cf0aa44 100644 --- a/lib/ansible/runner/connection_plugins/fireball.py +++ b/lib/ansible/runner/connection_plugins/fireball.py @@ -88,7 +88,7 @@ class Connection(object): response = utils.decrypt(self.key, response) response = utils.parse_json(response) - return ('', response.get('stdout',''), response.get('stderr','')) + return (response.get('rc',None), '', response.get('stdout',''), response.get('stderr','')) def put_file(self, in_path, out_path): diff --git a/lib/ansible/runner/connection_plugins/local.py b/lib/ansible/runner/connection_plugins/local.py index 2516cb2377..c7d720fa91 100644 --- a/lib/ansible/runner/connection_plugins/local.py +++ b/lib/ansible/runner/connection_plugins/local.py @@ -52,7 +52,7 @@ class Connection(object): p = subprocess.Popen(cmd, cwd=basedir, shell=True, stdin=None, stdout=subprocess.PIPE, stderr=subprocess.PIPE) stdout, stderr = p.communicate() - return ("", stdout, stderr) + return (p.returncode, '', stdout, stderr) def put_file(self, in_path, out_path): ''' transfer a file from local to local ''' diff --git a/lib/ansible/runner/connection_plugins/paramiko_ssh.py b/lib/ansible/runner/connection_plugins/paramiko_ssh.py index d6e7a5015d..4648269289 100644 --- a/lib/ansible/runner/connection_plugins/paramiko_ssh.py +++ b/lib/ansible/runner/connection_plugins/paramiko_ssh.py @@ -145,7 +145,7 @@ class Connection(object): except socket.timeout: raise errors.AnsibleError('ssh timed out waiting for sudo.\n' + sudo_output) - return (chan.makefile('wb', bufsize), chan.makefile('rb', bufsize), '') + return (chan.recv_exit_status(), chan.makefile('wb', bufsize), chan.makefile('rb', bufsize), chan.makefile_stderr('rb', bufsize)) def put_file(self, in_path, out_path): ''' transfer a file from local to remote ''' diff --git a/lib/ansible/runner/connection_plugins/ssh.py b/lib/ansible/runner/connection_plugins/ssh.py index 772ad46eca..6c85d68aed 100644 --- a/lib/ansible/runner/connection_plugins/ssh.py +++ b/lib/ansible/runner/connection_plugins/ssh.py @@ -108,11 +108,11 @@ class Connection(object): import pty master, slave = pty.openpty() p = subprocess.Popen(ssh_cmd, stdin=slave, - stdout=subprocess.PIPE, stderr=subprocess.STDOUT) + stdout=subprocess.PIPE, stderr=subprocess.PIPE) stdin = os.fdopen(master, 'w', 0) except: p = subprocess.Popen(ssh_cmd, stdin=subprocess.PIPE, - stdout=subprocess.PIPE, stderr=subprocess.STDOUT) + stdout=subprocess.PIPE, stderr=subprocess.PIPE) stdin = p.stdin self._send_password() @@ -137,22 +137,29 @@ class Connection(object): # We can't use p.communicate here because the ControlMaster may have stdout open as well stdout = '' + stderr = '' while True: - rfd, wfd, efd = select.select([p.stdout], [], [p.stdout], 1) + rfd, wfd, efd = select.select([p.stdout, p.stderr], [], [p.stdout, p.stderr], 1) if p.stdout in rfd: dat = os.read(p.stdout.fileno(), 9000) stdout += dat if dat == '': p.wait() break + elif p.stderr in rfd: + dat = os.read(p.stderr.fileno(), 9000) + stderr += dat + if dat == '': + p.wait() + break elif p.poll() is not None: break stdin.close() # close stdin after we read from stdout (see also issue #848) - if p.returncode != 0 and stdout.find('Bad configuration option: ControlPersist') != -1: + if p.returncode != 0 and stderr.find('Bad configuration option: ControlPersist') != -1: raise errors.AnsibleError('using -c ssh on certain older ssh versions may not support ControlPersist, set ANSIBLE_SSH_ARGS="" (or ansible_ssh_args in the config file) before running again') - return ('', stdout, '') + return (p.returncode, '', stdout, stderr) def put_file(self, in_path, out_path): ''' transfer a file from local to remote ''' diff --git a/library/fireball b/library/fireball index 16f3f58051..88248d6b00 100644 --- a/library/fireball +++ b/library/fireball @@ -159,7 +159,7 @@ def command(data): stderr = '' log("got stdout: %s" % stdout) - return dict(stdout=stdout, stderr=stderr) + return dict(rc=p.returncode, stdout=stdout, stderr=stderr) def fetch(data): if 'in_path' not in data: diff --git a/library/raw b/library/raw index 1b46f2f886..061d8eb066 100644 --- a/library/raw +++ b/library/raw @@ -13,11 +13,11 @@ description: all core modules require it. Another is speaking to any devices such as routers that do not have any Python installed. In any other case, using the M(shell) or M(command) module is much more appropriate. Arguments - given to M(raw) are run directly through the configured remote shell and - only output is returned. There is no error detection or change handler - support for this module + given to M(raw) are run directly through the configured remote shell. + Standard output, error output and return code are returned when + available. There is no change handler support for this module. examples: - - code: ansible newhost.example.com -m raw -a "yum -y install python-simplejson" - description: Example from C(/usr/bin/ansible) to bootstrap a legacy python 2.4 host + - description: Example from C(/usr/bin/ansible) to bootstrap a legacy python 2.4 host + code: ansible newhost.example.com -m raw -a "yum -y install python-simplejson" author: Michael DeHaan '''