From f840c0d1673761f15ab0118625766573ee5bf73d Mon Sep 17 00:00:00 2001 From: Brad Olson Date: Mon, 9 Apr 2012 23:12:05 +0000 Subject: [PATCH 01/19] Wired in Michael's usage string optparse style. --- bin/ansible | 2 +- lib/ansible/utils.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/bin/ansible b/bin/ansible index 4d0a3b9c4b6..21a09588bdc 100755 --- a/bin/ansible +++ b/bin/ansible @@ -57,7 +57,7 @@ class Cli(object): parser = utils.make_parser( options, - usage='ansible [options]', + usage='%prog [options]', runas_opts=True, async_opts=True, output_opts=True, diff --git a/lib/ansible/utils.py b/lib/ansible/utils.py index 5eb5d07e5d0..ce2a682e294 100755 --- a/lib/ansible/utils.py +++ b/lib/ansible/utils.py @@ -284,7 +284,9 @@ def make_parser(add_options, constants=C, usage="", output_opts=False, runas_opt ) options.update(add_options) + #NOTE: optparse deprecated in Python >= 2.7. parser = optparse.OptionParser() + parser.set_usage(usage) names = sorted(options.keys()) for n in names: data = options[n].copy() From 74b26da9ac7ba0702caa288569c90940ce0a8486 Mon Sep 17 00:00:00 2001 From: Brad Olson Date: Tue, 10 Apr 2012 14:45:44 +0000 Subject: [PATCH 02/19] began playbook tweak --- bin/ansible-playbook | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/ansible-playbook b/bin/ansible-playbook index cb344ad92ac..955888b2ece 100755 --- a/bin/ansible-playbook +++ b/bin/ansible-playbook @@ -32,7 +32,7 @@ def main(args): ''' run ansible-playbook operations ''' # create parser for CLI options - usage = "ansible-playbook playbook.yml [options]" + usage = "%prog playbook.yml [options]" options = { '-e' : dict(long='--extra-vars', dest='extra_vars', help='pass in extra key=value variables from outside the playbook'), From 5a4d4bc051a237c37ee3ba921db2fec818cfec90 Mon Sep 17 00:00:00 2001 From: Brad Olson Date: Tue, 10 Apr 2012 15:17:25 +0000 Subject: [PATCH 03/19] Added usage info to bin/ansible-playbook, now shows options on bare command line. NOTE: bin/ansible outputs usage to stdout, bin/ansible-playbook to stderr. Should they be consistent? --- bin/ansible-playbook | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/bin/ansible-playbook b/bin/ansible-playbook index 955888b2ece..7badfb65cec 100755 --- a/bin/ansible-playbook +++ b/bin/ansible-playbook @@ -32,7 +32,7 @@ def main(args): ''' run ansible-playbook operations ''' # create parser for CLI options - usage = "%prog playbook.yml [options]" + usage = "%prog [options]" options = { '-e' : dict(long='--extra-vars', dest='extra_vars', help='pass in extra key=value variables from outside the playbook'), @@ -43,7 +43,9 @@ def main(args): options, args = parser.parse_args(args) if len(args) == 0: - print >> sys.stderr, "playbook path is a required argument" + parser.print_help(file=sys.stderr) + #QUESTION for M.D. This would match bin/ansible's behavior. Do we want them consistent? + #parser.print_help() return 1 sshpass = None From 8ae71cc7b133b86fdaf5a508b81cc4b39c34235c Mon Sep 17 00:00:00 2001 From: Seth Vidal Date: Tue, 10 Apr 2012 13:51:58 -0400 Subject: [PATCH 04/19] go back to using a normal optparser to add options instead of the dict interface. add very small subclass of OptionParser to sort the options so mdehaan is happy --- bin/ansible | 23 +++-------- bin/ansible-playbook | 15 ++++--- lib/ansible/utils.py | 97 +++++++++++++++++--------------------------- 3 files changed, 51 insertions(+), 84 deletions(-) diff --git a/bin/ansible b/bin/ansible index 4d0a3b9c4b6..268ad37f6e4 100755 --- a/bin/ansible +++ b/bin/ansible @@ -47,23 +47,12 @@ class Cli(object): def parse(self): ''' create an options parser for bin/ansible ''' - - options = { - '-a' : dict(long='--args', dest='module_args', - help="module arguments", default=C.DEFAULT_MODULE_ARGS), - '-m' : dict(long='--module-name', dest='module_name', - help="module name to execute", default=C.DEFAULT_MODULE_NAME) - } - - parser = utils.make_parser( - options, - usage='ansible [options]', - runas_opts=True, - async_opts=True, - output_opts=True, - - ) - + parser = utils.base_parser(constants=C, runas_opts=True, async_opts=True, + output_opts=True, usage='ansible [options]') + parser.add_option('-a', '--args', dest='module_args', + help="module arguments", default=C.DEFAULT_MODULE_ARGS) + parser.add_option('-m', '--module-name', dest='module_name', + help="module name to execute", default=C.DEFAULT_MODULE_NAME) options, args = parser.parse_args() self.callbacks.options = options diff --git a/bin/ansible-playbook b/bin/ansible-playbook index cb344ad92ac..34aafa6b9f2 100755 --- a/bin/ansible-playbook +++ b/bin/ansible-playbook @@ -32,14 +32,13 @@ def main(args): ''' run ansible-playbook operations ''' # create parser for CLI options - usage = "ansible-playbook playbook.yml [options]" - options = { - '-e' : dict(long='--extra-vars', dest='extra_vars', - help='pass in extra key=value variables from outside the playbook'), - '-O' : dict(long='--override-hosts', dest="override_hosts", default=None, - help="run playbook against only hosts, ignorning the inventory file") - } - parser = utils.make_parser(options, constants=C, usage=usage) + usage = "ans-playbook playbook.yml" + parser = utils.base_parser(constants=C, usage=usage) + parser.add_option('-e', '--extra-vars', dest='extra_vars', + help='arguments to pass to the inventory script') + parser.add_option('-O', '--override-hosts', dest="override_hosts", default=None, + help="run playbook against these hosts regardless of inventory settings") + options, args = parser.parse_args(args) if len(args) == 0: diff --git a/lib/ansible/utils.py b/lib/ansible/utils.py index 5eb5d07e5d0..450da6871c5 100755 --- a/lib/ansible/utils.py +++ b/lib/ansible/utils.py @@ -24,7 +24,7 @@ import re import jinja2 import yaml import optparse - +from operator import methodcaller try: import json except ImportError: @@ -273,70 +273,49 @@ def parse_kv(args): options[k]=v return options -def make_parser(add_options, constants=C, usage="", output_opts=False, runas_opts=False, async_opts=False): - ''' create an options parser w/ common options for any ansible program ''' - - options = base_parser_options( - constants=constants, - output_opts=output_opts, - runas_opts=runas_opts, - async_opts=async_opts - ) - options.update(add_options) - - parser = optparse.OptionParser() - names = sorted(options.keys()) - for n in names: - data = options[n].copy() - long = data['long'] - del data['long'] - parser.add_option(n, long, **data) - return parser - -def base_parser_options(constants=C, output_opts=False, runas_opts=False, async_opts=False): - ''' creates common options for ansible programs ''' - - options = { - '-D': dict(long='--debug', default=False, action="store_true", - help='show debug/verbose module output'), - '-f': dict(long='--forks', dest='forks', default=constants.DEFAULT_FORKS, type='int', - help='number of parallel processes to use'), - '-i': dict(long='--inventory-file', dest='inventory', - help='path to inventory host file', default=constants.DEFAULT_HOST_LIST), - '-k': dict(long='--ask-pass', default=False, action='store_true', - help='ask for SSH password'), - '-M': dict(long='--module-path', dest='module_path', - help="path to module library directory", default=constants.DEFAULT_MODULE_PATH), - '-T': dict(long='--timeout', default=constants.DEFAULT_TIMEOUT, type='int', - dest='timeout', help='set the SSH connection timeout in seconds'), - '-p': dict(long='--port', default=constants.DEFAULT_REMOTE_PORT, type='int', - dest='remote_port', help='use this remote SSH port'), - } +class SortedOptParser(optparse.OptionParser): + '''Optparser which sorts the options by opt before outputting --help''' + def format_help(self, formatter=None): + self.option_list.sort(key=methodcaller('get_opt_string')) + return optparse.OptionParser.format_help(self, formatter=None) + +def base_parser(constants=C, usage="", output_opts=False, runas_opts=False, async_opts=False): + ''' create an options parser for any ansible script ''' + + parser = SortedOptParser(usage) + parser.add_option('-D','--debug', default=False, action="store_true", + help='enable standard error debugging of modules.') + parser.add_option('-f','--forks', dest='forks', default=constants.DEFAULT_FORKS, type='int', + help='number of parallel processes to use') + parser.add_option('-i', '--inventory-file', dest='inventory', + help='inventory host file', default=constants.DEFAULT_HOST_LIST) + parser.add_option('-k', '--ask-pass', default=False, action='store_true', + help='ask for SSH password') + parser.add_option('-M', '--module-path', dest='module_path', + help="path to module library", default=constants.DEFAULT_MODULE_PATH) + parser.add_option('-T', '--timeout', default=constants.DEFAULT_TIMEOUT, type='int', + dest='timeout', help='set the SSH timeout in seconds') + parser.add_option('-p', '--port', default=constants.DEFAULT_REMOTE_PORT, type='int', + dest='remote_port', help='set the remote ssh port') if output_opts: - options.update({ - '-o' : dict(long='--one-line', dest='one_line', action='store_true', - help='condense output'), - '-t' : dict(long='--tree', dest='tree', default=None, - help='log results to this directory') - }) + parser.add_option('-o', '--one-line', dest='one_line', action='store_true', + help='condense output') + parser.add_option('-t', '--tree', dest='tree', default=None, + help='log output to this directory') if runas_opts: - options.update({ - '-s' : dict(long="--sudo", default=False, action="store_true", - dest='sudo', help="run operations with sudo (nopasswd)"), - '-u' : dict(long='--user', default=constants.DEFAULT_REMOTE_USER, - dest='remote_user', help='connect as this user'), - }) + parser.add_option("-s", "--sudo", default=False, action="store_true", + dest='sudo', help="run operations with sudo (nopasswd)") + parser.add_option('-u', '--user', default=constants.DEFAULT_REMOTE_USER, + dest='remote_user', help='connect as this user') if async_opts: - options.update({ - '-P' : dict(long='--poll', default=constants.DEFAULT_POLL_INTERVAL, type='int', - dest='poll_interval', help='set the poll interval if using -B'), - '-B' : dict(long='--background', dest='seconds', type='int', default=0, - help='run asynchronously, failing after X seconds'), - }) + parser.add_option('-P', '--poll', default=constants.DEFAULT_POLL_INTERVAL, type='int', + dest='poll_interval', help='set the poll interval if using -B') + parser.add_option('-B', '--background', dest='seconds', type='int', default=0, + help='run asynchronously, failing after X seconds') - return options + return parser From 1d75a29ec92715a059fcf62bf2d8963d16775185 Mon Sep 17 00:00:00 2001 From: Michael DeHaan Date: Tue, 10 Apr 2012 20:58:40 -0400 Subject: [PATCH 05/19] Allow variables coming in from the playbook and the API to be expressed as dictionaries throughout their full life cycle such that nested data can be made available in templates and playbooks. --- lib/ansible/playbook.py | 11 ++------- lib/ansible/runner.py | 50 +++++++++++++++++++++++++++++++---------- library/setup | 35 ++++++++++++++++------------- test/playbook1.events | 19 ++++++++++++++-- 4 files changed, 77 insertions(+), 38 deletions(-) diff --git a/lib/ansible/playbook.py b/lib/ansible/playbook.py index 0eb0e4d685c..c75d27213ff 100755 --- a/lib/ansible/playbook.py +++ b/lib/ansible/playbook.py @@ -436,24 +436,17 @@ class PlayBook(object): else: self.callbacks.on_setup_primary() - # first run the setup task on every node, which gets the variables - # written to the JSON file and will also bubble facts back up via - # magic in Runner() - push_var_str='' - for (k,v) in vars.iteritems(): - push_var_str += "%s=\"%s\" " % (k,v) - host_list = [ h for h in self.host_list if not (h in self.stats.failures or h in self.stats.dark) ] # push any variables down to the system setup_results = ansible.runner.Runner( pattern=pattern, groups=self.groups, module_name='setup', - module_args=push_var_str, host_list=host_list, + module_args=vars, host_list=host_list, forks=self.forks, module_path=self.module_path, timeout=self.timeout, remote_user=user, remote_pass=self.remote_pass, remote_port=self.remote_port, setup_cache=SETUP_CACHE, - callbacks=self.runner_callbacks, sudo=sudo, + callbacks=self.runner_callbacks, sudo=sudo, debug=self.debug, ).run() self.stats.compute(setup_results, setup=True) diff --git a/lib/ansible/runner.py b/lib/ansible/runner.py index b2cf3518b99..ad7574452b9 100755 --- a/lib/ansible/runner.py +++ b/lib/ansible/runner.py @@ -113,8 +113,8 @@ class Runner(object): self.basedir = basedir self.sudo = sudo - if type(self.module_args) != str: - raise Exception("module_args must be a string: %s" % self.module_args) + if type(self.module_args) != str and type(self.module_args) != dict: + raise Exception("module_args must be a string or dict: %s" % self.module_args) self._tmp_paths = {} random.seed() @@ -271,6 +271,9 @@ class Runner(object): def _transfer_str(self, conn, tmp, name, args_str): ''' transfer arguments as a single file to be fed to the module. ''' + if type(args_str) == dict: + args_str = utils.smjson(args_str) + args_fd, args_file = tempfile.mkstemp() args_fo = os.fdopen(args_fd, 'w') args_fo.write(args_str) @@ -316,23 +319,43 @@ class Runner(object): def _add_setup_vars(self, inject, args): ''' setup module variables need special handling ''' + is_dict = False + if type(args) == dict: + is_dict = True + + # TODO: keep this as a dict through the whole path to simplify this code for (k,v) in inject.iteritems(): if not k.startswith('facter_') and not k.startswith('ohai_'): - if str(v).find(" ") != -1: - v = "\"%s\"" % v - args += " %s=%s" % (k, str(v).replace(" ","~~~")) + if not is_dict: + if str(v).find(" ") != -1: + v = "\"%s\"" % v + args += " %s=%s" % (k, str(v).replace(" ","~~~")) + else: + args[k]=v return args # ***************************************************** def _add_setup_metadata(self, args): ''' automatically determine where to store variables for the setup module ''' - - if args.find("metadata=") == -1: - if self.remote_user == 'root': - args = "%s metadata=/etc/ansible/setup" % args - else: - args = "%s metadata=/home/%s/.ansible/setup" % (args, self.remote_user) + + is_dict = False + if type(args) == dict: + is_dict = True + + # TODO: keep this as a dict through the whole path to simplify this code + if not is_dict: + if args.find("metadata=") == -1: + if self.remote_user == 'root': + args = "%s metadata=/etc/ansible/setup" % args + else: + args = "%s metadata=/home/%s/.ansible/setup" % (args, self.remote_user) + else: + if not 'metadata' in args: + if self.remote_user == 'root': + args['metadata'] = '/etc/ansible/setup' + else: + args['metadata'] = "/home/%s/.ansible/setup" % (self.remote_user) return args # ***************************************************** @@ -352,9 +375,11 @@ class Runner(object): args = self._add_setup_vars(inject, args) args = self._add_setup_metadata(args) + if type(args) == dict: + args = utils.bigjson(args) args = utils.template(args, inject) + module_name_tail = remote_module_path.split("/")[-1] - client_executed_str = "%s %s" % (module_name_tail, args.strip()) argsfile = self._transfer_str(conn, tmp, 'arguments', args) if async_jid is None: @@ -368,6 +393,7 @@ class Runner(object): res, err = self._exec_command(conn, cmd, tmp, sudoable=True) + client_executed_str = "%s %s" % (module_name_tail, args.strip()) return ( res, err, client_executed_str ) # ***************************************************** diff --git a/library/setup b/library/setup index a2b90b7400d..6efb5e973c0 100755 --- a/library/setup +++ b/library/setup @@ -23,6 +23,7 @@ import sys import os import shlex import subprocess +import traceback try: import json @@ -34,18 +35,22 @@ except ImportError: if len(sys.argv) == 1: sys.exit(1) + argfile = sys.argv[1] if not os.path.exists(argfile): sys.exit(1) -input_data = shlex.split(open(argfile, 'r').read()) - -# turn urlencoded k=v string (space delimited) to regular k=v directionary -splitted = [x.split('=',1) for x in input_data ] -splitted = [ (x[0], x[1].replace("~~~"," ")) for x in splitted ] -new_options = dict(splitted) - -ansible_file = new_options.get('metadata', DEFAULT_ANSIBLE_SETUP) +setup_options = open(argfile).read().strip() +try: + setup_options = json.loads(setup_options) +except: + list_options = shlex.split(setup_options) + setup_options = {} + for opt in list_options: + (k,v) = opt.split("=") + setup_options[k]=v + +ansible_file = setup_options.get('metadata', DEFAULT_ANSIBLE_SETUP) ansible_dir = os.path.dirname(ansible_file) # create the config dir if it doesn't exist @@ -74,7 +79,7 @@ if os.path.exists("/usr/bin/facter"): facter = False if facter: for (k,v) in facter_ds.items(): - new_options["facter_%s" % k] = v + setup_options["facter_%s" % k] = v # ditto for ohai, but just top level string keys # because it contains a lot of nested stuff we can't use for @@ -93,13 +98,13 @@ if os.path.exists("/usr/bin/ohai"): for (k,v) in ohai_ds.items(): if type(v) == str or type(v) == unicode: k2 = "ohai_%s" % k - new_options[k2] = v + setup_options[k2] = v # write the template/settings file using # instructions from server f = open(ansible_file, "w+") -reformat = json.dumps(new_options, sort_keys=True, indent=4) +reformat = json.dumps(setup_options, sort_keys=True, indent=4) f.write(reformat) f.close() @@ -108,9 +113,9 @@ md5sum2 = os.popen("md5sum %s" % ansible_file).read().split()[0] if md5sum != md5sum2: changed = True -new_options['written'] = ansible_file -new_options['changed'] = changed -new_options['md5sum'] = md5sum2 +setup_options['written'] = ansible_file +setup_options['changed'] = changed +setup_options['md5sum'] = md5sum2 -print json.dumps(new_options) +print json.dumps(setup_options) diff --git a/test/playbook1.events b/test/playbook1.events index b8ab78e634e..e5e9cd489b9 100644 --- a/test/playbook1.events +++ b/test/playbook1.events @@ -15,7 +15,7 @@ "answer": "Wuh, I think so, Brain, but if we didn't have ears, we'd look like weasels.", "changed": true, "metadata": "/etc/ansible/setup", - "port": "5150", + "port": 5150, "written": "/etc/ansible/setup" } ] @@ -44,7 +44,7 @@ "cow": "moo", "duck": "quack", "metadata": "/etc/ansible/setup", - "port": "5150", + "port": 5150, "testing": "default", "written": "/etc/ansible/setup" } @@ -228,6 +228,21 @@ "127.0.0.2" ] ], + [ + "ok", + [ + "127.0.0.2", + { + "started": 1 + } + ] + ], + [ + "async poll", + [ + "127.0.0.2" + ] + ], [ "ok", [ From 51e4faf7aa37a3567b713149c4b8915add5bbefc Mon Sep 17 00:00:00 2001 From: Michael DeHaan Date: Tue, 10 Apr 2012 21:17:02 -0400 Subject: [PATCH 06/19] Update test file -- we probably should not include the poll as the number of polls is changing between test runs (that's ok, it's not intended to be realtime accurate) --- test/playbook1.events | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/test/playbook1.events b/test/playbook1.events index e5e9cd489b9..feada7fe0a2 100644 --- a/test/playbook1.events +++ b/test/playbook1.events @@ -228,21 +228,6 @@ "127.0.0.2" ] ], - [ - "ok", - [ - "127.0.0.2", - { - "started": 1 - } - ] - ], - [ - "async poll", - [ - "127.0.0.2" - ] - ], [ "ok", [ From 611e3fec4c5ba80653bc3b14ce2b7ee005663031 Mon Sep 17 00:00:00 2001 From: Matthew Williams Date: Tue, 10 Apr 2012 20:19:23 -0700 Subject: [PATCH 07/19] fetch 'module' -- working with paramiko and local connections --- lib/ansible/connection.py | 13 ++++++++++++ lib/ansible/runner.py | 44 +++++++++++++++++++++++++++++++++++++++ library/fetch | 24 +++++++++++++++++++++ 3 files changed, 81 insertions(+) create mode 100755 library/fetch diff --git a/lib/ansible/connection.py b/lib/ansible/connection.py index 11faac402bd..e33e7e0e312 100755 --- a/lib/ansible/connection.py +++ b/lib/ansible/connection.py @@ -142,6 +142,15 @@ class ParamikoConnection(object): raise errors.AnsibleError("failed to transfer file to %s" % out_path) sftp.close() + def fetch_file(self, in_path, out_path): + sftp = self.ssh.open_sftp() + try: + sftp.get(in_path, out_path) + except IOError: + traceback.print_exc() + raise errors.AnsibleError("failed to transfer file from %s" % in_path) + sftp.close() + def close(self): ''' terminate the connection ''' @@ -184,6 +193,10 @@ class LocalConnection(object): traceback.print_exc() raise errors.AnsibleError("failed to transfer file to %s" % out_path) + def fetch_file(self, in_path, out_path): + ''' fetch a file from local to local -- for copatibility ''' + self.put_file(in_path, out_path) + def close(self): ''' terminate the connection; nothing to do here ''' diff --git a/lib/ansible/runner.py b/lib/ansible/runner.py index b2cf3518b99..e112081cf2f 100755 --- a/lib/ansible/runner.py +++ b/lib/ansible/runner.py @@ -460,6 +460,48 @@ class Runner(object): # ***************************************************** + def _execute_fetch(self, conn, host, tmp): + ''' handler for fetch operations ''' + + #load up options + options = utils.parse_kv(self.module_args) + source = options['src'] + dest = options['dest'] + + changed = False + failed = None + ok = True + error = None + + #get old md5 + local_md5 = None + if os.path.exists(dest): + local_md5 = os.popen("md5sum %s" % dest).read().split()[0] + + #get new md5 + remote_md5 = self._exec_command(conn, "md5sum %s" % source, tmp, True)[0].split()[0] + + if remote_md5 != local_md5: + #fetch the file and check for changes + conn.fetch_file(source, dest) + new_md5 = os.popen("md5sum %s" % dest).read().split()[0] + changed = (new_md5 != local_md5) + if new_md5 != remote_md5: + error = "new md5 does not match remote md5" + else: + new_md5 = local_md5 + + if error: + ok = False + failed = True + data = {'msg': error, 'failed': True, 'md5sum': new_md5, 'changed': changed} + else: + data = {'changed': changed, 'md5sum': new_md5} + + return (host, ok, data, failed) + + # ***************************************************** + def _chain_file_module(self, conn, tmp, data, err, options, executed): ''' handles changing file attribs after copy/template operations ''' @@ -549,6 +591,8 @@ class Runner(object): if self.module_name == 'copy': result = self._execute_copy(conn, host, tmp) + elif self.module_name == 'fetch': + result = self._execute_fetch(conn, host, tmp) elif self.module_name == 'template': result = self._execute_template(conn, host, tmp) else: diff --git a/library/fetch b/library/fetch new file mode 100755 index 00000000000..f26337a490a --- /dev/null +++ b/library/fetch @@ -0,0 +1,24 @@ +#!/usr/bin/python + +# (c) 2012, Michael DeHaan +# +# This file is part of Ansible +# +# Ansible is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Ansible is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Ansible. If not, see . +# + +### THIS FILE IS FOR REFERENCE OR FUTURE USE ### + +# See lib/ansible/runner.py for implementation of the fetch functionality # + From 31d3f52b286466c60db195ea6f23350fdc3812fc Mon Sep 17 00:00:00 2001 From: Matthew Williams Date: Wed, 11 Apr 2012 09:14:10 -0700 Subject: [PATCH 08/19] fetch to host specific directory --- lib/ansible/runner.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/ansible/runner.py b/lib/ansible/runner.py index e112081cf2f..183153fc266 100755 --- a/lib/ansible/runner.py +++ b/lib/ansible/runner.py @@ -466,7 +466,9 @@ class Runner(object): #load up options options = utils.parse_kv(self.module_args) source = options['src'] - dest = options['dest'] + + filename = os.path.basename(source) + dest = "%s/%s/%s" % (options['dest'], host, filename) changed = False failed = None @@ -482,6 +484,8 @@ class Runner(object): remote_md5 = self._exec_command(conn, "md5sum %s" % source, tmp, True)[0].split()[0] if remote_md5 != local_md5: + #create the containing directories, if needed + os.makedirs(os.path.dirname(dest)) #fetch the file and check for changes conn.fetch_file(source, dest) new_md5 = os.popen("md5sum %s" % dest).read().split()[0] From 245aa9bf8e397b331e39e9a4753d4ce946ca4a66 Mon Sep 17 00:00:00 2001 From: Michael DeHaan Date: Wed, 11 Apr 2012 20:12:01 -0400 Subject: [PATCH 09/19] Some tweaks to the fetch module. 'err' return was for stderr, so that should be empty string. Some minor code shortening. Added a test to TestRunner. --- lib/ansible/runner.py | 31 +++++++++---------------------- test/TestRunner.py | 8 ++++++++ 2 files changed, 17 insertions(+), 22 deletions(-) diff --git a/lib/ansible/runner.py b/lib/ansible/runner.py index 9683c17939d..deea5c98fd5 100755 --- a/lib/ansible/runner.py +++ b/lib/ansible/runner.py @@ -489,46 +489,33 @@ class Runner(object): def _execute_fetch(self, conn, host, tmp): ''' handler for fetch operations ''' - #load up options + # load up options options = utils.parse_kv(self.module_args) source = options['src'] + # files are saved in dest dir, with a subdir for each host, then the filename filename = os.path.basename(source) - dest = "%s/%s/%s" % (options['dest'], host, filename) - - changed = False - failed = None - ok = True - error = None + dest = "%s/%s/%s" % (utils.path_dwim(self.basedir, options['dest']), host, filename) - #get old md5 + # compare old and new md5 for support of change hooks local_md5 = None if os.path.exists(dest): local_md5 = os.popen("md5sum %s" % dest).read().split()[0] - - #get new md5 remote_md5 = self._exec_command(conn, "md5sum %s" % source, tmp, True)[0].split()[0] if remote_md5 != local_md5: - #create the containing directories, if needed + # create the containing directories, if needed os.makedirs(os.path.dirname(dest)) - #fetch the file and check for changes + # fetch the file and check for changes conn.fetch_file(source, dest) new_md5 = os.popen("md5sum %s" % dest).read().split()[0] changed = (new_md5 != local_md5) if new_md5 != remote_md5: - error = "new md5 does not match remote md5" - else: - new_md5 = local_md5 - - if error: - ok = False - failed = True - data = {'msg': error, 'failed': True, 'md5sum': new_md5, 'changed': changed} + return (host, True, dict(failed=True, msg="md5 mismatch", md5sum=new_md5), '') + return (host, True, dict(changed=True, md5sum=new_md5), '') else: - data = {'changed': changed, 'md5sum': new_md5} + return (host, True, dict(changed=False, md5sum=local_md5), '') - return (host, ok, data, failed) # ***************************************************** diff --git a/test/TestRunner.py b/test/TestRunner.py index 66acb6689d8..20c2c4e24cb 100644 --- a/test/TestRunner.py +++ b/test/TestRunner.py @@ -189,6 +189,14 @@ class TestRunner(unittest.TestCase): assert 'stdout' in result assert result['ansible_job_id'] == jid + def test_fetch(self): + input = self._get_test_file('sample.j2') + output = self._get_stage_file('127.0.0.2/sample.j2') + result = self._run('fetch', [ "src=%s" % input, "dest=%s" % self.stage_dir ]) + print "output file=%s" % output + assert os.path.exists(output) + assert open(input).read() == open(output).read() + def test_yum(self): result = self._run('yum', [ "list=repos" ]) assert 'failed' not in result From 95e045d153f3a079e6e3355a8068d6b33bd6747d Mon Sep 17 00:00:00 2001 From: Michael DeHaan Date: Wed, 11 Apr 2012 20:15:17 -0400 Subject: [PATCH 10/19] Remove remote logging as we're going to move this logging to the modules for performance reasons. --- lib/ansible/runner.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/lib/ansible/runner.py b/lib/ansible/runner.py index deea5c98fd5..188a00c27e8 100755 --- a/lib/ansible/runner.py +++ b/lib/ansible/runner.py @@ -387,11 +387,6 @@ class Runner(object): else: cmd = " ".join([str(x) for x in [remote_module_path, async_jid, async_limit, async_module, argsfile]]) - # log command as the full command not as the path to args file - helps with debugging - msg = '%s: "%s"' % (self.module_name, args) - conn.exec_command('/usr/bin/logger -t ansible -p auth.info "%s"' % msg, None) - - res, err = self._exec_command(conn, cmd, tmp, sudoable=True) client_executed_str = "%s %s" % (module_name_tail, args.strip()) return ( res, err, client_executed_str ) @@ -642,10 +637,6 @@ class Runner(object): def _exec_command(self, conn, cmd, tmp, sudoable=False): ''' execute a command string over SSH, return the output ''' - msg = '%s: %s' % (self.module_name, cmd) - # log remote command execution - conn.exec_command('/usr/bin/logger -t ansible -p auth.info "%s"' % msg, None) - # now run actual command stdin, stdout, stderr = conn.exec_command(cmd, tmp, sudoable=sudoable) if type(stderr) != str: From e103bdda93660315fa2a954ccd506bb559fb3958 Mon Sep 17 00:00:00 2001 From: Michael DeHaan Date: Wed, 11 Apr 2012 20:16:28 -0400 Subject: [PATCH 11/19] Rename test class to match what it is testing --- test/TestPlayBook.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/TestPlayBook.py b/test/TestPlayBook.py index b32c76780de..24568464223 100644 --- a/test/TestPlayBook.py +++ b/test/TestPlayBook.py @@ -90,7 +90,7 @@ class TestCallbacks(object): pass -class TestRunner(unittest.TestCase): +class TestPlaybook(unittest.TestCase): def setUp(self): self.user = getpass.getuser() From a0480a1bc59410faef8afb59b8950215b38a0403 Mon Sep 17 00:00:00 2001 From: Michael DeHaan Date: Wed, 11 Apr 2012 20:20:55 -0400 Subject: [PATCH 12/19] Block some paramiko warnings that are not relevant. --- lib/ansible/connection.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/ansible/connection.py b/lib/ansible/connection.py index e33e7e0e312..f85d44c0d73 100755 --- a/lib/ansible/connection.py +++ b/lib/ansible/connection.py @@ -18,7 +18,13 @@ ################################################ -import paramiko +import warnings +# prevent paramiko warning noise +# see http://stackoverflow.com/questions/3920502/ +with warnings.catch_warnings(): + warnings.simplefilter("ignore") + import paramiko + import traceback import os import time From b9e3b053f91ac91993951c8af90dd343803aa77d Mon Sep 17 00:00:00 2001 From: Michael DeHaan Date: Wed, 11 Apr 2012 20:27:17 -0400 Subject: [PATCH 13/19] Simplify playbook tests so things are not timing dependent in the poll section, which is leading to tests not being consistent between runs, even though there wasn't an error. Now we'll just check the final change counts, which should be just as solid and lead to less churn in the events file. --- test/TestPlayBook.py | 23 +-- test/playbook1.events | 322 ------------------------------------------ 2 files changed, 15 insertions(+), 330 deletions(-) delete mode 100644 test/playbook1.events diff --git a/test/TestPlayBook.py b/test/TestPlayBook.py index 24568464223..72c0b69ccae 100644 --- a/test/TestPlayBook.py +++ b/test/TestPlayBook.py @@ -139,20 +139,27 @@ class TestPlaybook(unittest.TestCase): callbacks = self.test_callbacks, runner_callbacks = self.test_callbacks ) - results = self.playbook.run() - return dict( - results = results, - events = EVENTS - ) + return self.playbook.run() def test_one(self): pb = os.path.join(self.test_dir, 'playbook1.yml') - expected = os.path.join(self.test_dir, 'playbook1.events') - expected = utils.json_loads(file(expected).read()) actual = self._run(pb) + # if different, this will output to screen + print "**ACTUAL**" print utils.bigjson(actual) - assert cmp(expected, actual) == 0, "expected events match actual events" + expected = { + "127.0.0.2": { + "changed": 9, + "failures": 0, + "ok": 12, + "skipped": 1, + "unreachable": 0 + } + } + print "**EXPECTED**" + print utils.bigjson(expected) + assert utils.bigjson(expected) == utils.bigjson(actual) # make sure the template module took options from the vars section data = file('/tmp/ansible_test_data_template.out').read() diff --git a/test/playbook1.events b/test/playbook1.events deleted file mode 100644 index feada7fe0a2..00000000000 --- a/test/playbook1.events +++ /dev/null @@ -1,322 +0,0 @@ -{ - "events": [ - "start", - [ - "play start", - [ - "all" - ] - ], - [ - "ok", - [ - "127.0.0.2", - { - "answer": "Wuh, I think so, Brain, but if we didn't have ears, we'd look like weasels.", - "changed": true, - "metadata": "/etc/ansible/setup", - "port": 5150, - "written": "/etc/ansible/setup" - } - ] - ], - [ - "import", - [ - "127.0.0.2", - "/home/mdehaan/ansible/test/common_vars.yml" - ] - ], - [ - "import", - [ - "127.0.0.2", - "/home/mdehaan/ansible/test/CentOS.yml" - ] - ], - [ - "ok", - [ - "127.0.0.2", - { - "answer": "Wuh, I think so, Brain, but if we didn't have ears, we'd look like weasels.", - "changed": true, - "cow": "moo", - "duck": "quack", - "metadata": "/etc/ansible/setup", - "port": 5150, - "testing": "default", - "written": "/etc/ansible/setup" - } - ] - ], - [ - "task start", - [ - "test basic success command", - false - ] - ], - [ - "ok", - [ - "127.0.0.2", - { - "changed": true, - "cmd": [ - "/bin/true" - ], - "rc": 0, - "stderr": "", - "stdout": "" - } - ] - ], - [ - "task start", - [ - "test basic success command 2", - false - ] - ], - [ - "ok", - [ - "127.0.0.2", - { - "changed": true, - "cmd": [ - "/bin/true" - ], - "rc": 0, - "stderr": "", - "stdout": "" - } - ] - ], - [ - "task start", - [ - "test basic shell, plus two ways to dereference a variable", - false - ] - ], - [ - "ok", - [ - "127.0.0.2", - { - "changed": true, - "cmd": "echo $HOME 5150 5150 ", - "rc": 0, - "stderr": "", - "stdout": "/root 5150 5150" - } - ] - ], - [ - "task start", - [ - "test vars_files imports", - false - ] - ], - [ - "ok", - [ - "127.0.0.2", - { - "changed": true, - "cmd": "echo quack moo default ", - "rc": 0, - "stderr": "", - "stdout": "quack moo default" - } - ] - ], - [ - "task start", - [ - "test copy", - false - ] - ], - [ - "ok", - [ - "127.0.0.2", - { - "changed": true, - "group": "root", - "mode": 420, - "path": "/tmp/ansible_test_data_copy.out", - "state": "file", - "user": "root" - } - ] - ], - [ - "notify", - [ - "127.0.0.2", - "on change 1" - ] - ], - [ - "task start", - [ - "test template", - false - ] - ], - [ - "ok", - [ - "127.0.0.2", - { - "changed": true, - "group": "root", - "mode": 420, - "path": "/tmp/ansible_test_data_template.out", - "state": "file", - "user": "root" - } - ] - ], - [ - "notify", - [ - "127.0.0.2", - "on change 1" - ] - ], - [ - "notify", - [ - "127.0.0.2", - "on change 2" - ] - ], - [ - "task start", - [ - "async poll test", - false - ] - ], - [ - "ok", - [ - "127.0.0.2", - { - "started": 1 - } - ] - ], - [ - "ok", - [ - "127.0.0.2", - { - "started": 1 - } - ] - ], - [ - "async poll", - [ - "127.0.0.2" - ] - ], - [ - "ok", - [ - "127.0.0.2", - { - "changed": true, - "cmd": "sleep 5 ", - "finished": 1, - "rc": 0, - "stderr": "", - "stdout": "" - } - ] - ], - [ - "task start", - [ - "this should be skipped", - false - ] - ], - [ - "skipped", - [ - "127.0.0.2" - ] - ], - [ - "task start", - [ - "on change 1", - true - ] - ], - [ - "ok", - [ - "127.0.0.2", - { - "changed": true, - "cmd": "echo 'this should fire once' ", - "rc": 0, - "stderr": "", - "stdout": "this should fire once" - } - ] - ], - [ - "ok", - [ - "127.0.0.2", - { - "changed": true, - "cmd": "echo 'this should fire once' ", - "rc": 0, - "stderr": "", - "stdout": "this should fire once" - } - ] - ], - [ - "task start", - [ - "on change 2", - true - ] - ], - [ - "ok", - [ - "127.0.0.2", - { - "changed": true, - "cmd": "echo 'this should fire once also' ", - "rc": 0, - "stderr": "", - "stdout": "this should fire once also" - } - ] - ] - ], - "results": { - "127.0.0.2": { - "changed": 9, - "failures": 0, - "ok": 12, - "skipped": 1, - "unreachable": 0 - } - } -} - From 529a1949501116db3784211340e0d38627962d5c Mon Sep 17 00:00:00 2001 From: Michael DeHaan Date: Wed, 11 Apr 2012 20:31:24 -0400 Subject: [PATCH 14/19] Upgrade apt message if no python-apt to intercept potential user questions. --- library/apt | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/library/apt b/library/apt index 0909f3ead18..1bce1931b1a 100755 --- a/library/apt +++ b/library/apt @@ -31,9 +31,7 @@ APT_PATH = "/usr/bin/apt-get" APT = "DEBIAN_PRIORITY=critical %s" % APT_PATH def debug(msg): - # ansible ignores stderr, so it's safe to use for debug print >>sys.stderr, msg - #pass def exit_json(rc=0, **kwargs): print json.dumps(kwargs) @@ -46,7 +44,7 @@ def fail_json(**kwargs): try: import apt except ImportError: - fail_json(msg="could not import apt") + fail_json(msg="could not import apt, please install the python-apt package on this host") def run_apt(command): try: From b418632a8d3c42b8ccbcb3e68815f9496336f5e2 Mon Sep 17 00:00:00 2001 From: Michael DeHaan Date: Wed, 11 Apr 2012 20:44:15 -0400 Subject: [PATCH 15/19] Allow yum module to use package or name as an alias for 'pkg' --- library/yum | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/library/yum b/library/yum index da0d5ab4922..87c2713d3e3 100755 --- a/library/yum +++ b/library/yum @@ -318,14 +318,14 @@ def main(): return 1, str(e) elif 'state' in params: - if 'pkg' not in params: + pkg = params.get('pkg', params.get('package', params.get('name', None))) + if 'pkg' is None: results['msg'] = "No pkg specified" else: try: my = yum_base(conf_file=params['conf_file'], cachedir=True) state = params['state'] - pkgspec = params['pkg'] - results = ensure(my, state, pkgspec) + results = ensure(my, state, pkg) except Exception, e: return 1, str(e) From 8152e44efdaa898c98a491293e4336f120cefaa7 Mon Sep 17 00:00:00 2001 From: Michael DeHaan Date: Wed, 11 Apr 2012 20:45:14 -0400 Subject: [PATCH 16/19] Also allow the apt module to use package or name as an alias for 'pkg' --- library/apt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/apt b/library/apt index 1bce1931b1a..90e4b17a1bd 100755 --- a/library/apt +++ b/library/apt @@ -113,7 +113,7 @@ for x in items: params[k] = v state = params.get('state','installed') -package = params.get('pkg', None) +package = params.get('pkg', params.get('package', params.get('name', None))) update_cache = params.get('update-cache', 'no') purge = params.get('purge', 'no') From ff5d3293745a858df3494d9a084eb22522cfb663 Mon Sep 17 00:00:00 2001 From: Michael DeHaan Date: Wed, 11 Apr 2012 20:47:38 -0400 Subject: [PATCH 17/19] Yum package state defaults to installed --- library/yum | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/library/yum b/library/yum index 87c2713d3e3..edb21019786 100755 --- a/library/yum +++ b/library/yum @@ -317,22 +317,18 @@ def main(): except Exception, e: return 1, str(e) - elif 'state' in params: + else: pkg = params.get('pkg', params.get('package', params.get('name', None))) if 'pkg' is None: results['msg'] = "No pkg specified" else: try: my = yum_base(conf_file=params['conf_file'], cachedir=True) - state = params['state'] + state = params.get('state', 'installed') results = ensure(my, state, pkg) except Exception, e: return 1, str(e) - else: - print json.dumps(dict(failed=True, msg='invalid module parameters')) - sys.exit(1) - print json.dumps(results) return 0, None From f3489a53cd259d15e5ab2d46931a0783b632d14a Mon Sep 17 00:00:00 2001 From: Michael DeHaan Date: Wed, 11 Apr 2012 20:57:41 -0400 Subject: [PATCH 18/19] English error messages if src and dest are left off the copy, template, or fetch modules --- lib/ansible/runner.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/lib/ansible/runner.py b/lib/ansible/runner.py index 188a00c27e8..edd9763e552 100755 --- a/lib/ansible/runner.py +++ b/lib/ansible/runner.py @@ -458,8 +458,10 @@ class Runner(object): # load up options options = utils.parse_kv(self.module_args) - source = options['src'] - dest = options['dest'] + source = options.get('src', None) + dest = options.get('dest', None) + if source is None or dest is None: + return (host, True, dict(failed=True, msg="src and dest are required"), '') # transfer the file to a remote tmp location tmp_src = tmp + source.split('/')[-1] @@ -486,11 +488,14 @@ class Runner(object): # load up options options = utils.parse_kv(self.module_args) - source = options['src'] + source = options.get('src', None) + dest = options.get('dest', None) + if source is None or dest is None: + return (host, True, dict(failed=True, msg="src and dest are required"), '') # files are saved in dest dir, with a subdir for each host, then the filename filename = os.path.basename(source) - dest = "%s/%s/%s" % (utils.path_dwim(self.basedir, options['dest']), host, filename) + dest = "%s/%s/%s" % (utils.path_dwim(self.basedir, dest), host, filename) # compare old and new md5 for support of change hooks local_md5 = None @@ -536,9 +541,11 @@ class Runner(object): # load up options options = utils.parse_kv(self.module_args) - source = options['src'] - dest = options['dest'] + source = options.get('src', None) + dest = options.get('dest', None) metadata = options.get('metadata', None) + if source is None or dest is None: + return (host, True, dict(failed=True, msg="src and dest are required"), '') if metadata is None: if self.remote_user == 'root': From 08c593bee19e0c6660c84b2a0c8477b90238a469 Mon Sep 17 00:00:00 2001 From: Michael DeHaan Date: Wed, 11 Apr 2012 21:05:46 -0400 Subject: [PATCH 19/19] Warn if no hosts matched --- lib/ansible/callbacks.py | 9 +++++++++ lib/ansible/runner.py | 1 + test/TestPlayBook.py | 2 ++ 3 files changed, 12 insertions(+) diff --git a/lib/ansible/callbacks.py b/lib/ansible/callbacks.py index 5e8ab823fd4..0b36a62ab33 100755 --- a/lib/ansible/callbacks.py +++ b/lib/ansible/callbacks.py @@ -95,6 +95,9 @@ class DefaultRunnerCallbacks(object): def on_unreachable(self, host, res): pass + def on_no_hosts(self): + pass + ######################################################################## class CliRunnerCallbacks(DefaultRunnerCallbacks): @@ -120,6 +123,9 @@ class CliRunnerCallbacks(DefaultRunnerCallbacks): def on_error(self, host, err): print >>sys.stderr, "stderr: [%s] => %s\n" % (host, err) + + def on_no_hosts(self): + print >>sys.stderr, "no hosts matched\n" def _on_any(self, host, result): print utils.host_report_msg(host, self.options.module_name, result, self.options.one_line) @@ -159,6 +165,9 @@ class PlaybookRunnerCallbacks(DefaultRunnerCallbacks): def on_skipped(self, host): print "skipping: [%s]\n" % host + def on_no_hosts(self): + print "no hosts matched or remaining\n" + ######################################################################## class PlaybookCallbacks(object): diff --git a/lib/ansible/runner.py b/lib/ansible/runner.py index edd9763e552..3396c550266 100755 --- a/lib/ansible/runner.py +++ b/lib/ansible/runner.py @@ -750,6 +750,7 @@ class Runner(object): # find hosts that match the pattern hosts = self._match_hosts(self.pattern) if len(hosts) == 0: + self.callbacks.on_no_hosts() return dict(contacted={}, dark={}) hosts = [ (self,x) for x in hosts ] diff --git a/test/TestPlayBook.py b/test/TestPlayBook.py index 72c0b69ccae..9e04e254320 100644 --- a/test/TestPlayBook.py +++ b/test/TestPlayBook.py @@ -89,6 +89,8 @@ class TestCallbacks(object): def on_setup_secondary(self): pass + def on_no_hosts(self): + pass class TestPlaybook(unittest.TestCase):