Clean up interpreter discovery (#84394)

* Clean up interpreter discovery

- Deprecated `auto_legacy` and `auto_legacy_silent`
- Removed obsolete platform fallback config and logic
- Replaced unit tests with integration tests
- Increased test coverage
pull/84796/merge
Matt Clay 8 months ago committed by GitHub
parent b7d76a93b2
commit 462affa7c4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,3 @@
deprecated_features:
- interpreter discovery - The ``auto_legacy`` and ``auto_legacy_silent`` options for ``INTERPRETER_PYTHON`` are deprecated.
Use ``auto`` or ``auto_silent`` options instead, as they have the same effect.

@ -1573,21 +1573,12 @@ INTERPRETER_PYTHON:
description:
- Path to the Python interpreter to be used for module execution on remote targets, or an automatic discovery mode.
Supported discovery modes are ``auto`` (the default), ``auto_silent``, ``auto_legacy``, and ``auto_legacy_silent``.
All discovery modes employ a lookup table to use the included system Python (on distributions known to include one),
falling back to a fixed ordered list of well-known Python interpreter locations if a platform-specific default is not
available. The fallback behavior will issue a warning that the interpreter should be set explicitly (since interpreters
installed later may change which one is used). This warning behavior can be disabled by setting ``auto_silent`` or
``auto_legacy_silent``. The value of ``auto_legacy`` provides all the same behavior, but for backward-compatibility
with older Ansible releases that always defaulted to ``/usr/bin/python``, will use that interpreter if present.
_INTERPRETER_PYTHON_DISTRO_MAP:
name: Mapping of known included platform pythons for various Linux distros
default:
# Entry only for testing
ansible test:
'99': /usr/bin/python99
version_added: "2.8"
# FUTURE: add inventory override once we're sure it can't be abused by a rogue target
# FUTURE: add a platform layer to the map so we could use for, eg, freebsd/macos/etc?
All discovery modes match against an ordered list of well-known Python interpreter locations.
The fallback behavior will issue a warning that the interpreter should be set explicitly (since interpreters
installed later may change which one is used). This warning behavior can be disabled by setting ``auto_silent``.
The ``auto_legacy`` modes are deprecated and behave the same as their respective ``auto`` modes.
They exist for backward-compatibility with older Ansible releases that always defaulted to ``/usr/bin/python3``,
which will use that interpreter if present.
INTERPRETER_PYTHON_FALLBACK:
name: Ordered list of Python interpreters to check for in discovery
default:

@ -1,47 +0,0 @@
# Copyright: (c) 2018 Ansible Project
# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
# FUTURE: this could be swapped out for our bundled version of distro to move more complete platform
# logic to the targets, so long as we maintain Py2.6 compat and don't need to do any kind of script assembly
from __future__ import annotations
import json
import platform
import io
import os
def read_utf8_file(path, encoding='utf-8'):
if not os.access(path, os.R_OK):
return None
with io.open(path, 'r', encoding=encoding) as fd:
content = fd.read()
return content
def get_platform_info():
result = dict(platform_dist_result=[])
if hasattr(platform, 'dist'):
result['platform_dist_result'] = platform.dist()
osrelease_content = read_utf8_file('/etc/os-release')
# try to fall back to /usr/lib/os-release
if not osrelease_content:
osrelease_content = read_utf8_file('/usr/lib/os-release')
result['osrelease_content'] = osrelease_content
return result
def main():
info = get_platform_info()
print(json.dumps(info))
if __name__ == '__main__':
main()

@ -3,25 +3,16 @@
from __future__ import annotations
import bisect
import json
import pkgutil
import re
from ansible import constants as C
from ansible.errors import AnsibleError
from ansible.module_utils.common.text.converters import to_native, to_text
from ansible.module_utils.distro import LinuxDistribution
from ansible.utils.display import Display
from ansible.utils.plugin_docs import get_versioned_doclink
from ansible.module_utils.compat.version import LooseVersion
from ansible.module_utils.facts.system.distribution import Distribution
from traceback import format_exc
OS_FAMILY_LOWER = {k.lower(): v.lower() for k, v in Distribution.OS_FAMILY.items()}
display = Display()
foundre = re.compile(r'(?s)PLATFORM[\r\n]+(.*)FOUND(.*)ENDFOUND')
foundre = re.compile(r'FOUND(.*)ENDFOUND', flags=re.DOTALL)
class InterpreterDiscoveryRequiredError(Exception):
@ -30,42 +21,28 @@ class InterpreterDiscoveryRequiredError(Exception):
self.interpreter_name = interpreter_name
self.discovery_mode = discovery_mode
def __str__(self):
return self.message
def __repr__(self):
# TODO: proper repr impl
return self.message
def discover_interpreter(action, interpreter_name, discovery_mode, task_vars):
# interpreter discovery is a 2-step process with the target. First, we use a simple shell-agnostic bootstrap to
# get the system type from uname, and find any random Python that can get us the info we need. For supported
# target OS types, we'll dispatch a Python script that calls platform.dist() (for older platforms, where available)
# and brings back /etc/os-release (if present). The proper Python path is looked up in a table of known
# distros/versions with included Pythons; if nothing is found, depending on the discovery mode, either the
# default fallback of /usr/bin/python is used (if we know it's there), or discovery fails.
# FUTURE: add logical equivalence for "python3" in the case of py3-only modules?
if interpreter_name != 'python':
raise ValueError('Interpreter discovery not supported for {0}'.format(interpreter_name))
"""Probe the target host for a Python interpreter from the `INTERPRETER_PYTHON_FALLBACK` list, returning the first found or `/usr/bin/python3` if none."""
host = task_vars.get('inventory_hostname', 'unknown')
res = None
platform_type = 'unknown'
found_interpreters = [u'/usr/bin/python3'] # fallback value
is_auto_legacy = discovery_mode.startswith('auto_legacy')
is_silent = discovery_mode.endswith('_silent')
if discovery_mode.startswith('auto_legacy'):
action._discovery_deprecation_warnings.append(dict(
msg=f"The '{discovery_mode}' option for 'INTERPRETER_PYTHON' now has the same effect as 'auto'.",
version='2.21',
))
try:
platform_python_map = C.config.get_config_value('_INTERPRETER_PYTHON_DISTRO_MAP', variables=task_vars)
bootstrap_python_list = C.config.get_config_value('INTERPRETER_PYTHON_FALLBACK', variables=task_vars)
display.vvv(msg=u"Attempting {0} interpreter discovery".format(interpreter_name), host=host)
display.vvv(msg=f"Attempting {interpreter_name} interpreter discovery.", host=host)
# not all command -v impls accept a list of commands, so we have to call it once per python
command_list = ["command -v '%s'" % py for py in bootstrap_python_list]
shell_bootstrap = "echo PLATFORM; uname; echo FOUND; {0}; echo ENDFOUND".format('; '.join(command_list))
shell_bootstrap = "echo FOUND; {0}; echo ENDFOUND".format('; '.join(command_list))
# FUTURE: in most cases we probably don't want to use become, but maybe sometimes we do?
res = action._low_level_execute_command(shell_bootstrap, sudoable=False)
@ -78,9 +55,7 @@ def discover_interpreter(action, interpreter_name, discovery_mode, task_vars):
display.debug(u'raw interpreter discovery output: {0}'.format(raw_stdout), host=host)
raise ValueError('unexpected output from Python interpreter discovery')
platform_type = match.groups()[0].lower().strip()
found_interpreters = [interp.strip() for interp in match.groups()[1].splitlines() if interp.startswith('/')]
found_interpreters = [interp.strip() for interp in match.groups()[0].splitlines() if interp.startswith('/')]
display.debug(u"found interpreters: {0}".format(found_interpreters), host=host)
@ -90,119 +65,20 @@ def discover_interpreter(action, interpreter_name, discovery_mode, task_vars):
u'host {0} (tried {1})'.format(host, bootstrap_python_list))
# this is lame, but returning None or throwing an exception is uglier
return u'/usr/bin/python3'
if platform_type != 'linux':
raise NotImplementedError('unsupported platform for extended discovery: {0}'.format(to_native(platform_type)))
platform_script = pkgutil.get_data('ansible.executor.discovery', 'python_target.py')
# FUTURE: respect pipelining setting instead of just if the connection supports it?
if action._connection.has_pipelining:
res = action._low_level_execute_command(found_interpreters[0], sudoable=False, in_data=platform_script)
else:
# FUTURE: implement on-disk case (via script action or ?)
raise NotImplementedError('pipelining support required for extended interpreter discovery')
platform_info = json.loads(res.get('stdout'))
distro, version = _get_linux_distro(platform_info)
if not distro or not version:
raise NotImplementedError('unable to get Linux distribution/version info')
family = OS_FAMILY_LOWER.get(distro.lower().strip())
version_map = platform_python_map.get(distro.lower().strip()) or platform_python_map.get(family)
if not version_map:
raise NotImplementedError('unsupported Linux distribution: {0}'.format(distro))
platform_interpreter = to_text(_version_fuzzy_match(version, version_map), errors='surrogate_or_strict')
# provide a transition period for hosts that were using /usr/bin/python previously (but shouldn't have been)
if is_auto_legacy:
if platform_interpreter != u'/usr/bin/python3' and u'/usr/bin/python3' in found_interpreters:
if not is_silent:
action._discovery_warnings.append(
u"Distribution {0} {1} on host {2} should use {3}, but is using "
u"/usr/bin/python3 for backward compatibility with prior Ansible releases. "
u"See {4} for more information"
.format(distro, version, host, platform_interpreter,
get_versioned_doclink('reference_appendices/interpreter_discovery.html')))
return u'/usr/bin/python3'
if platform_interpreter not in found_interpreters:
if platform_interpreter not in bootstrap_python_list:
# sanity check to make sure we looked for it
if not is_silent:
action._discovery_warnings \
.append(u"Platform interpreter {0} on host {1} is missing from bootstrap list"
.format(platform_interpreter, host))
if not is_silent:
action._discovery_warnings \
.append(u"Distribution {0} {1} on host {2} should use {3}, but is using {4}, since the "
u"discovered platform python interpreter was not present. See {5} "
u"for more information."
.format(distro, version, host, platform_interpreter, found_interpreters[0],
get_versioned_doclink('reference_appendices/interpreter_discovery.html')))
return found_interpreters[0]
return platform_interpreter
except NotImplementedError as ex:
display.vvv(msg=u'Python interpreter discovery fallback ({0})'.format(to_text(ex)), host=host)
except AnsibleError:
raise
except Exception as ex:
if not is_silent:
display.warning(msg=u'Unhandled error in Python interpreter discovery for host {0}: {1}'.format(host, to_text(ex)))
display.debug(msg=u'Interpreter discovery traceback:\n{0}'.format(to_text(format_exc())), host=host)
if res and res.get('stderr'):
display.vvv(msg=u'Interpreter discovery remote stderr:\n{0}'.format(to_text(res.get('stderr'))), host=host)
action._discovery_warnings.append(f'Unhandled error in Python interpreter discovery for host {host}: {ex}')
display.debug(msg=f'Interpreter discovery traceback:\n{format_exc()}', host=host)
if res and res.get('stderr'): # the current ssh plugin implementation always has stderr, making coverage of the false case difficult
display.vvv(msg=f"Interpreter discovery remote stderr:\n{res.get('stderr')}", host=host)
if not is_silent:
action._discovery_warnings \
.append(u"Platform {0} on host {1} is using the discovered Python interpreter at {2}, but future installation of "
u"another Python interpreter could change the meaning of that path. See {3} "
u"for more information."
.format(platform_type, host, found_interpreters[0],
get_versioned_doclink('reference_appendices/interpreter_discovery.html')))
return found_interpreters[0]
def _get_linux_distro(platform_info):
dist_result = platform_info.get('platform_dist_result', [])
if len(dist_result) == 3 and any(dist_result):
return dist_result[0], dist_result[1]
osrelease_content = platform_info.get('osrelease_content')
if not osrelease_content:
return u'', u''
osr = LinuxDistribution._parse_os_release_content(osrelease_content)
return osr.get('id', u''), osr.get('version_id', u'')
def _version_fuzzy_match(version, version_map):
# try exact match first
res = version_map.get(version)
if res:
return res
sorted_looseversions = sorted([LooseVersion(v) for v in version_map.keys()])
find_looseversion = LooseVersion(version)
# slot match; return nearest previous version we're newer than
kpos = bisect.bisect(sorted_looseversions, find_looseversion)
if kpos == 0:
# older than everything in the list, return the oldest version
# TODO: warning-worthy?
return version_map.get(sorted_looseversions[0].vstring)
# TODO: is "past the end of the list" warning-worthy too (at least if it's not a major version match)?
action._discovery_warnings.append(
f"Host {host} is using the discovered Python interpreter at {found_interpreters[0]}, "
"but future installation of another Python interpreter could change the meaning of that path. "
f"See {get_versioned_doclink('reference_appendices/interpreter_discovery.html')} for more information."
)
# return the next-oldest entry that we're newer than...
return version_map.get(sorted_looseversions[kpos - 1].vstring)
return found_interpreters[0]

@ -0,0 +1,23 @@
# Test discovery error handling when a connection failure is involved (raises AnsibleError).
- hosts: localhost
gather_facts: no
tasks:
- add_host:
name: bad_connection
ansible_connection: ssh
ansible_port: 1
ansible_host: localhost
ansible_python_interpreter: auto
ansible_pipelining: yes
- hosts: bad_connection
gather_facts: no
tasks:
- ping:
register: discovery
ignore_unreachable: yes
- assert:
that:
- discovery is unreachable

@ -0,0 +1,5 @@
- hosts: testhost
gather_facts: yes
tasks:
- include_tasks:
file: tasks/main.yml

@ -0,0 +1,10 @@
#!/usr/bin/env bash
set -eux
ansible-playbook discovery.yml -i ../../inventory "${@}"
# Run with -vvv to see the discovery message. This allows us to verify that discovery actually ran.
ansible-playbook bad-connection.yml -vvv 2>&1 | tee discovery.txt
grep 'Attempting python interpreter discovery.' discovery.txt

@ -66,7 +66,7 @@
- echoout_with_facts.ansible_facts is defined
- echoout_with_facts.running_python_interpreter == normalized_discovered_interpreter
- name: test that auto_legacy gives a dep warning when /usr/bin/python present but != auto result
- name: test that auto_legacy gives a deprecation warning
block:
- name: clear facts to force interpreter discovery to run
meta: clear_facts
@ -77,14 +77,100 @@
ping:
register: legacy
- name: check for warning (only on platforms where auto result is not /usr/bin/python and legacy is)
- name: check for warning
assert:
that:
- legacy.warnings | default([]) | length > 0
# only check for a dep warning if legacy returned /usr/bin/python and auto didn't
when: legacy.ansible_facts.discovered_interpreter_python == '/usr/bin/python' and
auto_out.ansible_facts.discovered_interpreter_python != '/usr/bin/python'
- legacy.deprecations | length == 1
- legacy.deprecations[0].msg is contains "The 'auto_legacy' option for 'INTERPRETER_PYTHON' now has the same effect as 'auto'."
- name: test no interpreter found behavior
block:
- name: clear facts to force interpreter discovery to run
meta: clear_facts
- name: trigger discovery with auto
vars:
ansible_python_interpreter: auto
ansible_interpreter_python_fallback:
- /usr/bin/does_not_exist
ping:
register: discovery
ignore_errors: yes # the fallback interpreter /usr/bin/python3 may not exist (e.g. FreeBSD)
- name: check for warning and default interpreter
assert:
that:
- discovery.warnings | length == 1
- discovery.warnings[0] is contains "No python interpreters found for host"
- discovery.ansible_facts.discovered_interpreter_python == '/usr/bin/python3'
- name: clear facts to force interpreter discovery to run
meta: clear_facts
- name: trigger discovery with auto_silent
vars:
ansible_python_interpreter: auto_silent
ansible_interpreter_python_fallback:
- /usr/bin/does_not_exist
ping:
register: discovery
ignore_errors: yes # the fallback interpreter /usr/bin/python3 may not exist (e.g. FreeBSD)
- name: verify auto_silent suppresses warning
assert:
that:
- discovery.warnings | default([]) | length == 0
- discovery.ansible_facts.discovered_interpreter_python == '/usr/bin/python3'
- name: test bad fallback interpreter
block:
- name: clear facts to force interpreter discovery to run
meta: clear_facts
- name: trigger discovery with auto
vars:
ansible_python_interpreter: auto
# This test takes advantage of the fact the existing code performs manual quoting instead of using shlex.quote.
# If that is ever fixed, it may be difficult (or impossible) to trigger the error condition covered here.
ansible_interpreter_python_fallback:
- "'i_have_a_single_quote"
ping:
register: discovery
ignore_errors: yes # the fallback interpreter /usr/bin/python3 may not exist (e.g. FreeBSD)
- debug:
var: discovery
- name: check for warning and default interpreter
assert:
that:
- discovery.warnings | length == 2
- discovery.warnings[0] is contains "Unhandled error in Python interpreter discovery for host"
- discovery.warnings[1] is contains "Host testhost is using the discovered Python interpreter at /usr/bin/python3"
- discovery.ansible_facts.discovered_interpreter_python == '/usr/bin/python3'
- name: clear facts to force interpreter discovery to run
meta: clear_facts
- name: trigger discovery with auto_silent
vars:
ansible_python_interpreter: auto_silent
# This test takes advantage of the fact the existing code performs manual quoting instead of using shlex.quote.
# If that is ever fixed, it may be difficult (or impossible) to trigger the error condition covered here.
ansible_interpreter_python_fallback:
- "'i_have_a_single_quote"
ping:
register: discovery
ignore_errors: yes # the fallback interpreter /usr/bin/python3 may not exist (e.g. FreeBSD)
- debug:
var: discovery
- name: verify auto_silent suppresses warning
assert:
that:
- discovery.warnings | default([]) | length == 0
- discovery.ansible_facts.discovered_interpreter_python == '/usr/bin/python3'
- name: test that auto_silent never warns and got the same answer as auto
block:
@ -102,7 +188,6 @@
- auto_silent_out.warnings is not defined
- auto_silent_out.ansible_facts.discovered_interpreter_python == auto_out.ansible_facts.discovered_interpreter_python
- name: test that auto_legacy_silent never warns and got the same answer as auto_legacy
block:
- name: clear facts to force interpreter discovery to run
@ -148,27 +233,25 @@
- name: fedora assertions
assert:
that:
- "'/bin/python3' in auto_out.ansible_facts.discovered_interpreter_python"
when: distro == 'fedora' and distro_version is version('23', '>=')
- auto_out.ansible_facts.discovered_interpreter_python|regex_search('^/usr/bin/python3')
when: distro == 'fedora'
- name: rhel assertions
assert:
that:
# rhel 9
- ('/bin/python3' in auto_out.ansible_facts.discovered_interpreter_python and distro_major_version is version('9','==')) or distro_major_version is version('9','!=')
- auto_out.ansible_facts.discovered_interpreter_python|regex_search('^/bin/python3')
when: distro == 'redhat'
- name: ubuntu assertions
assert:
that:
# ubuntu >= 16
- ('/bin/python3' in auto_out.ansible_facts.discovered_interpreter_python and distro_version is version('16.04','>=')) or distro_version is version('16.04','<')
- auto_out.ansible_facts.discovered_interpreter_python|regex_search('^/usr/bin/python3')
when: distro == 'ubuntu'
- name: mac assertions
assert:
that:
- auto_out.ansible_facts.discovered_interpreter_python == '/usr/bin/python'
- auto_out.ansible_facts.discovered_interpreter_python|regex_search('^/usr/bin/python3')
when: os_family == 'darwin'
always:

@ -1,96 +0,0 @@
# -*- coding: utf-8 -*-
# (c) 2019, Jordan Borean <jborean@redhat.com>
# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
from __future__ import annotations
import pytest
from unittest.mock import MagicMock
from ansible.executor.interpreter_discovery import discover_interpreter
from ansible.module_utils.common.text.converters import to_text
from ansible.errors import AnsibleConnectionFailure
mock_ubuntu_platform_res = to_text(
r'{"osrelease_content": "NAME=\"Ansible Test\"\nVERSION=\"100\"\nID=ansible-test\nID_LIKE=debian\n'
r'PRETTY_NAME=\"Ansible Test 100\"\nVERSION_ID=\"100\"\nHOME_URL=\"http://ansible.com/\"\n'
r'SUPPORT_URL=\"http://github.com/ansible/ansible\"\nBUG_REPORT_URL=\"http://github.com/ansible/ansible/\"\n'
r'VERSION_CODENAME=beans\nUBUNTU_CODENAME=beans\n", "platform_dist_result": ["Ansible Test", "100", "beans"]}'
)
def test_discovery_interpreter_linux_auto_legacy():
res1 = u'PLATFORM\nLinux\nFOUND\n/usr/bin/python99\n/usr/bin/python3\nENDFOUND'
mock_action = MagicMock()
mock_action._low_level_execute_command.side_effect = [{'stdout': res1}, {'stdout': mock_ubuntu_platform_res}]
actual = discover_interpreter(mock_action, 'python', 'auto_legacy', {'inventory_hostname': u'host-fóöbär'})
assert actual == u'/usr/bin/python3'
assert len(mock_action.method_calls) == 3
assert mock_action.method_calls[2][0] == '_discovery_warnings.append'
assert u'Distribution Ansible Test 100 on host host-fóöbär should use /usr/bin/python99, but is using /usr/bin/python3' \
u' for backward compatibility' in mock_action.method_calls[2][1][0]
def test_discovery_interpreter_linux_auto_legacy_silent():
res1 = u'PLATFORM\nLinux\nFOUND\n/usr/bin/python3.9\n/usr/bin/python3\nENDFOUND'
mock_action = MagicMock()
mock_action._low_level_execute_command.side_effect = [{'stdout': res1}, {'stdout': mock_ubuntu_platform_res}]
actual = discover_interpreter(mock_action, 'python', 'auto_legacy_silent', {'inventory_hostname': u'host-fóöbär'})
assert actual == u'/usr/bin/python3'
assert len(mock_action.method_calls) == 2
def test_discovery_interpreter_linux_auto():
res1 = u'PLATFORM\nLinux\nFOUND\n/usr/bin/python99\n/usr/bin/python3\nENDFOUND'
mock_action = MagicMock()
mock_action._low_level_execute_command.side_effect = [{'stdout': res1}, {'stdout': mock_ubuntu_platform_res}]
actual = discover_interpreter(mock_action, 'python', 'auto', {'inventory_hostname': u'host-fóöbär'})
assert actual == u'/usr/bin/python99'
assert len(mock_action.method_calls) == 2
def test_discovery_interpreter_non_linux():
mock_action = MagicMock()
mock_action._low_level_execute_command.return_value = \
{'stdout': u'PLATFORM\nDarwin\nFOUND\n/usr/bin/python3\nENDFOUND'}
actual = discover_interpreter(mock_action, 'python', 'auto_legacy', {'inventory_hostname': u'host-fóöbär'})
assert actual == u'/usr/bin/python3'
assert len(mock_action.method_calls) == 2
assert mock_action.method_calls[1][0] == '_discovery_warnings.append'
assert u'Platform darwin on host host-fóöbär is using the discovered Python interpreter at /usr/bin/python3, ' \
u'but future installation of another Python interpreter could change the meaning of that path' \
in mock_action.method_calls[1][1][0]
def test_no_interpreters_found():
mock_action = MagicMock()
mock_action._low_level_execute_command.return_value = {'stdout': u'PLATFORM\nWindows\nFOUND\nENDFOUND'}
actual = discover_interpreter(mock_action, 'python', 'auto_legacy', {'inventory_hostname': u'host-fóöbär'})
assert actual == u'/usr/bin/python3'
assert len(mock_action.method_calls) == 2
assert mock_action.method_calls[1][0] == '_discovery_warnings.append'
assert u'No python interpreters found for host host-fóöbär (tried' \
in mock_action.method_calls[1][1][0]
def test_ansible_error_exception():
mock_action = MagicMock()
mock_action._low_level_execute_command.side_effect = AnsibleConnectionFailure("host key mismatch")
with pytest.raises(AnsibleConnectionFailure) as context:
discover_interpreter(mock_action, 'python', 'auto_legacy', {'inventory_hostname': u'host'})
assert 'host key mismatch' == str(context.value)
Loading…
Cancel
Save