Repository: ansible/ansible-runner

Issue:
Stop using `distutils`
It's deprecated. Python 3.10 raises a `DeprecationWarning` and Python 3.12 won't have it in the stdlib anymore.
Why fix it now and not wait for another 2 years when this will produce an `ImportError`? Because it's nice to solve such problems ahead of time and also since ansible-runner is a library, it also causes these warnings (_or even errors_ with pytest's [`filterwarning = error`] or [`python -Werror`]&nbsp;[^-W-python-option]) for the projects that have Python 3.10 in their test matrixes. In particular, it's been observed in ansible-navigator.

[`filterwarning = error`]: https://docs.pytest.org/en/6.2.x/reference.html#confval-filterwarnings
[`python -Werror`]: https://docs.python.org/dev/library/warnings.html#the-warnings-filter
[^-W-python-option]: Learn more at https://docs.python.org/dev/using/cmdline.html#cmdoption-W

Discussion:
Shrews: The `copytree()` portion of code used for directory isolation does not seem to be covered by any tests. I'd like to add a test for that before merging this change. However, I discovered a bug in that bit of code that needs fixed before we can merge this (see issue #977).

Patch:
diff --git a/ansible_runner/config/runner.py b/ansible_runner/config/runner.py
index 84852e1..9fb96bb 100644
--- a/ansible_runner/config/runner.py
+++ b/ansible_runner/config/runner.py
@@ -23,7 +23,7 @@ import shlex
 import stat
 import tempfile
 import six
-import distutils.dir_util
+import shutil
 
 from six import string_types, text_type
 
@@ -146,7 +146,7 @@ class RunnerConfig(BaseConfig):
             if os.path.exists(self.project_dir):
                 output.debug("Copying directory tree from {} to {} for working directory isolation".format(self.project_dir,
                                                                                                            self.directory_isolation_path))
-                distutils.dir_util.copy_tree(self.project_dir, self.directory_isolation_path, preserve_symlinks=True)
+                shutil.copytree(self.project_dir, self.directory_isolation_path, symlinks=True)
 
         self.prepare_inventory()
         self.prepare_command()
diff --git a/ansible_runner/utils/__init__.py b/ansible_runner/utils/__init__.py
index 84b2b2a..f516db0 100644
--- a/ansible_runner/utils/__init__.py
+++ b/ansible_runner/utils/__init__.py
@@ -17,8 +17,6 @@ import codecs
 import atexit
 import signal
 
-from distutils.spawn import find_executable
-
 from ansible_runner.exceptions import ConfigurationError
 
 try:
@@ -474,7 +472,7 @@ def sanitize_json_response(data):
 
 
 def get_executable_path(name):
-    exec_path = find_executable(name)
+    exec_path = shutil.which(name)
     if exec_path is None:
         raise ConfigurationError(f"{name} command not found")
     return exec_path


Test patch:
diff --git a/test/conftest.py b/test/conftest.py
index 07676da..5e786f4 100644
--- a/test/conftest.py
+++ b/test/conftest.py
@@ -1,6 +1,5 @@
 import shutil
 
-from distutils.version import LooseVersion
 from pathlib import Path
 
 from ansible_runner import defaults
@@ -25,16 +24,6 @@ def change_save_path(tmp_path, mocker):
     mocker.patch.object(defaults, 'AUTO_CREATE_DIR', str(tmp_path))
 
 
-@pytest.fixture(scope='session')
-def is_pre_ansible28():
-    try:
-        if LooseVersion(pkg_resources.get_distribution('ansible').version) < LooseVersion('2.8'):
-            return True
-    except pkg_resources.DistributionNotFound:
-        # ansible-base (e.g. ansible 2.10 and beyond) is not accessible in this way
-        pass
-
-
 @pytest.fixture(scope='session')
 def is_pre_ansible211():
     """
@@ -51,12 +40,6 @@ def is_pre_ansible211():
         return True
 
 
-@pytest.fixture(scope='session')
-def skipif_pre_ansible28(is_pre_ansible28):
-    if is_pre_ansible28:
-        pytest.skip("Valid only on Ansible 2.8+")
-
-
 @pytest.fixture(scope='session')
 def skipif_pre_ansible211(is_pre_ansible211):
     if is_pre_ansible211:
diff --git a/test/integration/test___main__.py b/test/integration/test___main__.py
index f65c307..3772fae 100644
--- a/test/integration/test___main__.py
+++ b/test/integration/test___main__.py
@@ -98,7 +98,7 @@ def test_cmdline_role_with_playbook_option():
         assert exc == 1
 
 
-def test_cmdline_playbook(is_pre_ansible28, tmp_path):
+def test_cmdline_playbook(tmp_path):
     private_data_dir = tmp_path
     play = [{'hosts': 'all', 'tasks': [{'debug': {'msg': random_string()}}]}]
 
@@ -142,7 +142,7 @@ def test_cmdline_includes_one_option():
         assert exc == 1
 
 
-def test_cmdline_cmdline_override(is_pre_ansible28, tmp_path):
+def test_cmdline_cmdline_override(tmp_path):
     private_data_dir = tmp_path
     play = [{'hosts': 'all', 'tasks': [{'debug': {'msg': random_string()}}]}]
 
diff --git a/test/integration/test_display_callback.py b/test/integration/test_display_callback.py
index 8fedc0d..24bc907 100644
--- a/test/integration/test_display_callback.py
+++ b/test/integration/test_display_callback.py
@@ -13,7 +13,7 @@ HERE = os.path.abspath(os.path.dirname(__file__))
 
 
 @pytest.fixture()
-def executor(tmp_path, request, is_pre_ansible28):
+def executor(tmp_path, request):
     private_data_dir = tmp_path / 'foo'
     private_data_dir.mkdir()
 
@@ -176,7 +176,7 @@ def test_callback_plugin_no_log_filters(executor, playbook):
     - uri: url=https://example.org url_username="PUBLIC" url_password="PRIVATE"
 '''},  # noqa
 ])
-def test_callback_plugin_task_args_leak(executor, playbook, skipif_pre_ansible28):
+def test_callback_plugin_task_args_leak(executor, playbook):
     executor.run()
     events = list(executor.events)
     assert events[0]['event'] == 'playbook_on_start'
@@ -213,7 +213,7 @@ def test_callback_plugin_task_args_leak(executor, playbook, skipif_pre_ansible28
     - debug: msg="{{ command_register.results|map(attribute='stdout')|list }}"
 '''},  # noqa
 ])
-def test_callback_plugin_censoring_does_not_overwrite(executor, playbook, skipif_pre_ansible28):
+def test_callback_plugin_censoring_does_not_overwrite(executor, playbook):
     executor.run()
     events = list(executor.events)
     assert events[0]['event'] == 'playbook_on_start'
@@ -258,7 +258,7 @@ def test_callback_plugin_strips_task_environ_variables(executor, playbook):
           foo: "bar"
 '''},  # noqa
 ])
-def test_callback_plugin_saves_custom_stats(executor, playbook, skipif_pre_ansible28):
+def test_callback_plugin_saves_custom_stats(executor, playbook):
     executor.run()
     for event in executor.events:
         event_data = event.get('event_data', {})
@@ -284,7 +284,7 @@ def test_callback_plugin_saves_custom_stats(executor, playbook, skipif_pre_ansib
         - my_handler
 '''},  # noqa
 ])
-def test_callback_plugin_records_notify_events(executor, playbook, skipif_pre_ansible28):
+def test_callback_plugin_records_notify_events(executor, playbook):
     executor.run()
     assert len(list(executor.events))
     notify_events = [x for x in executor.events if x['event'] == 'playbook_on_notify']
@@ -308,7 +308,7 @@ def test_callback_plugin_records_notify_events(executor, playbook, skipif_pre_an
         url_password: "{{ pw }}"
 '''},  # noqa
 ])
-def test_module_level_no_log(executor, playbook, skipif_pre_ansible28):
+def test_module_level_no_log(executor, playbook):
     # It's possible for `no_log=True` to be defined at the _module_ level,
     # e.g., for the URI module password parameter
     # This test ensures that we properly redact those
@@ -398,7 +398,7 @@ def test_output_when_given_non_playbook_script(tmp_path):
         msg: "{{ ('F' * 150) | list }}"
 '''},  # noqa
 ])
-def test_large_stdout_parsing_when_using_json_output(executor, playbook, skipif_pre_ansible28):
+def test_large_stdout_parsing_when_using_json_output(executor, playbook):
     # When the json flag is used, it is possible to output more data than
     # pexpect's maxread default of 2000 characters.  As a result, if not
     # handled properly, the stdout can end up being corrupted with partial
diff --git a/test/integration/test_events.py b/test/integration/test_events.py
index 2e0b4c1..e91fc35 100644
--- a/test/integration/test_events.py
+++ b/test/integration/test_events.py
@@ -1,8 +1,7 @@
 import os
 import json
 
-from distutils.spawn import find_executable
-
+import shutil
 import pytest
 
 from ansible_runner import defaults, run, run_async
@@ -10,7 +9,7 @@ from ansible_runner import defaults, run, run_async
 
 @pytest.mark.test_all_runtimes
 @pytest.mark.parametrize('containerized', [True, False])
-def test_basic_events(containerized, is_pre_ansible28, runtime, tmp_path, is_run_async=False, g_facts=False):
+def test_basic_events(containerized, runtime, tmp_path, is_run_async=False, g_facts=False):
 
     inventory = 'localhost ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}"'
 
@@ -56,11 +55,11 @@ def test_basic_events(containerized, is_pre_ansible28, runtime, tmp_path, is_run
 
 @pytest.mark.test_all_runtimes
 @pytest.mark.parametrize('containerized', [True, False])
-def test_async_events(containerized, is_pre_ansible28, runtime, tmp_path):
-    test_basic_events(containerized, is_pre_ansible28, runtime, tmp_path, is_run_async=True, g_facts=True)
+def test_async_events(containerized, runtime, tmp_path):
+    test_basic_events(containerized, runtime, tmp_path, is_run_async=True, g_facts=True)
 
 
-def test_basic_serializeable(is_pre_ansible28, tmp_path):
+def test_basic_serializeable(tmp_path):
     inv = 'localhost ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}"'
     r = run(private_data_dir=str(tmp_path),
             inventory=inv,
@@ -69,7 +68,7 @@ def test_basic_serializeable(is_pre_ansible28, tmp_path):
     json.dumps(events)
 
 
-def test_event_omission(is_pre_ansible28, tmp_path):
+def test_event_omission(tmp_path):
     inv = 'localhost ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}"'
     r = run(private_data_dir=str(tmp_path),
             inventory=inv,
@@ -86,7 +85,7 @@ def test_event_omission(is_pre_ansible28, tmp_path):
     assert not any([x['event_data'] for x in events])
 
 
-def test_event_omission_except_failed(is_pre_ansible28, tmp_path):
+def test_event_omission_except_failed(tmp_path):
     inv = 'localhost ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}"'
     r = run(private_data_dir=str(tmp_path),
             inventory=inv,
@@ -105,7 +104,7 @@ def test_event_omission_except_failed(is_pre_ansible28, tmp_path):
     assert len(all_event_datas) == 1
 
 
-def test_runner_on_start(rc, skipif_pre_ansible28, tmp_path):
+def test_runner_on_start(rc, tmp_path):
     r = run(private_data_dir=str(tmp_path),
             inventory='localhost ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}"',
             playbook=[{'hosts': 'all', 'gather_facts': False, 'tasks': [{'debug': {'msg': "test"}}]}])
@@ -114,9 +113,7 @@ def test_runner_on_start(rc, skipif_pre_ansible28, tmp_path):
     assert len(start_events) == 1
 
 
-def test_playbook_on_stats_summary_fields(project_fixtures, is_pre_ansible28):
-    if is_pre_ansible28:
-        pytest.skip('Test is for post 2.8 status types.')
+def test_playbook_on_stats_summary_fields(project_fixtures):
     private_data_dir = project_fixtures / 'host_status'
 
     res = run(
@@ -147,9 +144,9 @@ def test_include_role_events(project_fixtures):
             assert event_data['res']['msg'] == 'Hello world!'
 
 
-@pytest.mark.skipif(find_executable('cgexec') is None,
+@pytest.mark.skipif(shutil.which('cgexec') is None,
                     reason="cgexec not available")
-def test_profile_data(skipif_pre_ansible28, tmp_path):
+def test_profile_data(tmp_path):
     try:
         r = run(private_data_dir=str(tmp_path),
                 inventory='localhost ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}"',
diff --git a/test/unit/config/test_runner.py b/test/unit/config/test_runner.py
index 4bc0b5b..afefe81 100644
--- a/test/unit/config/test_runner.py
+++ b/test/unit/config/test_runner.py
@@ -223,7 +223,7 @@ def test_prepare_env_directory_isolation_from_settings(mocker, project_fixtures)
     '''
     # Mock away the things that would actually prepare the isolation directory.
     mocker.patch('os.makedirs', return_value=True)
-    copy_tree = mocker.patch('distutils.dir_util.copy_tree')
+    copy_tree = mocker.patch('shutil.copytree')
     mkdtemp = mocker.patch('tempfile.mkdtemp')
     mkdtemp.return_value = '/tmp/runner/runner_di_XYZ'
     mocker.patch('ansible_runner.config.runner.RunnerConfig.build_process_isolation_temp_dir')
@@ -245,7 +245,7 @@ def test_prepare_env_directory_isolation_from_settings(mocker, project_fixtures)
     mkdtemp.assert_called_once_with(prefix='runner_di_', dir='/tmp/runner')
 
     # The project files should be copied to the isolation path.
-    copy_tree.assert_called_once_with(rc.project_dir, rc.directory_isolation_path, preserve_symlinks=True)
+    copy_tree.assert_called_once_with(rc.project_dir, rc.directory_isolation_path, symlinks=True)
 
 
 def test_prepare_inventory(mocker):