Adds optional filter arg to meta for flush_handlers (#25491)

[sean797]

SUMMARY

Adds optional filter arg to meta. Currently only used for the flush_handlers action but in the future I envision it being used by other actions.

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

meta

ANSIBLE VERSION

ansible 2.3.0.0

[ansibot]

The test ansible-test sanity --test pep8 failed with the following errors:

lib/ansible/plugins/strategy/__init__.py:718:59: E251 unexpected spaces around keyword / parameter equals
lib/ansible/plugins/strategy/__init__.py:718:61: E251 unexpected spaces around keyword / parameter equals
lib/ansible/plugins/strategy/__init__.py:730:28: E711 comparison to None should be 'if cond is not None:'

click here for bot help

[sean797]

ready_for_review

[cognifloyd]

@sean797 The tests are failing with a message about merging... Could you rebase this?
I would love to see this merged. I'll try to carve out some time next week to test this.

[cognifloyd]

I think this needs to deal with handlers that have an array of listeners. Multiple listeners is something I use quite a bit.

[cognifloyd]

I don't think this covers a handler that is listening to a list of handlers. right?

- name: handler_name
  listen:
    - listen_to_this
    - and_this
  command: echo

ansible/lib/ansible/executor/task_queue_manager.py

Lines 151 to 159 in 17c89d1

	if handler.listen:
	listeners = handler.listen
	if not isinstance(listeners, list):
	listeners = [listeners]
	for listener in listeners:
	if listener not in self._listening_handlers:
	self._listening_handlers[listener] = []
	display.debug("Adding handler %s to listening list" % handler.name)
	self._listening_handlers[listener].append(handler._uuid)

From reading the code, maybe something like:

            for handler in handler_block.block:
                if filters is not None:
                    handler_names = [handler.name]
                    if handler.listen:
                        if isinstance(handler.listen, list):
                            handler_names.extend(handler.listen)
                        else:
                            handler_names.append(handler.listen)
                    if all([name not in filters for name in handler_names]):
                        continue

What was the point of checking if listen is in _listening_handlers before checking if it was in filters (I dropped that)?

[sean797]

Thanks @cognifloyd Sorry for the delay, I've updated and rebased now. To me, it looks like the tests failures and unrelated to this change?

[cognifloyd]

Maybe add a test to make sure this covers listening to a list of handlers.

[sean797]

Added some more tests, thanks again @cognifloyd 👍

[cognifloyd]

Logically, this looks great. I look forward to using it in my playbooks! Thanks @sean797

[abadger]

k=v pairs is never recommended but the comment here isn't (and wasn't) really about k=v so it's probably more clear to drop the k=v mention:

old style meta tasks only had one value, the name of the meta action. This would be stored, unparsed in _raw_params. Meta tasks can now take separate fields for the action and options. If so, the meta_action will be stored in the 'name' dictionary field.

[bcoca]

rephrase of 'never used options, now it is recommended to always use options' .. which is 'format neutral', covering both k=v and k: v

[abadger]

If we decide to deprecate, then we should have additional logic to detect that the action came in via _raw_params rather than name and issue a deprecation warning.

However, we may want to do this via a "long deprecation cycle" which we've talked about the need for but haven't nailed down any rules for yet.

[sean797]

I'm not sure I would want to deprecate the raw_params method, I think it makes sense to support both, especially considering there are other meta actions that do no use any additional params.

[samdoran]

This was discussed in the IRC meeting on 2018-11-15 and the vote was to not include this feature. It adds complexity to meta in order to allow granular control of which handlers get executed. It is possible to get this same behavior using conditionals, block, include_tasks, or other existing mechanisms without the need to add complexity to meta.