ansible-test - Fix consistency of managed venvs. (#77028)

changelogs/fragments/ansible-test-managed-venv.yml

@@ -0,0 +1,15 @@
+bugfixes:
+  - ansible-test - Virtual environments managed by ansible-test now use consistent versions of ``pip``, ``setuptools`` and ``wheel``.
+                   This avoids issues with virtual environments containing outdated or dysfunctional versions of these tools.
+                   The initial bootstrapping of ``pip`` is done by ansible-test from an HTTPS endpoint instead of creating the virtual environment with it already present.
+  - ansible-test - Sanity tests run with the ``--requirements` option for Python 2.x now install ``virtualenv`` when it is missing or too old.
+                   Previously it was only installed if missing.
+                   Version 16.7.12 is now installed instead of the latest version.
+  - ansible-test - All virtual environments managed by ansible-test are marked as usable after being bootstrapped, to avoid errors caused by use of incomplete environments.
+                   Previously this was only done for sanity tests.
+                   Existing environments from previous versions of ansible-test will be recreated on demand due to lacking the new marker.
+minor_changes:
+  - ansible-test - The ``pip`` and ``wheel`` packages are removed from all sanity test virtual environments after installation completes to reduce their size.
+                   Previously they were only removed from the environments used for the ``import`` sanity test.
+  - ansible-test - The hash for all managed sanity test virtual environments has changed.
+                   Containers that include ``ansible-test sanity --prime-venvs`` will need to be rebuilt to continue using primed virtual environments.

test/lib/ansible_test/_internal/commands/sanity/__init__.py

@@ -1127,6 +1127,7 @@ def create_sanity_virtualenv(
 
         write_text_file(meta_install, virtualenv_install)
 
+        # false positive: pylint: disable=no-member
         if any(isinstance(command, PipInstall) and command.has_package('pyyaml') for command in commands):
             virtualenv_yaml = yamlcheck(virtualenv_python)
         else:

test/lib/ansible_test/_internal/commands/sanity/import.py

@@ -68,6 +68,10 @@ from ...host_configs import (
     PythonConfig,
 )
 
+from ...venv import (
+    get_virtualenv_version,
+)
+
 
 def _get_module_test(module_restrictions):  # type: (bool) -> t.Callable[[str], bool]
     """Create a predicate which tests whether a path can be used by modules or not."""
@@ -104,9 +108,10 @@ class ImportTest(SanityMultipleVersion):
 
         paths = [target.path for target in targets.include]
 
-        if python.version.startswith('2.'):
+        if python.version.startswith('2.') and (get_virtualenv_version(args, python.path) or (0,)) < (13,):
             # hack to make sure that virtualenv is available under Python 2.x
             # on Python 3.x we can use the built-in venv
+            # version 13+ is required to use the `--no-wheel` option
             try:
                 install_requirements(args, python, virtualenv=True, controller=False)  # sanity (import)
             except PipUnavailableError as ex:

test/lib/ansible_test/_internal/python_requirements.py

@@ -109,6 +109,13 @@ class PipVersion(PipCommand):
     """Details required to get the pip version."""
 
 
+@dataclasses.dataclass(frozen=True)
+class PipBootstrap(PipCommand):
+    """Details required to bootstrap pip."""
+    pip_version: str
+    packages: t.List[str]
+
+
 # Entry Points
 
 
@@ -168,10 +175,25 @@ def install_requirements(
 
     run_pip(args, python, commands, connection)
 
+    # false positive: pylint: disable=no-member
     if any(isinstance(command, PipInstall) and command.has_package('pyyaml') for command in commands):
         check_pyyaml(python)
 
 
+def collect_bootstrap(python):  # type: (PythonConfig) -> t.List[PipCommand]
+    """Return the details necessary to bootstrap pip into an empty virtual environment."""
+    infrastructure_packages = get_venv_packages(python)
+    pip_version = infrastructure_packages['pip']
+    packages = [f'{name}=={version}' for name, version in infrastructure_packages.items()]
+
+    bootstrap = PipBootstrap(
+        pip_version=pip_version,
+        packages=packages,
+    )
+
+    return [bootstrap]
+
+
 def collect_requirements(
         python,  # type: PythonConfig
         controller,  # type: bool
@@ -187,7 +209,9 @@ def collect_requirements(
     commands = []  # type: t.List[PipCommand]
 
     if virtualenv:
-        commands.extend(collect_package_install(packages=['virtualenv']))
+        # sanity tests on Python 2.x install virtualenv when it is too old or is not already installed and the `--requirements` option is given
+        # the last version of virtualenv with no dependencies is used to minimize the changes made outside a virtual environment
+        commands.extend(collect_package_install(packages=['virtualenv==16.7.12'], constraints=False))
 
     if coverage:
         commands.extend(collect_package_install(packages=[f'coverage=={COVERAGE_REQUIRED_VERSION}'], constraints=False))
@@ -207,15 +231,20 @@ def collect_requirements(
     if command in ('integration', 'windows-integration', 'network-integration'):
         commands.extend(collect_integration_install(command, controller))
 
-    if minimize:
-        # In some environments pkg_resources is installed as a separate pip package which needs to be removed.
-        # For example, using Python 3.8 on Ubuntu 18.04 a virtualenv is created with only pip and setuptools.
-        # However, a venv is created with an additional pkg-resources package which is independent of setuptools.
-        # Making sure pkg-resources is removed preserves the import test consistency between venv and virtualenv.
-        # Additionally, in the above example, the pyparsing package vendored with pkg-resources is out-of-date and generates deprecation warnings.
-        # Thus it is important to remove pkg-resources to prevent system installed packages from generating deprecation warnings.
-        commands.extend(collect_uninstall(packages=['pkg-resources'], ignore_errors=True))
-        commands.extend(collect_uninstall(packages=['setuptools', 'pip']))
+    if (sanity or minimize) and any(isinstance(command, PipInstall) for command in commands):
+        # bootstrap the managed virtual environment, which will have been created without any installed packages
+        # sanity tests which install no packages skip this step
+        commands = collect_bootstrap(python) + commands
+
+        # most infrastructure packages can be removed from sanity test virtual environments after they've been created
+        # removing them reduces the size of environments cached in containers
+        uninstall_packages = list(get_venv_packages(python))
+
+        if not minimize:
+            # installed packages may have run-time dependencies on setuptools
+            uninstall_packages.remove('setuptools')
+
+        commands.extend(collect_uninstall(packages=uninstall_packages))
 
     return commands
 
@@ -377,6 +406,41 @@ def collect_uninstall(packages, ignore_errors=False):  # type: (t.List[str], boo
 # Support
 
 
+def get_venv_packages(python):  # type: (PythonConfig) -> t.Dict[str, str]
+    """Return a dictionary of Python packages needed for a consistent virtual environment specific to the given Python version."""
+
+    # NOTE: This same information is needed for building the base-test-container image.
+    #       See: https://github.com/ansible/base-test-container/blob/main/files/installer.py
+
+    default_packages = dict(
+        pip='21.3.1',
+        setuptools='60.8.2',
+        wheel='0.37.1',
+    )
+
+    override_packages = {
+        '2.7': dict(
+            pip='20.3.4',  # 21.0 requires Python 3.6+
+            setuptools='44.1.1',  # 45.0.0 requires Python 3.5+
+            wheel=None,
+        ),
+        '3.5': dict(
+            pip='20.3.4',  # 21.0 requires Python 3.6+
+            setuptools='50.3.2',  # 51.0.0 requires Python 3.6+
+            wheel=None,
+        ),
+        '3.6': dict(
+            pip='21.3.1',  # 22.0 requires Python 3.7+
+            setuptools='59.6.0',  # 59.7.0 requires Python 3.7+
+            wheel=None,
+        ),
+    }
+
+    packages = {name: version or default_packages[name] for name, version in override_packages.get(python.version, default_packages).items()}
+
+    return packages
+
+
 def requirements_allowed(args, controller):  # type: (EnvironmentConfig, bool) -> bool
     """
     Return True if requirements can be installed, otherwise return False.

test/lib/ansible_test/_internal/venv.py

@@ -3,6 +3,7 @@ from __future__ import annotations
 
 import json
 import os
+import pathlib
 import sys
 import typing as t
 
@@ -31,6 +32,11 @@ from .host_configs import (
     PythonConfig,
 )
 
+from .python_requirements import (
+    collect_bootstrap,
+    run_pip,
+)
+
 
 def get_virtual_python(
         args,  # type: EnvironmentConfig
@@ -43,9 +49,28 @@ def get_virtual_python(
         suffix = ''
 
     virtual_environment_path = os.path.join(ResultType.TMP.path, 'delegation', f'python{python.version}{suffix}')
+    virtual_environment_marker = os.path.join(virtual_environment_path, 'marker.txt')
+
+    if os.path.exists(virtual_environment_marker):
+        display.info('Using existing Python %s virtual environment: %s' % (python.version, virtual_environment_path), verbosity=1)
+    else:
+        # a virtualenv without a marker is assumed to have been partially created
+        remove_tree(virtual_environment_path)
+
+        if not create_virtual_environment(args, python, virtual_environment_path, python.system_site_packages):
+            raise ApplicationError(f'Python {python.version} does not provide virtual environment support.')
 
-    if not create_virtual_environment(args, python, virtual_environment_path, python.system_site_packages):
-        raise ApplicationError(f'Python {python.version} does not provide virtual environment support.')
+        virtual_environment_python = VirtualPythonConfig(
+            version=python.version,
+            path=os.path.join(virtual_environment_path, 'bin', 'python'),
+        )
+
+        commands = collect_bootstrap(virtual_environment_python)
+
+        run_pip(args, virtual_environment_python, commands, None)  # get_virtual_python()
+
+    # touch the marker to keep track of when the virtualenv was last used
+    pathlib.Path(virtual_environment_marker).touch()
 
     return virtual_environment_path
 
@@ -54,13 +79,9 @@ def create_virtual_environment(args,  # type: EnvironmentConfig
                                python,  # type: PythonConfig
                                path,  # type: str
                                system_site_packages=False,  # type: bool
-                               pip=True,  # type: bool
+                               pip=False,  # type: bool
                                ):  # type: (...) -> bool
     """Create a virtual environment using venv or virtualenv for the requested Python version."""
-    if os.path.isdir(path):
-        display.info('Using existing Python %s virtual environment: %s' % (python.version, path), verbosity=1)
-        return True
-
     if not os.path.exists(python.path):
         # the requested python version could not be found
         return False
@@ -203,6 +224,9 @@ def run_virtualenv(args,  # type: EnvironmentConfig
 
     if not pip:
         cmd.append('--no-pip')
+        # these options provide consistency with venv, which does not install them without pip
+        cmd.append('--no-setuptools')
+        cmd.append('--no-wheel')
 
     cmd.append(path)
 

test/lib/ansible_test/_util/target/setup/quiet_pip.py

@@ -3,6 +3,7 @@ from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
 import logging
+import os
 import re
 import runpy
 import sys
@@ -52,8 +53,16 @@ def main():
         #   Python 2.7 cannot use the -W option to match warning text after a colon. This makes it impossible to match specific warning messages.
         warnings.filterwarnings('ignore', message_filter)
 
+    get_pip = os.environ.get('GET_PIP')
+
     try:
-        runpy.run_module('pip.__main__', run_name='__main__', alter_sys=True)
+        if get_pip:
+            directory, filename = os.path.split(get_pip)
+            module = os.path.splitext(filename)[0]
+            sys.path.insert(0, directory)
+            runpy.run_module(module, run_name='__main__', alter_sys=True)
+        else:
+            runpy.run_module('pip.__main__', run_name='__main__', alter_sys=True)
     except ImportError as ex:
         print('pip is unavailable: %s' % ex)
         sys.exit(1)

test/lib/ansible_test/_util/target/setup/requirements.py

@@ -38,6 +38,11 @@ except ImportError:
     # noinspection PyProtectedMember
     from pipes import quote as cmd_quote
 
+try:
+    from urllib.request import urlopen
+except ImportError:
+    from urllib import urlopen
+
 ENCODING = 'utf-8'
 PAYLOAD = b'{payload}'  # base-64 encoded JSON payload which will be populated before this script is executed
 
@@ -70,6 +75,38 @@ def main():  # type: () -> None
                 sys.exit(1)
 
 
+# noinspection PyUnusedLocal
+def bootstrap(pip, options):  # type: (str, t.Dict[str, t.Any]) -> None
+    """Bootstrap pip and related packages in an empty virtual environment."""
+    pip_version = options['pip_version']
+    packages = options['packages']
+
+    url = 'https://ci-files.testing.ansible.com/ansible-test/get-pip-%s.py' % pip_version
+    cache_path = os.path.expanduser('~/.ansible/test/cache/get_pip_%s.py' % pip_version.replace(".", "_"))
+    temp_path = cache_path + '.download'
+
+    if os.path.exists(cache_path):
+        log('Using cached pip %s bootstrap script: %s' % (pip_version, cache_path))
+    else:
+        log('Downloading pip %s bootstrap script: %s' % (pip_version, url))
+
+        make_dirs(os.path.dirname(cache_path))
+        download_file(url, temp_path)
+        shutil.move(temp_path, cache_path)
+
+        log('Cached pip %s bootstrap script: %s' % (pip_version, cache_path))
+
+    env = common_pip_environment()
+    env.update(GET_PIP=cache_path)
+
+    options = common_pip_options()
+    options.extend(packages)
+
+    command = [sys.executable, pip] + options
+
+    execute_command(command, env=env)
+
+
 def install(pip, options):  # type: (str, t.Dict[str, t.Any]) -> None
     """Perform a pip install."""
     requirements = options['requirements']
@@ -92,7 +129,9 @@ def install(pip, options):  # type: (str, t.Dict[str, t.Any]) -> None
 
         command = [sys.executable, pip, 'install'] + options
 
-        execute_command(command, tempdir)
+        env = common_pip_environment()
+
+        execute_command(command, env=env, cwd=tempdir)
     finally:
         remove_tree(tempdir)
 
@@ -107,8 +146,10 @@ def uninstall(pip, options):  # type: (str, t.Dict[str, t.Any]) -> None
 
     command = [sys.executable, pip, 'uninstall', '-y'] + options
 
+    env = common_pip_environment()
+
     try:
-        execute_command(command, capture=True)
+        execute_command(command, env=env, capture=True)
     except SubprocessError:
         if not ignore_errors:
             raise
@@ -123,7 +164,16 @@ def version(pip, options):  # type: (str, t.Dict[str, t.Any]) -> None
 
     command = [sys.executable, pip, '-V'] + options
 
-    execute_command(command, capture=True)
+    env = common_pip_environment()
+
+    execute_command(command, env=env, capture=True)
+
+
+def common_pip_environment():  # type: () -> t.Dict[str, str]
+    """Return common environment variables used to run pip."""
+    env = os.environ.copy()
+
+    return env
 
 
 def common_pip_options():  # type: () -> t.List[str]
@@ -143,6 +193,13 @@ def devnull():  # type: () -> t.IO[bytes]
     return devnull.file
 
 
+def download_file(url, path):  # type: (str, str) -> None
+    """Download the given URL to the specified file path."""
+    with open(to_bytes(path), 'wb') as saved_file:
+        download = urlopen(url)
+        shutil.copyfileobj(download, saved_file)
+
+
 class ApplicationError(Exception):
     """Base class for application exceptions."""
 
@@ -170,7 +227,7 @@ def log(message, verbosity=0):  # type: (str, int) -> None
     CONSOLE.flush()
 
 
-def execute_command(cmd, cwd=None, capture=False):  # type: (t.List[str], t.Optional[str], bool) -> None
+def execute_command(cmd, cwd=None, capture=False, env=None):  # type: (t.List[str], t.Optional[str], bool, t.Optional[t.Dict[str, str]]) -> None
     """Execute the specified command."""
     log('Execute command: %s' % ' '.join(cmd_quote(c) for c in cmd), verbosity=1)
 
@@ -183,7 +240,8 @@ def execute_command(cmd, cwd=None, capture=False):  # type: (t.List[str], t.Opti
         stdout = None
         stderr = None
 
-    process = subprocess.Popen(cmd_bytes, cwd=to_optional_bytes(cwd), stdin=devnull(), stdout=stdout, stderr=stderr)  # pylint: disable=consider-using-with
+    cwd_bytes = to_optional_bytes(cwd)
+    process = subprocess.Popen(cmd_bytes, cwd=cwd_bytes, stdin=devnull(), stdout=stdout, stderr=stderr, env=env)  # pylint: disable=consider-using-with
     stdout_bytes, stderr_bytes = process.communicate()
     stdout_text = to_optional_text(stdout_bytes) or u''
     stderr_text = to_optional_text(stderr_bytes) or u''

test/sanity/ignore.txt

@@ -206,6 +206,7 @@ test/integration/targets/win_script/files/test_script_with_args.ps1 pslint:PSAvo
 test/integration/targets/win_script/files/test_script_with_splatting.ps1 pslint:PSAvoidUsingWriteHost # Keep
 test/lib/ansible_test/_data/requirements/sanity.pslint.ps1 pslint:PSCustomUseLiteralPath # Uses wildcards on purpose
 test/lib/ansible_test/_util/target/setup/ConfigureRemotingForAnsible.ps1 pslint:PSCustomUseLiteralPath
+test/lib/ansible_test/_util/target/setup/requirements.py replace-urlopen
 test/support/integration/plugins/inventory/aws_ec2.py pylint:use-a-generator
 test/support/integration/plugins/modules/ec2_group.py pylint:use-a-generator
 test/support/integration/plugins/modules/timezone.py pylint:disallowed-name