Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not double fail hosts for includes #55913

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from
Open

Conversation

@mkrizek
Copy link
Contributor

@mkrizek mkrizek commented Apr 30, 2019

SUMMARY

Fixes #23161

- block:
  - include_tasks: "{{ item }}.yml"
    loop:
      - this_does_not_exist
      - this_does_not_exist_either
  rescue:
     - debug:
          msg: rescue

The above block (either with include_tasks or include_role) fails to execute the rescue section because we double mark hosts as failed for failed includes and so play iterator skips the rescue section.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME
lib/ansible/plugins/strategy/__init__.py
lib/ansible/plugins/strategy/free.py
lib/ansible/plugins/strategy/linear.py
if host.name not in self._tqm._failed_hosts:
self._tqm._failed_hosts[host.name] = True
iterator.mark_host_failed(host)
display.error(to_text(e), wrap_text=False)
Copy link
Contributor Author

@mkrizek mkrizek Apr 30, 2019

This makes the failures handling consistent with how linear strategy does it. I am not sure why that was different or if that was intentional.

@@ -924,8 +925,9 @@ def _do_handler_run(self, handler, handler_name, iterator, play_context, notifie
break
except AnsibleError as e:
for host in included_file._hosts:
iterator.mark_host_failed(host)
self._tqm._failed_hosts[host.name] = True
if host.name not in self._tqm._failed_hosts:
Copy link
Contributor Author

@mkrizek mkrizek Apr 30, 2019

I wonder if we should prevent double failing hosts in one place in iterator.mark_host_failed.

@mkrizek
Copy link
Contributor Author

@mkrizek mkrizek commented Apr 30, 2019

cc @sivel

jimi-c
jimi-c approved these changes Apr 30, 2019
@mkrizek mkrizek changed the title [WIP] Do not double fail hosts for includes Do not double fail hosts for includes Apr 30, 2019
@ansibot ansibot added core_review and removed WIP labels Apr 30, 2019
Copy link
Contributor

@s-hertel s-hertel left a comment

Tested, looks good to me.

Copy link
Contributor

@Andersson007 Andersson007 left a comment

LGTM
would be great to have integration tests for this case

@relrod
Copy link
Member

@relrod relrod commented Oct 30, 2020

@mkrizek is this still valid?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

7 participants