From 6a18075640cd97174e561e446e7aedac2228c1e8 Mon Sep 17 00:00:00 2001 From: Dietmar Schinnerl Date: Sat, 11 Aug 2012 15:13:07 +0200 Subject: [PATCH 1/6] Issue #848: Closing stdin after we read from stdout --- lib/ansible/runner/connection/ssh.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ansible/runner/connection/ssh.py b/lib/ansible/runner/connection/ssh.py index 8c84246e4ed..aaf429c54f1 100644 --- a/lib/ansible/runner/connection/ssh.py +++ b/lib/ansible/runner/connection/ssh.py @@ -98,12 +98,12 @@ class SSHConnection(object): stdout=subprocess.PIPE, stderr=subprocess.STDOUT) # We can't use p.communicate here because the ControlMaster may have stdout open as well - p.stdin.close() stdout = '' while p.poll() is None: rfd, wfd, efd = select.select([p.stdout], [], [p.stdout], 1) if p.stdout in rfd: stdout += os.read(p.stdout.fileno(), 1024) + p.stdin.close() # close stdin after we read from stdout (see also issue #848) # older versions of ssh generate this error which we ignore stdout=stdout.replace("tcgetattr: Invalid argument\n", "") # suppress Ubuntu 10.04/12.04 error on -tt option From bfed87df803f0bf65aac3d40eb2e04c9bfdab1a8 Mon Sep 17 00:00:00 2001 From: Dietmar Schinnerl Date: Sat, 11 Aug 2012 15:49:00 +0200 Subject: [PATCH 2/6] Removed leading blanks --- lib/ansible/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ansible/utils.py b/lib/ansible/utils.py index 9582ab9f823..e390070187c 100644 --- a/lib/ansible/utils.py +++ b/lib/ansible/utils.py @@ -307,10 +307,10 @@ def _gitinfo(): branch = f.readline().split('/')[-1].rstrip("\n") branch_path = os.path.join(repo_path, "refs", "heads", branch) with open(branch_path) as f: - commit = f.readline()[:10] + commit = f.readline()[:10] date = time.localtime(os.stat(branch_path).st_mtime) offset = time.timezone if (time.daylight == 0) else time.altzone - result = "({0} {1}) last updated {2} (GMT {3:+04d})".format(branch, commit, + result = "({0} {1}) last updated {2} (GMT {3:+04d})".format(branch, commit, time.strftime("%Y/%m/%d %H:%M:%S", date), offset / -36) return result From 993bb5c6f1d142aeff549328bb21eb8d5c2298e7 Mon Sep 17 00:00:00 2001 From: Dietmar Schinnerl Date: Sat, 11 Aug 2012 15:55:14 +0200 Subject: [PATCH 3/6] Added stub implementation of filters --- lib/ansible/runner/__init__.py | 4 ++-- lib/ansible/utils.py | 10 +++++++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/ansible/runner/__init__.py b/lib/ansible/runner/__init__.py index a1e566ce8ea..a31b34faf0e 100644 --- a/lib/ansible/runner/__init__.py +++ b/lib/ansible/runner/__init__.py @@ -655,7 +655,7 @@ class Runner(object): cmd = " || ".join(md5s) cmd = "%s; %s || (echo \"${rc} %s\")" % (test, cmd, path) - return self._low_level_exec_command(conn, cmd, tmp, sudoable=False).split()[0] + return utils.last_non_blank_line(self._low_level_exec_command(conn, cmd, tmp, sudoable=False)).split()[0] # ***************************************************** @@ -675,7 +675,7 @@ class Runner(object): cmd += ' && echo %s' % basetmp result = self._low_level_exec_command(conn, cmd, None, sudoable=False) - return result.split("\n")[0].strip() + '/' + return utils.last_non_blank_line(result.split("\n"))[0].strip() + '/' # ***************************************************** diff --git a/lib/ansible/utils.py b/lib/ansible/utils.py index e390070187c..7b2ef864b90 100644 --- a/lib/ansible/utils.py +++ b/lib/ansible/utils.py @@ -117,9 +117,11 @@ def json_loads(data): return json.loads(data) -def parse_json(data): +def parse_json(raw_data): ''' this version for module return data only ''' + data = filter_leading_garbage(raw_data) + try: return json.loads(data) except: @@ -409,3 +411,9 @@ def do_encrypt(result, encrypt, salt_size=None, salt=None): return result +def last_non_blank_line(lines): + return lines + +def filter_leading_garbage(lines): + return lines + From fbdddc7c747ce607b5c65cb40655b0701ba2eb1f Mon Sep 17 00:00:00 2001 From: Dietmar Schinnerl Date: Sat, 11 Aug 2012 16:14:19 +0200 Subject: [PATCH 4/6] Added utils.last_non_blank_line --- lib/ansible/runner/__init__.py | 4 ++-- lib/ansible/utils.py | 13 ++++++++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/lib/ansible/runner/__init__.py b/lib/ansible/runner/__init__.py index a31b34faf0e..5462b94628c 100644 --- a/lib/ansible/runner/__init__.py +++ b/lib/ansible/runner/__init__.py @@ -655,7 +655,7 @@ class Runner(object): cmd = " || ".join(md5s) cmd = "%s; %s || (echo \"${rc} %s\")" % (test, cmd, path) - return utils.last_non_blank_line(self._low_level_exec_command(conn, cmd, tmp, sudoable=False)).split()[0] + return utils.last_non_blank_line(self._low_level_exec_command(conn, cmd, tmp, sudoable=False)) # ***************************************************** @@ -675,7 +675,7 @@ class Runner(object): cmd += ' && echo %s' % basetmp result = self._low_level_exec_command(conn, cmd, None, sudoable=False) - return utils.last_non_blank_line(result.split("\n"))[0].strip() + '/' + return utils.last_non_blank_line(result).strip() + '/' # ***************************************************** diff --git a/lib/ansible/utils.py b/lib/ansible/utils.py index 7b2ef864b90..2e1109adfeb 100644 --- a/lib/ansible/utils.py +++ b/lib/ansible/utils.py @@ -412,7 +412,18 @@ def do_encrypt(result, encrypt, salt_size=None, salt=None): return result def last_non_blank_line(lines): - return lines + all_lines = lines.splitlines() + all_lines.reverse() + for line in all_lines: + if (len(line) > 0): + return line + + return "" # we shouldn't come here (no lines?) but let's pretend nothing happend + # We can't return all lines here because calling code expects only one + # line. + +def line_needs_filtering(line): + return line.startswith('#') or (len(line) == 0) def filter_leading_garbage(lines): return lines From 6b622beb4dd2430e81b7045730c01f7462f58116 Mon Sep 17 00:00:00 2001 From: Dietmar Schinnerl Date: Sat, 11 Aug 2012 16:24:16 +0200 Subject: [PATCH 5/6] Added filtering of non-JSON lines. --- lib/ansible/utils.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/lib/ansible/utils.py b/lib/ansible/utils.py index 2e1109adfeb..9c75cc19d4b 100644 --- a/lib/ansible/utils.py +++ b/lib/ansible/utils.py @@ -120,7 +120,7 @@ def json_loads(data): def parse_json(raw_data): ''' this version for module return data only ''' - data = filter_leading_garbage(raw_data) + data = filter_leading_non_json_lines(raw_data) try: return json.loads(data) @@ -420,11 +420,20 @@ def last_non_blank_line(lines): return "" # we shouldn't come here (no lines?) but let's pretend nothing happend # We can't return all lines here because calling code expects only one + # line. And since we don't know which line to return we return an empty # line. -def line_needs_filtering(line): - return line.startswith('#') or (len(line) == 0) +def is_valid_json_line(line): + return line.startswith('=') or line.startswith('{') or line.startswith('[') -def filter_leading_garbage(lines): - return lines +def filter_leading_non_json_lines(lines): + ''' we need to filter anything which starts not with '{', '[', ', '=' or is an empty line. + But we filter only leading lines since multiline JSON is valid. ''' + filtered_lines = '' + no_more_filtering = False + for line in lines.splitlines(): + if (no_more_filtering or is_valid_json_line(line)): + no_more_filtering = True + filtered_lines += line + '\n' + return filtered_lines From ac44c36e4f6b450da7b6c0b3f915a235e167a015 Mon Sep 17 00:00:00 2001 From: Dietmar Schinnerl Date: Sat, 11 Aug 2012 16:57:04 +0200 Subject: [PATCH 6/6] Removed unnecessary string replacements since parse_json already filters garbage lines --- lib/ansible/runner/connection/ssh.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/ansible/runner/connection/ssh.py b/lib/ansible/runner/connection/ssh.py index aaf429c54f1..51195aa1f3f 100644 --- a/lib/ansible/runner/connection/ssh.py +++ b/lib/ansible/runner/connection/ssh.py @@ -60,7 +60,7 @@ class SSHConnection(object): ssh_cmd = ["ssh", "-tt", "-q"] + self.common_args + [self.host] if self.runner.sudo and sudoable: # Rather than detect if sudo wants a password this time, -k makes - # sudo always ask for a password if one is required. + # sudo always ask for a password if one is required. # Passing a quoted compound command to sudo (or sudo -s) # directly doesn't work, so we shellquote it with pipes.quote() # and pass the quoted string to the user's shell. We loop reading @@ -104,10 +104,6 @@ class SSHConnection(object): if p.stdout in rfd: stdout += os.read(p.stdout.fileno(), 1024) p.stdin.close() # close stdin after we read from stdout (see also issue #848) - # older versions of ssh generate this error which we ignore - stdout=stdout.replace("tcgetattr: Invalid argument\n", "") - # suppress Ubuntu 10.04/12.04 error on -tt option - stdout=stdout.replace("tcgetattr: Inappropriate ioctl for device\n","") if p.returncode != 0 and stdout.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="" before running again')