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

Handlers not executed of roles referenced by dependency in combination with conditional role in playbook #83110

Closed
1 task done
alvinkwekel opened this issue Apr 19, 2024 · 10 comments
Labels
affects_2.16 bug This issue/PR relates to a bug.

Comments

@alvinkwekel
Copy link

alvinkwekel commented Apr 19, 2024

Summary

There seems to be an issue with role dependencies in combination with a conditional role in the playbook.

We have a playbook with multiple roles. Role A had a dependency on role B. But both role A and B are also defined in the playbook. They are defined in the order B and then A. And role A has a when condition so it's not executed on all hosts.

The handler of the role to which the dependency points (role B) is not executed on the hosts for which the when condition in the playbook for the role with the dependency (role A) was false. This only happens if the role with the dependency (role A) is defined later in the playbook

This is probably a strange setup which I've changed now, but nevertheless I felt it would be good to report this bug.

This setup used to work in Ansible 6 and stopped working once we upgraded to Ansible 8. It also doesn't work correctly on Ansible 9.4. In the expected result I've put the output of Ansible 6 in the actual result that of Ansible 9.4.0.

Issue Type

Bug Report

Component Name

core

Ansible Version

$ ansible --version
ansible [core 2.16.6]
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/runner/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/lib/python3.11/site-packages/ansible
  ansible collection location = /runner/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/local/bin/ansible
  python version = 3.11.7 (main, Jan 26 2024, 19:22:20) [GCC 8.5.0 20210514 (Red Hat 8.5.0-21)] (/usr/bin/python3.11)
  jinja version = 3.1.3
  libyaml = True

Configuration

# if using a version older than ansible-core 2.12 you should omit the '-t all'
$ ansible-config dump --only-changed -t all
Omitted until desired and not reproducible without.

OS / Environment

Ubuntu 20.04

Steps to Reproduce

test_playbook.yml

- name: Test playbook
  hosts: server-01,server-02
  become: true
  roles:
    - test_role_2
    - role: test_role_1
      when: inventory_hostname == "server-01"

roles/test_role_1/tasks/main.yml

- name: Test task 1
  debug:
    msg: Test task 1

roles/test_role_1/meta/main.yml

dependencies:
  - test_role_2

roles/test_role_2/tasks/main.yml

- name: Test task 2
  debug:
    msg: Test task 2
  notify: Test handler 2
  changed_when: true

roles/test_role_2/handlers/main.yml

- name: Test handler 2
  debug:
    msg: Test handler 2
  listen: Test handler 2

Expected Results

PLAY [Test playbook] ***********************************************************************************************************************************************************

TASK [Gathering Facts] *********************************************************************************************************************************************************
ok: [server-01]
ok: [server-02]

TASK [test_role_2 : Get ansible version] ***************************************************************************************************************************************
changed: [server-02 -> localhost]
changed: [server-01 -> localhost]

TASK [test_role_2 : Test task 2] ***********************************************************************************************************************************************
changed: [server-01] => {
    "msg": "Test task 2"
}
changed: [server-02] => {
    "msg": "Test task 2"
}

TASK [test_role_1 : Test task 1] ***********************************************************************************************************************************************
ok: [server-01] => {
    "msg": "Test task 1"
}
skipping: [server-02]

RUNNING HANDLER [test_role_2 : Test handler 2] *********************************************************************************************************************************
ok: [server-01] => {
    "msg": "Test handler 2"
}
ok: [server-02] => {
    "msg": "Test handler 2"
}

Actual Results

PLAY [Test playbook] ***********************************************************

TASK [Gathering Facts] *********************************************************
ok: [server-02]
ok: [server-01]

TASK [test_role_2 : Get ansible version] ***************************************
changed: [server-01 -> localhost]
changed: [server-02 -> localhost]

TASK [test_role_2 : Test task 2] ***********************************************
changed: [server-01] => {
    "msg": "Test task 2"
}
changed: [server-02] => {
    "msg": "Test task 2"
}

TASK [test_role_1 : Test task 1] ***********************************************
ok: [server-01] => {
    "msg": "Test task 1"
}
skipping: [server-02]

RUNNING HANDLER [test_role_2 : Test handler 2] *********************************
ok: [server-01] => {
    "msg": "Test handler 2"
}
skipping: [server-02]

Code of Conduct

  • I agree to follow the Ansible Code of Conduct
@ansibot ansibot added bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. affects_2.16 labels Apr 19, 2024
@ansibot
Copy link
Contributor

ansibot commented Apr 19, 2024

Files identified in the description:

None

If these files are incorrect, please update the component name section of the description or use the component bot command.

@alvinkwekel alvinkwekel changed the title No linear execution in handler after include_tasks with when condition Handlers not executed of roles referenced by dependency in combination with conditional role in playbook Apr 19, 2024
@s-hertel
Copy link
Contributor

This looks like a duplicate of #81013 (comment) / #80925 (which has a porting guide entry, though it focuses on include_role). If the handler didn't have the listen keyword, it should have always behaved as it does now, but there was a bug where the search order was backwards for listen, which was fixed in #81358.

@mkrizek
Copy link
Contributor

mkrizek commented Apr 22, 2024

This is the documented behavior. Handlers are play scoped, last one with the same name wins; in this case the last one inserted into the play inherits when: inventory_hostname == "server-01" which is why it is only run on server-01.

@MorphBonehunter
Copy link

Ok, maybe i didn't understand the docs right, but i can not see the fact that the handlers played scoped besides that all handlers are in global scope and handler definitions included later in the play with identical names overwrites one defined before.
The docs doesn't mention that conditional includes of roles defacto disable handler execution for all hosts which not match the condition as i mention in #83112 and which is also the reason for this issue.

As i also mentioned in the linked bug, this behavior changes over time as we do have an Debian 11 with ansible core 2.12.10 and there it works so this is some kind of regression, maybe with #77955.

We can workaround this with defining handlers per role which in fact do the same but with different names.

@mkrizek
Copy link
Contributor

mkrizek commented Apr 22, 2024

Ok, maybe i didn't understand the docs right, but i can not see the fact that the handlers played scoped besides that all handlers are in global scope and handler definitions included later in the play with identical names overwrites one defined before.

I feel like this is covered in https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_handlers.html#naming-handlers. But any suggestions on how to improve the docs are always welcome.

The docs doesn't mention that conditional includes of roles defacto disable handler execution for all hosts which not match the condition as i mention in #83112 and which is also the reason for this issue.

That is indeed not mentioned there but it is a general functionality in Ansible and not specific to handlers. Every task and handler within a role imported statically (either via roles: section or import_role) inherit the when conditional (and other keywords).

As i also mentioned in the linked bug, this behavior changes over time as we do have an Debian 11 with ansible core 2.12.10 and there it works so this is some kind of regression, maybe with #77955.

Please see the comment from @s-hertel above. It is not a regression but a fix for unintended behavior with an entry in the porting guide. Sorry this has caused your playbook to be incompatible a with newer ansible-core version, we try and list changes like this in a changelog and porting guide at least.

We can workaround this with defining handlers per role which in fact do the same but with different names.

You can target a handler from a specific role by using a special form of the handler name in notify, see Handler in roles in https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_handlers.html#handlers-in-roles.

Hope this helps.

@MorphBonehunter
Copy link

Ok, maybe i didn't understand the docs right, but i can not see the fact that the handlers played scoped besides that all handlers are in global scope and handler definitions included later in the play with identical names overwrites one defined before.

I feel like this is covered in https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_handlers.html#naming-handlers. But any suggestions on how to improve the docs are always welcome.

Ok, i see what you mean, the docs are good on this point, but my problem is, that this was not my point 😄

The docs doesn't mention that conditional includes of roles defacto disable handler execution for all hosts which not match the condition as i mention in #83112 and which is also the reason for this issue.

That is indeed not mentioned there but it is a general functionality in Ansible and not specific to handlers. Every task and handler within a role imported statically (either via roles: section or import_role) inherit the when conditional (and other keywords).

Yes, that true, i was just convinced that this does not apply to handlers.

As i also mentioned in the linked bug, this behavior changes over time as we do have an Debian 11 with ansible core 2.12.10 and there it works so this is some kind of regression, maybe with #77955.

Please see the comment from @s-hertel above. It is not a regression but a fix for unintended behavior with an entry in the porting guide. Sorry this has caused your playbook to be incompatible a with newer ansible-core version, we try and list changes like this in a changelog and porting guide at least.

I've seen the porting guide but honestly i did not think about it which impact this has.
Maybe a good Idea to link to this from the handler documentation, could be helpful.

We can workaround this with defining handlers per role which in fact do the same but with different names.

You can target a handler from a specific role by using a special form of the handler name in notify, see Handler in roles in https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_handlers.html#handlers-in-roles.

This doesn't work for our case, because of the single source of handler definition.

Hope this helps.

The hint for the porting guide was a good one, a reminder to read this more carefully, so yes i think this helps.
So maybe it's time to rethink some of our "old" role concepts.

@s-hertel
Copy link
Contributor

s-hertel commented Apr 22, 2024

This doesn't work for our case, because of the single source of handler definition.

You said you work around this by defining handlers per role which do the same but have different names, but could name them the same and notify them as rolename : handlername.

Another fix is to use a - meta: flush_handlers task to flush the notifications before importing the handler again. The implicit flush doesn't happen per role.

Another option is to use dynamic roles (include_role) instead of static (roles:/import_role), since you can make the role inclusion itself conditional, rather than the imported tasks.

- name: Test playbook
  hosts: server-01,server-02
  tasks:
    - include_role:
        name: test_role_2
    - include_role:
        name: test_role_1
      when: 'inventory_hostname == "server-01"'

If using include_role/import_role, the handlers_from option would allow you to import handlers from an alternate file location just once, which would avoid the issue of having future role invocations re-include the handler.

@MorphBonehunter
Copy link

This doesn't work for our case, because of the single source of handler definition.

You said you work around this by defining handlers per role which do the same but have different names, but could name them the same and notify them as rolename : handlername.

Yes, this also works.

Another fix is to use a - meta: flush_handlers task to flush the notifications before importing the handler again. The implicit flush doesn't happen per role.

Unfortunately this doesn't work.
I tried this in my minimal example in #83112 and as you can see the flush handler in role one isn't executed because of the condition of role two even bevor role two is executed.

Another option is to use dynamic roles (include_role) instead of static (roles:/import_role), since you can make the role inclusion itself conditional, rather than the imported tasks.

- name: Test playbook
  hosts: server-01,server-02
  tasks:
    - include_role:
        name: test_role_2
    - include_role:
        name: test_role_1
      when: 'inventory_hostname == "server-01"'

If using include_role/import_role, the handlers_from option would allow you to import handlers from an alternate file location just once, which would avoid the issue of having future role invocations re-include the handler.

I think we should generaly consider this as this could also speed up things.

@s-hertel
Copy link
Contributor

@MorphBonehunter Ah, sorry, I didn't look closely at #83112, these options could work for OPs reproducer. It looks like #83112 is due to #78661 (the underlying cause of this one too).

One more option could be using a variable in the handler name. The tricky part is not all variable sources are available during task-preprocessing, so you'd need to use a variable type with high enough precedence, like role parameters (#20 on https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_variables.html#understanding-variable-precedence).

# roles/dep/handlers/main.yml 
- name: Debug output dep {{ handler_id }}
  ansible.builtin.debug:
    msg: "Debug output dep"
  listen: Debug output dep

Role parameters can be set in the playbook or a role's meta/main.yml for dependencies like this:

# debug.yml
- name: Testcase
  gather_facts: false
  hosts: localhost
  vars:
    r_enable: false
  roles:
    - role: one
      handler_id: 1  # this is a role param
    - role: two
      when: r_enable
# roles/two/meta/main.yml
dependencies:
  - role: dep
    handler_id: 2  # this is also a role param

@mkrizek
Copy link
Contributor

mkrizek commented Apr 23, 2024

Closing as a duplicate of #80925.

If you have further questions please stop by IRC or the mailing list:

See this page for a complete and up to date list of communication channels and their purposes:

@mkrizek mkrizek closed this as completed Apr 23, 2024
@s-hertel s-hertel removed the needs_triage Needs a first human triage before being processed. label Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.16 bug This issue/PR relates to a bug.
Projects
None yet
Development

No branches or pull requests

5 participants