-
Notifications
You must be signed in to change notification settings - Fork 23.8k
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
Mentions 'meta: flush_handlers' task #79542
Conversation
Thanks for your Ansible docs contribution! We talk about Ansible documentation on matrix at #docs:ansible.im and on libera IRC at #ansible-docs if you ever want to join us and chat about the docs! We meet there on Tuesdays (see the Ansible calendar) and welcome additions to our weekly agenda items - scroll down to find the upcoming agenda and add a comment to put something new on that agenda. |
@@ -188,6 +188,8 @@ This is the great culmination of embedded tests: | |||
ansible.builtin.command: /usr/bin/add_back_to_pool {{ inventory_hostname }} | |||
delegate_to: 127.0.0.1 | |||
|
|||
If you need a handler to run before a particular role, use the ``meta`` module with the ``flush_handlers`` option. Consider placing this task in your test role itself. For more information, see :ref:`handlers`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be out of context. We do document the flush_handlers
meta task in the appropriate sections.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do mention them here if that helps - https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_handlers.html#controlling-when-handlers-run
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay rereading this and the issue that spawned this PR. Seems I put the wrong info in the issue. I think this new line likely belongs at https://github.com/ansible/ansible/blame/devel/docs/docsite/rst/reference_appendices/test_strategies.rst#L194 instead. What do you think @mkrizek ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think @samccann meant this https://docs.ansible.com/ansible/latest/collections/ansible/builtin/meta_module.html
@mkrizek, do you think we should remove the paragraph you highlighted and instead put there a reference to https://docs.ansible.com/ansible/latest/collections/ansible/builtin/meta_module.html as it mentions "flush_handlers
makes Ansible run any handler tasks which have..." ?
Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it would be better to just fix the example playbook rather than mention what could go wrong with it?
We could rewrite the playbook and move a "call" to apply_testing_checks
role into post_tasks
via include_role
(since flush_handlers
is implicitly called before post_tasks
is executed). Or run flush_handlers
explicitly to make the intent more obvious.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc: @mkrizek
I wonder if it would be better to just fix the example playbook rather... you mean to update the play/playbook that immediately precedes?
---
- hosts: webservers
serial: 5
pre_tasks:
- name: take out of load balancer pool
ansible.builtin.command: /usr/bin/take_out_of_pool {{ inventory_hostname }}
delegate_to: 127.0.0.1
roles:
- common
- webserver
- apply_testing_checks
post_tasks:
- name: add back to load balancer pool
ansible.builtin.command: /usr/bin/add_back_to_pool {{ inventory_hostname }}
delegate_to: 127.0.0.1
It sounds good that we update this playbook to make it explicit how flush_handlers
should work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for updating the example
I removed the sentence "If you need a handler to run before...", updated the
cc: @mkrizek |
So the two options I see are: In this example the - hosts: webservers
serial: 5
pre_tasks:
- name: take out of load balancer pool
ansible.builtin.command: /usr/bin/take_out_of_pool {{ inventory_hostname }}
delegate_to: 127.0.0.1
roles:
- common
- webserver
post_tasks:
- name: test the configuration
ansible.builtin.include_role:
name: apply_testing_checks
- name: add back to load balancer pool
ansible.builtin.command: /usr/bin/add_back_to_pool {{ inventory_hostname }}
delegate_to: 127.0.0.1 or run all roles via - hosts: webservers
serial: 5
pre_tasks:
- name: take out of load balancer pool
ansible.builtin.command: /usr/bin/take_out_of_pool {{ inventory_hostname }}
delegate_to: 127.0.0.1
tasks:
- ansible.builtin.include_role:
name: "{{ item }}"
loop:
- common
- webserver
- name: run any notified handlers
ansible.builtin.meta: flush_handlers
- name: test the configuration
ansible.builtin.include_role:
name: apply_testing_checks
post_tasks:
- name: add back to load balancer pool
ansible.builtin.command: /usr/bin/add_back_to_pool {{ inventory_hostname }}
delegate_to: 127.0.0.1 I think the second example that runs |
I went with the 2nd option -- run all roles via |
Thanks @JaroslavKlech for the Ansible docs fix! |
(cherry picked from commit d8dc76e)
* [Docs] add easyfix/good first issue/docs links (#79830) (cherry picked from commit 722fc05) * [Docs] add doc links to documentation_contributions.rst (#79840) (cherry picked from commit 58f0950) * Remove dev_guide stubs (#79795) * Remove dev_guide stubs * Remove Cisco ACI Dev Guide (cherry picked from commit 10f0e5f) * maintainers_guidelines.rst: add a link to collection release guidelines (#79859) (cherry picked from commit fa38267) * Mentions 'meta: flush_handlers' task (#79542) (cherry picked from commit d8dc76e) * Description for changing User ID to match user value (#79470) (cherry picked from commit 913e486) * Remove irrelevant line (#79865) Remove irrelevant comment line form example code (cherry picked from commit 1c01eab) --------- Co-authored-by: Andrew Klychkov <aaklychkov@mail.ru> Co-authored-by: Mario Lenz <m@riolenz.de> Co-authored-by: Andrew Klychkov <aklychko@redhat.com> Co-authored-by: JaroslavKlech <klechh@gmail.com> Co-authored-by: Tabah Baridule M <dulemartins07@gmail.com> Co-authored-by: Konrad Gawda <konrad.gawda.opensource@gmail.com>
SUMMARY
Encourage users to use
meta: flush_handlers
task in the test role itselfISSUE TYPE
COMPONENT NAME
docs.ansible.com
Fixes #59109