From 601a1cc6d918c9f1e332bd3406a658de7fcaa08e Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Tue, 25 Aug 2015 17:51:51 -0400 Subject: [PATCH] Multiple fixes for include statements and blocks in general Fixes #11981 Fixes #11995 Fixes #12039 Fixes #12077 --- lib/ansible/executor/play_iterator.py | 240 +++++++++++------- lib/ansible/playbook/block.py | 16 +- lib/ansible/playbook/task.py | 2 +- lib/ansible/plugins/strategies/linear.py | 34 ++- samples/test_block.yml | 1 - .../roles/test_includes/tasks/main.yml | 11 +- 6 files changed, 196 insertions(+), 108 deletions(-) diff --git a/lib/ansible/executor/play_iterator.py b/lib/ansible/executor/play_iterator.py index c6870e02247..89849988e35 100644 --- a/lib/ansible/executor/play_iterator.py +++ b/lib/ansible/executor/play_iterator.py @@ -31,22 +31,31 @@ from ansible.utils.boolean import boolean __all__ = ['PlayIterator'] +try: + from __main__ import display +except ImportError: + from ansible.utils.display import Display + display = Display() + + class HostState: def __init__(self, blocks): self._blocks = blocks[:] - self.cur_block = 0 - self.cur_regular_task = 0 - self.cur_rescue_task = 0 - self.cur_always_task = 0 - self.cur_role = None - self.run_state = PlayIterator.ITERATING_SETUP - self.fail_state = PlayIterator.FAILED_NONE - self.pending_setup = False - self.child_state = None + self.cur_block = 0 + self.cur_regular_task = 0 + self.cur_rescue_task = 0 + self.cur_always_task = 0 + self.cur_role = None + self.run_state = PlayIterator.ITERATING_SETUP + self.fail_state = PlayIterator.FAILED_NONE + self.pending_setup = False + self.tasks_child_state = None + self.rescue_child_state = None + self.always_child_state = None def __repr__(self): - return "HOST STATE: block=%d, task=%d, rescue=%d, always=%d, role=%s, run_state=%d, fail_state=%d, pending_setup=%s, child state? %s" % ( + return "HOST STATE: block=%d, task=%d, rescue=%d, always=%d, role=%s, run_state=%d, fail_state=%d, pending_setup=%s, tasks child state? %s, rescue child state? %s, always child state? %s" % ( self.cur_block, self.cur_regular_task, self.cur_rescue_task, @@ -55,7 +64,9 @@ class HostState: self.run_state, self.fail_state, self.pending_setup, - self.child_state, + self.tasks_child_state, + self.rescue_child_state, + self.always_child_state, ) def get_current_block(self): @@ -71,7 +82,12 @@ class HostState: new_state.run_state = self.run_state new_state.fail_state = self.fail_state new_state.pending_setup = self.pending_setup - new_state.child_state = self.child_state + if self.tasks_child_state is not None: + new_state.tasks_child_state = self.tasks_child_state.copy() + if self.rescue_child_state is not None: + new_state.rescue_child_state = self.rescue_child_state.copy() + if self.always_child_state is not None: + new_state.always_child_state = self.always_child_state.copy() return new_state class PlayIterator: @@ -126,10 +142,12 @@ class PlayIterator: def get_next_task_for_host(self, host, peek=False): + display.debug("getting the next task for host %s" % host.name) s = self.get_host_state(host) task = None if s.run_state == self.ITERATING_COMPLETE: + display.debug("host %s is done iterating, returning" % host.name) return (None, None) elif s.run_state == self.ITERATING_SETUP: s.run_state = self.ITERATING_TASKS @@ -169,6 +187,9 @@ class PlayIterator: if not peek: self._host_states[host.name] = s + display.debug("done getting next task for host %s" % host.name) + display.debug(" ^ task is: %s" % task) + display.debug(" ^ state is: %s" % s) return (s, task) @@ -176,15 +197,6 @@ class PlayIterator: task = None - # if we previously encountered a child block and we have a - # saved child state, try and get the next task from there - if state.child_state: - (state.child_state, task) = self._get_next_task_from_state(state.child_state, peek=peek) - if task: - return (state.child_state, task) - else: - state.child_state = None - # try and find the next task, given the current state. while True: # try to get the current block from the list of blocks, and @@ -207,7 +219,19 @@ class PlayIterator: state.run_state = self.ITERATING_ALWAYS else: task = block.block[state.cur_regular_task] - state.cur_regular_task += 1 + # if the current task is actually a child block, we dive into it + if isinstance(task, Block) or state.tasks_child_state is not None: + if state.tasks_child_state is None: + state.tasks_child_state = HostState(blocks=[task]) + state.tasks_child_state.run_state = self.ITERATING_TASKS + state.tasks_child_state.cur_role = state.cur_role + (state.tasks_child_state, task) = self._get_next_task_from_state(state.tasks_child_state, peek=peek) + if task is None: + state.tasks_child_state = None + state.cur_regular_task += 1 + continue + else: + state.cur_regular_task += 1 elif state.run_state == self.ITERATING_RESCUE: if state.fail_state & self.FAILED_RESCUE == self.FAILED_RESCUE: @@ -218,7 +242,18 @@ class PlayIterator: state.run_state = self.ITERATING_ALWAYS else: task = block.rescue[state.cur_rescue_task] - state.cur_rescue_task += 1 + if isinstance(task, Block) or state.rescue_child_state is not None: + if state.rescue_child_state is None: + state.rescue_child_state = HostState(blocks=[task]) + state.rescue_child_state.run_state = self.ITERATING_TASKS + state.rescue_child_state.cur_role = state.cur_role + (state.rescue_child_state, task) = self._get_next_task_from_state(state.rescue_child_state, peek=peek) + if task is None: + state.rescue_child_state = None + state.cur_rescue_task += 1 + continue + else: + state.cur_rescue_task += 1 elif state.run_state == self.ITERATING_ALWAYS: if state.cur_always_task >= len(block.always): @@ -233,38 +268,55 @@ class PlayIterator: state.child_state = None else: task = block.always[state.cur_always_task] - state.cur_always_task += 1 + if isinstance(task, Block) or state.always_child_state is not None: + if state.always_child_state is None: + state.always_child_state = HostState(blocks=[task]) + state.always_child_state.run_state = self.ITERATING_TASKS + state.always_child_state.cur_role = state.cur_role + (state.always_child_state, task) = self._get_next_task_from_state(state.always_child_state, peek=peek) + if task is None: + state.always_child_state = None + state.cur_always_task += 1 + continue + else: + state.cur_always_task += 1 elif state.run_state == self.ITERATING_COMPLETE: return (state, None) - # if the current task is actually a child block, we dive into it - if isinstance(task, Block): - state.child_state = HostState(blocks=[task]) - state.child_state.run_state = self.ITERATING_TASKS - state.child_state.cur_role = state.cur_role - (state.child_state, task) = self._get_next_task_from_state(state.child_state, peek=peek) - # if something above set the task, break out of the loop now if task: break return (state, task) + def _set_failed_state(self, state): + if state.pending_setup: + state.fail_state |= self.FAILED_SETUP + state.run_state = self.ITERATING_COMPLETE + elif state.run_state == self.ITERATING_TASKS: + if state.tasks_child_state is not None: + state.tasks_child_state = self._set_failed_state(state.tasks_child_state) + else: + state.fail_state |= self.FAILED_TASKS + state.run_state = self.ITERATING_RESCUE + elif state.run_state == self.ITERATING_RESCUE: + if state.rescue_child_state is not None: + state.rescue_child_state = self._set_failed_state(state.rescue_child_state) + else: + state.fail_state |= self.FAILED_RESCUE + state.run_state = self.ITERATING_ALWAYS + elif state.run_state == self.ITERATING_ALWAYS: + if state.always_child_state is not None: + state.always_child_state = self._set_failed_state(state.always_child_state) + else: + state.fail_state |= self.FAILED_ALWAYS + state.run_state = self.ITERATING_COMPLETE + return state + def mark_host_failed(self, host): s = self.get_host_state(host) - if s.pending_setup: - s.fail_state |= self.FAILED_SETUP - s.run_state = self.ITERATING_COMPLETE - elif s.run_state == self.ITERATING_TASKS: - s.fail_state |= self.FAILED_TASKS - s.run_state = self.ITERATING_RESCUE - elif s.run_state == self.ITERATING_RESCUE: - s.fail_state |= self.FAILED_RESCUE - s.run_state = self.ITERATING_ALWAYS - elif s.run_state == self.ITERATING_ALWAYS: - s.fail_state |= self.FAILED_ALWAYS - s.run_state = self.ITERATING_COMPLETE + s = self._set_failed_state(s) self._host_states[host.name] = s def get_failed_hosts(self): @@ -278,34 +330,37 @@ class PlayIterator: allows us to find the original task passed into the executor engine. ''' def _search_block(block, task): - for t in block.block: - if isinstance(t, Block): - res = _search_block(t, task) - if res: - return res - elif t._uuid == task._uuid: - return t - for t in block.rescue: - if isinstance(t, Block): - res = _search_block(t, task) - if res: - return res - elif t._uuid == task._uuid: - return t - for t in block.always: - if isinstance(t, Block): - res = _search_block(t, task) - if res: - return res - elif t._uuid == task._uuid: - return t + ''' + helper method to check a block's task lists (block/rescue/always) + for a given task uuid. If a Block is encountered in the place of a + task, it will be recursively searched (this happens when a task + include inserts one or more blocks into a task list). + ''' + for b in (block.block, block.rescue, block.always): + for t in b: + if isinstance(t, Block): + res = _search_block(t, task) + if res: + return res + elif t._uuid == task._uuid: + return t + return None + + def _search_state(state, task): + for block in state._blocks: + res = _search_block(block, task) + if res: + return res + for child_state in (state.tasks_child_state, state.rescue_child_state, state.always_child_state): + res = _search_state(child_state, task) + if res: + return res return None s = self.get_host_state(host) - for block in s._blocks: - res = _search_block(block, task) - if res: - return res + res = _search_state(s, task) + if res: + return res for block in self._play.handlers: res = _search_block(block, task) @@ -314,23 +369,36 @@ class PlayIterator: return None + def _insert_tasks_into_state(self, state, task_list): + if state.run_state == self.ITERATING_TASKS: + if state.tasks_child_state: + state.tasks_child_state = self._insert_tasks_into_state(state.tasks_child_state, task_list) + else: + target_block = state._blocks[state.cur_block].copy(exclude_parent=True) + before = target_block.block[:state.cur_regular_task] + after = target_block.block[state.cur_regular_task:] + target_block.block = before + task_list + after + state._blocks[state.cur_block] = target_block + elif state.run_state == self.ITERATING_RESCUE: + if state.rescue_child_state: + state.rescue_child_state = self._insert_tasks_into_state(state.rescue_child_state, task_list) + else: + target_block = state._blocks[state.cur_block].copy(exclude_parent=True) + before = target_block.rescue[:state.cur_rescue_task] + after = target_block.rescue[state.cur_rescue_task:] + target_block.rescue = before + task_list + after + state._blocks[state.cur_block] = target_block + elif state.run_state == self.ITERATING_ALWAYS: + if state.always_child_state: + state.always_child_state = self._insert_tasks_into_state(state.always_child_state, task_list) + else: + target_block = state._blocks[state.cur_block].copy(exclude_parent=True) + before = target_block.always[:state.cur_always_task] + after = target_block.always[state.cur_always_task:] + target_block.always = before + task_list + after + state._blocks[state.cur_block] = target_block + return state + def add_tasks(self, host, task_list): - s = self.get_host_state(host) - target_block = s._blocks[s.cur_block].copy(exclude_parent=True) - - if s.run_state == self.ITERATING_TASKS: - before = target_block.block[:s.cur_regular_task] - after = target_block.block[s.cur_regular_task:] - target_block.block = before + task_list + after - elif s.run_state == self.ITERATING_RESCUE: - before = target_block.rescue[:s.cur_rescue_task] - after = target_block.rescue[s.cur_rescue_task:] - target_block.rescue = before + task_list + after - elif s.run_state == self.ITERATING_ALWAYS: - before = target_block.always[:s.cur_always_task] - after = target_block.always[s.cur_always_task:] - target_block.always = before + task_list + after - - s._blocks[s.cur_block] = target_block - self._host_states[host.name] = s + self._host_states[host.name] = self._insert_tasks_into_state(self.get_host_state(host), task_list) diff --git a/lib/ansible/playbook/block.py b/lib/ansible/playbook/block.py index 6f492044148..400b2e7b730 100644 --- a/lib/ansible/playbook/block.py +++ b/lib/ansible/playbook/block.py @@ -325,16 +325,20 @@ class Block(Base, Become, Conditional, Taggable): def evaluate_and_append_task(target): tmp_list = [] for task in target: - if task.action in ('meta', 'include') or task.evaluate_tags(play_context.only_tags, play_context.skip_tags, all_vars=all_vars): + if isinstance(task, Block): + tmp_list.append(evaluate_block(task)) + elif task.action in ('meta', 'include') or task.evaluate_tags(play_context.only_tags, play_context.skip_tags, all_vars=all_vars): tmp_list.append(task) return tmp_list - new_block = self.copy() - new_block.block = evaluate_and_append_task(self.block) - new_block.rescue = evaluate_and_append_task(self.rescue) - new_block.always = evaluate_and_append_task(self.always) + def evaluate_block(block): + new_block = self.copy() + new_block.block = evaluate_and_append_task(block.block) + new_block.rescue = evaluate_and_append_task(block.rescue) + new_block.always = evaluate_and_append_task(block.always) + return new_block - return new_block + return evaluate_block(self) def has_tasks(self): return len(self.block) > 0 or len(self.rescue) > 0 or len(self.always) > 0 diff --git a/lib/ansible/playbook/task.py b/lib/ansible/playbook/task.py index 2eda23a7d70..7e6a5c68be2 100644 --- a/lib/ansible/playbook/task.py +++ b/lib/ansible/playbook/task.py @@ -256,7 +256,7 @@ class Task(Base, Conditional, Taggable, Become): new_me._task_include = None if self._task_include: - new_me._task_include = self._task_include.copy() + new_me._task_include = self._task_include.copy(exclude_block=exclude_block) return new_me diff --git a/lib/ansible/plugins/strategies/linear.py b/lib/ansible/plugins/strategies/linear.py index 527843c6923..2e6b3b23862 100644 --- a/lib/ansible/plugins/strategies/linear.py +++ b/lib/ansible/plugins/strategies/linear.py @@ -28,6 +28,13 @@ from ansible.plugins import action_loader from ansible.plugins.strategies import StrategyBase from ansible.template import Templar +try: + from __main__ import display +except ImportError: + from ansible.utils.display import Display + display = Display() + + class StrategyModule(StrategyBase): def _get_next_task_lockstep(self, hosts, iterator): @@ -43,8 +50,10 @@ class StrategyModule(StrategyBase): noop_task.set_loader(iterator._play._loader) host_tasks = {} + display.debug("building list of next tasks for hosts") for host in hosts: host_tasks[host.name] = iterator.get_next_task_for_host(host, peek=True) + display.debug("done building task lists") num_setups = 0 num_tasks = 0 @@ -53,6 +62,7 @@ class StrategyModule(StrategyBase): lowest_cur_block = len(iterator._blocks) + display.debug("counting tasks in each state of execution") for (k, v) in host_tasks.iteritems(): if v is None: continue @@ -72,6 +82,7 @@ class StrategyModule(StrategyBase): num_rescue += 1 elif s.run_state == PlayIterator.ITERATING_ALWAYS: num_always += 1 + display.debug("done counting tasks in each state of execution") def _advance_selected_hosts(hosts, cur_block, cur_state): ''' @@ -83,6 +94,7 @@ class StrategyModule(StrategyBase): # we return the values in the order they were originally # specified in the given hosts array rvals = [] + display.debug("starting to advance hosts") for host in hosts: host_state_task = host_tasks[host.name] if host_state_task is None: @@ -92,36 +104,39 @@ class StrategyModule(StrategyBase): continue if s.run_state == cur_state and s.cur_block == cur_block: new_t = iterator.get_next_task_for_host(host) - #if new_t != t: - # raise AnsibleError("iterator error, wtf?") FIXME rvals.append((host, t)) else: rvals.append((host, noop_task)) + display.debug("done advancing hosts to next task") return rvals - # if any hosts are in ITERATING_SETUP, return the setup task # while all other hosts get a noop if num_setups: + display.debug("advancing hosts in ITERATING_SETUP") return _advance_selected_hosts(hosts, lowest_cur_block, PlayIterator.ITERATING_SETUP) # if any hosts are in ITERATING_TASKS, return the next normal # task for these hosts, while all other hosts get a noop if num_tasks: + display.debug("advancing hosts in ITERATING_TASKS") return _advance_selected_hosts(hosts, lowest_cur_block, PlayIterator.ITERATING_TASKS) # if any hosts are in ITERATING_RESCUE, return the next rescue # task for these hosts, while all other hosts get a noop if num_rescue: + display.debug("advancing hosts in ITERATING_RESCUE") return _advance_selected_hosts(hosts, lowest_cur_block, PlayIterator.ITERATING_RESCUE) # if any hosts are in ITERATING_ALWAYS, return the next always # task for these hosts, while all other hosts get a noop if num_always: + display.debug("advancing hosts in ITERATING_ALWAYS") return _advance_selected_hosts(hosts, lowest_cur_block, PlayIterator.ITERATING_ALWAYS) # at this point, everything must be ITERATING_COMPLETE, so we # return None for all hosts in the list + display.debug("all hosts are done, so returning None's for all hosts") return [(host, None) for host in hosts] def run(self, iterator, play_context): @@ -200,15 +215,22 @@ class StrategyModule(StrategyBase): self._display.debug("done getting variables") if not callback_sent: - temp_task = task.copy() + display.debug("sending task start callback, copying the task so we can template it temporarily") + saved_name = task.name + display.debug("done copying, going to template now") try: - temp_task.name = unicode(templar.template(temp_task.name, fail_on_undefined=False)) + task.name = unicode(templar.template(task.name, fail_on_undefined=False)) + display.debug("done templating") except: # just ignore any errors during task name templating, # we don't care if it just shows the raw name + display.debug("templating failed for some reason") pass - self._tqm.send_callback('v2_playbook_on_task_start', temp_task, is_conditional=False) + display.debug("here goes the callback...") + self._tqm.send_callback('v2_playbook_on_task_start', task, is_conditional=False) + task.name = saved_name callback_sent = True + display.debug("sending task start callback") self._blocked_hosts[host.get_name()] = True self._queue_task(host, task, task_vars, play_context) diff --git a/samples/test_block.yml b/samples/test_block.yml index 25c90030823..626fef5e332 100644 --- a/samples/test_block.yml +++ b/samples/test_block.yml @@ -1,5 +1,4 @@ - hosts: all - connection: local gather_facts: yes tasks: - block: diff --git a/test/integration/roles/test_includes/tasks/main.yml b/test/integration/roles/test_includes/tasks/main.yml index b4808412bef..33aefe89599 100644 --- a/test/integration/roles/test_includes/tasks/main.yml +++ b/test/integration/roles/test_includes/tasks/main.yml @@ -31,11 +31,6 @@ b: 102 c: 103 -# Params specified via k=v values are strings, while those -# that come from variables will keep the type they were previously. -# Prior to v2.0, facts too priority over include params, however -# this is no longer the case. - - include: included_task1.yml a={{a}} b={{b}} c=103 - name: verify variable include params @@ -43,10 +38,10 @@ that: - "ca == 101" - "cb == 102" - - "cc == '103'" + - "cc == 103" # Test that strings are not turned into numbers -- set_fact: +- set_fact: a: "101" b: "102" c: "103" @@ -54,7 +49,7 @@ - include: included_task1.yml a={{a}} b={{b}} c=103 - name: verify variable include params - assert: + assert: that: - "ca == '101'" - "cb == '102'"