Don't immediately return failed for any_errors_fatal tasks
Instead of immediately returning a failed code (indicating a break in the play execution), we internally 'or' that failure code with the result (now an integer flag instead of a boolean) so that we can properly handle the rescue/always portions of blocks and still remember that the break condition was hit. Fixes #16937
This commit is contained in:
parent
b44eb402bd
commit
c669a381d1
5 changed files with 26 additions and 25 deletions
|
@ -147,7 +147,7 @@ class PlaybookExecutor:
|
|||
result = self._tqm.run(play=play)
|
||||
|
||||
# break the play if the result equals the special return code
|
||||
if result == self._tqm.RUN_FAILED_BREAK_PLAY:
|
||||
if result & self._tqm.RUN_FAILED_BREAK_PLAY != 0:
|
||||
result = self._tqm.RUN_FAILED_HOSTS
|
||||
break_play = True
|
||||
|
||||
|
|
|
@ -61,8 +61,8 @@ class TaskQueueManager:
|
|||
RUN_OK = 0
|
||||
RUN_ERROR = 1
|
||||
RUN_FAILED_HOSTS = 2
|
||||
RUN_UNREACHABLE_HOSTS = 3
|
||||
RUN_FAILED_BREAK_PLAY = 4
|
||||
RUN_UNREACHABLE_HOSTS = 4
|
||||
RUN_FAILED_BREAK_PLAY = 8
|
||||
RUN_UNKNOWN_ERROR = 255
|
||||
|
||||
def __init__(self, inventory, variable_manager, loader, options, passwords, stdout_callback=None, run_additional_callbacks=True, run_tree=False):
|
||||
|
|
|
@ -119,14 +119,18 @@ class StrategyBase:
|
|||
# outstanding tasks still in queue
|
||||
self._blocked_hosts = dict()
|
||||
|
||||
def run(self, iterator, play_context, result=True):
|
||||
def run(self, iterator, play_context, result=0):
|
||||
# save the failed/unreachable hosts, as the run_handlers()
|
||||
# method will clear that information during its execution
|
||||
failed_hosts = iterator.get_failed_hosts()
|
||||
unreachable_hosts = self._tqm._unreachable_hosts.keys()
|
||||
|
||||
display.debug("running handlers")
|
||||
result &= self.run_handlers(iterator, play_context)
|
||||
handler_result = self.run_handlers(iterator, play_context)
|
||||
if isinstance(handler_result, bool) and not handler_result:
|
||||
result |= self._tqm.RUN_ERROR
|
||||
elif not handler_result:
|
||||
result |= handler_result
|
||||
|
||||
# now update with the hosts (if any) that failed or were
|
||||
# unreachable during the handler execution phase
|
||||
|
@ -140,8 +144,6 @@ class StrategyBase:
|
|||
return self._tqm.RUN_UNREACHABLE_HOSTS
|
||||
elif len(failed_hosts) > 0:
|
||||
return self._tqm.RUN_FAILED_HOSTS
|
||||
elif isinstance(result, bool) and not result:
|
||||
return self._tqm.RUN_ERROR
|
||||
else:
|
||||
return self._tqm.RUN_OK
|
||||
|
||||
|
@ -296,7 +298,11 @@ class StrategyBase:
|
|||
display.debug("marking %s as failed" % original_host.name)
|
||||
if original_task.run_once:
|
||||
# if we're using run_once, we have to fail every host here
|
||||
[iterator.mark_host_failed(h) for h in self._inventory.get_hosts(iterator._play.hosts) if h.name not in self._tqm._unreachable_hosts]
|
||||
for h in self._inventory.get_hosts(iterator._play.hosts):
|
||||
if h.name not in self._tqm._unreachable_hosts:
|
||||
state, _ = iterator.get_next_task_for_host(h, peek=True)
|
||||
iterator.mark_host_failed(h)
|
||||
state, new_task = iterator.get_next_task_for_host(h, peek=True)
|
||||
else:
|
||||
iterator.mark_host_failed(original_host)
|
||||
|
||||
|
@ -631,7 +637,7 @@ class StrategyBase:
|
|||
Runs handlers on those hosts which have been notified.
|
||||
'''
|
||||
|
||||
result = True
|
||||
result = self._tqm.RUN_OK
|
||||
|
||||
for handler_block in iterator._play.handlers:
|
||||
# FIXME: handlers need to support the rescue/always portions of blocks too,
|
||||
|
@ -671,7 +677,7 @@ class StrategyBase:
|
|||
|
||||
host_results = []
|
||||
for host in notified_hosts:
|
||||
if not handler.has_triggered(host) and (host.name not in self._tqm._failed_hosts or play_context.force_handlers):
|
||||
if not handler.has_triggered(host) and (not iterator.is_failed(host) or play_context.force_handlers):
|
||||
task_vars = self._variable_manager.get_vars(loader=self._loader, play=iterator._play, host=host, task=handler)
|
||||
self.add_tqm_variables(task_vars, play=iterator._play)
|
||||
self._queue_task(host, handler, task_vars, play_context)
|
||||
|
|
|
@ -157,7 +157,7 @@ class StrategyModule(StrategyBase):
|
|||
'''
|
||||
|
||||
# iteratate over each task, while there is one left to run
|
||||
result = True
|
||||
result = self._tqm.RUN_OK
|
||||
work_to_do = True
|
||||
while work_to_do and not self._tqm._terminated:
|
||||
|
||||
|
@ -269,12 +269,6 @@ class StrategyModule(StrategyBase):
|
|||
results += self._wait_on_pending_results(iterator)
|
||||
host_results.extend(results)
|
||||
|
||||
if not work_to_do and len(iterator.get_failed_hosts()) > 0:
|
||||
display.debug("out of hosts to run on")
|
||||
self._tqm.send_callback('v2_playbook_on_no_hosts_remaining')
|
||||
result = self._tqm.RUN_ERROR
|
||||
break
|
||||
|
||||
try:
|
||||
included_files = IncludedFile.process_include_results(
|
||||
host_results,
|
||||
|
@ -285,6 +279,7 @@ class StrategyModule(StrategyBase):
|
|||
variable_manager=self._variable_manager
|
||||
)
|
||||
except AnsibleError as e:
|
||||
# this is a fatal error, so we abort here regardless of block state
|
||||
return self._tqm.RUN_ERROR
|
||||
|
||||
include_failure = False
|
||||
|
@ -360,13 +355,10 @@ class StrategyModule(StrategyBase):
|
|||
# if any_errors_fatal and we had an error, mark all hosts as failed
|
||||
if any_errors_fatal and (len(failed_hosts) > 0 or len(unreachable_hosts) > 0):
|
||||
for host in hosts_left:
|
||||
# don't double-mark hosts, or the iterator will potentially
|
||||
# fail them out of the rescue/always states
|
||||
if host.name not in failed_hosts:
|
||||
(s, _) = iterator.get_next_task_for_host(host, peek=True)
|
||||
if s.run_state != iterator.ITERATING_RESCUE:
|
||||
self._tqm._failed_hosts[host.name] = True
|
||||
iterator.mark_host_failed(host)
|
||||
self._tqm.send_callback('v2_playbook_on_no_hosts_remaining')
|
||||
return self._tqm.RUN_FAILED_BREAK_PLAY
|
||||
result |= self._tqm.RUN_FAILED_BREAK_PLAY
|
||||
display.debug("done checking for any_errors_fatal")
|
||||
|
||||
display.debug("checking for max_fail_percentage")
|
||||
|
@ -381,7 +373,7 @@ class StrategyModule(StrategyBase):
|
|||
self._tqm._failed_hosts[host.name] = True
|
||||
iterator.mark_host_failed(host)
|
||||
self._tqm.send_callback('v2_playbook_on_no_hosts_remaining')
|
||||
return self._tqm.RUN_FAILED_BREAK_PLAY
|
||||
result |= self._tqm.RUN_FAILED_BREAK_PLAY
|
||||
display.debug("done checking for max_fail_percentage")
|
||||
|
||||
except (IOError, EOFError) as e:
|
||||
|
|
|
@ -60,6 +60,9 @@ class TestStrategyBase(unittest.TestCase):
|
|||
mock_tqm._listening_handlers = {}
|
||||
mock_tqm.send_callback.return_value = None
|
||||
|
||||
for attr in ('RUN_OK', 'RUN_ERROR', 'RUN_FAILED_HOSTS', 'RUN_UNREACHABLE_HOSTS'):
|
||||
setattr(mock_tqm, attr, getattr(TaskQueueManager, attr))
|
||||
|
||||
mock_iterator = MagicMock()
|
||||
mock_iterator._play = MagicMock()
|
||||
mock_iterator._play.handlers = []
|
||||
|
@ -77,7 +80,7 @@ class TestStrategyBase(unittest.TestCase):
|
|||
mock_host.name = 'host1'
|
||||
|
||||
self.assertEqual(strategy_base.run(iterator=mock_iterator, play_context=mock_play_context), mock_tqm.RUN_OK)
|
||||
self.assertEqual(strategy_base.run(iterator=mock_iterator, play_context=mock_play_context, result=False), mock_tqm.RUN_ERROR)
|
||||
self.assertEqual(strategy_base.run(iterator=mock_iterator, play_context=mock_play_context, result=TaskQueueManager.RUN_ERROR), mock_tqm.RUN_ERROR)
|
||||
mock_tqm._failed_hosts = dict(host1=True)
|
||||
mock_iterator.get_failed_hosts.return_value = [mock_host]
|
||||
self.assertEqual(strategy_base.run(iterator=mock_iterator, play_context=mock_play_context, result=False), mock_tqm.RUN_FAILED_HOSTS)
|
||||
|
|
Loading…
Reference in a new issue