ansible / ansible-runner

A tool and python library that helps when interfacing with Ansible directly or as part of another system whether that be through a container image interface, as a standalone tool, or as a Python module that can be imported. The goal is to provide a stable and consistent interface abstraction to Ansible.
Other
937 stars 347 forks source link

runner 2.2 breaks callbacks that use `get_option` #1077

Open evgeni opened 2 years ago

evgeni commented 2 years ago

Ohai,

ansible runner 2.2.0 breaks runs that use callbacks which utilize get_option to load options, like the tree callback in ansible-core.

import os

import ansible_runner

extra_env = {}
extra_env['ANSIBLE_CALLBACKS_ENABLED'] = "tree"
extra_env['ANSIBLE_STDOUT_CALLBACK'] = "tree"
os.environ.update(extra_env)

playbook = os.path.join(os.getcwd(), 'test.yml')
ansible_runner.run(playbook=playbook)
---
- hosts: localhost
  tasks:
    - name: test
      debug:
        msg: test

Running the above Python code results in:

[WARNING]: provided hosts list is empty, only localhost is available. Note that
the implicit localhost does not match 'all'
ERROR! Unexpected Exception, this is probably a bug: 'directory'
to see the full traceback, use -vvv

And with increased verbosity:

ansible-playbook [core 2.12.5]
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/home/evgeni/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/evgeni/Devel/theforeman/foreman-ansible-modules/venv-new/lib64/python3.10/site-packages/ansible
  ansible collection location = /home/evgeni/.ansible/collections:/usr/share/ansible/collections
  executable location = /home/evgeni/Devel/theforeman/foreman-ansible-modules/venv-new/bin/ansible-playbook
  python version = 3.10.4 (main, Mar 25 2022, 00:00:00) [GCC 11.2.1 20220127 (Red Hat 11.2.1-9)]
  jinja version = 3.1.2
  libyaml = True
Using /etc/ansible/ansible.cfg as config file
setting up inventory plugins
host_list declined parsing /etc/ansible/hosts as it did not pass its verify_file() method
script declined parsing /etc/ansible/hosts as it did not pass its verify_file() method
auto declined parsing /etc/ansible/hosts as it did not pass its verify_file() method
Parsed /etc/ansible/hosts inventory source with ini plugin
[WARNING]: provided hosts list is empty, only localhost is available. Note that
the implicit localhost does not match 'all'
Loading callback plugin tree of type aggregate, v2.0 from /home/evgeni/Devel/theforeman/foreman-ansible-modules/venv-new/lib64/python3.10/site-packages/ansible/plugins/callback/tree.py
Loading callback plugin awx_display of type stdout, v2.0 from /home/evgeni/Devel/theforeman/foreman-ansible-modules/venv-new/lib64/python3.10/site-packages/ansible_runner/display_callback/callback/awx_display.py
ERROR! Unexpected Exception, this is probably a bug: 'directory'
the full traceback was:

Traceback (most recent call last):
  File "/home/evgeni/Devel/theforeman/foreman-ansible-modules/venv-new/bin/ansible-playbook", line 128, in <module>
    exit_code = cli.run()
  File "/home/evgeni/Devel/theforeman/foreman-ansible-modules/venv-new/lib64/python3.10/site-packages/ansible/cli/playbook.py", line 137, in run
    results = pbex.run()
  File "/home/evgeni/Devel/theforeman/foreman-ansible-modules/venv-new/lib64/python3.10/site-packages/ansible/executor/playbook_executor.py", line 119, in run
    self._tqm.load_callbacks()
  File "/home/evgeni/Devel/theforeman/foreman-ansible-modules/venv-new/lib64/python3.10/site-packages/ansible/executor/task_queue_manager.py", line 174, in load_callbacks
    self._stdout_callback.set_options()
  File "/home/evgeni/Devel/theforeman/foreman-ansible-modules/venv-new/lib64/python3.10/site-packages/ansible/plugins/callback/tree.py", line 58, in set_options
    self.tree = self.get_option('directory')
  File "/home/evgeni/Devel/theforeman/foreman-ansible-modules/venv-new/lib64/python3.10/site-packages/ansible/plugins/callback/__init__.py", line 82, in get_option
    return self._plugin_options[k]
KeyError: 'directory'

Downgrading to ansible-runner-2.1.3 makes the same code work again.

Shrews commented 2 years ago

@shanemcd @AlanCoding This seems to be an issue with the awx_display callback since it overrides the original callback being specified.

evgeni commented 2 years ago

The backtrace does contain the right ansible/plugins/callback/tree.py tho?

AlanCoding commented 2 years ago

I don't think ANSIBLE_STDOUT_CALLBACK is right, because "tree" is not a standard out callback. It saves the data to files, it doesn't output it to the console. You can see it in the plugin code CALLBACK_TYPE = 'aggregate', not CALLBACK_TYPE = 'stdout'.

In these cases you still need to set ANSIBLE_CALLBACKS_ENABLED. I still don't know how to make your script work correctly, but there's not necessarily an ansible-runner bug.

ansibot commented 2 years ago

@evgeni: Greetings! Thanks for taking the time to open this issue. In order for the community to handle your issue effectively, we need a bit more information.

Here are the items we could not find in your description:

Please set the description of this issue with an appropriate template from: https://github.com/ansible/ansible/tree/devel/.github/ISSUE_TEMPLATE

click here for bot help

sivel commented 2 years ago

This is most certainly a runner bug. The problem is that the DOCUMENTATION for awx_display only extends from default_callback and the fact that there is dynamic class building means that the config defs for the inherited callback are not included in the config defs of the awx_display callback.

This means that the awx_display as is can currently only utilize a callback that is effectively the default callback.

Here is a POC that fixes the issue:

Expand Patch...

```diff diff --git a/ansible_runner/display_callback/callback/awx_display.py b/ansible_runner/display_callback/callback/awx_display.py index 79a56f9..10fe3ac 100644 --- a/ansible_runner/display_callback/callback/awx_display.py +++ b/ansible_runner/display_callback/callback/awx_display.py @@ -61,7 +61,7 @@ elif IS_ADHOC: else: default_stdout_callback = 'default' -DefaultCallbackModule = callback_loader.get(default_stdout_callback).__class__ +DefaultCallbackModule = callback_loader.get(default_stdout_callback, class_only=True) CENSORED = "the output has been hidden due to the fact that 'no_log: true' was specified for this result" @@ -348,6 +348,16 @@ class CallbackModule(DefaultCallbackModule): self.play_uuids = set() self.duplicate_play_counts = collections.defaultdict(lambda: 1) + def set_options(self, task_keys=None, var_options=None, direct=None): + base_config = C.config.get_configuration_definition(DefaultCallbackModule._load_name, plugin_type='callback') + my_config = C.config.get_configuration_definition(self._load_name, plugin_type='callback') + C.config.initialize_plugin_configuration_definitions('callback', self._load_name, base_config | my_config) + return super().set_options(task_keys=task_keys, var_options=var_options, direct=direct) + @contextlib.contextmanager def capture_event_data(self, event, **event_data): event_data.setdefault('uuid', str(uuid.uuid4())) ```

bcoca commented 2 years ago

patch looks good, though i might reverse the order depending on which options you want to have precedence in a conflict case

AlanCoding commented 2 years ago

I don't disagree with the patch, but I lack a genuine test case. The command:

ANSIBLE_CALLBACKS_ENABLED=tree ANSIBLE_STDOUT_CALLBACK=tree ansible-playbook -i localhost, ping.yml --connection=local

Results in no output and the data saved to ~/.ansible/tree. It is somewhat surprising that it runs.

ANSIBLE_CALLBACKS_ENABLED=tree ansible-playbook -i localhost, ping.yml --connection=local

Results in normal stdout and the same data saved to ~/.ansible/tree.

I can find no example of a stdout callback that sets non-default options.

sivel commented 2 years ago

fwiw, this was how I tested with just ansible core:

export ANSIBLE_CALLBACKS_ENABLED=tree,awx_display
export ANSIBLE_STDOUT_CALLBACK=awx_display
export ORIGINAL_STDOUT_CALLBACK=tree
export AWX_ISOLATED_DATA_DIR=$PWD/cache
mkdir -p $AWX_ISOLATED_DATA_DIR

RUNNER=$PWD/ansible-runner/ansible_runner
if [ -e "${RUNNER}/callbacks" ]; then
    export ANSIBLE_CALLBACK_PLUGINS=${RUNNER}/callbacks
else
    export ANSIBLE_CALLBACK_PLUGINS=${RUNNER}/display_callback/callback
fi

ansible-playbook -vvv 77819.yml
bcoca commented 2 years ago

iirc, this one should have non standard options only https://docs.ansible.com/ansible/latest/collections/community/general/selective_callback.html