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

[backport][2.2] update how callback plugin gets copied and added to job container #1097

Closed
wants to merge 8 commits into from
12 changes: 6 additions & 6 deletions .zuul.d/jobs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
- github.com/ansible/ansible
tags:
# If zuul.tag is defined: [ '3', '3.19', '3.19.0' ]. Only works for 3-component tags.
# Otherwise: ['devel']
"{{ zuul.tag is defined | ternary([zuul.get('tag', '').split('.')[0], '.'.join(zuul.get('tag', '').split('.')[:2]), zuul.get('tag', '')], ['devel']) }}"
# Otherwise: ['latest']
"{{ zuul.tag is defined | ternary([zuul.get('tag', '').split('.')[0], '.'.join(zuul.get('tag', '').split('.')[:2]), zuul.get('tag', '')], ['latest']) }}"
docker_images: *container_images

- job:
Expand Down Expand Up @@ -79,7 +79,7 @@
siblings:
- github.com/ansible/ansible
tags:
- stable-2.12-devel
- stable-2.12-latest
docker_images: *container_images_stable_2_12

- job:
Expand Down Expand Up @@ -126,7 +126,7 @@
siblings:
- github.com/ansible/ansible
tags:
- stable-2.11-devel
- stable-2.11-latest
docker_images: *container_images_stable_2_11

- job:
Expand Down Expand Up @@ -173,7 +173,7 @@
siblings:
- github.com/ansible/ansible
tags:
- stable-2.10-devel
- stable-2.10-latest
docker_images: *container_images_stable_2_10

- job:
Expand Down Expand Up @@ -221,7 +221,7 @@
siblings:
- github.com/ansible/ansible
tags:
- stable-2.9-devel
- stable-2.9-latest
docker_images: *container_images_stable_2_9

- job:
Expand Down
3 changes: 0 additions & 3 deletions .zuul.d/project.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@
periodic:
jobs:
- ansible-buildset-registry
- ansible-runner-upload-container-image:
vars:
upload_container_image_promote: false
- ansible-runner-upload-container-image-stable-2.9:
vars:
upload_container_image_promote: false
Expand Down
4 changes: 4 additions & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ RUN for dir in \

WORKDIR /runner

# NB: this appears to be necessary for container builds based on this container, since we can't rely on the entrypoint
# script to run during a build to fix up /etc/passwd. This envvar value, and the fact that all user homedirs are
# set to /home/runner is an implementation detail that may change with future versions of runner and should not be
# assumed by other code or tools.
ENV HOME=/home/runner

ADD utils/entrypoint.sh /bin/entrypoint
Expand Down
25 changes: 18 additions & 7 deletions ansible_runner/config/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import re
import stat
import tempfile
import shutil

from base64 import b64encode
from uuid import uuid4
Expand All @@ -39,7 +40,6 @@
from ansible_runner.defaults import registry_auth_prefix
from ansible_runner.loader import ArtifactLoader
from ansible_runner.utils import (
callback_mount,
get_callback_dir,
open_fifo_write,
args2cmdline,
Expand Down Expand Up @@ -224,6 +224,7 @@ def _prepare_env(self, runner_mode='pexpect'):
self.env['AWX_ISOLATED_DATA_DIR'] = artifact_dir
if self.fact_cache_type == 'jsonfile':
self.env['ANSIBLE_CACHE_PLUGIN_CONNECTION'] = os.path.join(artifact_dir, 'fact_cache')

else:
# seed env with existing shell env
self.env = os.environ.copy()
Expand All @@ -234,7 +235,7 @@ def _prepare_env(self, runner_mode='pexpect'):
try:
envvars = self.loader.load_file('env/envvars', Mapping)
if envvars:
self.env.update({str(k): str(v) for k, v in envvars.items()})
self.env.update(envvars)
except ConfigurationError:
debug("Not loading environment vars")
# Still need to pass default environment to pexpect
Expand Down Expand Up @@ -262,7 +263,18 @@ def _prepare_env(self, runner_mode='pexpect'):
self.fact_cache = os.path.join(self.artifact_dir, self.settings['fact_cache'])

# Use local callback directory
if not self.containerized:
if self.containerized:
# when containerized, copy the callback dir to $private_data_dir/artifacts/<job_id>/callback
# then append to env['ANSIBLE_CALLBACK_PLUGINS'] with the copied location.
callback_dir = os.path.join(self.artifact_dir, 'callback')
# if callback dir already exists (on repeat execution with the same ident), remove it first.
if os.path.exists(callback_dir):
shutil.rmtree(callback_dir)
shutil.copytree(get_callback_dir(), callback_dir)

container_callback_dir = os.path.join("/runner/artifacts", "{}".format(self.ident), "callback")
self.env['ANSIBLE_CALLBACK_PLUGINS'] = ':'.join(filter(None, (self.env.get('ANSIBLE_CALLBACK_PLUGINS'), container_callback_dir)))
else:
callback_dir = self.env.get('AWX_LIB_DIRECTORY', os.getenv('AWX_LIB_DIRECTORY'))
if callback_dir is None:
callback_dir = get_callback_dir()
Expand Down Expand Up @@ -293,6 +305,9 @@ def _prepare_env(self, runner_mode='pexpect'):
if not self.containerized:
self.env['ANSIBLE_CACHE_PLUGIN_CONNECTION'] = self.fact_cache

# Pexpect will error with non-string envvars types, so we ensure string types
self.env = {str(k): str(v) for k, v in self.env.items()}

debug('env:')
for k, v in sorted(self.env.items()):
debug(f' {k}: {v}')
Expand Down Expand Up @@ -501,10 +516,6 @@ def wrap_args_for_containerization(self, args, execution_mode, cmdline_args):
# custom show paths inside private_data_dir do not make sense
self._update_volume_mount_paths(new_args, "{}".format(self.private_data_dir), dst_mount_path="/runner", labels=":Z")

# Mount the stdout callback plugin from the ansible-runner code base
mount_paths = callback_mount(copy_if_needed=True)
self._update_volume_mount_paths(new_args, mount_paths[0], dst_mount_path=mount_paths[1], labels=":Z")

if self.container_auth_data:
# Pull in the necessary registry auth info, if there is a container cred
self.registry_auth_path, registry_auth_conf_file = self._generate_container_auth_dir(self.container_auth_data)
Expand Down
23 changes: 1 addition & 22 deletions ansible_runner/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,27 +61,6 @@ def is_dir_owner(directory):
return bool(current_user == callback_owner)


def callback_mount(copy_if_needed=False):
'''
Return a tuple that gives mount points for the standard out callback
in the form of (<host location>, <location in container>)
if copy_if_needed is set, and the install is owned by another user,
it will copy the plugin to a tmpdir for the mount in anticipation of SELinux problems
'''
container_dot_ansible = '/home/runner/.ansible'
rel_path = ('callback', '',)
host_path = os.path.join(get_plugin_dir(), *rel_path)
if copy_if_needed:
callback_dir = get_callback_dir()
if not is_dir_owner(callback_dir):
tmp_path = tempfile.mkdtemp(prefix='ansible_runner_plugins_')
register_for_cleanup(tmp_path)
host_path = os.path.join(tmp_path, 'callback')
shutil.copytree(callback_dir, host_path)
container_path = os.path.join(container_dot_ansible, 'plugins', *rel_path)
return (host_path, container_path)


class Bunch(object):

'''
Expand Down Expand Up @@ -432,7 +411,7 @@ def open_fifo_write(path, data):
'''
os.mkfifo(path, stat.S_IRUSR | stat.S_IWUSR)
# If the data is a string instead of bytes, convert it before writing the fifo
if type(data) == str:
if isinstance(data, string_types):
data = data.encode()
threading.Thread(target=lambda p, d: open(p, 'wb').write(d),
args=(path, data)).start()
Expand Down
2 changes: 1 addition & 1 deletion ansible_runner/utils/streaming.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
def stream_dir(source_directory, stream):
with tempfile.NamedTemporaryFile() as tmp:
with zipfile.ZipFile(
tmp.name, "w", compression=zipfile.ZIP_DEFLATED, allowZip64=True
tmp.name, "w", compression=zipfile.ZIP_DEFLATED, allowZip64=True, strict_timestamps=False
) as archive:
if source_directory:
for dirpath, dirs, files in os.walk(source_directory):
Expand Down
22 changes: 22 additions & 0 deletions docs/python_interface.rst
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,28 @@ Usage examples
print("Final status:")
print(r.stats)


.. code-block:: python

import ansible_runner

def my_status_handler(data, runner_config):
# Do something here
print(data)

r = ansible_runner.run(private_data_dir='/tmp/demo', playbook='test.yml', status_handler=my_status_handler)


.. code-block:: python

import ansible_runner

def my_event_handler(data):
# Do something here
print(data)

r = ansible_runner.run(private_data_dir='/tmp/demo', playbook='test.yml', event_handler=my_event_handler)

.. code-block:: python

import ansible_runner
Expand Down
18 changes: 15 additions & 3 deletions test/unit/config/test__base.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
from ansible_runner.config._base import BaseConfig, BaseExecutionMode
from ansible_runner.loader import ArtifactLoader
from ansible_runner.exceptions import ConfigurationError
from ansible_runner.utils import callback_mount

try:
Pattern = re._pattern_type
Expand Down Expand Up @@ -73,7 +72,7 @@ def test_base_config_project_dir(tmp_path):
assert rc.project_dir == tmp_path.joinpath('project').as_posix()


def test_prepare_environment_vars_only_strings(mocker):
def test_prepare_environment_vars_only_strings_from_file(mocker):
rc = BaseConfig(envvars=dict(D='D'))

value = dict(A=1, B=True, C="foo")
Expand All @@ -92,6 +91,20 @@ def test_prepare_environment_vars_only_strings(mocker):
assert rc.env['D'] == 'D'


def test_prepare_environment_vars_only_strings_from_interface():
rc = BaseConfig(envvars=dict(D='D', A=1, B=True, C="foo"))
rc._prepare_env()

assert 'A' in rc.env
assert isinstance(rc.env['A'], six.string_types)
assert 'B' in rc.env
assert isinstance(rc.env['B'], six.string_types)
assert 'C' in rc.env
assert isinstance(rc.env['C'], six.string_types)
assert 'D' in rc.env
assert rc.env['D'] == 'D'


def test_prepare_environment_pexpect_defaults():
rc = BaseConfig()
rc._prepare_env()
Expand Down Expand Up @@ -317,7 +330,6 @@ def test_containerization_settings(tmp_path, runtime, mocker):
expected_command_start.extend([
'-v', '{}/artifacts/:/runner/artifacts/:Z'.format(rc.private_data_dir),
'-v', '{}/:/runner/:Z'.format(rc.private_data_dir),
'-v', '{0}:{1}:Z'.format(*callback_mount()),
'--env-file', '{}/env.list'.format(rc.artifact_dir),
])

Expand Down
3 changes: 1 addition & 2 deletions test/unit/config/test_ansible_cfg.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from ansible_runner.config.ansible_cfg import AnsibleCfgConfig
from ansible_runner.config._base import BaseExecutionMode
from ansible_runner.exceptions import ConfigurationError
from ansible_runner.utils import get_executable_path, callback_mount
from ansible_runner.utils import get_executable_path


def test_ansible_cfg_init_defaults(tmp_path, patch_private_data_dir):
Expand Down Expand Up @@ -91,7 +91,6 @@ def test_prepare_config_command_with_containerization(tmp_path, runtime, mocker)
expected_command_start.extend([
'-v', '{}/artifacts/:/runner/artifacts/:Z'.format(rc.private_data_dir),
'-v', '{}/:/runner/:Z'.format(rc.private_data_dir),
'-v', '{0}:{1}:Z'.format(*callback_mount()),
'--env-file', '{}/env.list'.format(rc.artifact_dir),
])

Expand Down
2 changes: 0 additions & 2 deletions test/unit/config/test_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from ansible_runner.config.command import CommandConfig
from ansible_runner.config._base import BaseExecutionMode
from ansible_runner.exceptions import ConfigurationError
from ansible_runner.utils import callback_mount


def test_ansible_config_defaults(tmp_path, patch_private_data_dir):
Expand Down Expand Up @@ -106,7 +105,6 @@ def test_prepare_run_command_with_containerization(tmp_path, runtime, mocker):
expected_command_start.extend([
'-v', '{}/artifacts/:/runner/artifacts/:Z'.format(rc.private_data_dir),
'-v', '{}/:/runner/:Z'.format(rc.private_data_dir),
'-v', '{0}:{1}:Z'.format(*callback_mount()),
'--env-file', '{}/env.list'.format(rc.artifact_dir),
])

Expand Down
4 changes: 1 addition & 3 deletions test/unit/config/test_doc.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from ansible_runner.config.doc import DocConfig
from ansible_runner.config._base import BaseExecutionMode
from ansible_runner.exceptions import ConfigurationError
from ansible_runner.utils import get_executable_path, callback_mount
from ansible_runner.utils import get_executable_path


def test_ansible_doc_defaults(tmp_path, patch_private_data_dir):
Expand Down Expand Up @@ -101,7 +101,6 @@ def test_prepare_plugin_docs_command_with_containerization(tmp_path, runtime, mo
expected_command_start.extend([
'-v', '{}/artifacts/:/runner/artifacts/:Z'.format(rc.private_data_dir),
'-v', '{}/:/runner/:Z'.format(rc.private_data_dir),
'-v', '{0}:{1}:Z'.format(*callback_mount()),
'--env-file', '{}/env.list'.format(rc.artifact_dir),
])

Expand Down Expand Up @@ -170,7 +169,6 @@ def test_prepare_plugin_list_command_with_containerization(tmp_path, runtime, mo
expected_command_start.extend([
'-v', '{}/artifacts/:/runner/artifacts/:Z'.format(rc.private_data_dir),
'-v', '{}/:/runner/:Z'.format(rc.private_data_dir),
'-v', '{0}:{1}:Z'.format(*callback_mount()),
'--env-file', '{}/env.list'.format(rc.artifact_dir),
])

Expand Down
3 changes: 1 addition & 2 deletions test/unit/config/test_inventory.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from ansible_runner.config.inventory import InventoryConfig
from ansible_runner.config._base import BaseExecutionMode
from ansible_runner.exceptions import ConfigurationError
from ansible_runner.utils import get_executable_path, callback_mount
from ansible_runner.utils import get_executable_path


def test_ansible_inventory_init_defaults(tmp_path, patch_private_data_dir):
Expand Down Expand Up @@ -126,7 +126,6 @@ def test_prepare_inventory_command_with_containerization(tmp_path, runtime, mock
expected_command_start.extend([
'-v', '{}/artifacts/:/runner/artifacts/:Z'.format(rc.private_data_dir),
'-v', '{}/:/runner/:Z'.format(rc.private_data_dir),
'-v', '{0}:{1}:Z'.format(*callback_mount()),
'--env-file', '{}/env.list'.format(rc.artifact_dir),
])

Expand Down
15 changes: 13 additions & 2 deletions test/unit/config/test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
from ansible_runner.interface import init_runner
from ansible_runner.loader import ArtifactLoader
from ansible_runner.exceptions import ConfigurationError
from ansible_runner.utils import callback_mount

try:
Pattern = re._pattern_type
Expand Down Expand Up @@ -715,6 +714,11 @@ def test_containerization_settings(tmp_path, runtime, mocker):
mock_containerized = mocker.patch('ansible_runner.runner_config.RunnerConfig.containerized', new_callable=mocker.PropertyMock)
mock_containerized.return_value = True

# In this test get_callback_dir() will not return a callback plugin dir that exists
# mock shutil.copytree and shutil.rmtree to just return True instead of trying to copy
mocker.patch('shutil.copytree', return_value=True)
mocker.patch('shutil.rmtree', return_value=True)

rc = RunnerConfig(tmp_path)
rc.ident = 'foo'
rc.playbook = 'main.yaml'
Expand All @@ -725,6 +729,14 @@ def test_containerization_settings(tmp_path, runtime, mocker):
rc.container_volume_mounts = ['/host1:/container1', '/host2:/container2']
rc.prepare()

# validate ANSIBLE_CALLBACK_PLUGINS env var is set
assert rc.env.get('ANSIBLE_CALLBACK_PLUGINS', None) is not None

# validate ANSIBLE_CALLBACK_PLUGINS contains callback plugin dir
callback_plugins = rc.env['ANSIBLE_CALLBACK_PLUGINS'].split(':')
callback_dir = os.path.join("/runner/artifacts", "{}".format(rc.ident), "callback")
assert callback_dir in callback_plugins

extra_container_args = []
if runtime == 'podman':
extra_container_args = ['--quiet']
Expand All @@ -733,7 +745,6 @@ def test_containerization_settings(tmp_path, runtime, mocker):

expected_command_start = [runtime, 'run', '--rm', '--tty', '--interactive', '--workdir', '/runner/project'] + \
['-v', '{}/:/runner/:Z'.format(rc.private_data_dir)] + \
['-v', '{0}:{1}:Z'.format(*callback_mount())] + \
['-v', '/host1/:/container1/', '-v', '/host2/:/container2/'] + \
['--env-file', '{}/env.list'.format(rc.artifact_dir)] + \
extra_container_args + \
Expand Down