From 39b3fc0926645b26d60588af6a068c7570b778cf Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Thu, 8 Aug 2019 17:21:38 -0700 Subject: [PATCH] Update ansible-test layout and payload handling. --- .../dev_guide/testing/sanity/bin-symlinks.rst | 7 ++ .../ansible_test/_internal/ansible_util.py | 7 +- test/lib/ansible_test/_internal/data.py | 76 +++++++++------ test/lib/ansible_test/_internal/delegation.py | 14 +-- test/lib/ansible_test/_internal/payload.py | 85 +++++++++++++--- .../_internal/provider/layout/__init__.py | 4 - .../_internal/provider/source/installed.py | 41 ++++++++ .../_internal/provider/source/unversioned.py | 1 + .../_internal/sanity/bin_symlinks.py | 97 +++++++++++++++++++ test/lib/ansible_test/_internal/util.py | 2 + 10 files changed, 272 insertions(+), 62 deletions(-) create mode 100644 docs/docsite/rst/dev_guide/testing/sanity/bin-symlinks.rst create mode 100644 test/lib/ansible_test/_internal/provider/source/installed.py create mode 100644 test/lib/ansible_test/_internal/sanity/bin_symlinks.py diff --git a/docs/docsite/rst/dev_guide/testing/sanity/bin-symlinks.rst b/docs/docsite/rst/dev_guide/testing/sanity/bin-symlinks.rst new file mode 100644 index 00000000000..fd63318ac20 --- /dev/null +++ b/docs/docsite/rst/dev_guide/testing/sanity/bin-symlinks.rst @@ -0,0 +1,7 @@ +bin-symlinks +============ + +The ``bin/`` directory in Ansible must contain only symbolic links to executable files. +These files must reside in the ``lib/ansible/`` or ``test/lib/ansible_test/`` directories. + +This is required to allow ``ansible-test`` to work with containers and remote hosts when running from an installed version of Ansible. diff --git a/test/lib/ansible_test/_internal/ansible_util.py b/test/lib/ansible_test/_internal/ansible_util.py index 75be193152c..c3b10becb9e 100644 --- a/test/lib/ansible_test/_internal/ansible_util.py +++ b/test/lib/ansible_test/_internal/ansible_util.py @@ -17,6 +17,7 @@ from .util import ( ANSIBLE_ROOT, ANSIBLE_LIB_ROOT, ANSIBLE_TEST_DATA_ROOT, + ANSIBLE_BIN_PATH, ) from .util_common import ( @@ -45,10 +46,8 @@ def ansible_environment(args, color=True, ansible_config=None): env = common_environment() path = env['PATH'] - ansible_path = os.path.join(ANSIBLE_ROOT, 'bin') - - if not path.startswith(ansible_path + os.path.pathsep): - path = ansible_path + os.path.pathsep + path + if not path.startswith(ANSIBLE_BIN_PATH + os.path.pathsep): + path = ANSIBLE_BIN_PATH + os.path.pathsep + path if ansible_config: pass diff --git a/test/lib/ansible_test/_internal/data.py b/test/lib/ansible_test/_internal/data.py index 0b777e4f854..6beb8f63569 100644 --- a/test/lib/ansible_test/_internal/data.py +++ b/test/lib/ansible_test/_internal/data.py @@ -12,6 +12,8 @@ from .util import ( ANSIBLE_ROOT, is_subdir, ANSIBLE_IS_INSTALLED, + ANSIBLE_LIB_ROOT, + ANSIBLE_TEST_ROOT, ) from .provider import ( @@ -28,9 +30,12 @@ from .provider.source.unversioned import ( UnversionedSource, ) +from .provider.source.installed import ( + InstalledSource, +) + from .provider.layout import ( ContentLayout, - InstallLayout, LayoutProvider, ) @@ -50,33 +55,29 @@ class DataContext: content_path = os.environ.get('ANSIBLE_TEST_CONTENT_ROOT') current_path = os.getcwd() - self.__layout_providers = get_path_provider_classes(LayoutProvider) - self.__source_providers = get_path_provider_classes(SourceProvider) + layout_providers = get_path_provider_classes(LayoutProvider) + source_providers = get_path_provider_classes(SourceProvider) + + self.__source_providers = source_providers + self.__ansible_source = None # type: t.Optional[t.Tuple[t.Tuple[str, str], ...]] + self.payload_callbacks = [] # type: t.List[t.Callable[t.List[t.Tuple[str, str]], None]] if content_path: - content = self.create_content_layout(self.__layout_providers, self.__source_providers, content_path, False) - - if content.is_ansible: - install = InstallLayout(ANSIBLE_ROOT, content.all_files()) - else: - install = None + content = self.__create_content_layout(layout_providers, source_providers, content_path, False) elif is_subdir(current_path, ANSIBLE_ROOT): - content = self.create_content_layout(self.__layout_providers, self.__source_providers, ANSIBLE_ROOT, False) - install = InstallLayout(ANSIBLE_ROOT, content.all_files()) + content = self.__create_content_layout(layout_providers, source_providers, ANSIBLE_ROOT, False) else: - content = self.create_content_layout(self.__layout_providers, self.__source_providers, current_path, True) - install = None + content = self.__create_content_layout(layout_providers, source_providers, current_path, True) - self.__install = install # type: t.Optional[InstallLayout] self.content = content # type: ContentLayout @staticmethod - def create_content_layout(layout_providers, # type: t.List[t.Type[LayoutProvider]] - source_providers, # type: t.List[t.Type[SourceProvider]] - root, # type: str - walk, # type: bool - ): # type: (...) -> ContentLayout + def __create_content_layout(layout_providers, # type: t.List[t.Type[LayoutProvider]] + source_providers, # type: t.List[t.Type[SourceProvider]] + root, # type: str + walk, # type: bool + ): # type: (...) -> ContentLayout """Create a content layout using the given providers and root path.""" layout_provider = find_path_provider(LayoutProvider, layout_providers, root, walk) @@ -92,25 +93,38 @@ class DataContext: return layout - @staticmethod - def create_install_layout(source_providers): # type: (t.List[t.Type[SourceProvider]]) -> InstallLayout - """Create an install layout using the given source provider.""" + def __create_ansible_source(self): + """Return a tuple of Ansible source files with both absolute and relative paths.""" + if ANSIBLE_IS_INSTALLED: + sources = [] + + source_provider = InstalledSource(ANSIBLE_LIB_ROOT) + sources.extend((os.path.join(source_provider.root, path), os.path.join('lib', 'ansible', path)) + for path in source_provider.get_paths(source_provider.root)) + + source_provider = InstalledSource(ANSIBLE_TEST_ROOT) + sources.extend((os.path.join(source_provider.root, path), os.path.join('test', 'lib', 'ansible_test', path)) + for path in source_provider.get_paths(source_provider.root)) + + return tuple(sources) + + if self.content.is_ansible: + return tuple((os.path.join(self.content.root, path), path) for path in self.content.all_files()) + try: - source_provider = find_path_provider(SourceProvider, source_providers, ANSIBLE_ROOT, False) + source_provider = find_path_provider(SourceProvider, self.__source_providers, ANSIBLE_ROOT, False) except ProviderNotFoundForPath: source_provider = UnversionedSource(ANSIBLE_ROOT) - paths = source_provider.get_paths(ANSIBLE_ROOT) - - return InstallLayout(ANSIBLE_ROOT, paths) + return tuple((os.path.join(source_provider.root, path), path) for path in source_provider.get_paths(source_provider.root)) @property - def install(self): # type: () -> InstallLayout - """Return the install context, loaded on demand.""" - if not self.__install: - self.__install = self.create_install_layout(self.__source_providers) + def ansible_source(self): # type: () -> t.Tuple[t.Tuple[str, str], ...] + """Return a tuple of Ansible source files with both absolute and relative paths.""" + if not self.__ansible_source: + self.__ansible_source = self.__create_ansible_source() - return self.__install + return self.__ansible_source def register_payload_callback(self, callback): # type: (t.Callable[t.List[t.Tuple[str, str]], None]) -> None """Register the given payload callback.""" diff --git a/test/lib/ansible_test/_internal/delegation.py b/test/lib/ansible_test/_internal/delegation.py index 35e09fa73aa..7fdee7c112c 100644 --- a/test/lib/ansible_test/_internal/delegation.py +++ b/test/lib/ansible_test/_internal/delegation.py @@ -42,7 +42,7 @@ from .util import ( common_environment, pass_vars, display, - ANSIBLE_ROOT, + ANSIBLE_BIN_PATH, ANSIBLE_TEST_DATA_ROOT, ) @@ -170,7 +170,7 @@ def delegate_tox(args, exclude, require, integration_targets): tox.append('--') - cmd = generate_command(args, None, ANSIBLE_ROOT, data_context().content.root, options, exclude, require) + cmd = generate_command(args, None, ANSIBLE_BIN_PATH, data_context().content.root, options, exclude, require) if not args.python: cmd += ['--python', version] @@ -239,7 +239,7 @@ def delegate_docker(args, exclude, require, integration_targets): else: content_root = install_root - cmd = generate_command(args, python_interpreter, install_root, content_root, options, exclude, require) + cmd = generate_command(args, python_interpreter, os.path.join(install_root, 'bin'), content_root, options, exclude, require) if isinstance(args, TestConfig): if args.coverage and not args.coverage_label: @@ -431,7 +431,7 @@ def delegate_remote(args, exclude, require, integration_targets): else: content_root = install_root - cmd = generate_command(args, python_interpreter, install_root, content_root, options, exclude, require) + cmd = generate_command(args, python_interpreter, os.path.join(install_root, 'bin'), content_root, options, exclude, require) if httptester_id: cmd += ['--inject-httptester'] @@ -478,11 +478,11 @@ def delegate_remote(args, exclude, require, integration_targets): docker_rm(args, httptester_id) -def generate_command(args, python_interpreter, install_root, content_root, options, exclude, require): +def generate_command(args, python_interpreter, ansible_bin_path, content_root, options, exclude, require): """ :type args: EnvironmentConfig :type python_interpreter: str | None - :type install_root: str + :type ansible_bin_path: str :type content_root: str :type options: dict[str, int] :type exclude: list[str] @@ -491,7 +491,7 @@ def generate_command(args, python_interpreter, install_root, content_root, optio """ options['--color'] = 1 - cmd = [os.path.join(install_root, 'bin/ansible-test')] + cmd = [os.path.join(ansible_bin_path, 'ansible-test')] if python_interpreter: cmd = [python_interpreter] + cmd diff --git a/test/lib/ansible_test/_internal/payload.py b/test/lib/ansible_test/_internal/payload.py index f3c59ec30a7..db1df19031a 100644 --- a/test/lib/ansible_test/_internal/payload.py +++ b/test/lib/ansible_test/_internal/payload.py @@ -2,10 +2,14 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type +import atexit import os import tarfile +import tempfile import time +from . import types as t + from .config import ( IntegrationConfig, ShellConfig, @@ -14,6 +18,9 @@ from .config import ( from .util import ( display, ANSIBLE_ROOT, + ANSIBLE_IS_INSTALLED, + remove_tree, + is_subdir, ) from .data import ( @@ -28,38 +35,69 @@ from .util_common import ( tarfile.pwd = None tarfile.grp = None +# this bin symlink map must exactly match the contents of the bin directory +# it is necessary for payload creation to reconstruct the bin directory when running ansible-test from an installed version of ansible +ANSIBLE_BIN_SYMLINK_MAP = { + 'ansible': '../lib/ansible/cli/scripts/ansible_cli_stub.py', + 'ansible-config': 'ansible', + 'ansible-connection': '../lib/ansible/cli/scripts/ansible_connection_cli_stub.py', + 'ansible-console': 'ansible', + 'ansible-doc': 'ansible', + 'ansible-galaxy': 'ansible', + 'ansible-inventory': 'ansible', + 'ansible-playbook': 'ansible', + 'ansible-pull': 'ansible', + 'ansible-test': '../test/lib/ansible_test/_data/cli/ansible_test_cli_stub.py', + 'ansible-vault': 'ansible', +} + def create_payload(args, dst_path): # type: (CommonConfig, str) -> None """Create a payload for delegation.""" if args.explain: return - files = [(os.path.join(ANSIBLE_ROOT, path), path) for path in data_context().install.all_files()] + files = list(data_context().ansible_source) + + if ANSIBLE_IS_INSTALLED: + # reconstruct the bin directory which is not available when running from an ansible install + files.extend(create_temporary_bin_files(args)) if not data_context().content.is_ansible: + # exclude unnecessary files when not testing ansible itself files = [f for f in files if - f[1].startswith('bin/') or - f[1].startswith('lib/') or - f[1].startswith('test/lib/') or - f[1] in ( - 'test/integration/integration.cfg', - 'test/integration/integration_config.yml', - 'test/integration/inventory', - 'test/integration/network-integration.cfg', - 'test/integration/target-prefixes.network', - 'test/integration/windows-integration.cfg', - )] + is_subdir(f[1], 'bin/') or + is_subdir(f[1], 'lib/ansible/') or + (is_subdir(f[1], 'test/lib/ansible_test/') and not is_subdir(f[1], 'test/lib/ansible_test/tests/'))] if not isinstance(args, (ShellConfig, IntegrationConfig)): - files = [f for f in files if not f[1].startswith('lib/ansible/modules/') or f[1] == 'lib/ansible/modules/__init__.py'] + # exclude built-in ansible modules when they are not needed + files = [f for f in files if not is_subdir(f[1], 'lib/ansible/modules/') or f[1] == 'lib/ansible/modules/__init__.py'] - if data_context().content.collection: - files.extend((os.path.join(data_context().content.root, path), os.path.join(data_context().content.collection.directory, path)) - for path in data_context().content.all_files()) + if data_context().content.collection: + # include collections content for testing + files.extend((os.path.join(data_context().content.root, path), os.path.join(data_context().content.collection.directory, path)) + for path in data_context().content.all_files()) + + # these files need to be migrated to the ansible-test data directory + hack_files_to_keep = ( + 'test/integration/integration.cfg', + 'test/integration/integration_config.yml', + 'test/integration/inventory', + 'test/integration/network-integration.cfg', + 'test/integration/target-prefixes.network', + 'test/integration/windows-integration.cfg', + ) + + # temporary solution to include files not yet present in the ansible-test data directory + files.extend([(os.path.join(ANSIBLE_ROOT, path), path) for path in hack_files_to_keep]) for callback in data_context().payload_callbacks: callback(files) + # maintain predictable file order + files = sorted(files) + display.info('Creating a payload archive containing %d files...' % len(files), verbosity=1) start = time.time() @@ -73,3 +111,18 @@ def create_payload(args, dst_path): # type: (CommonConfig, str) -> None payload_size_bytes = os.path.getsize(dst_path) display.info('Created a %d byte payload archive containing %d files in %d seconds.' % (payload_size_bytes, len(files), duration), verbosity=1) + + +def create_temporary_bin_files(args): # type: (CommonConfig) -> t.Tuple[t.Tuple[str, str], ...] + """Create a temporary ansible bin directory populated using the symlink map.""" + if args.explain: + temp_path = '/tmp/ansible-tmp-bin' + else: + temp_path = tempfile.mkdtemp(prefix='ansible', suffix='bin') + atexit.register(remove_tree, temp_path) + + for name, dest in ANSIBLE_BIN_SYMLINK_MAP.items(): + path = os.path.join(temp_path, name) + os.link(dest, path) + + return tuple((os.path.join(temp_path, name), os.path.join('bin', name)) for name in sorted(ANSIBLE_BIN_SYMLINK_MAP)) diff --git a/test/lib/ansible_test/_internal/provider/layout/__init__.py b/test/lib/ansible_test/_internal/provider/layout/__init__.py index 3daf28e4cce..fae057bf764 100644 --- a/test/lib/ansible_test/_internal/provider/layout/__init__.py +++ b/test/lib/ansible_test/_internal/provider/layout/__init__.py @@ -64,10 +64,6 @@ class Layout: return item[1] if item else [] -class InstallLayout(Layout): - """Information about the current Ansible install.""" - - class ContentLayout(Layout): """Information about the current Ansible content being tested.""" def __init__(self, diff --git a/test/lib/ansible_test/_internal/provider/source/installed.py b/test/lib/ansible_test/_internal/provider/source/installed.py new file mode 100644 index 00000000000..0823a6ee6f7 --- /dev/null +++ b/test/lib/ansible_test/_internal/provider/source/installed.py @@ -0,0 +1,41 @@ +"""Source provider for content which has been installed.""" +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +import os + +from ... import types as t + +from . import ( + SourceProvider, +) + + +class InstalledSource(SourceProvider): + """Source provider for content which has been installed.""" + sequence = 0 # disable automatic detection + + @staticmethod + def is_content_root(path): # type: (str) -> bool + """Return True if the given path is a content root for this provider.""" + return False + + def get_paths(self, path): # type: (str) -> t.List[str] + """Return the list of available content paths under the given path.""" + paths = [] + + kill_extensions = ( + '.pyc', + '.pyo', + ) + + for root, _dummy, file_names in os.walk(path): + rel_root = os.path.relpath(root, path) + + if rel_root == '.': + rel_root = '' + + paths.extend([os.path.join(rel_root, file_name) for file_name in file_names + if not os.path.splitext(file_name)[1] in kill_extensions]) + + return paths diff --git a/test/lib/ansible_test/_internal/provider/source/unversioned.py b/test/lib/ansible_test/_internal/provider/source/unversioned.py index 2975af627ee..ade6e01be3e 100644 --- a/test/lib/ansible_test/_internal/provider/source/unversioned.py +++ b/test/lib/ansible_test/_internal/provider/source/unversioned.py @@ -56,6 +56,7 @@ class UnversionedSource(SourceProvider): kill_extensions = ( '.pyc', + '.pyo', '.retry', ) diff --git a/test/lib/ansible_test/_internal/sanity/bin_symlinks.py b/test/lib/ansible_test/_internal/sanity/bin_symlinks.py new file mode 100644 index 00000000000..54fe515810d --- /dev/null +++ b/test/lib/ansible_test/_internal/sanity/bin_symlinks.py @@ -0,0 +1,97 @@ +"""Sanity test for symlinks in the bin directory.""" +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +import os + +from .. import types as t + +from ..sanity import ( + SanityVersionNeutral, + SanityMessage, + SanityFailure, + SanitySuccess, +) + +from ..config import ( + SanityConfig, +) + +from ..data import ( + data_context, +) + +from ..payload import ( + ANSIBLE_BIN_SYMLINK_MAP, + __file__ as symlink_map_full_path, +) + + +class BinSymlinksTest(SanityVersionNeutral): + """Sanity test for symlinks in the bin directory.""" + ansible_only = True + + @property + def can_ignore(self): # type: () -> bool + """True if the test supports ignore entries.""" + return False + + @property + def no_targets(self): # type: () -> bool + """True if the test does not use test targets. Mutually exclusive with all_targets.""" + return True + + # noinspection PyUnusedLocal + def test(self, args, targets): # pylint: disable=locally-disabled, unused-argument + """ + :type args: SanityConfig + :type targets: SanityTargets + :rtype: TestResult + """ + bin_root = os.path.join(data_context().content.root, 'bin') + bin_names = os.listdir(bin_root) + bin_paths = sorted(os.path.join(bin_root, path) for path in bin_names) + + errors = [] # type: t.List[t.Tuple[str, str]] + + symlink_map_path = os.path.relpath(symlink_map_full_path, data_context().content.root) + + for bin_path in bin_paths: + if not os.path.islink(bin_path): + errors.append((bin_path, 'not a symbolic link')) + continue + + dest = os.readlink(bin_path) + + if not os.path.exists(bin_path): + errors.append((bin_path, 'points to non-existent path "%s"' % dest)) + continue + + if not os.path.isfile(bin_path): + errors.append((bin_path, 'points to non-file "%s"' % dest)) + continue + + map_dest = ANSIBLE_BIN_SYMLINK_MAP.get(os.path.basename(bin_path)) + + if not map_dest: + errors.append((bin_path, 'missing from ANSIBLE_BIN_SYMLINK_MAP in file "%s"' % symlink_map_path)) + continue + + if dest != map_dest: + errors.append((bin_path, 'points to "%s" instead of "%s" from ANSIBLE_BIN_SYMLINK_MAP in file "%s"' % (dest, map_dest, symlink_map_path))) + continue + + if not os.access(bin_path, os.X_OK): + errors.append((bin_path, 'points to non-executable file "%s"' % dest)) + continue + + for bin_path, dest in ANSIBLE_BIN_SYMLINK_MAP.items(): + if bin_path not in bin_names: + errors.append((bin_path, 'missing symlink to "%s" defined in ANSIBLE_BIN_SYMLINK_MAP in file "%s"' % (dest, symlink_map_path))) + + messages = [SanityMessage(message=message, path=os.path.relpath(path, data_context().content.root), confidence=100) for path, message in errors] + + if errors: + return SanityFailure(self.name, messages=messages) + + return SanitySuccess(self.name) diff --git a/test/lib/ansible_test/_internal/util.py b/test/lib/ansible_test/_internal/util.py index ccad8e20a0f..b13c39034d0 100644 --- a/test/lib/ansible_test/_internal/util.py +++ b/test/lib/ansible_test/_internal/util.py @@ -66,12 +66,14 @@ ANSIBLE_TEST_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) # assume running from install ANSIBLE_ROOT = os.path.dirname(ANSIBLE_TEST_ROOT) +ANSIBLE_BIN_PATH = os.path.dirname(os.path.abspath(sys.argv[0])) ANSIBLE_LIB_ROOT = os.path.join(ANSIBLE_ROOT, 'ansible') ANSIBLE_IS_INSTALLED = True if not os.path.exists(ANSIBLE_LIB_ROOT): # running from source ANSIBLE_ROOT = os.path.dirname(os.path.dirname(os.path.dirname(ANSIBLE_TEST_ROOT))) + ANSIBLE_BIN_PATH = os.path.join(ANSIBLE_ROOT, 'bin') ANSIBLE_LIB_ROOT = os.path.join(ANSIBLE_ROOT, 'lib', 'ansible') ANSIBLE_IS_INSTALLED = False