From 966d449924f79cb7bf4e0c80aba2258079114e81 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Wed, 13 Mar 2024 17:18:32 -0500 Subject: [PATCH 1/6] Address event correlation issues in the awx_display callback, fix callback options --- .../display_callback/callback/awx_display.py | 126 +++++++++--------- 1 file changed, 65 insertions(+), 61 deletions(-) diff --git a/src/ansible_runner/display_callback/callback/awx_display.py b/src/ansible_runner/display_callback/callback/awx_display.py index 007a493f3..aba3edaff 100644 --- a/src/ansible_runner/display_callback/callback/awx_display.py +++ b/src/ansible_runner/display_callback/callback/awx_display.py @@ -19,10 +19,9 @@ from __future__ import (absolute_import, division, print_function) -# Python +import inspect import json import stat -import multiprocessing import threading import base64 import functools @@ -32,13 +31,15 @@ import os import sys import uuid -from copy import copy # Ansible from ansible import constants as C from ansible.plugins.callback import CallbackBase from ansible.plugins.loader import callback_loader from ansible.utils.display import Display +from ansible.utils.multiprocessing import context as multiprocessing + +display = Display() DOCUMENTATION = ''' @@ -48,8 +49,7 @@ description: - This callback is necessary for ansible-runner to work type: stdout - extends_documentation_fragment: - - default_callback + options: {} requirements: - Set as stdout in config ''' @@ -64,7 +64,7 @@ else: default_stdout_callback = 'default' -DefaultCallbackModule: CallbackBase = callback_loader.get(default_stdout_callback).__class__ +DefaultCallbackModule: CallbackBase = 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" @@ -132,11 +132,16 @@ def __init__(self): self._local = threading.local() if os.getenv('AWX_ISOLATED_DATA_DIR'): self.cache = IsolatedFileWrite() + self._global_ctx = {} def add_local(self, **kwargs): - tls = vars(self._local) - ctx = tls.setdefault('_ctx', {}) - ctx.update(kwargs) + try: + ctx = self._local._ctx + except AttributeError: + ctx = self._local._ctx = {} + + # Don't overwrite an earlier event context, this is only additive + ctx.update(kwargs | ctx) def remove_local(self, **kwargs): for key in kwargs: @@ -172,22 +177,13 @@ def get_global(self): return self._global_ctx def get(self): - ctx = {} - ctx.update(self.get_global()) - ctx.update(self.get_local()) - return ctx + return self.get_global() | self.get_local() def get_begin_dict(self): omit_event_data = os.getenv("RUNNER_OMIT_EVENTS", "False").lower() == "true" include_only_failed_event_data = os.getenv("RUNNER_ONLY_FAILED_EVENTS", "False").lower() == "true" event_data = self.get() event = event_data.pop('event', None) - if not event: - event = 'verbose' - for key in ('debug', 'verbose', 'deprecated', 'warning', 'system_warning', 'error'): - if event_data.get(key, False): - event = key - break event_dict = {'event': event} should_process_event_data = (include_only_failed_event_data and event in ('runner_on_failed', 'runner_on_async_failed', 'runner_on_item_failed')) \ or not include_only_failed_event_data @@ -247,62 +243,50 @@ def dump_end(self, fileobj): event_context = EventContext() -def with_context(**context): - global event_context # pylint: disable=W0602 - - def wrap(f): - @functools.wraps(f) - def wrapper(*args, **kwargs): - with event_context.set_local(**context): - return f(*args, **kwargs) - return wrapper - return wrap - - -for attr in dir(Display): - if attr.startswith('_') or 'cow' in attr or 'prompt' in attr: - continue - if attr in ('display', 'v', 'vv', 'vvv', 'vvvv', 'vvvvv', 'vvvvvv', 'verbose'): - continue - if not callable(getattr(Display, attr)): - continue - setattr(Display, attr, with_context(**{attr: True})(getattr(Display, attr))) - - -def with_verbosity(f): - global event_context # pylint: disable=W0602 +def display_context(f): @functools.wraps(f) def wrapper(*args, **kwargs): - host = args[2] if len(args) >= 3 else kwargs.get('host', None) - caplevel = args[3] if len(args) >= 4 else kwargs.get('caplevel', 2) - context = {'verbose': True, 'verbosity': (caplevel + 1)} - if host is not None: - context['remote_addr'] = host - with event_context.set_local(**context): + if multiprocessing.parent_process() is not None: return f(*args, **kwargs) - return wrapper + name = f.__name__ + ctx = {'event': name} + callargs = inspect.getcallargs(f, *args, **kwargs) + host = callargs.get('host') + caplevel = callargs.get('caplevel') + log_only = callargs.get('log_only') + stderr = callargs.get('stderr') + if host: + ctx['remote_addr'] = host + if caplevel: + ctx['verbosity'] = caplevel + 1 -Display.verbose = with_verbosity(Display.verbose) + dump = True + if caplevel and display.verbosity <= caplevel: + dump = False + if name == 'debug' and not C.DEFAULT_DEBUG: + dump = False -def display_with_context(f): + if name == 'system_warning' and not C.SYSTEM_WARNINGS: + dump = False + + if name == "deprecated" and not C.DEPRECATION_WARNINGS: + dump = False + + if not dump: + return f(*args, **kwargs) - @functools.wraps(f) - def wrapper(*args, **kwargs): - log_only = args[5] if len(args) >= 6 else kwargs.get('log_only', False) - stderr = args[3] if len(args) >= 4 else kwargs.get('stderr', False) event_uuid = event_context.get().get('uuid', None) with event_context.display_lock: - # If writing only to a log file or there is already an event UUID - # set (from a callback module method), skip dumping the event data. if log_only or event_uuid: return f(*args, **kwargs) try: fileobj = sys.stderr if stderr else sys.stdout event_context.add_local(uuid=str(uuid.uuid4())) - event_context.dump_begin(fileobj) + with event_context.set_local(**ctx): + event_context.dump_begin(fileobj) return f(*args, **kwargs) finally: event_context.dump_end(fileobj) @@ -311,7 +295,15 @@ def wrapper(*args, **kwargs): return wrapper -Display.display = display_with_context(Display.display) +for _display_attr in ('banner', 'deprecated', 'display', 'error', + 'system_warning', 'verbose', 'warning'): + setattr( + Display, + _display_attr, + display_context( + getattr(Display, _display_attr) + ) + ) class CallbackModule(DefaultCallbackModule): @@ -352,6 +344,18 @@ def __init__(self): # NOTE: Ansible doesn't generate a UUID for playbook_on_start so do it for them. self.playbook_uuid = str(uuid.uuid4()) + def set_options(self, *args, **kwargs): + 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(*args, **kwargs) + @contextlib.contextmanager def capture_event_data(self, event, **event_data): event_data.setdefault('uuid', str(uuid.uuid4())) @@ -365,7 +369,7 @@ def capture_event_data(self, event, **event_data): if event_data['res'].get('_ansible_no_log', False): event_data['res'] = {'censored': CENSORED} if event_data['res'].get('results', []): - event_data['res']['results'] = copy(event_data['res']['results']) + event_data['res']['results'] = event_data['res']['results'][:] for i, item in enumerate(event_data['res'].get('results', [])): if isinstance(item, dict) and item.get('_ansible_no_log', False): event_data['res']['results'][i] = {'censored': CENSORED} From cd1ecd8b0902f45a755b5cd207afe5beaa647d30 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Wed, 13 Mar 2024 17:18:56 -0500 Subject: [PATCH 2/6] Make it apparent that pre-monkeypatched display events are unclassified --- src/ansible_runner/utils/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ansible_runner/utils/__init__.py b/src/ansible_runner/utils/__init__.py index f4a661ee5..c4583c327 100644 --- a/src/ansible_runner/utils/__init__.py +++ b/src/ansible_runner/utils/__init__.py @@ -386,14 +386,14 @@ def _emit_event(self, event_data = self._current_event_data stdout_chunks = [buffered_stdout] elif buffered_stdout: - event_data = dict({'event': 'verbose'}) + event_data = dict({'event': 'unclassified_display'}) stdout_chunks = buffered_stdout.splitlines(True) else: event_data = {} stdout_chunks = [] for stdout_chunk in stdout_chunks: - if event_data.get('event') == 'verbose': + if event_data.get('event') == 'unclassified_display': event_data['uuid'] = str(uuid.uuid4()) self._counter += 1 event_data['counter'] = self._counter From a3f0f42f6a6b7428558d3f8ed10a88262b7b94bc Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Wed, 13 Mar 2024 17:34:32 -0500 Subject: [PATCH 3/6] fix linting issue --- src/ansible_runner/display_callback/callback/awx_display.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ansible_runner/display_callback/callback/awx_display.py b/src/ansible_runner/display_callback/callback/awx_display.py index aba3edaff..18cc20ce4 100644 --- a/src/ansible_runner/display_callback/callback/awx_display.py +++ b/src/ansible_runner/display_callback/callback/awx_display.py @@ -256,7 +256,7 @@ def wrapper(*args, **kwargs): host = callargs.get('host') caplevel = callargs.get('caplevel') log_only = callargs.get('log_only') - stderr = callargs.get('stderr') + stderr = callargs.get('stderr') if host: ctx['remote_addr'] = host if caplevel: From 6477050615770e524b76149c545c4b47cd6e161d Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Wed, 20 Mar 2024 16:59:54 -0500 Subject: [PATCH 4/6] Don't use deprecated method --- src/ansible_runner/display_callback/callback/awx_display.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ansible_runner/display_callback/callback/awx_display.py b/src/ansible_runner/display_callback/callback/awx_display.py index 18cc20ce4..214edf055 100644 --- a/src/ansible_runner/display_callback/callback/awx_display.py +++ b/src/ansible_runner/display_callback/callback/awx_display.py @@ -252,7 +252,9 @@ def wrapper(*args, **kwargs): name = f.__name__ ctx = {'event': name} - callargs = inspect.getcallargs(f, *args, **kwargs) + ba = inspect.signature(f).bind(*args, **kwargs) + ba.apply_defaults() + callargs = ba.arguments host = callargs.get('host') caplevel = callargs.get('caplevel') log_only = callargs.get('log_only') From 2d6772f78df4c9d36eb5de998bb86126e06f720b Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Wed, 20 Mar 2024 17:17:28 -0500 Subject: [PATCH 5/6] disable no-member for ansible.constants --- src/ansible_runner/display_callback/callback/awx_display.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ansible_runner/display_callback/callback/awx_display.py b/src/ansible_runner/display_callback/callback/awx_display.py index 214edf055..57bf43eda 100644 --- a/src/ansible_runner/display_callback/callback/awx_display.py +++ b/src/ansible_runner/display_callback/callback/awx_display.py @@ -268,13 +268,13 @@ def wrapper(*args, **kwargs): if caplevel and display.verbosity <= caplevel: dump = False - if name == 'debug' and not C.DEFAULT_DEBUG: + if name == 'debug' and not C.DEFAULT_DEBUG: # pylint: disable=no-member dump = False - if name == 'system_warning' and not C.SYSTEM_WARNINGS: + if name == 'system_warning' and not C.SYSTEM_WARNINGS: # pylint: disable=no-member dump = False - if name == "deprecated" and not C.DEPRECATION_WARNINGS: + if name == "deprecated" and not C.DEPRECATION_WARNINGS: # pylint: disable=no-member dump = False if not dump: From 9cf4ed1779d0dd75490190c8539d21298e036923 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Wed, 20 Mar 2024 17:43:37 -0500 Subject: [PATCH 6/6] Performance and type hints --- .../display_callback/callback/awx_display.py | 30 ++++++++++++------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/src/ansible_runner/display_callback/callback/awx_display.py b/src/ansible_runner/display_callback/callback/awx_display.py index 57bf43eda..769eff7f1 100644 --- a/src/ansible_runner/display_callback/callback/awx_display.py +++ b/src/ansible_runner/display_callback/callback/awx_display.py @@ -19,17 +19,19 @@ from __future__ import (absolute_import, division, print_function) -import inspect -import json -import stat -import threading import base64 -import functools import collections +import collections.abc as c import contextlib import datetime +import functools +import inspect +import json import os +import stat import sys +import threading +import typing as t import uuid # Ansible @@ -39,6 +41,8 @@ from ansible.utils.display import Display from ansible.utils.multiprocessing import context as multiprocessing +P = t.ParamSpec('P') + display = Display() @@ -243,18 +247,24 @@ def dump_end(self, fileobj): event_context = EventContext() -def display_context(f): +def _getcallargs(sig: inspect.Signature, *args: P.args, **kwargs: P.kwargs) -> dict: + ba = sig.bind(*args, **kwargs) + ba.apply_defaults() + return ba.arguments + + +def display_context(f: c.Callable[t.Concatenate[Display, P], None]) -> c.Callable[..., None]: + + sig = inspect.signature(f) @functools.wraps(f) - def wrapper(*args, **kwargs): + def wrapper(*args: P.args, **kwargs: P.kwargs) -> None: if multiprocessing.parent_process() is not None: return f(*args, **kwargs) name = f.__name__ ctx = {'event': name} - ba = inspect.signature(f).bind(*args, **kwargs) - ba.apply_defaults() - callargs = ba.arguments + callargs = _getcallargs(sig, *args, **kwargs) host = callargs.get('host') caplevel = callargs.get('caplevel') log_only = callargs.get('log_only')