From 1825b4a9c74417764f0d6f0c4a4a515a55a07735 Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Fri, 15 Jan 2016 13:14:27 -0500 Subject: [PATCH] Fix any_errors_fatal incorrect implementation in 2.0 Also adds that flag to blocks. Fixes #13744 --- lib/ansible/executor/playbook_executor.py | 4 +--- lib/ansible/playbook/block.py | 11 +++++++++++ lib/ansible/playbook/task.py | 19 +++++++++++-------- lib/ansible/plugins/strategy/free.py | 2 ++ lib/ansible/plugins/strategy/linear.py | 12 ++++++++++++ 5 files changed, 37 insertions(+), 11 deletions(-) diff --git a/lib/ansible/executor/playbook_executor.py b/lib/ansible/executor/playbook_executor.py index bdb08993e3..bcfe1bebbe 100644 --- a/lib/ansible/executor/playbook_executor.py +++ b/lib/ansible/executor/playbook_executor.py @@ -151,9 +151,7 @@ class PlaybookExecutor: # conditions are met, we break out, otherwise we only break out if the entire # batch failed failed_hosts_count = len(self._tqm._failed_hosts) + len(self._tqm._unreachable_hosts) - if new_play.any_errors_fatal and failed_hosts_count > 0: - break - elif new_play.max_fail_percentage is not None and \ + if new_play.max_fail_percentage is not None and \ int((new_play.max_fail_percentage)/100.0 * len(batch)) > int((len(batch) - failed_hosts_count) / len(batch) * 100.0): break elif len(batch) == failed_hosts_count: diff --git a/lib/ansible/playbook/block.py b/lib/ansible/playbook/block.py index f2d9c82833..be73c5d8ac 100644 --- a/lib/ansible/playbook/block.py +++ b/lib/ansible/playbook/block.py @@ -35,6 +35,7 @@ class Block(Base, Become, Conditional, Taggable): _always = FieldAttribute(isa='list', default=[]) _delegate_to = FieldAttribute(isa='list') _delegate_facts = FieldAttribute(isa='bool', default=False) + _any_errors_fatal = FieldAttribute(isa='bool') # for future consideration? this would be functionally # similar to the 'else' clause for exceptions @@ -330,6 +331,16 @@ class Block(Base, Become, Conditional, Taggable): return environment + def _get_attr_any_errors_fatal(self): + ''' + Override for the 'tags' getattr fetcher, used from Base. + ''' + any_errors_fatal = self._attributes['any_errors_fatal'] + if hasattr(self, '_get_parent_attribute'): + if self._get_parent_attribute('any_errors_fatal'): + any_errors_fatal = True + return any_errors_fatal + def filter_tagged_tasks(self, play_context, all_vars): ''' Creates a new block, with task lists filtered based on the tags contained diff --git a/lib/ansible/playbook/task.py b/lib/ansible/playbook/task.py index 62b8cbc999..154ff53d5e 100644 --- a/lib/ansible/playbook/task.py +++ b/lib/ansible/playbook/task.py @@ -216,14 +216,6 @@ class Task(Base, Conditional, Taggable, Become): return super(Task, self).preprocess_data(new_ds) - def _load_any_errors_fatal(self, attr, value): - ''' - Exists only to show a deprecation warning, as this attribute is not valid - at the task level. - ''' - display.deprecated("Setting any_errors_fatal on a task is no longer supported. This should be set at the play level only") - return None - def post_validate(self, templar): ''' Override of base class post_validate, to also do final validation on @@ -422,3 +414,14 @@ class Task(Base, Conditional, Taggable, Become): if parent_environment is not None: environment = self._extend_value(environment, parent_environment) return environment + + def _get_attr_any_errors_fatal(self): + ''' + Override for the 'tags' getattr fetcher, used from Base. + ''' + any_errors_fatal = self._attributes['any_errors_fatal'] + if hasattr(self, '_get_parent_attribute'): + if self._get_parent_attribute('any_errors_fatal'): + any_errors_fatal = True + return any_errors_fatal + diff --git a/lib/ansible/plugins/strategy/free.py b/lib/ansible/plugins/strategy/free.py index 976d33abba..da123ce3b7 100644 --- a/lib/ansible/plugins/strategy/free.py +++ b/lib/ansible/plugins/strategy/free.py @@ -122,6 +122,8 @@ class StrategyModule(StrategyBase): else: # handle step if needed, skip meta actions as they are used internally if not self._step or self._take_step(task, host_name): + if task.any_errors_fatal: + 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) diff --git a/lib/ansible/plugins/strategy/linear.py b/lib/ansible/plugins/strategy/linear.py index f441b88fe3..40c435ca53 100644 --- a/lib/ansible/plugins/strategy/linear.py +++ b/lib/ansible/plugins/strategy/linear.py @@ -280,6 +280,7 @@ class StrategyModule(StrategyBase): except AnsibleError as e: return False + include_failure = False if len(included_files) > 0: display.debug("we have included files to process") noop_task = Task() @@ -325,6 +326,7 @@ class StrategyModule(StrategyBase): self._tqm._failed_hosts[host.name] = True iterator.mark_host_failed(host) display.error(e, wrap_text=False) + include_failure = True continue # finally go through all of the hosts and append the @@ -338,6 +340,16 @@ class StrategyModule(StrategyBase): display.debug("done processing included files") display.debug("results queue empty") + + display.debug("checking for any_errors_fatal") + had_failure = include_failure + for res in results: + if res.is_failed() or res.is_unreachable(): + had_failure = True + break + if task and task.any_errors_fatal and had_failure: + return False + except (IOError, EOFError) as e: display.debug("got IOError/EOFError in task loop: %s" % e) # most likely an abort, return failed