From 699a854bf30784cd7514f4c1b6f8bd742868f7eb Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Wed, 3 Feb 2016 18:42:27 -0500 Subject: [PATCH] Fixing bugs in play iteration and strategies * Fixed a bug in PlayIterator when ITERATING_ALWAYS, where the block was advanced but the incorrect data structure elements were cleared * Cleaned up the logic of is_failed() in PlayIterator * Fixed a bug in the free strategy which had not been updated to use the base strategy _execute_meta() method * Stopped strategies from using is_failed() to determine if tasks should still be fetched for a host Fixes #14040 --- lib/ansible/executor/play_iterator.py | 21 +++++++++++---------- lib/ansible/plugins/strategy/free.py | 17 ++++------------- lib/ansible/plugins/strategy/linear.py | 3 +-- 3 files changed, 16 insertions(+), 25 deletions(-) diff --git a/lib/ansible/executor/play_iterator.py b/lib/ansible/executor/play_iterator.py index e46a8d1507..ec85ce2f33 100644 --- a/lib/ansible/executor/play_iterator.py +++ b/lib/ansible/executor/play_iterator.py @@ -275,7 +275,7 @@ class PlayIterator: if state.pending_setup: state.pending_setup = False - if state.fail_state & self.FAILED_TASKS == self.FAILED_TASKS: + if self._check_failed_state(state): state.run_state = self.ITERATING_RESCUE elif state.cur_regular_task >= len(block.block): state.run_state = self.ITERATING_ALWAYS @@ -335,7 +335,9 @@ class PlayIterator: state.cur_rescue_task = 0 state.cur_always_task = 0 state.run_state = self.ITERATING_TASKS - state.child_state = None + state.tasks_child_state = None + state.rescue_child_state = None + state.always_child_state = None else: task = block.always[state.cur_always_task] if isinstance(task, Block) or state.always_child_state is not None: @@ -365,7 +367,7 @@ class PlayIterator: return (state, task) def _set_failed_state(self, state): - if state.pending_setup: + if state.run_state == self.ITERATING_SETUP: state.fail_state |= self.FAILED_SETUP state.run_state = self.ITERATING_COMPLETE elif state.run_state == self.ITERATING_TASKS: @@ -407,19 +409,18 @@ class PlayIterator: def _check_failed_state(self, state): if state is None: return False + elif state.fail_state != self.FAILED_NONE: + if state.run_state == self.ITERATING_RESCUE and state.fail_state&self.FAILED_RESCUE == 0 or \ + state.run_state == self.ITERATING_ALWAYS and state.fail_state&self.FAILED_ALWAYS == 0: + return False + else: + return True elif state.run_state == self.ITERATING_TASKS and self._check_failed_state(state.tasks_child_state): return True elif state.run_state == self.ITERATING_RESCUE and self._check_failed_state(state.rescue_child_state): return True elif state.run_state == self.ITERATING_ALWAYS and self._check_failed_state(state.always_child_state): return True - elif state.run_state == self.ITERATING_COMPLETE and state.fail_state != self.FAILED_NONE: - if state.run_state == self.ITERATING_RESCUE and state.fail_state&self.FAILED_RESCUE == 0: - return False - elif state.run_state == self.ITERATING_ALWAYS and state.fail_state&self.FAILED_ALWAYS == 0: - return False - else: - return True return False def is_failed(self, host): diff --git a/lib/ansible/plugins/strategy/free.py b/lib/ansible/plugins/strategy/free.py index da123ce3b7..17516c91bc 100644 --- a/lib/ansible/plugins/strategy/free.py +++ b/lib/ansible/plugins/strategy/free.py @@ -78,7 +78,7 @@ class StrategyModule(StrategyBase): (state, task) = iterator.get_next_task_for_host(host, peek=True) display.debug("free host state: %s" % state) display.debug("free host task: %s" % task) - if not iterator.is_failed(host) and host_name not in self._tqm._unreachable_hosts and task: + if host_name not in self._tqm._unreachable_hosts and task: # set the flag so the outer loop knows we've still found # some work which needs to be done @@ -106,18 +106,7 @@ class StrategyModule(StrategyBase): continue if task.action == 'meta': - # meta tasks store their args in the _raw_params field of args, - # since they do not use k=v pairs, so get that - meta_action = task.args.get('_raw_params') - if meta_action == 'noop': - continue - elif meta_action == 'flush_handlers': - # FIXME: in the 'free' mode, flushing handlers should result in - # only those handlers notified for the host doing the flush - self.run_handlers(iterator, play_context) - else: - raise AnsibleError("invalid meta action requested: %s" % meta_action, obj=task._ds) - + self._execute_meta(task, play_context, iterator) self._blocked_hosts[host_name] = False else: # handle step if needed, skip meta actions as they are used internally @@ -126,6 +115,8 @@ class StrategyModule(StrategyBase): display.warning("Using any_errors_fatal with the free strategy is not supported, as tasks are executed independently on each host") self._tqm.send_callback('v2_playbook_on_task_start', task, is_conditional=False) self._queue_task(host, task, task_vars, play_context) + else: + display.debug("%s is blocked, skipping for now" % host_name) # move on to the next host and make sure we # haven't gone past the end of our hosts list diff --git a/lib/ansible/plugins/strategy/linear.py b/lib/ansible/plugins/strategy/linear.py index 804cfadc77..00c8e15749 100644 --- a/lib/ansible/plugins/strategy/linear.py +++ b/lib/ansible/plugins/strategy/linear.py @@ -54,8 +54,7 @@ class StrategyModule(StrategyBase): host_tasks = {} display.debug("building list of next tasks for hosts") for host in hosts: - if not iterator.is_failed(host): - host_tasks[host.name] = iterator.get_next_task_for_host(host, peek=True) + host_tasks[host.name] = iterator.get_next_task_for_host(host, peek=True) display.debug("done building task lists") num_setups = 0