Improve ansible-test python interpreter selection. (#54445)

test/runner/completion/docker.txt

@@ -1,11 +1,11 @@
-default name=quay.io/ansible/default-test-container:1.6.0 python=3
-centos6 name=quay.io/ansible/centos6-test-container:1.4.0 seccomp=unconfined
-centos7 name=quay.io/ansible/centos7-test-container:1.4.0 seccomp=unconfined
-fedora28 name=quay.io/ansible/fedora28-test-container:1.5.0
-fedora29 name=quay.io/ansible/fedora29-test-container:1.5.0 python=3
-opensuse15py2 name=quay.io/ansible/opensuse15py2-test-container:1.7.0
-opensuse15 name=quay.io/ansible/opensuse15-test-container:1.7.0 python=3
-ubuntu1404 name=quay.io/ansible/ubuntu1404-test-container:1.4.0 seccomp=unconfined
-ubuntu1604 name=quay.io/ansible/ubuntu1604-test-container:1.4.0 seccomp=unconfined
-ubuntu1604py3 name=quay.io/ansible/ubuntu1604py3-test-container:1.4.0 seccomp=unconfined python=3
-ubuntu1804 name=quay.io/ansible/ubuntu1804-test-container:1.6.0 seccomp=unconfined python=3
+default name=quay.io/ansible/default-test-container:1.6.0 python=3.6,2.6,2.7,3.5,3.7,3.8 python3.8=/usr/local/bin/python3.8 seccomp=unconfined
+centos6 name=quay.io/ansible/centos6-test-container:1.4.0 python=2.6 seccomp=unconfined
+centos7 name=quay.io/ansible/centos7-test-container:1.4.0 python=2.7 seccomp=unconfined
+fedora28 name=quay.io/ansible/fedora28-test-container:1.5.0 python=2.7
+fedora29 name=quay.io/ansible/fedora29-test-container:1.5.0 python=3.7
+opensuse15py2 name=quay.io/ansible/opensuse15py2-test-container:1.7.0 python=2.7
+opensuse15 name=quay.io/ansible/opensuse15-test-container:1.7.0 python=3.6
+ubuntu1404 name=quay.io/ansible/ubuntu1404-test-container:1.4.0 python=2.7 seccomp=unconfined
+ubuntu1604 name=quay.io/ansible/ubuntu1604-test-container:1.4.0 python=2.7 seccomp=unconfined
+ubuntu1604py3 name=quay.io/ansible/ubuntu1604py3-test-container:1.4.0 python=3.5 seccomp=unconfined
+ubuntu1804 name=quay.io/ansible/ubuntu1804-test-container:1.6.0 python=3.6 seccomp=unconfined

test/runner/completion/remote.txt

@@ -1,5 +1,5 @@
-freebsd/11.1
-freebsd/12.0
-osx/10.11
-rhel/7.6
-rhel/8.0
+freebsd/11.1 python=2.7,3.6 python_dir=/usr/local/bin
+freebsd/12.0 python=2.7,3.6 python_dir=/usr/local/bin
+osx/10.11 python=2.7 python_dir=/usr/local/bin
+rhel/7.6 python=2.7
+rhel/8.0 python=3.6

test/runner/lib/cli.py

@@ -17,12 +17,14 @@ from lib.util import (
     display,
     raw_command,
     get_docker_completion,
+    get_remote_completion,
     generate_pip_command,
     read_lines_without_comments,
     MAXFD,
 )
 
 from lib.delegation import (
+    check_delegation_args,
     delegate,
 )
 
@@ -96,6 +98,7 @@ def main():
         display.color = config.color
         display.info_stderr = (isinstance(config, SanityConfig) and config.lint) or (isinstance(config, IntegrationConfig) and config.list_targets)
         check_startup()
+        check_delegation_args(config)
         configure_timeout(config)
 
         display.info('RLIMIT_NOFILE: %s' % (CURRENT_RLIMIT_NOFILE,), verbosity=2)
@@ -423,6 +426,11 @@ def parse_args():
                                   parents=[common],
                                   help='open an interactive shell')
 
+    shell.add_argument('--python',
+                       metavar='VERSION',
+                       choices=SUPPORTED_PYTHON_VERSIONS + ('default',),
+                       help='python version: %s' % ', '.join(SUPPORTED_PYTHON_VERSIONS))
+
     shell.set_defaults(func=command_shell,
                        config=ShellConfig)
 
@@ -584,6 +592,11 @@ def add_environments(parser, tox_version=False, tox_only=False):
                         action='store_true',
                         help='install command requirements')
 
+    parser.add_argument('--python-interpreter',
+                        metavar='PATH',
+                        default=None,
+                        help='path to the docker or remote python interpreter')
+
     environments = parser.add_mutually_exclusive_group()
 
     environments.add_argument('--local',
@@ -617,6 +630,7 @@ def add_environments(parser, tox_version=False, tox_only=False):
             remote_provider=None,
             remote_aws_region=None,
             remote_terminate=None,
+            python_interpreter=None,
         )
 
         return
@@ -752,7 +766,7 @@ def complete_remote(prefix, parsed_args, **_):
     """
     del parsed_args
 
-    images = read_lines_without_comments('test/runner/completion/remote.txt', remove_blank_lines=True)
+    images = sorted(get_remote_completion().keys())
 
     return [i for i in images if i.startswith(prefix)]
 
@@ -765,7 +779,7 @@ def complete_remote_shell(prefix, parsed_args, **_):
     """
     del parsed_args
 
-    images = read_lines_without_comments('test/runner/completion/remote.txt', remove_blank_lines=True)
+    images = sorted(get_remote_completion().keys())
 
     # 2008 doesn't support SSH so we do not add to the list of valid images
     images.extend(["windows/%s" % i for i in read_lines_without_comments('test/runner/completion/windows.txt', remove_blank_lines=True) if i != '2008'])

test/runner/lib/config.py

@@ -68,6 +68,7 @@ class EnvironmentConfig(CommonConfig):
             self.python = None
 
         self.python_version = self.python or '.'.join(str(i) for i in sys.version_info[:2])
+        self.python_interpreter = args.python_interpreter
 
         self.delegate = self.tox or self.docker or self.remote
         self.delegate_args = []  # type: list[str]

test/runner/lib/delegation.py

@@ -16,6 +16,10 @@ from lib.executor import (
     create_shell_command,
     run_httptester,
     start_httptester,
+    get_python_interpreter,
+    get_python_version,
+    get_docker_completion,
+    get_remote_completion,
 )
 
 from lib.config import (
@@ -65,6 +69,19 @@ from lib.target import (
 )
 
 
+def check_delegation_args(args):
+    """
+    :type args: CommonConfig
+    """
+    if not isinstance(args, EnvironmentConfig):
+        return
+
+    if args.docker:
+        get_python_version(args, get_docker_completion(), args.docker_raw)
+    elif args.remote:
+        get_python_version(args, get_remote_completion(), args.remote)
+
+
 def delegate(args, exclude, require, integration_targets):
     """
     :type args: EnvironmentConfig
@@ -143,7 +160,7 @@ def delegate_tox(args, exclude, require, integration_targets):
 
         tox.append('--')
 
-        cmd = generate_command(args, os.path.abspath('bin/ansible-test'), options, exclude, require)
+        cmd = generate_command(args, None, os.path.abspath('bin/ansible-test'), options, exclude, require)
 
         if not args.python:
             cmd += ['--python', version]
@@ -195,7 +212,8 @@ def delegate_docker(args, exclude, require, integration_targets):
         '--docker-util': 1,
     }
 
-    cmd = generate_command(args, '/root/ansible/bin/ansible-test', options, exclude, require)
+    python_interpreter = get_python_interpreter(args, get_docker_completion(), args.docker_raw)
+    cmd = generate_command(args, python_interpreter, '/root/ansible/bin/ansible-test', options, exclude, require)
 
     if isinstance(args, TestConfig):
         if args.coverage and not args.coverage_label:
@@ -369,7 +387,8 @@ def delegate_remote(args, exclude, require, integration_targets):
                 '--remote': 1,
             }
 
-            cmd = generate_command(args, 'ansible/bin/ansible-test', options, exclude, require)
+            python_interpreter = get_python_interpreter(args, get_remote_completion(), args.remote)
+            cmd = generate_command(args, python_interpreter, 'ansible/bin/ansible-test', options, exclude, require)
 
             if httptester_id:
                 cmd += ['--inject-httptester']
@@ -388,7 +407,8 @@ def delegate_remote(args, exclude, require, integration_targets):
 
             manage = ManagePosixCI(core_ci)
 
-        manage.setup()
+        python_version = get_python_version(args, get_remote_completion(), args.remote)
+        manage.setup(python_version)
 
         if isinstance(args, IntegrationConfig):
             cloud_platforms = get_cloud_providers(args)
@@ -420,9 +440,10 @@ def delegate_remote(args, exclude, require, integration_targets):
             docker_rm(args, httptester_id)
 
 
-def generate_command(args, path, options, exclude, require):
+def generate_command(args, python_interpreter, path, options, exclude, require):
     """
     :type args: EnvironmentConfig
+    :type python_interpreter: str | None
     :type path: str
     :type options: dict[str, int]
     :type exclude: list[str]
@@ -433,6 +454,9 @@ def generate_command(args, path, options, exclude, require):
 
     cmd = [path]
 
+    if python_interpreter:
+        cmd = [python_interpreter] + cmd
+
     # Force the encoding used during delegation.
     # This is only needed because ansible-test relies on Python's file system encoding.
     # Environments that do not have the locale configured are thus unable to work with unicode file paths.

test/runner/lib/executor.py

@@ -58,6 +58,7 @@ from lib.util import (
     generate_pip_command,
     find_python,
     get_docker_completion,
+    get_remote_completion,
     named_temporary_file,
     COVERAGE_OUTPUT_PATH,
 )
@@ -1623,17 +1624,7 @@ def get_integration_local_filter(args, targets):
             display.warning('Excluding tests marked "%s" which require --allow-destructive or prefixing with "destructive/" to run locally: %s'
                             % (skip.rstrip('/'), ', '.join(skipped)))
 
-    if args.python_version.startswith('3'):
-        python_version = 3
-    else:
-        python_version = 2
-
-    skip = 'skip/python%d/' % python_version
-    skipped = [target.name for target in targets if skip in target.aliases]
-    if skipped:
-        exclude.append(skip)
-        display.warning('Excluding tests marked "%s" which are not supported on python %d: %s'
-                        % (skip.rstrip('/'), python_version, ', '.join(skipped)))
+    exclude_targets_by_python_version(targets, args.python_version, exclude)
 
     return exclude
 
@@ -1663,22 +1654,9 @@ def get_integration_docker_filter(args, targets):
             display.warning('Excluding tests marked "%s" which require --docker-privileged to run under docker: %s'
                             % (skip.rstrip('/'), ', '.join(skipped)))
 
-    python_version = 2  # images are expected to default to python 2 unless otherwise specified
-
-    python_version = int(get_docker_completion().get(args.docker_raw, {}).get('python', str(python_version)))
+    python_version = get_python_version(args, get_docker_completion(), args.docker_raw)
 
-    if args.python:  # specifying a numeric --python option overrides the default python
-        if args.python.startswith('3'):
-            python_version = 3
-        elif args.python.startswith('2'):
-            python_version = 2
-
-    skip = 'skip/python%d/' % python_version
-    skipped = [target.name for target in targets if skip in target.aliases]
-    if skipped:
-        exclude.append(skip)
-        display.warning('Excluding tests marked "%s" which are not supported on python %d: %s'
-                        % (skip.rstrip('/'), python_version, ', '.join(skipped)))
+    exclude_targets_by_python_version(targets, python_version, exclude)
 
     return exclude
 
@@ -1711,16 +1689,99 @@ def get_integration_remote_filter(args, targets):
         display.warning('Excluding tests marked "%s" which are not supported on %s: %s'
                         % (skip.rstrip('/'), args.remote.replace('/', ' '), ', '.join(skipped)))
 
-    python_version = 2  # remotes are expected to default to python 2
+    python_version = get_python_version(args, get_remote_completion(), args.remote)
+
+    exclude_targets_by_python_version(targets, python_version, exclude)
 
-    skip = 'skip/python%d/' % python_version
+    return exclude
+
+
+def exclude_targets_by_python_version(targets, python_version, exclude):
+    """
+    :type targets: tuple[IntegrationTarget]
+    :type python_version: str
+    :type exclude: list[str]
+    """
+    if not python_version:
+        display.warning('Python version unknown. Unable to skip tests based on Python version.')
+        return
+
+    python_major_version = python_version.split('.')[0]
+
+    skip = 'skip/python%s/' % python_version
     skipped = [target.name for target in targets if skip in target.aliases]
     if skipped:
         exclude.append(skip)
-        display.warning('Excluding tests marked "%s" which are not supported on python %d: %s'
+        display.warning('Excluding tests marked "%s" which are not supported on python %s: %s'
                         % (skip.rstrip('/'), python_version, ', '.join(skipped)))
 
-    return exclude
+    skip = 'skip/python%s/' % python_major_version
+    skipped = [target.name for target in targets if skip in target.aliases]
+    if skipped:
+        exclude.append(skip)
+        display.warning('Excluding tests marked "%s" which are not supported on python %s: %s'
+                        % (skip.rstrip('/'), python_version, ', '.join(skipped)))
+
+
+def get_python_version(args, configs, name):
+    """
+    :type args: EnvironmentConfig
+    :type configs: dict[str, dict[str, str]]
+    :type name: str
+    """
+    config = configs.get(name, {})
+    config_python = config.get('python')
+
+    if not config or not config_python:
+        if args.python:
+            return args.python
+
+        display.warning('No Python version specified. '
+                        'Use completion config or the --python option to specify one.', unique=True)
+
+        return ''  # failure to provide a version may result in failures or reduced functionality later
+
+    supported_python_versions = config_python.split(',')
+    default_python_version = supported_python_versions[0]
+
+    if args.python and args.python not in supported_python_versions:
+        raise ApplicationError('Python %s is not supported by %s. Supported Python version(s) are: %s' % (
+            args.python, name, ', '.join(sorted(supported_python_versions))))
+
+    python_version = args.python or default_python_version
+
+    return python_version
+
+
+def get_python_interpreter(args, configs, name):
+    """
+    :type args: EnvironmentConfig
+    :type configs: dict[str, dict[str, str]]
+    :type name: str
+    """
+    if args.python_interpreter:
+        return args.python_interpreter
+
+    config = configs.get(name, {})
+
+    if not config:
+        if args.python:
+            guess = 'python%s' % args.python
+        else:
+            guess = 'python'
+
+        display.warning('Using "%s" as the Python interpreter. '
+                        'Use completion config or the --python-interpreter option to specify the path.' % guess, unique=True)
+
+        return guess
+
+    python_version = get_python_version(args, configs, name)
+
+    python_dir = config.get('python_dir', '/usr/bin')
+    python_interpreter = os.path.join(python_dir, 'python%s' % python_version)
+    python_interpreter = config.get('python%s' % python_version, python_interpreter)
+
+    return python_interpreter
 
 
 class EnvironmentDescription(object):

test/runner/lib/manage_ci.py

@@ -49,8 +49,10 @@ class ManageWindowsCI(object):
         for ssh_option in sorted(ssh_options):
             self.ssh_args += ['-o', '%s=%s' % (ssh_option, ssh_options[ssh_option])]
 
-    def setup(self):
-        """Used in delegate_remote to setup the host, no action is required for Windows."""
+    def setup(self, python_version):
+        """Used in delegate_remote to setup the host, no action is required for Windows.
+        :type python_version: str
+        """
         pass
 
     def wait(self):
@@ -204,15 +206,17 @@ class ManagePosixCI(object):
         elif self.core_ci.platform == 'rhel':
             self.become = ['sudo', '-in', 'bash', '-c']
 
-    def setup(self):
-        """Start instance and wait for it to become ready and respond to an ansible ping."""
+    def setup(self, python_version):
+        """Start instance and wait for it to become ready and respond to an ansible ping.
+        :type python_version: str
+        """
         self.wait()
 
         if isinstance(self.core_ci.args, ShellConfig):
             if self.core_ci.args.raw:
                 return
 
-        self.configure()
+        self.configure(python_version)
         self.upload_source()
 
     def wait(self):
@@ -227,10 +231,12 @@ class ManagePosixCI(object):
         raise ApplicationError('Timeout waiting for %s/%s instance %s.' %
                                (self.core_ci.platform, self.core_ci.version, self.core_ci.instance_id))
 
-    def configure(self):
-        """Configure remote host for testing."""
+    def configure(self, python_version):
+        """Configure remote host for testing.
+        :type python_version: str
+        """
         self.upload('test/runner/setup/remote.sh', '/tmp')
-        self.ssh('chmod +x /tmp/remote.sh && /tmp/remote.sh %s' % self.core_ci.platform)
+        self.ssh('chmod +x /tmp/remote.sh && /tmp/remote.sh %s %s' % (self.core_ci.platform, python_version))
 
     def upload_source(self):
         """Upload and extract source."""

test/runner/lib/util.py

@@ -38,7 +38,8 @@ except ImportError:
     # noinspection PyCompatibility
     from configparser import ConfigParser
 
-DOCKER_COMPLETION = {}
+DOCKER_COMPLETION = {}  # type: dict[str, dict[str, str]]
+REMOTE_COMPLETION = {}  # type: dict[str, dict[str, str]]
 PYTHON_PATHS = {}  # type: dict[str, str]
 
 try:
@@ -66,17 +67,33 @@ MODE_DIRECTORY_WRITE = MODE_DIRECTORY | stat.S_IWGRP | stat.S_IWOTH
 
 def get_docker_completion():
     """
-    :rtype: dict[str, str]
+    :rtype: dict[str, dict[str, str]]
+    """
+    return get_parameterized_completion(DOCKER_COMPLETION, 'docker')
+
+
+def get_remote_completion():
+    """
+    :rtype: dict[str, dict[str, str]]
+    """
+    return get_parameterized_completion(REMOTE_COMPLETION, 'remote')
+
+
+def get_parameterized_completion(cache, name):
+    """
+    :type cache: dict[str, dict[str, str]]
+    :type name: str
+    :rtype: dict[str, dict[str, str]]
     """
-    if not DOCKER_COMPLETION:
-        images = read_lines_without_comments('test/runner/completion/docker.txt', remove_blank_lines=True)
+    if not cache:
+        images = read_lines_without_comments('test/runner/completion/%s.txt' % name, remove_blank_lines=True)
 
-        DOCKER_COMPLETION.update(dict(kvp for kvp in [parse_docker_completion(i) for i in images] if kvp))
+        cache.update(dict(kvp for kvp in [parse_parameterized_completion(i) for i in images] if kvp))
 
-    return DOCKER_COMPLETION
+    return cache
 
 
-def parse_docker_completion(value):
+def parse_parameterized_completion(value):
     """
     :type value: str
     :rtype: tuple[str, dict[str, str]]

test/runner/setup/docker.sh

@@ -5,17 +5,6 @@ set -eu
 # Required for newer mysql-server packages to install/upgrade on Ubuntu 16.04.
 rm -f /usr/sbin/policy-rc.d
 
-# Support images with only python3 installed.
-if [ ! -f /usr/bin/python ] && [ -f /usr/bin/python3 ]; then
-    ln -s /usr/bin/python3 /usr/bin/python
-fi
-if [ ! -f /usr/bin/pip ] && [ -f /usr/bin/pip3 ]; then
-    ln -s /usr/bin/pip3 /usr/bin/pip
-fi
-if [ ! -f /usr/bin/virtualenv ] && [ -f /usr/bin/virtualenv-3 ]; then
-    ln -s /usr/bin/virtualenv-3 /usr/bin/virtualenv
-fi
-
 # Improve prompts on remote host for interactive use.
 # shellcheck disable=SC1117
 cat << EOF > ~/.bashrc

test/runner/setup/remote.sh

@@ -3,28 +3,32 @@
 set -eu
 
 platform="$1"
+python_version="$2"
+python_interpreter="python${python_version}"
 
 cd ~/
 
 install_pip () {
-    if ! pip --version --disable-pip-version-check 2>/dev/null; then
+    if ! "${python_interpreter}" -m pip.__main__ --version --disable-pip-version-check 2>/dev/null; then
         curl --silent --show-error https://bootstrap.pypa.io/get-pip.py -o /tmp/get-pip.py
-        python /tmp/get-pip.py --disable-pip-version-check --quiet
+        "${python_interpreter}" /tmp/get-pip.py --disable-pip-version-check --quiet
         rm /tmp/get-pip.py
     fi
 }
 
 if [ "${platform}" = "freebsd" ]; then
+    py_version="$(echo "${python_version}" | tr -d '.')"
+
     while true; do
         env ASSUME_ALWAYS_YES=YES pkg bootstrap && \
         pkg install -q -y \
             bash \
             curl \
             gtar \
-            python \
-            py27-Jinja2 \
-            py27-virtualenv \
-            py27-cryptography \
+            "python${py_version}" \
+            "py${py_version}-Jinja2" \
+            "py${py_version}-virtualenv" \
+            "py${py_version}-cryptography" \
             sudo \
         && break
         echo "Failed to install packages. Sleeping before trying again..."
@@ -55,18 +59,6 @@ elif [ "${platform}" = "rhel" ]; then
             echo "Failed to install packages. Sleeping before trying again..."
             sleep 10
         done
-
-        # When running from source our python shebang is: #!/usr/bin/env python
-        # To avoid modifying all of our scripts while running tests we make sure `python` is in our PATH.
-        if [ ! -f /usr/bin/python ]; then
-            ln -s /usr/bin/python3 /usr/bin/python
-        fi
-        if [ ! -f /usr/bin/pip ]; then
-            ln -s /usr/bin/pip3 /usr/bin/pip
-        fi
-        if [ ! -f /usr/bin/virtualenv ]; then
-            ln -s /usr/bin/virtualenv-3 /usr/bin/virtualenv
-        fi
     else
         while true; do
             yum install -q -y \