From 64c51ade1e5485dc90ebdf8dfa81166fb7a3e756 Mon Sep 17 00:00:00 2001 From: Michael DeHaan Date: Sat, 14 Jul 2012 19:18:33 -0400 Subject: [PATCH] Change the way we do with_items to make them happen next to each other in runner, which eliminates the problem of with_items and vars_files sometimes not playing nice with each other. (Also a fix for the user module error handling when the user is not present at the time of the return. This can only really be caused by multiple ansible executions). --- lib/ansible/playbook/play.py | 20 +++++------ lib/ansible/runner/__init__.py | 64 +++++++++++++++++++++++++++++----- library/user | 7 ++++ 3 files changed, 72 insertions(+), 19 deletions(-) diff --git a/lib/ansible/playbook/play.py b/lib/ansible/playbook/play.py index 44b9320b60..09462ab5f3 100644 --- a/lib/ansible/playbook/play.py +++ b/lib/ansible/playbook/play.py @@ -104,17 +104,15 @@ class Play(object): for y in data: items = y.get('with_items',None) if items is None: - items = [ '' ] + items = [ ] elif isinstance(items, basestring): - items = utils.varLookup(items, task_vars) - for item in items: - item = utils.template(item, task_vars) - if self._has_vars_in(item): - raise errors.AnsibleError("parse error: unbound variable in with_items: %s" % item) - - mv = task_vars.copy() - mv['item'] = item - results.append(Task(self,y,module_vars=mv)) + #items = utils.varLookup(items, task_vars) + if type(items) != list: + raise errors.AnsibleError("with_items must be a list, got: %s" % items) + # items = [ utils.template(item, task_vars) for item in items] + mv = task_vars.copy() + mv['items'] = items + results.append(Task(self,y,module_vars=mv)) for x in results: if self.tags is not None: @@ -245,6 +243,8 @@ class Play(object): if host is not None: filename3 = utils.template(filename2, self.playbook.SETUP_CACHE[host]) filename4 = utils.path_dwim(self.playbook.basedir, filename3) + if self._has_vars_in(filename4): + return new_vars = utils.parse_yaml_from_file(filename4) if new_vars: if type(new_vars) != dict: diff --git a/lib/ansible/runner/__init__.py b/lib/ansible/runner/__init__.py index e4daa5825c..486d1b6be1 100644 --- a/lib/ansible/runner/__init__.py +++ b/lib/ansible/runner/__init__.py @@ -71,13 +71,12 @@ def _executor_hook(job_queue, result_queue): class ReturnData(object): - __slots__ = [ 'result', 'comm_ok', 'executed_str', 'host' ] + __slots__ = [ 'result', 'comm_ok', 'host' ] - def __init__(self, host=None, result=None, comm_ok=True, executed_str=''): + def __init__(self, host=None, result=None, comm_ok=True): self.host = host self.result = result self.comm_ok = comm_ok - self.executed_str = executed_str if type(self.result) in [ str, unicode ]: self.result = utils.parse_json(self.result) @@ -135,7 +134,7 @@ class Runner(object): conditional : only execute if this string, evaluated, is True callbacks : output callback class sudo : log in as remote user and immediately sudo to root - module_vars : provides additional variables to a template. FIXME: factor this out + module_vars : provides additional variables to a template. is_playbook : indicates Runner is being used by a playbook. affects behavior in various ways. inventory : inventory object, if host_list is not provided """ @@ -279,10 +278,59 @@ class Runner(object): return args # ***************************************************** - + def _execute_module(self, conn, tmp, remote_module_path, args, async_jid=None, async_module=None, async_limit=None): + items = self.module_vars.get('items', None) + if items is None or len(items) == 0: + # executing a single item + return self._execute_module_internal( + conn, tmp, remote_module_path, args, + async_jid=async_jid, async_module=async_module, async_limit=async_limit + ) + else: + # executing using with_items, so make multiple calls + # TODO: refactor + aggregrate = {} + all_comm_ok = True + all_changed = False + all_failed = False + results = [] + for x in items: + self.module_vars['item'] = x + result = self._execute_module_internal( + conn, tmp, remote_module_path, args, + async_jid=async_jid, async_module=async_module, async_limit=async_limit + ) + results.append(result.result) + if result.comm_ok == False: + all_comm_ok = False + break + for x in results: + if x.get('changed') == True: + all_changed = True + if (x.get('failed') == True) or (('rc' in x) and (x['rc'] != 0)): + all_failed = True + break + msg = 'All items succeeded' + if all_failed: + msg = "One or more items failed." + rd_result = dict( + failed = all_failed, + changed = all_changed, + results = results, + msg = msg + ) + if not all_failed: + del rd_result['failed'] + return ReturnData(host=conn.host, comm_ok=all_comm_ok, result=rd_result) + + # ***************************************************** + + def _execute_module_internal(self, conn, tmp, remote_module_path, args, + async_jid=None, async_module=None, async_limit=None): + ''' runs a module that has already been transferred ''' inject = self.setup_cache.get(conn.host,{}).copy() @@ -304,6 +352,7 @@ class Runner(object): if type(args) == dict: args = utils.bigjson(args) + args = utils.template(args, inject, self.setup_cache) module_name_tail = remote_module_path.split("/")[-1] @@ -316,9 +365,7 @@ class Runner(object): res = self._low_level_exec_command(conn, cmd, tmp, sudoable=True) - executed_str = "%s %s" % (module_name_tail, args.strip()) - - return ReturnData(host=conn.host, result=res, executed_str=executed_str) + return ReturnData(host=conn.host, result=res) # ***************************************************** @@ -597,7 +644,6 @@ class Runner(object): # modify file attribs if needed if exec_rc.comm_ok: - exec_rc.executed_str = exec_rc.executed_str.replace("copy","template",1) return self._chain_file_module(conn, tmp, exec_rc, options) else: return exec_rc diff --git a/library/user b/library/user index da33f30cb0..61bb80ce9c 100755 --- a/library/user +++ b/library/user @@ -54,6 +54,13 @@ def add_user_info(kwargs): if user_exists(name): kwargs['state'] = 'present' info = user_info(name) + if info == False: + if 'failed' in kwargs: + kwargs['notice'] = "failed to look up user name: %s" % name + else: + kwargs['msg'] = "failed to look up user name: %s" % name + kwargs['failed'] = True + return kwargs kwargs['uid'] = info[2] kwargs['group'] = info[3] kwargs['comment'] = info[4]