ansible-test - Parse content config only once. (#78418)

changelogs/fragments/ansible-test-content-config.yml

@@ -0,0 +1,3 @@
+bugfixes:
+  - "ansible-test - Test configuration for collections is now parsed only once, prior to delegation.
+                    Fixes issue: https://github.com/ansible/ansible/issues/78334"

test/integration/targets/ansible-test-config-invalid/aliases

@@ -0,0 +1,4 @@
+shippable/posix/group1  # runs in the distro test containers
+shippable/generic/group1  # runs in the default test container
+context/controller
+needs/target/collection

test/integration/targets/ansible-test-config-invalid/ansible_collections/ns/col/tests/config.yml

@@ -0,0 +1 @@
+invalid

test/integration/targets/ansible-test-config-invalid/ansible_collections/ns/col/tests/integration/targets/test/aliases

@@ -0,0 +1 @@
+context/controller

test/integration/targets/ansible-test-config-invalid/ansible_collections/ns/col/tests/integration/targets/test/runme.sh

@@ -0,0 +1 @@
+#!/usr/bin/env bash

test/integration/targets/ansible-test-config-invalid/ansible_collections/ns/col/tests/unit/plugins/module_utils/test_test.py

@@ -0,0 +1,2 @@
+def test_me():
+    pass

test/integration/targets/ansible-test-config-invalid/runme.sh

@@ -0,0 +1,12 @@
+#!/usr/bin/env bash
+# Make sure that ansible-test continues to work when content config is invalid.
+
+set -eu
+
+source ../collection/setup.sh
+
+set -x
+
+ansible-test sanity --test import --python "${ANSIBLE_TEST_PYTHON_VERSION}" --color --venv -v
+ansible-test units  --python "${ANSIBLE_TEST_PYTHON_VERSION}" --color --venv -v
+ansible-test integration --color --venv -v

test/integration/targets/ansible-test-config/aliases

@@ -0,0 +1,4 @@
+shippable/posix/group1  # runs in the distro test containers
+shippable/generic/group1  # runs in the default test container
+context/controller
+needs/target/collection

test/integration/targets/ansible-test-config/ansible_collections/ns/col/plugins/module_utils/test.py

@@ -0,0 +1,14 @@
+import sys
+import os
+
+
+def version_to_str(value):
+    return '.'.join(str(v) for v in value)
+
+
+controller_min_python_version = tuple(int(v) for v in os.environ['ANSIBLE_CONTROLLER_MIN_PYTHON_VERSION'].split('.'))
+current_python_version = sys.version_info[:2]
+
+if current_python_version < controller_min_python_version:
+    raise Exception('Current Python version %s is lower than the minimum controller Python version of %s. '
+                    'Did the collection config get ignored?' % (version_to_str(current_python_version), version_to_str(controller_min_python_version)))

test/integration/targets/ansible-test-config/ansible_collections/ns/col/tests/config.yml

@@ -0,0 +1,2 @@
+modules:
+  python_requires: controller  # allow tests to pass when run against a Python version not supported by the controller

test/integration/targets/ansible-test-config/ansible_collections/ns/col/tests/unit/plugins/module_utils/test_test.py

@@ -0,0 +1,5 @@
+from ansible_collections.ns.col.plugins.module_utils import test
+
+
+def test_me():
+    assert test

test/integration/targets/ansible-test-config/runme.sh

@@ -0,0 +1,15 @@
+#!/usr/bin/env bash
+# Make sure that ansible-test is able to parse collection config when using a venv.
+
+set -eu
+
+source ../collection/setup.sh
+
+set -x
+
+# On systems with a Python version below the minimum controller Python version, such as the default container, this test
+# will verify that the content config is working properly after delegation. Otherwise it will only verify that no errors
+# occur while trying to access content config (such as missing requirements).
+
+ansible-test sanity --test import --color --venv -v
+ansible-test units --color --venv -v

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

@@ -162,6 +162,8 @@ def command_sanity(args):  # type: (SanityConfig) -> None
     targets_use_pypi = any(isinstance(test, SanityMultipleVersion) and test.needs_pypi for test in tests) and not args.list_tests
     host_state = prepare_profiles(args, targets_use_pypi=targets_use_pypi)  # sanity
 
+    get_content_config(args)  # make sure content config has been parsed prior to delegation
+
     if args.delegate:
         raise Delegate(host_state=host_state, require=changes, exclude=args.exclude)
 
@@ -220,7 +222,7 @@ def command_sanity(args):  # type: (SanityConfig) -> None
                 all_targets = SanityTargets.filter_and_inject_targets(test, all_targets)
                 usable_targets = SanityTargets.filter_and_inject_targets(test, usable_targets)
 
-                usable_targets = sorted(test.filter_targets_by_version(list(usable_targets), version))
+                usable_targets = sorted(test.filter_targets_by_version(args, list(usable_targets), version))
                 usable_targets = settings.filter_skipped_targets(usable_targets)
                 sanity_targets = SanityTargets(tuple(all_targets), tuple(usable_targets))
 
@@ -362,12 +364,12 @@ class SanityIgnoreParser:
                 for python_version in test.supported_python_versions:
                     test_name = '%s-%s' % (test.name, python_version)
 
-                    paths_by_test[test_name] = set(target.path for target in test.filter_targets_by_version(test_targets, python_version))
+                    paths_by_test[test_name] = set(target.path for target in test.filter_targets_by_version(args, test_targets, python_version))
                     tests_by_name[test_name] = test
             else:
                 unversioned_test_names.update(dict(('%s-%s' % (test.name, python_version), test.name) for python_version in SUPPORTED_PYTHON_VERSIONS))
 
-                paths_by_test[test.name] = set(target.path for target in test.filter_targets_by_version(test_targets, ''))
+                paths_by_test[test.name] = set(target.path for target in test.filter_targets_by_version(args, test_targets, ''))
                 tests_by_name[test.name] = test
 
         for line_no, line in enumerate(lines, start=1):
@@ -761,7 +763,7 @@ class SanityTest(metaclass=abc.ABCMeta):
 
         raise NotImplementedError('Sanity test "%s" must implement "filter_targets" or set "no_targets" to True.' % self.name)
 
-    def filter_targets_by_version(self, targets, python_version):  # type: (t.List[TestTarget], str) -> t.List[TestTarget]
+    def filter_targets_by_version(self, args, targets, python_version):  # type: (SanityConfig, t.List[TestTarget], str) -> t.List[TestTarget]
         """Return the given list of test targets, filtered to include only those relevant for the test, taking into account the Python version."""
         del python_version  # python_version is not used here, but derived classes may make use of it
 
@@ -769,7 +771,7 @@ class SanityTest(metaclass=abc.ABCMeta):
 
         if self.py2_compat:
             # This sanity test is a Python 2.x compatibility test.
-            content_config = get_content_config()
+            content_config = get_content_config(args)
 
             if content_config.py2_support:
                 # This collection supports Python 2.x.
@@ -1061,15 +1063,15 @@ class SanityMultipleVersion(SanityTest, metaclass=abc.ABCMeta):
         """A tuple of supported Python versions or None if the test does not depend on specific Python versions."""
         return SUPPORTED_PYTHON_VERSIONS
 
-    def filter_targets_by_version(self, targets, python_version):  # type: (t.List[TestTarget], str) -> t.List[TestTarget]
+    def filter_targets_by_version(self, args, targets, python_version):  # type: (SanityConfig, t.List[TestTarget], str) -> t.List[TestTarget]
         """Return the given list of test targets, filtered to include only those relevant for the test, taking into account the Python version."""
         if not python_version:
             raise Exception('python_version is required to filter multi-version tests')
 
-        targets = super().filter_targets_by_version(targets, python_version)
+        targets = super().filter_targets_by_version(args, targets, python_version)
 
         if python_version in REMOTE_ONLY_PYTHON_VERSIONS:
-            content_config = get_content_config()
+            content_config = get_content_config(args)
 
             if python_version not in content_config.modules.python_versions:
                 # when a remote-only python version is not supported there are no paths to test

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

@@ -103,7 +103,7 @@ def command_units(args):  # type: (UnitsConfig) -> None
 
     paths = [target.path for target in include]
 
-    content_config = get_content_config()
+    content_config = get_content_config(args)
     supported_remote_python_versions = content_config.modules.python_versions
 
     if content_config.modules.controller_only:

test/lib/ansible_test/_internal/config.py

@@ -1,6 +1,7 @@
 """Configuration classes."""
 from __future__ import annotations
 
+import dataclasses
 import enum
 import os
 import sys
@@ -48,6 +49,22 @@ class TerminateMode(enum.Enum):
         return self.name.lower()
 
 
+@dataclasses.dataclass(frozen=True)
+class ModulesConfig:
+    """Configuration for modules."""
+    python_requires: str
+    python_versions: tuple[str, ...]
+    controller_only: bool
+
+
+@dataclasses.dataclass(frozen=True)
+class ContentConfig:
+    """Configuration for all content."""
+    modules: ModulesConfig
+    python_versions: tuple[str, ...]
+    py2_support: bool
+
+
 class EnvironmentConfig(CommonConfig):
     """Configuration common to all commands which execute in an environment."""
     def __init__(self, args, command):  # type: (t.Any, str) -> None
@@ -59,6 +76,10 @@ class EnvironmentConfig(CommonConfig):
         self.pypi_proxy = args.pypi_proxy  # type: bool
         self.pypi_endpoint = args.pypi_endpoint  # type: t.Optional[str]
 
+        # Populated by content_config.get_content_config on the origin.
+        # Serialized and passed to delegated instances to avoid parsing a second time.
+        self.content_config = None  # type: t.Optional[ContentConfig]
+
         # Set by check_controller_python once HostState has been created by prepare_profiles.
         # This is here for convenience, to avoid needing to pass HostState to some functions which already have access to EnvironmentConfig.
         self.controller_python = None  # type: t.Optional[PythonConfig]
@@ -97,9 +118,11 @@ class EnvironmentConfig(CommonConfig):
             if config.host_path:
                 settings_path = os.path.join(config.host_path, 'settings.dat')
                 state_path = os.path.join(config.host_path, 'state.dat')
+                config_path = os.path.join(config.host_path, 'config.dat')
 
                 files.append((os.path.abspath(settings_path), settings_path))
                 files.append((os.path.abspath(state_path), state_path))
+                files.append((os.path.abspath(config_path), config_path))
 
         data_context().register_payload_callback(host_callback)
 

test/lib/ansible_test/_internal/content_config.py

@@ -2,6 +2,7 @@
 from __future__ import annotations
 
 import os
+import pickle
 import typing as t
 
 from .constants import (
@@ -21,6 +22,7 @@ from .compat.yaml import (
 )
 
 from .io import (
+    open_binary_file,
     read_text_file,
 )
 
@@ -28,54 +30,59 @@ from .util import (
     ApplicationError,
     display,
     str_to_version,
-    cache,
 )
 
 from .data import (
     data_context,
 )
 
+from .config import (
+    EnvironmentConfig,
+    ContentConfig,
+    ModulesConfig,
+)
 
 MISSING = object()
 
 
-class BaseConfig:
-    """Base class for content configuration."""
-    def __init__(self, data):  # type: (t.Any) -> None
-        if not isinstance(data, dict):
-            raise Exception('config must be type `dict` not `%s`' % type(data))
-
+def parse_modules_config(data: t.Any) -> ModulesConfig:
+    """Parse the given dictionary as module config and return it."""
+    if not isinstance(data, dict):
+        raise Exception('config must be type `dict` not `%s`' % type(data))
 
-class ModulesConfig(BaseConfig):
-    """Configuration for modules."""
-    def __init__(self, data):  # type: (t.Any) -> None
-        super().__init__(data)
+    python_requires = data.get('python_requires', MISSING)
 
-        python_requires = data.get('python_requires', MISSING)
+    if python_requires == MISSING:
+        raise KeyError('python_requires is required')
 
-        if python_requires == MISSING:
-            raise KeyError('python_requires is required')
+    return ModulesConfig(
+        python_requires=python_requires,
+        python_versions=parse_python_requires(python_requires),
+        controller_only=python_requires == 'controller',
+    )
 
-        self.python_requires = python_requires
-        self.python_versions = parse_python_requires(python_requires)
-        self.controller_only = python_requires == 'controller'
 
+def parse_content_config(data: t.Any) -> ContentConfig:
+    """Parse the given dictionary as content config and return it."""
+    if not isinstance(data, dict):
+        raise Exception('config must be type `dict` not `%s`' % type(data))
 
-class ContentConfig(BaseConfig):
-    """Configuration for all content."""
-    def __init__(self, data):  # type: (t.Any) -> None
-        super().__init__(data)
+    # Configuration specific to modules/module_utils.
+    modules = parse_modules_config(data.get('modules', {}))
 
-        # Configuration specific to modules/module_utils.
-        self.modules = ModulesConfig(data.get('modules', {}))
+    # Python versions supported by the controller, combined with Python versions supported by modules/module_utils.
+    # Mainly used for display purposes and to limit the Python versions used for sanity tests.
+    python_versions = tuple(version for version in SUPPORTED_PYTHON_VERSIONS
+                            if version in CONTROLLER_PYTHON_VERSIONS or version in modules.python_versions)
 
-        # Python versions supported by the controller, combined with Python versions supported by modules/module_utils.
-        # Mainly used for display purposes and to limit the Python versions used for sanity tests.
-        self.python_versions = [version for version in SUPPORTED_PYTHON_VERSIONS
-                                if version in CONTROLLER_PYTHON_VERSIONS or version in self.modules.python_versions]
+    # True if Python 2.x is supported.
+    py2_support = any(version for version in python_versions if str_to_version(version)[0] == 2)
 
-        # True if Python 2.x is supported.
-        self.py2_support = any(version for version in self.python_versions if str_to_version(version)[0] == 2)
+    return ContentConfig(
+        modules=modules,
+        python_versions=python_versions,
+        py2_support=py2_support,
+    )
 
 
 def load_config(path):  # type: (str) -> t.Optional[ContentConfig]
@@ -95,7 +102,7 @@ def load_config(path):  # type: (str) -> t.Optional[ContentConfig]
         return None
 
     try:
-        config = ContentConfig(yaml_value)
+        config = parse_content_config(yaml_value)
     except Exception as ex:  # pylint: disable=broad-except
         display.warning('Ignoring config "%s" due a config parsing error: %s' % (path, ex))
         return None
@@ -105,13 +112,18 @@ def load_config(path):  # type: (str) -> t.Optional[ContentConfig]
     return config
 
 
-@cache
-def get_content_config():  # type: () -> ContentConfig
+def get_content_config(args):  # type: (EnvironmentConfig) -> ContentConfig
     """
     Parse and return the content configuration (if any) for the current collection.
     For ansible-core, a default configuration is used.
     Results are cached.
     """
+    if args.host_path:
+        args.content_config = deserialize_content_config(os.path.join(args.host_path, 'config.dat'))
+
+    if args.content_config:
+        return args.content_config
+
     collection_config_path = 'tests/config.yml'
 
     config = None
@@ -120,7 +132,7 @@ def get_content_config():  # type: () -> ContentConfig
         config = load_config(collection_config_path)
 
     if not config:
-        config = ContentConfig(dict(
+        config = parse_content_config(dict(
             modules=dict(
                 python_requires='default',
             ),
@@ -132,20 +144,36 @@ def get_content_config():  # type: () -> ContentConfig
                                'This collection provides the Python requirement: %s' % (
                                    ', '.join(SUPPORTED_PYTHON_VERSIONS), config.modules.python_requires))
 
+    args.content_config = config
+
     return config
 
 
-def parse_python_requires(value):  # type: (t.Any) -> t.List[str]
+def parse_python_requires(value):  # type: (t.Any) -> tuple[str, ...]
     """Parse the given 'python_requires' version specifier and return the matching Python versions."""
     if not isinstance(value, str):
         raise ValueError('python_requires must must be of type `str` not type `%s`' % type(value))
 
+    versions: tuple[str, ...]
+
     if value == 'default':
-        versions = list(SUPPORTED_PYTHON_VERSIONS)
+        versions = SUPPORTED_PYTHON_VERSIONS
     elif value == 'controller':
-        versions = list(CONTROLLER_PYTHON_VERSIONS)
+        versions = CONTROLLER_PYTHON_VERSIONS
     else:
         specifier_set = SpecifierSet(value)
-        versions = [version for version in SUPPORTED_PYTHON_VERSIONS if specifier_set.contains(Version(version))]
+        versions = tuple(version for version in SUPPORTED_PYTHON_VERSIONS if specifier_set.contains(Version(version)))
 
     return versions
+
+
+def serialize_content_config(args: EnvironmentConfig, path: str) -> None:
+    """Serialize the content config to the given path. If the config has not been loaded, an empty config will be serialized."""
+    with open_binary_file(path, 'wb') as config_file:
+        pickle.dump(args.content_config, config_file)
+
+
+def deserialize_content_config(path: str) -> ContentConfig:
+    """Deserialize content config from the path."""
+    with open_binary_file(path) as config_file:
+        return pickle.load(config_file)

test/lib/ansible_test/_internal/delegation.py

@@ -72,6 +72,10 @@ from .provisioning import (
     HostState,
 )
 
+from .content_config import (
+    serialize_content_config,
+)
+
 
 @contextlib.contextmanager
 def delegation_context(args, host_state):  # type: (EnvironmentConfig, HostState) -> t.Iterator[None]
@@ -85,6 +89,7 @@ def delegation_context(args, host_state):  # type: (EnvironmentConfig, HostState
     with tempfile.TemporaryDirectory(prefix='host-', dir=ResultType.TMP.path) as host_dir:
         args.host_settings.serialize(os.path.join(host_dir, 'settings.dat'))
         host_state.serialize(os.path.join(host_dir, 'state.dat'))
+        serialize_content_config(args, os.path.join(host_dir, 'config.dat'))
 
         args.host_path = os.path.join(ResultType.TMP.relative_path, os.path.basename(host_dir))