ansible / proposals

Repository for sharing and tracking progress on enhancement proposals for Ansible.
Creative Commons Zero v1.0 Universal
93 stars 19 forks source link

Support cancellation of pending handlers #105

Closed flowerysong closed 5 years ago

flowerysong commented 6 years ago

Proposal: handler-denotify

Author: Paul Arthur <@flowerysong>

Date: 2018/04/13

Motivation

Ansible currently allows handlers to be run early (using meta: flush_handlers), but does not provide a way to cancel running a handler once it's been notified.

Problems

It's very common to have a series of tasks to install and configure a service, which notify a handler to restart the service if a new version is installed or a configuration file changed.

tasks:
- package:
    name: red
    state: latest
  notify: Restart red

- copy:
    src: pink
    dest: /etc/pink
  notify: Rebuild pink

- service:
    name: red
    state: started

- template:
    src: red.conf.j2
    dest: /etc/red.conf.d/extra
  notify: Restart red

handlers:
- name: Restart red
  service:
    name: red
    state: restarted

- name: Rebuild pink
  command: /usr/bin/newpink

This works, but if the initial state of the service was stopped it's started by the final task, then potentially restarted by the handler. Furthermore, there's an overlap in handler functionality: red rebuilds pink on startup, so if the first handler ran there's no need to run the second one.

It's possible to avoid extra restarts/work to a certain extent with registered variables, but this is non-obvious, requires structuring your playbook very carefully, and can quickly get ugly:

tasks:
- package:
    name: red
    state: latest
  notify: Restart red

- copy:
    src: pink
    dest: /etc/pink
  notify: Rebuild pink

- service:
    name: red
    state: started
  register: red_start

- template:
    src: red.conf.j2
    dest: /etc/red.conf.d/extra
  notify: Restart red
  register: red_extra

handlers:
- name: Restart red
  service:
    name: red
    state: restarted
  when: (red_start | default({})) is not changed or (red_extra | default({})) is changed
  register: red_restart

- name: Rebuild pink
  command: /usr/bin/newpink
  when:
    - (red_restart | default({})) is not changed
    - (red_restart | default({})) is not skipped

Solution proposal

Rather than registering a bunch of variables and running the handler conditionally, I propose enhancing the existing notification functionality by allowing changed tasks to cancel previous notifications.

tasks:
- package:
    name: red
    state: latest
  notify: Restart red

- template:
    src: red.conf.j2
    dest: /etc/red.conf.d/extra
  notify: Restart red

- copy:
    src: pink
    dest: /etc/pink
  notify: Rebuild pink

- service:
    name: red
    state: started
  denotify:
    - Restart red
    - Rebuild pink

- template:
    src: red.conf.j2
    dest: /etc/red.conf.d/extra
  notify: Restart red

handlers:
- name: Restart red
  service:
    name: red
    state: restarted
  denotify: Rebuild pink

- name: Rebuild pink
  command: /usr/bin/newpink

This is fairly simple to implement (https://github.com/flowerysong/ansible/commit/f9ae651becffe3f44e315892c3841932bbfca91a), though there may be complications that weren't uncovered in my cursory testing.

Testing

The implementation needs integration tests.

Documentation

The feature would need to be added to the handler documentation, along with at least one example.

Nomenclature

My POC used denotify as the keyword. While to me this is fairly intuitive, it is not a common word. cancel might be a better name even though it lacks symmetry with notify.

bcoca commented 6 years ago

You can also define the handler:

- name: myhandler
  actoin: dostuff
  when: not dontrun|default(False)

and set the dontrun variable in the task that you want to cancel the handler execution

flowerysong commented 6 years ago

@bcoca I'm not sure what you mean by "set the dontrun variable in the task"; as far as I know you would have to do this in a second task (though it's certainly cleaner logic than registering multiple variables and having the handler check all of them.)

- service: { name: red, state: started }
  register: result

- set_fact:
    dontrun: true
  when: result is changed

The disadvantage of this approach compared to canceling the notification is that it creates a parallel notification system instead of extending the existing one; any later tasks that notify that handler need to also reset dontrun or their notification will be ignored.

bcoca commented 6 years ago

or just change the condition on the handler, to look at the result(s)

I'm not saying this is a perfect substitute for all your cases, but it can cover most of them for now

zutnop commented 6 years ago

As a newer Ansible user, I was looking exactly for this functionality, denotify!

nboullis commented 5 years ago

I’m also a new Ansible user, and I was looking for such a feature. My use case is a role that configures a PostgreSQL server and then starts it if it is not running yet, with handlers to restart the server or reload its configuration. The configuration task would notify the restart or reload handlers when some configuration options are changed; the server start would cancel both notifications; the restart would cancel the reload notifications. I hope @flowerysong’s proposal will find its way to Ansible.

flowerysong commented 5 years ago

Meeting consensus was against this proposal; similar functionality might be accepted with a different/more comprehensive design.

mback2k commented 4 years ago

I would really like to see something like this implemented. E.g. for the following use case.

  1. Task changes configuration and triggers systemd unit restart via notify handler.
  2. Service was not running in the first place, so:
  3. Task to install/setup systemd unit enables and starts service.
  4. Playbook / role ends and therefore the handler is run, so:
  5. The just started service is interrupted to be restarted.

This can cause quite some issues with service which shouldn't be interrupted in early startup phase.