From 3f068e7c83cb2ce67f2073153d900d3d92009b78 Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Thu, 9 Jan 2025 13:35:41 +0000 Subject: [PATCH 01/12] ci: Remove unneeded aws command entries CI container imagesa re now hosted by GitHub Container Register. There is nothing that needs to run `aws`. fixes #1036 --- tox.ini | 2 -- 1 file changed, 2 deletions(-) diff --git a/tox.ini b/tox.ini index ea988aa0..779d1e1a 100644 --- a/tox.ini +++ b/tox.ini @@ -140,7 +140,6 @@ allowlist_externals = # Added: Tox 3.18: Tox 4.0+ *_install.py *_tests.py - aws docker docker-credential-secretservice echo @@ -150,7 +149,6 @@ whitelist_externals = # Deprecated: Tox 3.18+; Removed: Tox 4.0 *_install.py *_tests.py - aws docker docker-credential-secretservice echo From e564944c5b5189db768a10a06ff26d0f63a56f6f Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Thu, 9 Jan 2025 14:02:22 +0000 Subject: [PATCH 02/12] tests: Stricter playbook and inventory parsing --- tests/ansible/ansible.cfg | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/ansible/ansible.cfg b/tests/ansible/ansible.cfg index 37edfd94..48a4eec3 100644 --- a/tests/ansible/ansible.cfg +++ b/tests/ansible/ansible.cfg @@ -10,6 +10,7 @@ callbacks_enabled = callback_whitelist = profile_tasks, timer +duplicate_dict_key = error inventory = hosts gathering = explicit strategy_plugins = ../../ansible_mitogen/plugins/strategy @@ -46,9 +47,13 @@ timeout = 30 host_key_checking = False [inventory] +# Fatal error if any inventory source is unparsed by every available plugin. any_unparsed_is_failed = true +# Fatal error if no inventory sources have a match for a host pattern. host_pattern_mismatch = error ignore_extensions = ~, .bak, .disabled +# Fatal error if no inventory sources are successfully parsed. +unparsed_is_failed = true [callback_profile_tasks] task_output_limit = 10 From 6698f4bcd9ab4abd30d8a6671b51044a45291a68 Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Thu, 9 Jan 2025 14:03:35 +0000 Subject: [PATCH 03/12] tests: Remove unused tasks fragment --- tests/ansible/integration/runner/_reset_conn.yml | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 tests/ansible/integration/runner/_reset_conn.yml diff --git a/tests/ansible/integration/runner/_reset_conn.yml b/tests/ansible/integration/runner/_reset_conn.yml deleted file mode 100644 index 30f1b0c0..00000000 --- a/tests/ansible/integration/runner/_reset_conn.yml +++ /dev/null @@ -1,2 +0,0 @@ - -- meta: reset_connection From 90779fe846359cce58ecbcd6ee6ab23bb7ee7583 Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Thu, 9 Jan 2025 14:14:02 +0000 Subject: [PATCH 04/12] ci: Enable Python warnings --- docs/changelog.rst | 1 + tox.ini | 2 ++ 2 files changed, 3 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index ce84593c..2f31dfd8 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -21,6 +21,7 @@ To avail of fixes in an unreleased version, please download a ZIP file In progress (unreleased) ------------------------ +* :gh:issue:`1213` tests: Enable default Python warnings v0.3.21 (2025-01-20) diff --git a/tox.ini b/tox.ini index 779d1e1a..bd2d65e9 100644 --- a/tox.ini +++ b/tox.ini @@ -105,6 +105,8 @@ setenv = NOCOVERAGE_ERASE = 1 NOCOVERAGE_REPORT = 1 PIP_CONSTRAINT={toxinidir}/tests/constraints.txt + # Print warning on the first occurence at each module:linenno in Mitogen. Available Python 2.7, 3.2+. + PYTHONWARNINGS=default:::ansible_mitogen,default:::mitogen # Ansible 6 - 8 (ansible-core 2.13 - 2.15) require Python 2.7 or >= 3.5 on targets ansible6: MITOGEN_TEST_DISTRO_SPECS=centos7 centos8 debian9 debian10 debian11 ubuntu1604 ubuntu1804 ubuntu2004 ansible7: MITOGEN_TEST_DISTRO_SPECS=centos7 centos8 debian9 debian10 debian11 ubuntu1604 ubuntu1804 ubuntu2004 From 6fcb7aae96ddfaf69c612ef999a3e6cdcd0389ae Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Thu, 9 Jan 2025 15:47:35 +0000 Subject: [PATCH 05/12] mitogen: Replace uses of deprecated `pkgutil.find_loader()` fixes #1111 --- docs/changelog.rst | 2 ++ mitogen/master.py | 26 +++++++++++++++++++------- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 2f31dfd8..a63b1163 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -22,6 +22,8 @@ In progress (unreleased) ------------------------ * :gh:issue:`1213` tests: Enable default Python warnings +* :gh:issue:`1111` :mod:`mitogen`: Replace uses of deprecated + :py:func:`pkgutil.find_loader` v0.3.21 (2025-01-20) diff --git a/mitogen/master.py b/mitogen/master.py index 865c9dc1..927ccaf1 100644 --- a/mitogen/master.py +++ b/mitogen/master.py @@ -54,21 +54,33 @@ try: import importlib.machinery import importlib.util from _imp import is_builtin as _is_builtin + + def _find_loader(fullname): + try: + maybe_spec = importlib.util.find_spec(fullname) + except (ImportError, AttributeError, TypeError, ValueError): + exc = sys.exc_info()[1] + raise ImportError(*exc.args) + try: + return maybe_spec.loader + except AttributeError: + return None except ImportError: # Python < 3.4, PEP 302 Import Hooks import imp from imp import is_builtin as _is_builtin + try: + from pkgutil import find_loader as _find_loader + except ImportError: + # Python < 2.5 + from mitogen.compat.pkgutil import find_loader as _find_loader + try: import sysconfig except ImportError: sysconfig = None -if not hasattr(pkgutil, 'find_loader'): - # find_loader() was new in >=2.5, but the modern pkgutil.py syntax has - # been kept intentionally 2.3 compatible so we can reuse it. - from mitogen.compat import pkgutil - import mitogen import mitogen.core import mitogen.minify @@ -175,7 +187,7 @@ def get_child_modules(path, fullname): return [to_text(name) for _, name, _ in pkgutil.iter_modules([mod_path])] else: # we loaded some weird package in memory, so we'll see if it has a custom loader we can use - loader = pkgutil.find_loader(fullname) + loader = _find_loader(fullname) return [to_text(name) for name, _ in loader.iter_modules(None)] if loader else [] @@ -528,7 +540,7 @@ class PkgutilMethod(FinderMethod): # then the containing package is imported. # Pre-'import spec' this returned None, in Python3.6 it raises # ImportError. - loader = pkgutil.find_loader(fullname) + loader = _find_loader(fullname) except ImportError: e = sys.exc_info()[1] LOG.debug('%r: find_loader(%r) failed: %s', self, fullname, e) From 67219c309a836216ea65ea30bf4aa546d35c3523 Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Thu, 23 Jan 2025 12:48:25 +0000 Subject: [PATCH 06/12] mitogen: Fix unclosed file in first stage --- docs/changelog.rst | 1 + mitogen/parent.py | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index a63b1163..5167ef26 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -24,6 +24,7 @@ In progress (unreleased) * :gh:issue:`1213` tests: Enable default Python warnings * :gh:issue:`1111` :mod:`mitogen`: Replace uses of deprecated :py:func:`pkgutil.find_loader` +* :gh:issue:`1213` :mod:`mitogen`: Fix unclosed file in first stage v0.3.21 (2025-01-20) diff --git a/mitogen/parent.py b/mitogen/parent.py index fa3092c1..f301a42c 100644 --- a/mitogen/parent.py +++ b/mitogen/parent.py @@ -1429,7 +1429,9 @@ class Connection(object): os.environ['ARGV0']=sys.executable os.execl(sys.executable,sys.executable+'(mitogen:CONTEXT_NAME)') os.write(1,'MITO000\n'.encode()) - C=zlib.decompress(os.fdopen(0,'rb').read(PREAMBLE_COMPRESSED_LEN)) + fp=os.fdopen(0,'rb') + C=zlib.decompress(fp.read(PREAMBLE_COMPRESSED_LEN)) + fp.close() fp=os.fdopen(W,'wb',0) fp.write(C) fp.close() From 9342186b220790ce1ea9b87bc877d9ec2ad86408 Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Thu, 23 Jan 2025 12:49:40 +0000 Subject: [PATCH 07/12] tests: Fix unclosed file in fd_check script --- docs/changelog.rst | 1 + tests/data/fd_check.py | 22 ++++++++++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 5167ef26..d1edc1b5 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -25,6 +25,7 @@ In progress (unreleased) * :gh:issue:`1111` :mod:`mitogen`: Replace uses of deprecated :py:func:`pkgutil.find_loader` * :gh:issue:`1213` :mod:`mitogen`: Fix unclosed file in first stage +* :gh:issue:`1213` tests: Fix unclosed file in fd_check script v0.3.21 (2025-01-20) diff --git a/tests/data/fd_check.py b/tests/data/fd_check.py index 0a87a95e..f3684443 100755 --- a/tests/data/fd_check.py +++ b/tests/data/fd_check.py @@ -26,6 +26,7 @@ def controlling_tty(): return None +out_path = sys.argv[1] fd = int(sys.argv[2]) st = os.fstat(fd) @@ -35,7 +36,7 @@ if sys.argv[3] == 'write': else: buf = os.read(fd, 4).decode() -open(sys.argv[1], 'w').write(repr({ +output = repr({ 'buf': buf, 'flags': fcntl.fcntl(fd, fcntl.F_GETFL), 'st_mode': st.st_mode, @@ -43,4 +44,21 @@ open(sys.argv[1], 'w').write(repr({ 'st_ino': st.st_ino, 'ttyname': ttyname(fd), 'controlling_tty': controlling_tty(), -})) +}) + +try: + out_f = open(out_path, 'w') +except Exception: + exc = sys.exc_info()[1] + sys.stderr.write("Failed to open %r: %r" % (out_path, exc)) + sys.exit(1) + +try: + out_f.write(output) +except Exception: + out_f.close() + exc = sys.exc_info()[1] + sys.stderr.write("Failed to write to %r: %r" % (out_path, exc)) + sys.exit(2) + +out_f.close() From d3da3ff7693e251dc4a4833bc33efef414a23ddb Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Thu, 23 Jan 2025 12:52:21 +0000 Subject: [PATCH 08/12] ansible_mitogen: Don't redeclare interpreter discovery attributes Duplicated effort on Ansible 2.10, and a potential source of future error --- ansible_mitogen/mixins.py | 5 ----- docs/changelog.rst | 2 ++ 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/ansible_mitogen/mixins.py b/ansible_mitogen/mixins.py index 3953eb52..b916afe1 100644 --- a/ansible_mitogen/mixins.py +++ b/ansible_mitogen/mixins.py @@ -105,11 +105,6 @@ class ActionModuleMixin(ansible.plugins.action.ActionBase): connection.templar = self._templar self._finding_python_interpreter = False self._rediscovered_python = False - # redeclaring interpreter discovery vars here in case running ansible < 2.8.0 - self._discovered_interpreter_key = None - self._discovered_interpreter = False - self._discovery_deprecation_warnings = [] - self._discovery_warnings = [] def run(self, tmp=None, task_vars=None): """ diff --git a/docs/changelog.rst b/docs/changelog.rst index d1edc1b5..1b3df255 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -26,6 +26,8 @@ In progress (unreleased) :py:func:`pkgutil.find_loader` * :gh:issue:`1213` :mod:`mitogen`: Fix unclosed file in first stage * :gh:issue:`1213` tests: Fix unclosed file in fd_check script +* :gh:issue:`1213` :mod:`ansible_mitogen`: Don't redeclare Ansible interpreter + discovery attributes v0.3.21 (2025-01-20) From 1b8b2c8b1af8226b00ad2e26f0644c58f98c2fa2 Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Thu, 23 Jan 2025 12:58:12 +0000 Subject: [PATCH 09/12] ansible_mitogen: Rename Mitogen interpreter discovery attributes This makes their nature and ownership/responsibility much more explicit. --- ansible_mitogen/mixins.py | 12 +++++++----- ansible_mitogen/transport_config.py | 10 +++++----- docs/changelog.rst | 2 ++ tests/ansible/tests/connection_test.py | 4 +++- 4 files changed, 17 insertions(+), 11 deletions(-) diff --git a/ansible_mitogen/mixins.py b/ansible_mitogen/mixins.py index b916afe1..d1def2f6 100644 --- a/ansible_mitogen/mixins.py +++ b/ansible_mitogen/mixins.py @@ -103,8 +103,10 @@ class ActionModuleMixin(ansible.plugins.action.ActionBase): # required for python interpreter discovery connection.templar = self._templar - self._finding_python_interpreter = False - self._rediscovered_python = False + + self._mitogen_discovering_interpreter = False + self._mitogen_interpreter_candidate = None + self._mitogen_rediscovered_interpreter = False def run(self, tmp=None, task_vars=None): """ @@ -397,7 +399,7 @@ class ActionModuleMixin(ansible.plugins.action.ActionBase): # only cache discovered_interpreter if we're not running a rediscovery # rediscovery happens in places like docker connections that could have different # python interpreters than the main host - if not self._rediscovered_python: + if not self._mitogen_rediscovered_interpreter: result['ansible_facts'][self._discovered_interpreter_key] = self._discovered_interpreter if self._discovery_warnings: @@ -457,7 +459,7 @@ class ActionModuleMixin(ansible.plugins.action.ActionBase): # calling exec_command until we run into the right python we'll use # chicken-and-egg issue, mitogen needs a python to run low_level_execute_command # which is required by Ansible's discover_interpreter function - if self._finding_python_interpreter: + if self._mitogen_discovering_interpreter: possible_pythons = [ '/usr/bin/python', 'python3', @@ -484,7 +486,7 @@ class ActionModuleMixin(ansible.plugins.action.ActionBase): for possible_python in possible_pythons: try: - self._possible_python_interpreter = possible_python + self._mitogen_interpreter_candidate = possible_python rc, stdout, stderr = _run_cmd() # TODO: what exception is thrown? except: diff --git a/ansible_mitogen/transport_config.py b/ansible_mitogen/transport_config.py index effb4d62..22afd197 100644 --- a/ansible_mitogen/transport_config.py +++ b/ansible_mitogen/transport_config.py @@ -87,8 +87,8 @@ def run_interpreter_discovery_if_necessary(s, task_vars, action, rediscover_pyth it could be different than what's ran on the host """ # keep trying different interpreters until we don't error - if action._finding_python_interpreter: - return action._possible_python_interpreter + if action._mitogen_discovering_interpreter: + return action._mitogen_interpreter_candidate if s in ['auto', 'auto_legacy', 'auto_silent', 'auto_legacy_silent']: # python is the only supported interpreter_name as of Ansible 2.8.8 @@ -102,13 +102,13 @@ def run_interpreter_discovery_if_necessary(s, task_vars, action, rediscover_pyth # if we're rediscovering python then chances are we're running something like a docker connection # this will handle scenarios like running a playbook that does stuff + then dynamically creates a docker container, # then runs the rest of the playbook inside that container, and then rerunning the playbook again - action._rediscovered_python = True + action._mitogen_rediscovered_interpreter = True # blow away the discovered_interpreter_config cache and rediscover del task_vars['ansible_facts'][discovered_interpreter_config] if discovered_interpreter_config not in task_vars['ansible_facts']: - action._finding_python_interpreter = True + action._mitogen_discovering_interpreter = True # fake pipelining so discover_interpreter can be happy action._connection.has_pipelining = True s = ansible.executor.interpreter_discovery.discover_interpreter( @@ -128,7 +128,7 @@ def run_interpreter_discovery_if_necessary(s, task_vars, action, rediscover_pyth action._discovered_interpreter_key = discovered_interpreter_config action._discovered_interpreter = s - action._finding_python_interpreter = False + action._mitogen_discovering_interpreter = False return s diff --git a/docs/changelog.rst b/docs/changelog.rst index 1b3df255..e5f3340b 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -28,6 +28,8 @@ In progress (unreleased) * :gh:issue:`1213` tests: Fix unclosed file in fd_check script * :gh:issue:`1213` :mod:`ansible_mitogen`: Don't redeclare Ansible interpreter discovery attributes +* :gh:issue:`1213` :mod:`ansible_mitogen`: Rename Mitogen interpreter discovery + attributes v0.3.21 (2025-01-20) diff --git a/tests/ansible/tests/connection_test.py b/tests/ansible/tests/connection_test.py index 649f8b65..856c6b1c 100644 --- a/tests/ansible/tests/connection_test.py +++ b/tests/ansible/tests/connection_test.py @@ -45,7 +45,9 @@ class ConnectionMixin(MuxProcessMixin): conn = self.klass(play_context, new_stdin=False) # conn functions don't fetch ActionModuleMixin objs from _get_task_vars() # through the usual walk-the-stack approach so we'll not run interpreter discovery here - conn._action = mock.MagicMock(_possible_python_interpreter=testlib.base_executable()) + conn._action = mock.MagicMock( + _mitogen_interpreter_candidate=testlib.base_executable(), + ) conn.on_action_run( task_vars={}, delegate_to_hostname=None, From 356be2e65f76e045143b8b9cab306ed511afe2c9 Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Thu, 23 Jan 2025 12:59:36 +0000 Subject: [PATCH 10/12] ansible_mitogen: Remove unneeded internal _run_cmd() --- ansible_mitogen/mixins.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/ansible_mitogen/mixins.py b/ansible_mitogen/mixins.py index d1def2f6..2f66961b 100644 --- a/ansible_mitogen/mixins.py +++ b/ansible_mitogen/mixins.py @@ -476,18 +476,12 @@ class ActionModuleMixin(ansible.plugins.action.ActionBase): # not used, just adding a filler value possible_pythons = ['python'] - def _run_cmd(): - return self._connection.exec_command( - cmd=cmd, - in_data=in_data, - sudoable=sudoable, - mitogen_chdir=chdir, - ) - for possible_python in possible_pythons: try: self._mitogen_interpreter_candidate = possible_python - rc, stdout, stderr = _run_cmd() + rc, stdout, stderr = self._connection.exec_command( + cmd, in_data, sudoable, mitogen_chdir=chdir, + ) # TODO: what exception is thrown? except: # we've reached the last python attempted and failed From 51c7b789f7ed9140b31617f0d8e0998221711793 Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Thu, 23 Jan 2025 13:03:42 +0000 Subject: [PATCH 11/12] ansible_mitogen: Decouple possible_pythons order & error handling 'python' could now be tried earlier, or not at all. --- ansible_mitogen/mixins.py | 3 +-- docs/changelog.rst | 2 ++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/ansible_mitogen/mixins.py b/ansible_mitogen/mixins.py index 2f66961b..c34aa68a 100644 --- a/ansible_mitogen/mixins.py +++ b/ansible_mitogen/mixins.py @@ -485,8 +485,7 @@ class ActionModuleMixin(ansible.plugins.action.ActionBase): # TODO: what exception is thrown? except: # we've reached the last python attempted and failed - # TODO: could use enumerate(), need to check which version of python first had it though - if possible_python == 'python': + if possible_python == possible_pythons[-1]: raise else: continue diff --git a/docs/changelog.rst b/docs/changelog.rst index e5f3340b..c6320bae 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -30,6 +30,8 @@ In progress (unreleased) discovery attributes * :gh:issue:`1213` :mod:`ansible_mitogen`: Rename Mitogen interpreter discovery attributes +* :gh:issue:`1213` :mod:`ansible_mitogen`: Decouple possible_pythons order & + error handling v0.3.21 (2025-01-20) From e97d20c9d1c90614bc30343a94a6ee9bb9b3e66a Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Thu, 23 Jan 2025 13:13:18 +0000 Subject: [PATCH 12/12] ansible_mitogen: Return stderr_lines from _low_level_execute_command() Vanilla Ansible has returned stderr since v1.9 or earlier, stderr_lines was added in v2.6.0 (https://github.com/ansible/ansible/pull/40079). --- ansible_mitogen/mixins.py | 4 +++- docs/changelog.rst | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/ansible_mitogen/mixins.py b/ansible_mitogen/mixins.py index c34aa68a..dadf2c17 100644 --- a/ansible_mitogen/mixins.py +++ b/ansible_mitogen/mixins.py @@ -491,10 +491,12 @@ class ActionModuleMixin(ansible.plugins.action.ActionBase): continue stdout_text = to_text(stdout, errors=encoding_errors) + stderr_text = to_text(stderr, errors=encoding_errors) return { 'rc': rc, 'stdout': stdout_text, 'stdout_lines': stdout_text.splitlines(), - 'stderr': stderr, + 'stderr': stderr_text, + 'stderr_lines': stderr_text.splitlines(), } diff --git a/docs/changelog.rst b/docs/changelog.rst index c6320bae..ef575c35 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -32,6 +32,8 @@ In progress (unreleased) attributes * :gh:issue:`1213` :mod:`ansible_mitogen`: Decouple possible_pythons order & error handling +* :gh:issue:`1213` :mod:`ansible_mitogen`: Return ``stderr_lines`` from + ``_low_level_execute_command()`` v0.3.21 (2025-01-20)