diff options
| author | Matt Clay <matt@mystile.com> | 2022-06-02 16:39:52 -0700 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2022-06-02 16:39:52 -0700 |
| commit | ae380e3bef989cfce16ce3ab70d7b5fc28fb6c86 (patch) | |
| tree | f3dfe5f6fdef020093d07529449af08037eb57e6 /test | |
| parent | f3b56ec661bab9b205de43c5a942432e69a19fbc (diff) | |
| download | ansible-ae380e3bef989cfce16ce3ab70d7b5fc28fb6c86.tar.gz | |
[stable-2.13] ansible-test - Multiple backports (#77951)
* ansible-test - Backport `InternalError`
NOTE: This is a partial backport, including only one new class.
(cherry picked from commit b9606417598217106e394c12c776d8c5ede9cd98)
* ansible-test - Fix subprocess management. (#77641)
* Run code-smell sanity tests in UTF-8 Mode.
* Update subprocess use in sanity test programs.
* Use raw_command instead of run_command with always=True set.
* Add more capture=True usage.
* Don't expose stdin to subprocesses.
* Capture more output. Warn on retry.
* Add more captures.
* Capture coverage cli output.
* Capture windows and network host checks.
* Be explicit about interactive usage.
* Use a shell for non-captured, non-interactive subprocesses.
* Add integration test to assert no TTY.
* Add unit test to assert no TTY.
* Require blocking stdin/stdout/stderr.
* Use subprocess.run in ansible-core sanity tests.
* Remove unused arg.
* Be explicit with subprocess.run check=False.
* Add changelog.
* Use a Python subprocess instead of a shell.
* Use InternalError instead of Exception.
* Require capture argument.
* Check for invalid raw_command arguments.
* Removed pointless communicate=True usage.
* Relocate stdout w/o capture check.
* Use threads instead of a subprocess for IO.
(cherry picked from commit 5c2d830dea986a8c7bd8c286b86bdce326cd7eb1)
* ansible-test - Add support for remote Ubuntu VMs.
(cherry picked from commit 6513453310cdfc42c44a4b879535a8f0795c0295)
* ansible-test - Fix remote completion validation.
(cherry picked from commit e2200e8dfc5b2a51db4f77800090fe859811f80b)
* ansible-test - Add multi-arch remote support.
(cherry picked from commit 2cc74b04c49338b48af070ddd811b25b5d801c12)
* ansible-test - Enhance the shell command. (#77734)
* ansible-test - Add shell --export option.
* ansible-test - Support cmd args for shell command.
Also allow shell to be used without a valid layout if no delegation is required.
* ansible-test - Improve stderr/stdout consistency.
By default all output goes to stdout only, with the exception of a fatal error.
When using any of the following, all output defaults to stderr instead:
* sanity with the `--lint` option -- sanity messages to stdout
* coverage analyze -- output to stdout if the output file is `/dev/stdout`
* shell -- shell output to stdout
This fixes issues two main issues:
* Unpredictable output order when using both info and error/warning messages.
* Mixing of lint/command/shell output with bootstrapping messages on stdout.
* ansible-test - Add changelog fragment.
(cherry picked from commit fe349a1ccd658d86cfcf10eecdce9d48ece6176c)
* ansible-test - Fix remote args restriction.
The platform-specific and global fallbacks were not working with the `--remote` option.
This regression was introduced by https://github.com/ansible/ansible/pull/77711
(cherry picked from commit 76ead1e7680a0341261d45e17aeb57774935e5b4)
Diffstat (limited to 'test')
52 files changed, 713 insertions, 232 deletions
diff --git a/test/integration/targets/ansible-test-no-tty/aliases b/test/integration/targets/ansible-test-no-tty/aliases new file mode 100644 index 0000000000..0ac86c9200 --- /dev/null +++ b/test/integration/targets/ansible-test-no-tty/aliases @@ -0,0 +1,2 @@ +context/controller +shippable/posix/group1 diff --git a/test/integration/targets/ansible-test-no-tty/runme.py b/test/integration/targets/ansible-test-no-tty/runme.py new file mode 100755 index 0000000000..c8c5cfccce --- /dev/null +++ b/test/integration/targets/ansible-test-no-tty/runme.py @@ -0,0 +1,7 @@ +#!/usr/bin/env python + +import sys + +assert not sys.stdin.isatty() +assert not sys.stdout.isatty() +assert not sys.stderr.isatty() diff --git a/test/integration/targets/ansible-test-no-tty/runme.sh b/test/integration/targets/ansible-test-no-tty/runme.sh new file mode 100755 index 0000000000..bae5220e2c --- /dev/null +++ b/test/integration/targets/ansible-test-no-tty/runme.sh @@ -0,0 +1,5 @@ +#!/usr/bin/env bash + +set -eux + +./runme.py diff --git a/test/lib/ansible_test/_data/completion/network.txt b/test/lib/ansible_test/_data/completion/network.txt index 8c6243e9a1..1d6b0c196a 100644 --- a/test/lib/ansible_test/_data/completion/network.txt +++ b/test/lib/ansible_test/_data/completion/network.txt @@ -1,2 +1,2 @@ -ios/csr1000v collection=cisco.ios connection=ansible.netcommon.network_cli provider=aws -vyos/1.1.8 collection=vyos.vyos connection=ansible.netcommon.network_cli provider=aws +ios/csr1000v collection=cisco.ios connection=ansible.netcommon.network_cli provider=aws arch=x86_64 +vyos/1.1.8 collection=vyos.vyos connection=ansible.netcommon.network_cli provider=aws arch=x86_64 diff --git a/test/lib/ansible_test/_data/completion/remote.txt b/test/lib/ansible_test/_data/completion/remote.txt index 3009cf045d..c07f2d1dbb 100644 --- a/test/lib/ansible_test/_data/completion/remote.txt +++ b/test/lib/ansible_test/_data/completion/remote.txt @@ -1,9 +1,11 @@ -freebsd/12.3 python=3.8 python_dir=/usr/local/bin provider=aws -freebsd/13.0 python=3.7,2.7,3.8,3.9 python_dir=/usr/local/bin provider=aws -freebsd python_dir=/usr/local/bin provider=aws -macos/12.0 python=3.10 python_dir=/usr/local/bin provider=parallels -macos python_dir=/usr/local/bin provider=parallels -rhel/7.9 python=2.7 provider=aws -rhel/8.5 python=3.6,3.8,3.9 provider=aws -rhel/9.0 python=3.9 provider=aws -rhel provider=aws +freebsd/12.3 python=3.8 python_dir=/usr/local/bin provider=aws arch=x86_64 +freebsd/13.0 python=3.7,2.7,3.8,3.9 python_dir=/usr/local/bin provider=aws arch=x86_64 +freebsd python_dir=/usr/local/bin provider=aws arch=x86_64 +macos/12.0 python=3.10 python_dir=/usr/local/bin provider=parallels arch=x86_64 +macos python_dir=/usr/local/bin provider=parallels arch=x86_64 +rhel/7.9 python=2.7 provider=aws arch=x86_64 +rhel/8.5 python=3.6,3.8,3.9 provider=aws arch=x86_64 +rhel/9.0 python=3.9 provider=aws arch=x86_64 +rhel provider=aws arch=x86_64 +ubuntu/22.04 python=3.10 provider=aws arch=x86_64 +ubuntu provider=aws arch=x86_64 diff --git a/test/lib/ansible_test/_data/completion/windows.txt b/test/lib/ansible_test/_data/completion/windows.txt index 280ad97f13..767c36cbcb 100644 --- a/test/lib/ansible_test/_data/completion/windows.txt +++ b/test/lib/ansible_test/_data/completion/windows.txt @@ -1,6 +1,6 @@ -windows/2012 provider=aws -windows/2012-R2 provider=aws -windows/2016 provider=aws -windows/2019 provider=aws -windows/2022 provider=aws -windows provider=aws +windows/2012 provider=aws arch=x86_64 +windows/2012-R2 provider=aws arch=x86_64 +windows/2016 provider=aws arch=x86_64 +windows/2019 provider=aws arch=x86_64 +windows/2022 provider=aws arch=x86_64 +windows provider=aws arch=x86_64 diff --git a/test/lib/ansible_test/_internal/__init__.py b/test/lib/ansible_test/_internal/__init__.py index e663c45e79..43d10c027d 100644 --- a/test/lib/ansible_test/_internal/__init__.py +++ b/test/lib/ansible_test/_internal/__init__.py @@ -57,7 +57,7 @@ def main(cli_args=None): # type: (t.Optional[t.List[str]]) -> None display.truncate = config.truncate display.redact = config.redact display.color = config.color - display.info_stderr = config.info_stderr + display.fd = sys.stderr if config.display_stderr else sys.stdout configure_timeout(config) display.info('RLIMIT_NOFILE: %s' % (CURRENT_RLIMIT_NOFILE,), verbosity=2) @@ -66,7 +66,9 @@ def main(cli_args=None): # type: (t.Optional[t.List[str]]) -> None target_names = None try: - data_context().check_layout() + if config.check_layout: + data_context().check_layout() + args.func(config) except PrimeContainers: pass @@ -82,7 +84,7 @@ def main(cli_args=None): # type: (t.Optional[t.List[str]]) -> None if target_names: for target_name in target_names: - print(target_name) # info goes to stderr, this should be on stdout + print(target_name) # display goes to stderr, this should be on stdout display.review_warnings() config.success = True @@ -90,7 +92,7 @@ def main(cli_args=None): # type: (t.Optional[t.List[str]]) -> None display.warning(u'%s' % ex) sys.exit(0) except ApplicationError as ex: - display.error(u'%s' % ex) + display.fatal(u'%s' % ex) sys.exit(1) except KeyboardInterrupt: sys.exit(2) diff --git a/test/lib/ansible_test/_internal/ansible_util.py b/test/lib/ansible_test/_internal/ansible_util.py index a3582dc89f..0fe3db26ae 100644 --- a/test/lib/ansible_test/_internal/ansible_util.py +++ b/test/lib/ansible_test/_internal/ansible_util.py @@ -22,11 +22,11 @@ from .util import ( ANSIBLE_SOURCE_ROOT, ANSIBLE_TEST_TOOLS_ROOT, get_ansible_version, + raw_command, ) from .util_common import ( create_temp_dir, - run_command, ResultType, intercept_python, get_injector_path, @@ -258,12 +258,12 @@ class CollectionDetailError(ApplicationError): self.reason = reason -def get_collection_detail(args, python): # type: (EnvironmentConfig, PythonConfig) -> CollectionDetail +def get_collection_detail(python): # type: (PythonConfig) -> CollectionDetail """Return collection detail.""" collection = data_context().content.collection directory = os.path.join(collection.root, collection.directory) - stdout = run_command(args, [python.path, os.path.join(ANSIBLE_TEST_TOOLS_ROOT, 'collection_detail.py'), directory], capture=True, always=True)[0] + stdout = raw_command([python.path, os.path.join(ANSIBLE_TEST_TOOLS_ROOT, 'collection_detail.py'), directory], capture=True)[0] result = json.loads(stdout) error = result.get('error') @@ -282,15 +282,15 @@ def run_playbook( args, # type: EnvironmentConfig inventory_path, # type: str playbook, # type: str - run_playbook_vars=None, # type: t.Optional[t.Dict[str, t.Any]] - capture=False, # type: bool + capture, # type: bool + variables=None, # type: t.Optional[t.Dict[str, t.Any]] ): # type: (...) -> None """Run the specified playbook using the given inventory file and playbook variables.""" playbook_path = os.path.join(ANSIBLE_TEST_DATA_ROOT, 'playbooks', playbook) cmd = ['ansible-playbook', '-i', inventory_path, playbook_path] - if run_playbook_vars: - cmd.extend(['-e', json.dumps(run_playbook_vars)]) + if variables: + cmd.extend(['-e', json.dumps(variables)]) if args.verbosity: cmd.append('-%s' % ('v' * args.verbosity)) diff --git a/test/lib/ansible_test/_internal/cli/commands/shell.py b/test/lib/ansible_test/_internal/cli/commands/shell.py index 301ff70e90..7d52b39e05 100644 --- a/test/lib/ansible_test/_internal/cli/commands/shell.py +++ b/test/lib/ansible_test/_internal/cli/commands/shell.py @@ -39,9 +39,21 @@ def do_shell( shell = parser.add_argument_group(title='shell arguments') shell.add_argument( + 'cmd', + nargs='*', + help='run the specified command', + ) + + shell.add_argument( '--raw', action='store_true', help='direct to shell with no setup', ) + shell.add_argument( + '--export', + metavar='PATH', + help='export inventory instead of opening a shell', + ) + add_environments(parser, completer, ControllerMode.DELEGATED, TargetMode.SHELL) # shell diff --git a/test/lib/ansible_test/_internal/cli/compat.py b/test/lib/ansible_test/_internal/cli/compat.py index 21bc904561..0a23c2306f 100644 --- a/test/lib/ansible_test/_internal/cli/compat.py +++ b/test/lib/ansible_test/_internal/cli/compat.py @@ -115,6 +115,7 @@ class LegacyHostOptions: venv_system_site_packages: t.Optional[bool] = None remote: t.Optional[str] = None remote_provider: t.Optional[str] = None + remote_arch: t.Optional[str] = None docker: t.Optional[str] = None docker_privileged: t.Optional[bool] = None docker_seccomp: t.Optional[str] = None @@ -374,33 +375,34 @@ def get_legacy_host_config( if remote_config.controller_supported: if controller_python(options.python) or not options.python: - controller = PosixRemoteConfig(name=options.remote, python=native_python(options), provider=options.remote_provider) + controller = PosixRemoteConfig(name=options.remote, python=native_python(options), provider=options.remote_provider, + arch=options.remote_arch) targets = controller_targets(mode, options, controller) else: controller_fallback = f'remote:{options.remote}', f'--remote {options.remote} --python {options.python}', FallbackReason.PYTHON - controller = PosixRemoteConfig(name=options.remote, provider=options.remote_provider) + controller = PosixRemoteConfig(name=options.remote, provider=options.remote_provider, arch=options.remote_arch) targets = controller_targets(mode, options, controller) else: context, reason = f'--remote {options.remote}', FallbackReason.ENVIRONMENT controller = None - targets = [PosixRemoteConfig(name=options.remote, python=native_python(options), provider=options.remote_provider)] + targets = [PosixRemoteConfig(name=options.remote, python=native_python(options), provider=options.remote_provider, arch=options.remote_arch)] elif mode == TargetMode.SHELL and options.remote.startswith('windows/'): if options.python and options.python not in CONTROLLER_PYTHON_VERSIONS: raise ControllerNotSupportedError(f'--python {options.python}') controller = OriginConfig(python=native_python(options)) - targets = [WindowsRemoteConfig(name=options.remote, provider=options.remote_provider)] + targets = [WindowsRemoteConfig(name=options.remote, provider=options.remote_provider, arch=options.remote_arch)] else: if not options.python: raise PythonVersionUnspecifiedError(f'--remote {options.remote}') if controller_python(options.python): - controller = PosixRemoteConfig(name=options.remote, python=native_python(options), provider=options.remote_provider) + controller = PosixRemoteConfig(name=options.remote, python=native_python(options), provider=options.remote_provider, arch=options.remote_arch) targets = controller_targets(mode, options, controller) else: context, reason = f'--remote {options.remote} --python {options.python}', FallbackReason.PYTHON controller = None - targets = [PosixRemoteConfig(name=options.remote, python=native_python(options), provider=options.remote_provider)] + targets = [PosixRemoteConfig(name=options.remote, python=native_python(options), provider=options.remote_provider, arch=options.remote_arch)] if not controller: if docker_available(): @@ -458,12 +460,13 @@ def handle_non_posix_targets( """Return a list of non-POSIX targets if the target mode is non-POSIX.""" if mode == TargetMode.WINDOWS_INTEGRATION: if options.windows: - targets = [WindowsRemoteConfig(name=f'windows/{version}', provider=options.remote_provider) for version in options.windows] + targets = [WindowsRemoteConfig(name=f'windows/{version}', provider=options.remote_provider, arch=options.remote_arch) + for version in options.windows] else: targets = [WindowsInventoryConfig(path=options.inventory)] elif mode == TargetMode.NETWORK_INTEGRATION: if options.platform: - network_targets = [NetworkRemoteConfig(name=platform, provider=options.remote_provider) for platform in options.platform] + network_targets = [NetworkRemoteConfig(name=platform, provider=options.remote_provider, arch=options.remote_arch) for platform in options.platform] for platform, collection in options.platform_collection or []: for entry in network_targets: diff --git a/test/lib/ansible_test/_internal/cli/environments.py b/test/lib/ansible_test/_internal/cli/environments.py index 5709c7c1ec..e3e759fda5 100644 --- a/test/lib/ansible_test/_internal/cli/environments.py +++ b/test/lib/ansible_test/_internal/cli/environments.py @@ -13,6 +13,10 @@ from ..constants import ( SUPPORTED_PYTHON_VERSIONS, ) +from ..util import ( + REMOTE_ARCHITECTURES, +) + from ..completion import ( docker_completion, network_completion, @@ -532,6 +536,13 @@ def add_environment_remote( help=suppress or 'remote provider to use: %(choices)s', ) + environments_parser.add_argument( + '--remote-arch', + metavar='ARCH', + choices=REMOTE_ARCHITECTURES, + help=suppress or 'remote arch to use: %(choices)s', + ) + def complete_remote_stage(prefix: str, **_) -> t.List[str]: """Return a list of supported stages matching the given prefix.""" diff --git a/test/lib/ansible_test/_internal/cli/parsers/key_value_parsers.py b/test/lib/ansible_test/_internal/cli/parsers/key_value_parsers.py index b22705f731..8f71e7635e 100644 --- a/test/lib/ansible_test/_internal/cli/parsers/key_value_parsers.py +++ b/test/lib/ansible_test/_internal/cli/parsers/key_value_parsers.py @@ -10,6 +10,10 @@ from ...constants import ( SUPPORTED_PYTHON_VERSIONS, ) +from ...util import ( + REMOTE_ARCHITECTURES, +) + from ...host_configs import ( OriginConfig, ) @@ -126,6 +130,7 @@ class PosixRemoteKeyValueParser(KeyValueParser): """Return a dictionary of key names and value parsers.""" return dict( provider=ChoicesParser(REMOTE_PROVIDERS), + arch=ChoicesParser(REMOTE_ARCHITECTURES), python=PythonParser(versions=self.versions, allow_venv=False, allow_default=self.allow_default), ) @@ -137,6 +142,7 @@ class PosixRemoteKeyValueParser(KeyValueParser): state.sections[f'{"controller" if self.controller else "target"} {section_name} (comma separated):'] = '\n'.join([ f' provider={ChoicesParser(REMOTE_PROVIDERS).document(state)}', + f' arch={ChoicesParser(REMOTE_ARCHITECTURES).document(state)}', f' python={python_parser.document(state)}', ]) @@ -149,6 +155,7 @@ class WindowsRemoteKeyValueParser(KeyValueParser): """Return a dictionary of key names and value parsers.""" return dict( provider=ChoicesParser(REMOTE_PROVIDERS), + arch=ChoicesParser(REMOTE_ARCHITECTURES), ) def document(self, state): # type: (DocumentationState) -> t.Optional[str] @@ -157,6 +164,7 @@ class WindowsRemoteKeyValueParser(KeyValueParser): state.sections[f'target {section_name} (comma separated):'] = '\n'.join([ f' provider={ChoicesParser(REMOTE_PROVIDERS).document(state)}', + f' arch={ChoicesParser(REMOTE_ARCHITECTURES).document(state)}', ]) return f'{{{section_name}}}' @@ -168,6 +176,7 @@ class NetworkRemoteKeyValueParser(KeyValueParser): """Return a dictionary of key names and value parsers.""" return dict( provider=ChoicesParser(REMOTE_PROVIDERS), + arch=ChoicesParser(REMOTE_ARCHITECTURES), collection=AnyParser(), connection=AnyParser(), ) @@ -178,7 +187,8 @@ class NetworkRemoteKeyValueParser(KeyValueParser): state.sections[f'target {section_name} (comma separated):'] = '\n'.join([ f' provider={ChoicesParser(REMOTE_PROVIDERS).document(state)}', - ' collection={collecton}', + f' arch={ChoicesParser(REMOTE_ARCHITECTURES).document(state)}', + ' collection={collection}', ' connection={connection}', ]) diff --git a/test/lib/ansible_test/_internal/commands/coverage/__init__.py b/test/lib/ansible_test/_internal/commands/coverage/__init__.py index 1e59ac6fde..88128c46ea 100644 --- a/test/lib/ansible_test/_internal/commands/coverage/__init__.py +++ b/test/lib/ansible_test/_internal/commands/coverage/__init__.py @@ -95,7 +95,16 @@ def run_coverage(args, host_state, output_file, command, cmd): # type: (Coverag cmd = ['python', '-m', 'coverage.__main__', command, '--rcfile', COVERAGE_CONFIG_PATH] + cmd - intercept_python(args, host_state.controller_profile.python, cmd, env) + stdout, stderr = intercept_python(args, host_state.controller_profile.python, cmd, env, capture=True) + + stdout = (stdout or '').strip() + stderr = (stderr or '').strip() + + if stdout: + display.info(stdout) + + if stderr: + display.warning(stderr) def get_all_coverage_files(): # type: () -> t.List[str] diff --git a/test/lib/ansible_test/_internal/commands/coverage/analyze/__init__.py b/test/lib/ansible_test/_internal/commands/coverage/analyze/__init__.py index db169fd7a0..16521bef4f 100644 --- a/test/lib/ansible_test/_internal/commands/coverage/analyze/__init__.py +++ b/test/lib/ansible_test/_internal/commands/coverage/analyze/__init__.py @@ -14,4 +14,4 @@ class CoverageAnalyzeConfig(CoverageConfig): # avoid mixing log messages with file output when using `/dev/stdout` for the output file on commands # this may be worth considering as the default behavior in the future, instead of being dependent on the command or options used - self.info_stderr = True + self.display_stderr = True diff --git a/test/lib/ansible_test/_internal/commands/coverage/analyze/targets/__init__.py b/test/lib/ansible_test/_internal/commands/coverage/analyze/targets/__init__.py index f94b7360ec..412414050f 100644 --- a/test/lib/ansible_test/_internal/commands/coverage/analyze/targets/__init__.py +++ b/test/lib/ansible_test/_internal/commands/coverage/analyze/targets/__init__.py @@ -32,7 +32,7 @@ class CoverageAnalyzeTargetsConfig(CoverageAnalyzeConfig): def __init__(self, args): # type: (t.Any) -> None super().__init__(args) - self.info_stderr = True + self.display_stderr = True def make_report(target_indexes, arcs, lines): # type: (TargetIndexes, Arcs, Lines) -> t.Dict[str, t.Any] diff --git a/test/lib/ansible_test/_internal/commands/coverage/combine.py b/test/lib/ansible_test/_internal/commands/coverage/combine.py index 8cf4c1054b..8458081f05 100644 --- a/test/lib/ansible_test/_internal/commands/coverage/combine.py +++ b/test/lib/ansible_test/_internal/commands/coverage/combine.py @@ -18,11 +18,11 @@ from ...util import ( ANSIBLE_TEST_TOOLS_ROOT, display, ApplicationError, + raw_command, ) from ...util_common import ( ResultType, - run_command, write_json_file, write_json_test_results, ) @@ -194,7 +194,7 @@ def _command_coverage_combine_powershell(args): # type: (CoverageCombineConfig) cmd = ['pwsh', os.path.join(ANSIBLE_TEST_TOOLS_ROOT, 'coverage_stub.ps1')] cmd.extend(source_paths) - stubs = json.loads(run_command(args, cmd, capture=True, always=True)[0]) + stubs = json.loads(raw_command(cmd, capture=True)[0]) return dict((d['Path'], dict((line, 0) for line in d['Lines'])) for d in stubs) diff --git a/test/lib/ansible_test/_internal/commands/integration/__init__.py b/test/lib/ansible_test/_internal/commands/integration/__init__.py index 247bce0820..d5f497b541 100644 --- a/test/lib/ansible_test/_internal/commands/integration/__init__.py +++ b/test/lib/ansible_test/_internal/commands/integration/__init__.py @@ -619,7 +619,7 @@ def command_integration_script( cmd += ['-e', '@%s' % config_path] env.update(coverage_manager.get_environment(target.name, target.aliases)) - cover_python(args, host_state.controller_profile.python, cmd, target.name, env, cwd=cwd) + cover_python(args, host_state.controller_profile.python, cmd, target.name, env, cwd=cwd, capture=False) def command_integration_role( @@ -738,7 +738,7 @@ def command_integration_role( env['ANSIBLE_ROLES_PATH'] = test_env.targets_dir env.update(coverage_manager.get_environment(target.name, target.aliases)) - cover_python(args, host_state.controller_profile.python, cmd, target.name, env, cwd=cwd) + cover_python(args, host_state.controller_profile.python, cmd, target.name, env, cwd=cwd, capture=False) def run_setup_targets( diff --git a/test/lib/ansible_test/_internal/commands/integration/cloud/aws.py b/test/lib/ansible_test/_internal/commands/integration/cloud/aws.py index b2b02095f3..a67a0f89a9 100644 --- a/test/lib/ansible_test/_internal/commands/integration/cloud/aws.py +++ b/test/lib/ansible_test/_internal/commands/integration/cloud/aws.py @@ -21,6 +21,7 @@ from ....target import ( from ....core_ci import ( AnsibleCoreCI, + CloudResource, ) from ....host_configs import ( @@ -91,7 +92,7 @@ class AwsCloudProvider(CloudProvider): def _create_ansible_core_ci(self): # type: () -> AnsibleCoreCI """Return an AWS instance of AnsibleCoreCI.""" - return AnsibleCoreCI(self.args, 'aws', 'aws', 'aws', persist=False) + return AnsibleCoreCI(self.args, CloudResource(platform='aws')) class AwsCloudEnvironment(CloudEnvironment): diff --git a/test/lib/ansible_test/_internal/commands/integration/cloud/azure.py b/test/lib/ansible_test/_internal/commands/integration/cloud/azure.py index cf16c7f54a..f67d1adf25 100644 --- a/test/lib/ansible_test/_internal/commands/integration/cloud/azure.py +++ b/test/lib/ansible_test/_internal/commands/integration/cloud/azure.py @@ -19,6 +19,7 @@ from ....target import ( from ....core_ci import ( AnsibleCoreCI, + CloudResource, ) from . import ( @@ -97,7 +98,7 @@ class AzureCloudProvider(CloudProvider): def _create_ansible_core_ci(self): # type: () -> AnsibleCoreCI """Return an Azure instance of AnsibleCoreCI.""" - return AnsibleCoreCI(self.args, 'azure', 'azure', 'azure', persist=False) + return AnsibleCoreCI(self.args, CloudResource(platform='azure')) class AzureCloudEnvironment(CloudEnvironment): diff --git a/test/lib/ansible_test/_internal/commands/integration/cloud/cs.py b/test/lib/ansible_test/_internal/commands/integration/cloud/cs.py index f20a7d887e..8ffcabfb32 100644 --- a/test/lib/ansible_test/_internal/commands/integration/cloud/cs.py +++ b/test/lib/ansible_test/_internal/commands/integration/cloud/cs.py @@ -106,7 +106,7 @@ class CsCloudProvider(CloudProvider): # apply work-around for OverlayFS issue # https://github.com/docker/for-linux/issues/72#issuecomment-319904698 - docker_exec(self.args, self.DOCKER_SIMULATOR_NAME, ['find', '/var/lib/mysql', '-type', 'f', '-exec', 'touch', '{}', ';']) + docker_exec(self.args, self.DOCKER_SIMULATOR_NAME, ['find', '/var/lib/mysql', '-type', 'f', '-exec', 'touch', '{}', ';'], capture=True) if self.args.explain: values = dict( diff --git a/test/lib/ansible_test/_internal/commands/integration/cloud/hcloud.py b/test/lib/ansible_test/_internal/commands/integration/cloud/hcloud.py index 28b07e7230..6912aff36d 100644 --- a/test/lib/ansible_test/_internal/commands/integration/cloud/hcloud.py +++ b/test/lib/ansible_test/_internal/commands/integration/cloud/hcloud.py @@ -18,6 +18,7 @@ from ....target import ( from ....core_ci import ( AnsibleCoreCI, + CloudResource, ) from . import ( @@ -78,7 +79,7 @@ class HcloudCloudProvider(CloudProvider): def _create_ansible_core_ci(self): # type: () -> AnsibleCoreCI """Return a Heztner instance of AnsibleCoreCI.""" - return AnsibleCoreCI(self.args, 'hetzner', 'hetzner', 'hetzner', persist=False) + return AnsibleCoreCI(self.args, CloudResource(platform='hetzner')) class HcloudCloudEnvironment(CloudEnvironment): diff --git a/test/lib/ansible_test/_internal/commands/integration/coverage.py b/test/lib/ansible_test/_internal/commands/integration/coverage.py index 6b8a0a6ea2..3146181afc 100644 --- a/test/lib/ansible_test/_internal/commands/integration/coverage.py +++ b/test/lib/ansible_test/_internal/commands/integration/coverage.py @@ -118,7 +118,7 @@ class CoverageHandler(t.Generic[THostConfig], metaclass=abc.ABCMeta): def run_playbook(self, playbook, variables): # type: (str, t.Dict[str, str]) -> None """Run the specified playbook using the current inventory.""" self.create_inventory() - run_playbook(self.args, self.inventory_path, playbook, variables) + run_playbook(self.args, self.inventory_path, playbook, capture=False, variables=variables) class PosixCoverageHandler(CoverageHandler[PosixConfig]): diff --git a/test/lib/ansible_test/_internal/commands/integration/filters.py b/test/lib/ansible_test/_internal/commands/integration/filters.py index 0396ce9231..35acae52c8 100644 --- a/test/lib/ansible_test/_internal/commands/integration/filters.py +++ b/test/lib/ansible_test/_internal/commands/integration/filters.py @@ -10,6 +10,7 @@ from ...config import ( from ...util import ( cache, + detect_architecture, display, get_type_map, ) @@ -223,6 +224,14 @@ class NetworkInventoryTargetFilter(TargetFilter[NetworkInventoryConfig]): class OriginTargetFilter(PosixTargetFilter[OriginConfig]): """Target filter for localhost.""" + def filter_targets(self, targets, exclude): # type: (t.List[IntegrationTarget], t.Set[str]) -> None + """Filter the list of targets, adding any which this host profile cannot support to the provided exclude list.""" + super().filter_targets(targets, exclude) + + arch = detect_architecture(self.config.python.path) + + if arch: + self.skip(f'skip/{arch}', f'which are not supported by {arch}', targets, exclude) @cache @@ -247,10 +256,7 @@ def get_target_filter(args, configs, controller): # type: (IntegrationConfig, t def get_remote_skip_aliases(config): # type: (RemoteConfig) -> t.Dict[str, str] """Return a dictionary of skip aliases and the reason why they apply.""" - if isinstance(config, PosixRemoteConfig): - return get_platform_skip_aliases(config.platform, config.version, config.arch) - - return get_platform_skip_aliases(config.platform, config.version, None) + return get_platform_skip_aliases(config.platform, config.version, config.arch) def get_platform_skip_aliases(platform, version, arch): # type: (str, str, t.Optional[str]) -> t.Dict[str, str] diff --git a/test/lib/ansible_test/_internal/commands/sanity/__init__.py b/test/lib/ansible_test/_internal/commands/sanity/__init__.py index d819c37e33..ff3a61699d 100644 --- a/test/lib/ansible_test/_internal/commands/sanity/__init__.py +++ b/test/lib/ansible_test/_internal/commands/sanity/__init__.py @@ -179,7 +179,7 @@ def command_sanity(args): # type: (SanityConfig) -> None for test in tests: if args.list_tests: - display.info(test.name) + print(test.name) # display goes to stderr, this should be on stdout continue for version in SUPPORTED_PYTHON_VERSIONS: @@ -952,6 +952,7 @@ class SanityCodeSmellTest(SanitySingleVersion): cmd = [python.path, self.path] env = ansible_environment(args, color=False) + env.update(PYTHONUTF8='1') # force all code-smell sanity tests to run with Python UTF-8 Mode enabled pattern = None data = None diff --git a/test/lib/ansible_test/_internal/commands/sanity/pylint.py b/test/lib/ansible_test/_internal/commands/sanity/pylint.py index 0e6ace8ea9..370e899843 100644 --- a/test/lib/ansible_test/_internal/commands/sanity/pylint.py +++ b/test/lib/ansible_test/_internal/commands/sanity/pylint.py @@ -141,7 +141,7 @@ class PylintTest(SanitySingleVersion): if data_context().content.collection: try: - collection_detail = get_collection_detail(args, python) + collection_detail = get_collection_detail(python) if not collection_detail.version: display.warning('Skipping pylint collection version checks since no collection version was found.') diff --git a/test/lib/ansible_test/_internal/commands/sanity/validate_modules.py b/test/lib/ansible_test/_internal/commands/sanity/validate_modules.py index e0fbac6439..f1d448804c 100644 --- a/test/lib/ansible_test/_internal/commands/sanity/validate_modules.py +++ b/test/lib/ansible_test/_internal/commands/sanity/validate_modules.py @@ -121,7 +121,7 @@ class ValidateModulesTest(SanitySingleVersion): cmd.extend(['--collection', data_context().content.collection.directory]) try: - collection_detail = get_collection_detail(args, python) + collection_detail = get_collection_detail(python) if collection_detail.version: cmd.extend(['--collection-version', collection_detail.version]) diff --git a/test/lib/ansible_test/_internal/commands/shell/__init__.py b/test/lib/ansible_test/_internal/commands/shell/__init__.py index 4b205171a3..4bd3fde02a 100644 --- a/test/lib/ansible_test/_internal/commands/shell/__init__.py +++ b/test/lib/ansible_test/_internal/commands/shell/__init__.py @@ -2,6 +2,7 @@ from __future__ import annotations import os +import sys import typing as t from ...util import ( @@ -38,12 +39,20 @@ from ...host_configs import ( OriginConfig, ) +from ...inventory import ( + create_controller_inventory, + create_posix_inventory, +) + def command_shell(args): # type: (ShellConfig) -> None """Entry point for the `shell` command.""" if args.raw and isinstance(args.targets[0], ControllerConfig): raise ApplicationError('The --raw option has no effect on the controller.') + if not args.export and not args.cmd and not sys.stdin.isatty(): + raise ApplicationError('Standard input must be a TTY to launch a shell.') + host_state = prepare_profiles(args, skip_setup=args.raw) # shell if args.delegate: @@ -57,10 +66,25 @@ def command_shell(args): # type: (ShellConfig) -> None if isinstance(target_profile, ControllerProfile): # run the shell locally unless a target was requested con = LocalConnection(args) # type: Connection + + if args.export: + display.info('Configuring controller inventory.', verbosity=1) + create_controller_inventory(args, args.export, host_state.controller_profile) else: # a target was requested, connect to it over SSH con = target_profile.get_controller_target_connections()[0] + if args.export: + display.info('Configuring target inventory.', verbosity=1) + create_posix_inventory(args, args.export, host_state.target_profiles, True) + + if args.export: + return + + if args.cmd: + con.run(args.cmd, capture=False, interactive=False, force_stdout=True) + return + if isinstance(con, SshConnection) and args.raw: cmd = [] # type: t.List[str] elif isinstance(target_profile, PosixProfile): @@ -87,4 +111,4 @@ def command_shell(args): # type: (ShellConfig) -> None else: cmd = [] - con.run(cmd) + con.run(cmd, capture=False, interactive=True) diff --git a/test/lib/ansible_test/_internal/commands/units/__init__.py b/test/lib/ansible_test/_internal/commands/units/__init__.py index 8eb3130d48..ef65df29d4 100644 --- a/test/lib/ansible_test/_internal/commands/units/__init__.py +++ b/test/lib/ansible_test/_internal/commands/units/__init__.py @@ -280,7 +280,7 @@ def command_units(args): # type: (UnitsConfig) -> None display.info('Unit test %s with Python %s' % (test_context, python.version)) try: - cover_python(args, python, cmd, test_context, env) + cover_python(args, python, cmd, test_context, env, capture=False) except SubprocessError as ex: # pytest exits with status code 5 when all tests are skipped, which isn't an error for our use case if ex.status != 5: diff --git a/test/lib/ansible_test/_internal/completion.py b/test/lib/ansible_test/_internal/completion.py index 7aee99ed66..73aee2f25f 100644 --- a/test/lib/ansible_test/_internal/completion.py +++ b/test/lib/ansible_test/_internal/completion.py @@ -79,6 +79,7 @@ class PythonCompletionConfig(PosixCompletionConfig, metaclass=abc.ABCMeta): class RemoteCompletionConfig(CompletionConfig): """Base class for completion configuration of remote environments provisioned through Ansible Core CI.""" provider: t.Optional[str] = None + arch: t.Optional[str] = None @property def platform(self): @@ -99,6 +100,9 @@ class RemoteCompletionConfig(CompletionConfig): if not self.provider: raise Exception(f'Remote completion entry "{self.name}" must provide a "provider" setting.') + if not self.arch: + raise Exception(f'Remote completion entry "{self.name}" must provide a "arch" setting.') + @dataclasses.dataclass(frozen=True) class InventoryCompletionConfig(CompletionConfig): @@ -152,6 +156,11 @@ class NetworkRemoteCompletionConfig(RemoteCompletionConfig): """Configuration for remote network platforms.""" collection: str = '' connection: str = '' + placeholder: bool = False + + def __post_init__(self): + if not self.placeholder: + super().__post_init__() @dataclasses.dataclass(frozen=True) @@ -160,6 +169,9 @@ class PosixRemoteCompletionConfig(RemoteCompletionConfig, PythonCompletionConfig placeholder: bool = False def __post_init__(self): + if not self.placeholder: + super().__post_init__() + if not self.supported_pythons: if self.version and not self.placeholder: raise Exception(f'POSIX remote completion entry "{self.name}" must provide a "python" setting.') diff --git a/test/lib/ansible_test/_internal/config.py b/test/lib/ansible_test/_internal/config.py index 0a14a806ca..e5320f48f9 100644 --- a/test/lib/ansible_test/_internal/config.py +++ b/test/lib/ansible_test/_internal/config.py @@ -48,29 +48,6 @@ class TerminateMode(enum.Enum): return self.name.lower() -class ParsedRemote: - """A parsed version of a "remote" string.""" - def __init__(self, arch, platform, version): # type: (t.Optional[str], str, str) -> None - self.arch = arch - self.platform = platform - self.version = version - - @staticmethod - def parse(value): # type: (str) -> t.Optional['ParsedRemote'] - """Return a ParsedRemote from the given value or None if the syntax is invalid.""" - parts = value.split('/') - - if len(parts) == 2: - arch = None - platform, version = parts - elif len(parts) == 3: - arch, platform, version = parts - else: - return None - - return ParsedRemote(arch, platform, version) - - class EnvironmentConfig(CommonConfig): """Configuration common to all commands which execute in an environment.""" def __init__(self, args, command): # type: (t.Any, str) -> None @@ -237,7 +214,12 @@ class ShellConfig(EnvironmentConfig): def __init__(self, args): # type: (t.Any) -> None super().__init__(args, 'shell') + self.cmd = args.cmd # type: t.List[str] self.raw = args.raw # type: bool + self.check_layout = self.delegate # allow shell to be used without a valid layout as long as no delegation is required + self.interactive = True + self.export = args.export # type: t.Optional[str] + self.display_stderr = True class SanityConfig(TestConfig): @@ -253,7 +235,7 @@ class SanityConfig(TestConfig): self.keep_git = args.keep_git # type: bool self.prime_venvs = args.prime_venvs # type: bool - self.info_stderr = self.lint + self.display_stderr = self.lint or self.list_tests if self.keep_git: def git_callback(files): # type: (t.List[t.Tuple[str, str]]) -> None @@ -292,7 +274,7 @@ class IntegrationConfig(TestConfig): if self.list_targets: self.explain = True - self.info_stderr = True + self.display_stderr = True def get_ansible_config(self): # type: () -> str """Return the path to the Ansible config for the given config.""" diff --git a/test/lib/ansible_test/_internal/connections.py b/test/lib/ansible_test/_internal/connections.py index 14234b2d93..026058abb4 100644 --- a/test/lib/ansible_test/_internal/connections.py +++ b/test/lib/ansible_test/_internal/connections.py @@ -3,7 +3,6 @@ from __future__ import annotations import abc import shlex -import sys import tempfile import typing as t @@ -46,10 +45,12 @@ class Connection(metaclass=abc.ABCMeta): @abc.abstractmethod def run(self, command, # type: t.List[str] - capture=False, # type: bool + capture, # type: bool + interactive=False, # type: bool data=None, # type: t.Optional[str] stdin=None, # type: t.Optional[t.IO[bytes]] stdout=None, # type: t.Optional[t.IO[bytes]] + force_stdout=False, # type: bool ): # type: (...) -> t.Tuple[t.Optional[str], t.Optional[str]] """Run the specified command and return the result.""" @@ -60,7 +61,7 @@ class Connection(metaclass=abc.ABCMeta): """Extract the given archive file stream in the specified directory.""" tar_cmd = ['tar', 'oxzf', '-', '-C', chdir] - retry(lambda: self.run(tar_cmd, stdin=src)) + retry(lambda: self.run(tar_cmd, stdin=src, capture=True)) def create_archive(self, chdir, # type: str @@ -82,7 +83,7 @@ class Connection(metaclass=abc.ABCMeta): sh_cmd = ['sh', '-c', ' | '.join(' '.join(shlex.quote(cmd) for cmd in command) for command in commands)] - retry(lambda: self.run(sh_cmd, stdout=dst)) + retry(lambda: self.run(sh_cmd, stdout=dst, capture=True)) class LocalConnection(Connection): @@ -92,10 +93,12 @@ class LocalConnection(Connection): def run(self, command, # type: t.List[str] - capture=False, # type: bool + capture, # type: bool + interactive=False, # type: bool data=None, # type: t.Optional[str] stdin=None, # type: t.Optional[t.IO[bytes]] stdout=None, # type: t.Optional[t.IO[bytes]] + force_stdout=False, # type: bool ): # type: (...) -> t.Tuple[t.Optional[str], t.Optional[str]] """Run the specified command and return the result.""" return run_command( @@ -105,6 +108,8 @@ class LocalConnection(Connection): data=data, stdin=stdin, stdout=stdout, + interactive=interactive, + force_stdout=force_stdout, ) @@ -130,10 +135,12 @@ class SshConnection(Connection): def run(self, command, # type: t.List[str] - capture=False, # type: bool + capture, # type: bool + interactive=False, # type: bool data=None, # type: t.Optional[str] stdin=None, # type: t.Optional[t.IO[bytes]] stdout=None, # type: t.Optional[t.IO[bytes]] + force_stdout=False, # type: bool ): # type: (...) -> t.Tuple[t.Optional[str], t.Optional[str]] """Run the specified command and return the result.""" options = list(self.options) @@ -143,7 +150,7 @@ class SshConnection(Connection): options.append('-q') - if not data and not stdin and not stdout and sys.stdin.isatty(): + if interactive: options.append('-tt') with tempfile.NamedTemporaryFile(prefix='ansible-test-ssh-debug-', suffix='.log') as ssh_logfile: @@ -166,6 +173,8 @@ class SshConnection(Connection): data=data, stdin=stdin, stdout=stdout, + interactive=interactive, + force_stdout=force_stdout, error_callback=error_callback, ) @@ -208,10 +217,12 @@ class DockerConnection(Connection): def run(self, command, # type: t.List[str] - capture=False, # type: bool + capture, # type: bool + interactive=False, # type: bool data=None, # type: t.Optional[str] stdin=None, # type: t.Optional[t.IO[bytes]] stdout=None, # type: t.Optional[t.IO[bytes]] + force_stdout=False, # type: bool ): # type: (...) -> t.Tuple[t.Optional[str], t.Optional[str]] """Run the specified command and return the result.""" options = [] @@ -219,7 +230,7 @@ class DockerConnection(Connection): if self.user: options.extend(['--user', self.user]) - if not data and not stdin and not stdout and sys.stdin.isatty(): + if interactive: options.append('-it') return docker_exec( @@ -231,6 +242,8 @@ class DockerConnection(Connection): data=data, stdin=stdin, stdout=stdout, + interactive=interactive, + force_stdout=force_stdout, ) def inspect(self): # type: () -> DockerInspect diff --git a/test/lib/ansible_test/_internal/containers.py b/test/lib/ansible_test/_internal/containers.py index 5e29c6ac05..f4d16714a6 100644 --- a/test/lib/ansible_test/_internal/containers.py +++ b/test/lib/ansible_test/_internal/containers.py @@ -794,7 +794,7 @@ def forward_ssh_ports( inventory = generate_ssh_inventory(ssh_connections) with named_temporary_file(args, 'ssh-inventory-', '.json', None, inventory) as inventory_path: # type: str - run_playbook(args, inventory_path, playbook, dict(hosts_entries=hosts_entries)) + run_playbook(args, inventory_path, playbook, capture=False, variables=dict(hosts_entries=hosts_entries)) ssh_processes = [] # type: t.List[SshProcess] @@ -827,7 +827,7 @@ def cleanup_ssh_ports( inventory = generate_ssh_inventory(ssh_connections) with named_temporary_file(args, 'ssh-inventory-', '.json', None, inventory) as inventory_path: # type: str - run_playbook(args, inventory_path, playbook, dict(hosts_entries=hosts_entries)) + run_playbook(args, inventory_path, playbook, capture=False, variables=dict(hosts_entries=hosts_entries)) if ssh_processes: for process in ssh_processes: diff --git a/test/lib/ansible_test/_internal/core_ci.py b/test/lib/ansible_test/_internal/core_ci.py index dbb428aeeb..62d063b2b7 100644 --- a/test/lib/ansible_test/_internal/core_ci.py +++ b/test/lib/ansible_test/_internal/core_ci.py @@ -1,6 +1,8 @@ """Access Ansible Core CI remote services.""" from __future__ import annotations +import abc +import dataclasses import json import os import re @@ -48,6 +50,65 @@ from .data import ( ) +@dataclasses.dataclass(frozen=True) +class Resource(metaclass=abc.ABCMeta): + """Base class for Ansible Core CI resources.""" + @abc.abstractmethod + def as_tuple(self) -> t.Tuple[str, str, str, str]: + """Return the resource as a tuple of platform, version, architecture and provider.""" + + @abc.abstractmethod + def get_label(self) -> str: + """Return a user-friendly label for this resource.""" + + @property + @abc.abstractmethod + def persist(self) -> bool: + """True if the resource is persistent, otherwise false.""" + + +@dataclasses.dataclass(frozen=True) +class VmResource(Resource): + """Details needed to request a VM from Ansible Core CI.""" + platform: str + version: str + architecture: str + provider: str + tag: str + + def as_tuple(self) -> t.Tuple[str, str, str, str]: + """Return the resource as a tuple of platform, version, architecture and provider.""" + return self.platform, self.version, self.architecture, self.provider + + def get_label(self) -> str: + """Return a user-friendly label for this resource.""" + return f'{self.platform} {self.version} ({self.architecture}) [{self.tag}] @{self.provider}' + + @property + def persist(self) -> bool: + """True if the resource is persistent, otherwise false.""" + return True + + +@dataclasses.dataclass(frozen=True) +class CloudResource(Resource): + """Details needed to request cloud credentials from Ansible Core CI.""" + platform: str + + def as_tuple(self) -> t.Tuple[str, str, str, str]: + """Return the resource as a tuple of platform, version, architecture and provider.""" + return self.platform, '', '', self.platform + + def get_label(self) -> str: + """Return a user-friendly label for this resource.""" + return self.platform + + @property + def persist(self) -> bool: + """True if the resource is persistent, otherwise false.""" + return False + + class AnsibleCoreCI: """Client for Ansible Core CI services.""" DEFAULT_ENDPOINT = 'https://ansible-core-ci.testing.ansible.com' @@ -55,16 +116,12 @@ class AnsibleCoreCI: def __init__( self, args, # type: EnvironmentConfig - platform, # type: str - version, # type: str - provider, # type: str - persist=True, # type: bool + resource, # type: Resource load=True, # type: bool - suffix=None, # type: t.Optional[str] ): # type: (...) -> None self.args = args - self.platform = platform - self.version = version + self.resource = resource + self.platform, self.version, self.arch, self.provider = self.resource.as_tuple() self.stage = args.remote_stage self.client = HttpClient(args) self.connection = None @@ -73,35 +130,33 @@ class AnsibleCoreCI: self.default_endpoint = args.remote_endpoint or self.DEFAULT_ENDPOINT self.retries = 3 self.ci_provider = get_ci_provider() - self.provider = provider - self.name = '%s-%s' % (self.platform, self.version) + self.label = self.resource.get_label() - if suffix: - self.name += '-' + suffix + stripped_label = re.sub('[^A-Za-z0-9_.]+', '-', self.label).strip('-') - self.path = os.path.expanduser('~/.ansible/test/instances/%s-%s-%s' % (self.name, self.provider, self.stage)) + self.name = f"{stripped_label}-{self.stage}" # turn the label into something suitable for use as a filename + + self.path = os.path.expanduser(f'~/.ansible/test/instances/{self.name}') self.ssh_key = SshKey(args) - if persist and load and self._load(): + if self.resource.persist and load and self._load(): try: - display.info('Checking existing %s/%s instance %s.' % (self.platform, self.version, self.instance_id), - verbosity=1) + display.info(f'Checking existing {self.label} instance using: {self._uri}', verbosity=1) self.connection = self.get(always_raise_on=[404]) - display.info('Loaded existing %s/%s from: %s' % (self.platform, self.version, self._uri), verbosity=1) + display.info(f'Loaded existing {self.label} instance.', verbosity=1) except HttpError as ex: if ex.status != 404: raise self._clear() - display.info('Cleared stale %s/%s instance %s.' % (self.platform, self.version, self.instance_id), - verbosity=1) + display.info(f'Cleared stale {self.label} instance.', verbosity=1) self.instance_id = None self.endpoint = None - elif not persist: + elif not self.resource.persist: self.instance_id = None self.endpoint = None self._clear() @@ -126,8 +181,7 @@ class AnsibleCoreCI: def start(self): """Start instance.""" if self.started: - display.info('Skipping started %s/%s instance %s.' % (self.platform, self.version, self.instance_id), - verbosity=1) + display.info(f'Skipping started {self.label} instance.', verbosity=1) return None return self._start(self.ci_provider.prepare_core_ci_auth()) @@ -135,22 +189,19 @@ class AnsibleCoreCI: def stop(self): """Stop instance.""" if not self.started: - display.info('Skipping invalid %s/%s instance %s.' % (self.platform, self.version, self.instance_id), - verbosity=1) + display.info(f'Skipping invalid {self.label} instance.', verbosity=1) return response = self.client.delete(self._uri) if response.status_code == 404: self._clear() - display.info('Cleared invalid %s/%s instance %s.' % (self.platform, self.version, self.instance_id), - verbosity=1) + display.info(f'Cleared invalid {self.label} instance.', verbosity=1) return if response.status_code == 200: self._clear() - display.info('Stopped running %s/%s instance %s.' % (self.platform, self.version, self.instance_id), - verbosity=1) + display.info(f'Stopped running {self.label} instance.', verbosity=1) return raise self._create_http_error(response) @@ -158,8 +209,7 @@ class AnsibleCoreCI: def get(self, tries=3, sleep=15, always_raise_on=None): # type: (int, int, t.Optional[t.List[int]]) -> t.Optional[InstanceConnection] """Get instance connection information.""" if not self.started: - display.info('Skipping invalid %s/%s instance %s.' % (self.platform, self.version, self.instance_id), - verbosity=1) + display.info(f'Skipping invalid {self.label} instance.', verbosity=1) return None if not always_raise_on: @@ -180,7 +230,7 @@ class AnsibleCoreCI: if not tries or response.status_code in always_raise_on: raise error - display.warning('%s. Trying again after %d seconds.' % (error, sleep)) + display.warning(f'{error}. Trying again after {sleep} seconds.') time.sleep(sleep) if self.args.explain: @@ -216,9 +266,7 @@ class AnsibleCoreCI: status = 'running' if self.connection.running else 'starting' - display.info('Status update: %s/%s on instance %s is %s.' % - (self.platform, self.version, self.instance_id, status), - verbosity=1) + display.info(f'The {self.label} instance is {status}.', verbosity=1) return self.connection @@ -229,16 +277,15 @@ class AnsibleCoreCI: return time.sleep(10) - raise ApplicationError('Timeout waiting for %s/%s instance %s.' % - (self.platform, self.version, self.instance_id)) + raise ApplicationError(f'Timeout waiting for {self.label} instance.') @property def _uri(self): - return '%s/%s/%s/%s' % (self.endpoint, self.stage, self.provider, self.instance_id) + return f'{self.endpoint}/{self.stage}/{self.provider}/{self.instance_id}' def _start(self, auth): """Start instance.""" - display.info('Initializing new %s/%s instance %s.' % (self.platform, self.version, self.instance_id), verbosity=1) + display.info(f'Initializing new {self.label} instance using: {self._uri}', verbosity=1) if self.platform == 'windows': winrm_config = read_text_file(os.path.join(ANSIBLE_TEST_TARGET_ROOT, 'setup', 'ConfigureRemotingForAnsible.ps1')) @@ -249,6 +296,7 @@ class AnsibleCoreCI: config=dict( platform=self.platform, version=self.version, + architecture=self.arch, public_key=self.ssh_key.pub_contents, query=False, winrm_config=winrm_config, @@ -266,7 +314,7 @@ class AnsibleCoreCI: self.started = True self._save() - display.info('Started %s/%s from: %s' % (self.platform, self.version, self._uri), verbosity=1) + display.info(f'Started {self.label} instance.', verbosity=1) if self.args.explain: return {} @@ -277,8 +325,6 @@ class AnsibleCoreCI: tries = self.retries sleep = 15 - display.info('Trying endpoint: %s' % self.endpoint, verbosity=1) - while True: tries -= 1 response = self.client.put(self._uri, data=json.dumps(data), headers=headers) @@ -294,7 +340,7 @@ class AnsibleCoreCI: if not tries: raise error - display.warning('%s. Trying again after %d seconds.' % (error, sleep)) + display.warning(f'{error}. Trying again after {sleep} seconds.') time.sleep(sleep) def _clear(self): @@ -345,14 +391,14 @@ class AnsibleCoreCI: def save(self): # type: () -> t.Dict[str, str] """Save instance details and return as a dictionary.""" return dict( - platform_version='%s/%s' % (self.platform, self.version), + label=self.resource.get_label(), instance_id=self.instance_id, endpoint=self.endpoint, ) @staticmethod def _create_http_error(response): # type: (HttpResponse) -> ApplicationError - """Return an exception created from the given HTTP resposne.""" + """Return an exception created from the given HTTP response.""" response_json = response.json() stack_trace = '' @@ -369,7 +415,7 @@ class AnsibleCoreCI: traceback_lines = traceback.format_list(traceback_lines) trace = '\n'.join([x.rstrip() for x in traceback_lines]) - stack_trace = ('\nTraceback (from remote server):\n%s' % trace) + stack_trace = f'\nTraceback (from remote server):\n{trace}' else: message = str(response_json) @@ -379,7 +425,7 @@ class AnsibleCoreCI: class CoreHttpError(HttpError): """HTTP response as an error.""" def __init__(self, status, remote_message, remote_stack_trace): # type: (int, str, str) -> None - super().__init__(status, '%s%s' % (remote_message, remote_stack_trace)) + super().__init__(status, f'{remote_message}{remote_stack_trace}') self.remote_message = remote_message self.remote_stack_trace = remote_stack_trace @@ -388,8 +434,8 @@ class CoreHttpError(HttpError): class SshKey: """Container for SSH key used to connect to remote instances.""" KEY_TYPE = 'rsa' # RSA is used to maintain compatibility with paramiko and EC2 - KEY_NAME = 'id_%s' % KEY_TYPE - PUB_NAME = '%s.pub' % KEY_NAME + KEY_NAME = f'id_{KEY_TYPE}' + PUB_NAME = f'{KEY_NAME}.pub' @mutex def __init__(self, args): # type: (EnvironmentConfig) -> None @@ -469,7 +515,7 @@ class SshKey: make_dirs(os.path.dirname(key)) if not os.path.isfile(key) or not os.path.isfile(pub): - run_command(args, ['ssh-keygen', '-m', 'PEM', '-q', '-t', self.KEY_TYPE, '-N', '', '-f', key]) + run_command(args, ['ssh-keygen', '-m', 'PEM', '-q', '-t', self.KEY_TYPE, '-N', '', '-f', key], capture=True) if args.explain: return key, pub @@ -502,6 +548,6 @@ class InstanceConnection: def __str__(self): if self.password: - return '%s:%s [%s:%s]' % (self.hostname, self.port, self.username, self.password) + return f'{self.hostname}:{self.port} [{self.username}:{self.password}]' - return '%s:%s [%s]' % (self.hostname, self.port, self.username) + return f'{self.hostname}:{self.port} [{self.username}]' diff --git a/test/lib/ansible_test/_internal/coverage_util.py b/test/lib/ansible_test/_internal/coverage_util.py index 5c489a0240..c514079ade 100644 --- a/test/lib/ansible_test/_internal/coverage_util.py +++ b/test/lib/ansible_test/_internal/coverage_util.py @@ -48,7 +48,7 @@ def cover_python( cmd, # type: t.List[str] target_name, # type: str env, # type: t.Dict[str, str] - capture=False, # type: bool + capture, # type: bool data=None, # type: t.Optional[str] cwd=None, # type: t.Optional[str] ): # type: (...) -> t.Tuple[t.Optional[str], t.Optional[str]] diff --git a/test/lib/ansible_test/_internal/delegation.py b/test/lib/ansible_test/_internal/delegation.py index 6298bf2405..33e3f621d9 100644 --- a/test/lib/ansible_test/_internal/delegation.py +++ b/test/lib/ansible_test/_internal/delegation.py @@ -160,12 +160,13 @@ def delegate_command(args, host_state, exclude, require): # type: (EnvironmentC os.path.join(content_root, ResultType.COVERAGE.relative_path), ] - con.run(['mkdir', '-p'] + writable_dirs) - con.run(['chmod', '777'] + writable_dirs) - con.run(['chmod', '755', working_directory]) - con.run(['chmod', '644', os.path.join(content_root, args.metadata_path)]) - con.run(['useradd', pytest_user, '--create-home']) - con.run(insert_options(command, options + ['--requirements-mode', 'only'])) + con.run(['mkdir', '-p'] + writable_dirs, capture=True) + con.run(['chmod', '777'] + writable_dirs, capture=True) + con.run(['chmod', '755', working_directory], capture=True) + con.run(['chmod', '644', os.path.join(content_root, args.metadata_path)], capture=True) + con.run(['useradd', pytest_user, '--create-home'], capture=True) + + con.run(insert_options(command, options + ['--requirements-mode', 'only']), capture=False) container = con.inspect() networks = container.get_network_names() @@ -191,7 +192,7 @@ def delegate_command(args, host_state, exclude, require): # type: (EnvironmentC success = False try: - con.run(insert_options(command, options)) + con.run(insert_options(command, options), capture=False, interactive=args.interactive) success = True finally: if host_delegation: diff --git a/test/lib/ansible_test/_internal/docker_util.py b/test/lib/ansible_test/_internal/docker_util.py index 1250907677..0a17bf4c66 100644 --- a/test/lib/ansible_test/_internal/docker_util.py +++ b/test/lib/ansible_test/_internal/docker_util.py @@ -268,7 +268,7 @@ def docker_pull(args, image): # type: (EnvironmentConfig, str) -> None for _iteration in range(1, 10): try: - docker_command(args, ['pull', image]) + docker_command(args, ['pull', image], capture=False) return except SubprocessError: display.warning('Failed to pull docker image "%s". Waiting a few seconds before trying again.' % image) @@ -279,7 +279,7 @@ def docker_pull(args, image): # type: (EnvironmentConfig, str) -> None def docker_cp_to(args, container_id, src, dst): # type: (EnvironmentConfig, str, str, str) -> None """Copy a file to the specified container.""" - docker_command(args, ['cp', src, '%s:%s' % (container_id, dst)]) + docker_command(args, ['cp', src, '%s:%s' % (container_id, dst)], capture=True) def docker_run( @@ -510,10 +510,12 @@ def docker_exec( args, # type: EnvironmentConfig container_id, # type: str cmd, # type: t.List[str] + capture, # type: bool options=None, # type: t.Optional[t.List[str]] - capture=False, # type: bool stdin=None, # type: t.Optional[t.IO[bytes]] stdout=None, # type: t.Optional[t.IO[bytes]] + interactive=False, # type: bool + force_stdout=False, # type: bool data=None, # type: t.Optional[str] ): # type: (...) -> t.Tuple[t.Optional[str], t.Optional[str]] """Execute the given command in the specified container.""" @@ -523,7 +525,8 @@ def docker_exec( if data or stdin or stdout: options.append('-i') - return docker_command(args, ['exec'] + options + [container_id] + cmd, capture=capture, stdin=stdin, stdout=stdout, data=data) + return docker_command(args, ['exec'] + options + [container_id] + cmd, capture=capture, stdin=stdin, stdout=stdout, interactive=interactive, + force_stdout=force_stdout, data=data) def docker_info(args): # type: (CommonConfig) -> t.Dict[str, t.Any] @@ -541,18 +544,23 @@ def docker_version(args): # type: (CommonConfig) -> t.Dict[str, t.Any] def docker_command( args, # type: CommonConfig cmd, # type: t.List[str] - capture=False, # type: bool + capture, # type: bool stdin=None, # type: t.Optional[t.IO[bytes]] stdout=None, # type: t.Optional[t.IO[bytes]] + interactive=False, # type: bool + force_stdout=False, # type: bool always=False, # type: bool data=None, # type: t.Optional[str] ): # type: (...) -> t.Tuple[t.Optional[str], t.Optional[str]] """Run the specified docker command.""" env = docker_environment() command = [require_docker().command] + if command[0] == 'podman' and _get_podman_remote(): command.append('--remote') - return run_command(args, command + cmd, env=env, capture=capture, stdin=stdin, stdout=stdout, always=always, data=data) + + return run_command(args, command + cmd, env=env, capture=capture, stdin=stdin, stdout=stdout, interactive=interactive, always=always, + force_stdout=force_stdout, data=data) def docker_environment(): # type: () -> t.Dict[str, str] diff --git a/test/lib/ansible_test/_internal/host_configs.py b/test/lib/ansible_test/_internal/host_configs.py index fee741e8b9..11a4506465 100644 --- a/test/lib/ansible_test/_internal/host_configs.py +++ b/test/lib/ansible_test/_internal/host_configs.py @@ -39,6 +39,7 @@ from .util import ( get_available_python_versions, str_to_version, version_to_str, + Architecture, ) @@ -206,6 +207,7 @@ class RemoteConfig(HostConfig, metaclass=abc.ABCMeta): """Base class for remote host configuration.""" name: t.Optional[str] = None provider: t.Optional[str] = None + arch: t.Optional[str] = None @property def platform(self): # type: () -> str @@ -227,6 +229,7 @@ class RemoteConfig(HostConfig, metaclass=abc.ABCMeta): self.provider = None self.provider = self.provider or defaults.provider or 'aws' + self.arch = self.arch or defaults.arch or Architecture.X86_64 @property def is_managed(self): # type: () -> bool @@ -330,8 +333,6 @@ class DockerConfig(ControllerHostConfig, PosixConfig): @dataclasses.dataclass class PosixRemoteConfig(RemoteConfig, ControllerHostConfig, PosixConfig): """Configuration for a POSIX remote host.""" - arch: t.Optional[str] = None - def get_defaults(self, context): # type: (HostContext) -> PosixRemoteCompletionConfig """Return the default settings.""" return filter_completion(remote_completion()).get(self.name) or remote_completion().get(self.platform) or PosixRemoteCompletionConfig( @@ -388,6 +389,7 @@ class NetworkRemoteConfig(RemoteConfig, NetworkConfig): """Return the default settings.""" return filter_completion(network_completion()).get(self.name) or NetworkRemoteCompletionConfig( name=self.name, + placeholder=True, ) def apply_defaults(self, context, defaults): # type: (HostContext, CompletionConfig) -> None diff --git a/test/lib/ansible_test/_internal/host_profiles.py b/test/lib/ansible_test/_internal/host_profiles.py index 9079c7e924..b15d7d356e 100644 --- a/test/lib/ansible_test/_internal/host_profiles.py +++ b/test/lib/ansible_test/_internal/host_profiles.py @@ -40,6 +40,7 @@ from .host_configs import ( from .core_ci import ( AnsibleCoreCI, SshKey, + VmResource, ) from .util import ( @@ -50,6 +51,7 @@ from .util import ( get_type_map, sanitize_host_name, sorted_versions, + InternalError, ) from .util_common import ( @@ -148,7 +150,7 @@ class Inventory: inventory_text = inventory_text.strip() if not args.explain: - write_text_file(path, inventory_text) + write_text_file(path, inventory_text + '\n') display.info(f'>>> Inventory\n{inventory_text}', verbosity=3) @@ -295,12 +297,18 @@ class RemoteProfile(SshTargetHostProfile[TRemoteConfig], metaclass=abc.ABCMeta): def create_core_ci(self, load): # type: (bool) -> AnsibleCoreCI """Create and return an AnsibleCoreCI instance.""" + if not self.config.arch: + raise InternalError(f'No arch specified for config: {self.config}') + return AnsibleCoreCI( args=self.args, - platform=self.config.platform, - version=self.config.version, - provider=self.config.provider, - suffix='controller' if self.controller else 'target', + resource=VmResource( + platform=self.config.platform, + version=self.config.version, + architecture=self.config.arch, + provider=self.config.provider, + tag='controller' if self.controller else 'target', + ), load=load, ) @@ -362,7 +370,7 @@ class DockerProfile(ControllerHostProfile[DockerConfig], SshTargetHostProfile[Do setup_sh = bootstrapper.get_script() shell = setup_sh.splitlines()[0][2:] - docker_exec(self.args, self.container_name, [shell], data=setup_sh) + docker_exec(self.args, self.container_name, [shell], data=setup_sh, capture=False) def deprovision(self): # type: () -> None """Deprovision the host after delegation has completed.""" @@ -484,8 +492,9 @@ class NetworkRemoteProfile(RemoteProfile[NetworkRemoteConfig]): for dummy in range(1, 90): try: - intercept_python(self.args, self.args.controller_python, cmd, env) - except SubprocessError: + intercept_python(self.args, self.args.controller_python, cmd, env, capture=True) + except SubprocessError as ex: + display.warning(str(ex)) time.sleep(10) else: return @@ -547,7 +556,7 @@ class PosixRemoteProfile(ControllerHostProfile[PosixRemoteConfig], RemoteProfile shell = setup_sh.splitlines()[0][2:] ssh = self.get_origin_controller_connection() - ssh.run([shell], data=setup_sh) + ssh.run([shell], data=setup_sh, capture=False) def get_ssh_connection(self): # type: () -> SshConnection """Return an SSH connection for accessing the host.""" @@ -570,6 +579,8 @@ class PosixRemoteProfile(ControllerHostProfile[PosixRemoteConfig], RemoteProfile become = Sudo() elif self.config.platform == 'rhel': become = Sudo() + elif self.config.platform == 'ubuntu': + become = Sudo() else: raise NotImplementedError(f'Become support has not been implemented for platform "{self.config.platform}" and user "{settings.user}" is not root.') @@ -717,8 +728,9 @@ class WindowsRemoteProfile(RemoteProfile[WindowsRemoteConfig]): for dummy in range(1, 120): try: - intercept_python(self.args, self.args.controller_python, cmd, env) - except SubprocessError: + intercept_python(self.args, self.args.controller_python, cmd, env, capture=True) + except SubprocessError as ex: + display.warning(str(ex)) time.sleep(10) else: return diff --git a/test/lib/ansible_test/_internal/pypi_proxy.py b/test/lib/ansible_test/_internal/pypi_proxy.py index 51974d261a..f0d778157e 100644 --- a/test/lib/ansible_test/_internal/pypi_proxy.py +++ b/test/lib/ansible_test/_internal/pypi_proxy.py @@ -126,7 +126,8 @@ def configure_target_pypi_proxy(args, profile, pypi_endpoint, pypi_hostname): # force = 'yes' if profile.config.is_managed else 'no' - run_playbook(args, inventory_path, 'pypi_proxy_prepare.yml', dict(pypi_endpoint=pypi_endpoint, pypi_hostname=pypi_hostname, force=force), capture=True) + run_playbook(args, inventory_path, 'pypi_proxy_prepare.yml', capture=True, variables=dict( + pypi_endpoint=pypi_endpoint, pypi_hostname=pypi_hostname, force=force)) atexit.register(cleanup_pypi_proxy) diff --git a/test/lib/ansible_test/_internal/python_requirements.py b/test/lib/ansible_test/_internal/python_requirements.py index f67f659811..9086c7981c 100644 --- a/test/lib/ansible_test/_internal/python_requirements.py +++ b/test/lib/ansible_test/_internal/python_requirements.py @@ -261,7 +261,7 @@ def run_pip( if not args.explain: try: - connection.run([python.path], data=script) + connection.run([python.path], data=script, capture=False) except SubprocessError: script = prepare_pip_script([PipVersion()]) diff --git a/test/lib/ansible_test/_internal/test.py b/test/lib/ansible_test/_internal/test.py index 3e149b15bf..6d2cf2a344 100644 --- a/test/lib/ansible_test/_internal/test.py +++ b/test/lib/ansible_test/_internal/test.py @@ -265,10 +265,10 @@ class TestFailure(TestResult): message = 'The test `%s` failed. See stderr output for details.' % command path = '' message = TestMessage(message, path) - print(message) + print(message) # display goes to stderr, this should be on stdout else: for message in self.messages: - print(message) + print(message) # display goes to stderr, this should be on stdout def write_junit(self, args): # type: (TestConfig) -> None """Write results to a junit XML file.""" diff --git a/test/lib/ansible_test/_internal/util.py b/test/lib/ansible_test/_internal/util.py index 0ad78882de..25cca59d40 100644 --- a/test/lib/ansible_test/_internal/util.py +++ b/test/lib/ansible_test/_internal/util.py @@ -1,12 +1,15 @@ """Miscellaneous utility functions and classes.""" from __future__ import annotations +import abc import errno import fcntl import importlib.util import inspect +import json import keyword import os +import platform import pkgutil import random import re @@ -41,6 +44,7 @@ from .io import ( from .thread import ( mutex, + WrappedThread, ) from .constants import ( @@ -96,6 +100,18 @@ MODE_DIRECTORY = MODE_READ | stat.S_IWUSR | stat.S_IXUSR | stat.S_IXGRP | stat.S MODE_DIRECTORY_WRITE = MODE_DIRECTORY | stat.S_IWGRP | stat.S_IWOTH +class Architecture: + """ + Normalized architecture names. + These are the architectures supported by ansible-test, such as when provisioning remote instances. + """ + X86_64 = 'x86_64' + AARCH64 = 'aarch64' + + +REMOTE_ARCHITECTURES = list(value for key, value in Architecture.__dict__.items() if not key.startswith('__')) + + def is_valid_identifier(value: str) -> bool: """Return True if the given value is a valid non-keyword Python identifier, otherwise return False.""" return value.isidentifier() and not keyword.iskeyword(value) @@ -119,6 +135,58 @@ def cache(func): # type: (t.Callable[[], TValue]) -> t.Callable[[], TValue] return wrapper +@mutex +def detect_architecture(python: str) -> t.Optional[str]: + """Detect the architecture of the specified Python and return a normalized version, or None if it cannot be determined.""" + results: t.Dict[str, t.Optional[str]] + + try: + results = detect_architecture.results # type: ignore[attr-defined] + except AttributeError: + results = detect_architecture.results = {} # type: ignore[attr-defined] + + if python in results: + return results[python] + + if python == sys.executable or os.path.realpath(python) == os.path.realpath(sys.executable): + uname = platform.uname() + else: + data = raw_command([python, '-c', 'import json, platform; print(json.dumps(platform.uname()));'], capture=True)[0] + uname = json.loads(data) + + translation = { + 'x86_64': Architecture.X86_64, # Linux, macOS + 'amd64': Architecture.X86_64, # FreeBSD + 'aarch64': Architecture.AARCH64, # Linux, FreeBSD + 'arm64': Architecture.AARCH64, # FreeBSD + } + + candidates = [] + + if len(uname) >= 5: + candidates.append(uname[4]) + + if len(uname) >= 6: + candidates.append(uname[5]) + + candidates = sorted(set(candidates)) + architectures = sorted(set(arch for arch in [translation.get(candidate) for candidate in candidates] if arch)) + + architecture: t.Optional[str] = None + + if not architectures: + display.warning(f'Unable to determine architecture for Python interpreter "{python}" from: {candidates}') + elif len(architectures) == 1: + architecture = architectures[0] + display.info(f'Detected architecture {architecture} for Python interpreter: {python}', verbosity=1) + else: + display.warning(f'Conflicting architectures detected ({architectures}) for Python interpreter "{python}" from: {candidates}') + + results[python] = architecture + + return architecture + + def filter_args(args, filters): # type: (t.List[str], t.Dict[str, int]) -> t.List[str] """Return a filtered version of the given command line arguments.""" remaining = 0 @@ -254,18 +322,44 @@ def get_available_python_versions(): # type: () -> t.Dict[str, str] def raw_command( cmd, # type: t.Iterable[str] - capture=False, # type: bool + capture, # type: bool env=None, # type: t.Optional[t.Dict[str, str]] data=None, # type: t.Optional[str] cwd=None, # type: t.Optional[str] explain=False, # type: bool stdin=None, # type: t.Optional[t.Union[t.IO[bytes], int]] stdout=None, # type: t.Optional[t.Union[t.IO[bytes], int]] + interactive=False, # type: bool + force_stdout=False, # type: bool cmd_verbosity=1, # type: int str_errors='strict', # type: str error_callback=None, # type: t.Optional[t.Callable[[SubprocessError], None]] ): # type: (...) -> t.Tuple[t.Optional[str], t.Optional[str]] """Run the specified command and return stdout and stderr as a tuple.""" + if capture and interactive: + raise InternalError('Cannot combine capture=True with interactive=True.') + + if data and interactive: + raise InternalError('Cannot combine data with interactive=True.') + + if stdin and interactive: + raise InternalError('Cannot combine stdin with interactive=True.') + + if stdout and interactive: + raise InternalError('Cannot combine stdout with interactive=True.') + + if stdin and data: + raise InternalError('Cannot combine stdin with data.') + + if stdout and not capture: + raise InternalError('Redirection of stdout requires capture=True to avoid redirection of stderr to stdout.') + + if force_stdout and capture: + raise InternalError('Cannot combine force_stdout=True with capture=True.') + + if force_stdout and interactive: + raise InternalError('Cannot combine force_stdout=True with interactive=True.') + if not cwd: cwd = os.getcwd() @@ -276,7 +370,30 @@ def raw_command( escaped_cmd = ' '.join(shlex.quote(c) for c in cmd) - display.info('Run command: %s' % escaped_cmd, verbosity=cmd_verbosity, truncate=True) + if capture: + description = 'Run' + elif interactive: + description = 'Interactive' + else: + description = 'Stream' + + description += ' command' + + with_types = [] + + if data: + with_types.append('data') + + if stdin: + with_types.append('stdin') + + if stdout: + with_types.append('stdout') + + if with_types: + description += f' with {"/".join(with_types)}' + + display.info(f'{description}: {escaped_cmd}', verbosity=cmd_verbosity, truncate=True) display.info('Working directory: %s' % cwd, verbosity=2) program = find_executable(cmd[0], cwd=cwd, path=env['PATH'], required='warning') @@ -294,17 +411,23 @@ def raw_command( if stdin is not None: data = None - communicate = True elif data is not None: stdin = subprocess.PIPE communicate = True - - if stdout: - communicate = True - - if capture: + elif interactive: + pass # allow the subprocess access to our stdin + else: + stdin = subprocess.DEVNULL + + if not interactive: + # When not running interactively, send subprocess stdout/stderr through a pipe. + # This isolates the stdout/stderr of the subprocess from the current process, and also hides the current TTY from it, if any. + # This prevents subprocesses from sharing stdout/stderr with the current process or each other. + # Doing so allows subprocesses to safely make changes to their file handles, such as making them non-blocking (ssh does this). + # This also maintains consistency between local testing and CI systems, which typically do not provide a TTY. + # To maintain output ordering, a single pipe is used for both stdout/stderr when not capturing output. stdout = stdout or subprocess.PIPE - stderr = subprocess.PIPE + stderr = subprocess.PIPE if capture else subprocess.STDOUT communicate = True else: stderr = None @@ -324,7 +447,8 @@ def raw_command( if communicate: data_bytes = to_optional_bytes(data) - stdout_bytes, stderr_bytes = process.communicate(data_bytes) + stdout_bytes, stderr_bytes = communicate_with_process(process, data_bytes, stdout == subprocess.PIPE, stderr == subprocess.PIPE, capture=capture, + force_stdout=force_stdout) stdout_text = to_optional_text(stdout_bytes, str_errors) or u'' stderr_text = to_optional_text(stderr_bytes, str_errors) or u'' else: @@ -347,6 +471,122 @@ def raw_command( raise SubprocessError(cmd, status, stdout_text, stderr_text, runtime, error_callback) +def communicate_with_process( + process: subprocess.Popen, + stdin: t.Optional[bytes], + stdout: bool, + stderr: bool, + capture: bool, + force_stdout: bool +) -> t.Tuple[bytes, bytes]: + """Communicate with the specified process, handling stdin/stdout/stderr as requested.""" + threads: t.List[WrappedThread] = [] + reader: t.Type[ReaderThread] + + if capture: + reader = CaptureThread + else: + reader = OutputThread + + if stdin is not None: + threads.append(WriterThread(process.stdin, stdin)) + + if stdout: + stdout_reader = reader(process.stdout, force_stdout) + threads.append(stdout_reader) + else: + stdout_reader = None + + if stderr: + stderr_reader = reader(process.stderr, force_stdout) + threads.append(stderr_reader) + else: + stderr_reader = None + + for thread in threads: + thread.start() + + for thread in threads: + try: + thread.wait_for_result() + except Exception as ex: # pylint: disable=broad-except + display.error(str(ex)) + + if isinstance(stdout_reader, ReaderThread): + stdout_bytes = b''.join(stdout_reader.lines) + else: + stdout_bytes = b'' + + if isinstance(stderr_reader, ReaderThread): + stderr_bytes = b''.join(stderr_reader.lines) + else: + stderr_bytes = b'' + + process.wait() + + return stdout_bytes, stderr_bytes + + +class WriterThread(WrappedThread): + """Thread to write data to stdin of a subprocess.""" + def __init__(self, handle: t.IO[bytes], data: bytes) -> None: + super().__init__(self._run) + + self.handle = handle + self.data = data + + def _run(self) -> None: + """Workload to run on a thread.""" + try: + self.handle.write(self.data) + self.handle.flush() + finally: + self.handle.close() + + +class ReaderThread(WrappedThread, metaclass=abc.ABCMeta): + """Thread to read stdout from a subprocess.""" + def __init__(self, handle: t.IO[bytes], force_stdout: bool) -> None: + super().__init__(self._run) + + self.handle = handle + self.force_stdout = force_stdout + self.lines = [] # type: t.List[bytes] + + @abc.abstractmethod + def _run(self) -> None: + """Workload to run on a thread.""" + + +class CaptureThread(ReaderThread): + """Thread to capture stdout from a subprocess into a buffer.""" + def _run(self) -> None: + """Workload to run on a thread.""" + src = self.handle + dst = self.lines + + try: + for line in src: + dst.append(line) + finally: + src.close() + + +class OutputThread(ReaderThread): + """Thread to pass stdout from a subprocess to stdout.""" + def _run(self) -> None: + """Workload to run on a thread.""" + src = self.handle + dst = sys.stdout.buffer if self.force_stdout else display.fd.buffer + + try: + for line in src: + dst.write(line) + dst.flush() + finally: + src.close() + + def common_environment(): """Common environment used for executing all programs.""" env = dict( @@ -516,7 +756,7 @@ class Display: self.color = sys.stdout.isatty() self.warnings = [] self.warnings_unique = set() - self.info_stderr = False + self.fd = sys.stderr # default to stderr until config is initialized to avoid early messages going to stdout self.rows = 0 self.columns = 0 self.truncate = 0 @@ -528,7 +768,7 @@ class Display: def __warning(self, message): # type: (str) -> None """Internal implementation for displaying a warning message.""" - self.print_message('WARNING: %s' % message, color=self.purple, fd=sys.stderr) + self.print_message('WARNING: %s' % message, color=self.purple) def review_warnings(self): # type: () -> None """Review all warnings which previously occurred.""" @@ -556,23 +796,27 @@ class Display: def notice(self, message): # type: (str) -> None """Display a notice level message.""" - self.print_message('NOTICE: %s' % message, color=self.purple, fd=sys.stderr) + self.print_message('NOTICE: %s' % message, color=self.purple) def error(self, message): # type: (str) -> None """Display an error level message.""" - self.print_message('ERROR: %s' % message, color=self.red, fd=sys.stderr) + self.print_message('ERROR: %s' % message, color=self.red) + + def fatal(self, message): # type: (str) -> None + """Display a fatal level message.""" + self.print_message('FATAL: %s' % message, color=self.red, stderr=True) def info(self, message, verbosity=0, truncate=False): # type: (str, int, bool) -> None """Display an info level message.""" if self.verbosity >= verbosity: color = self.verbosity_colors.get(verbosity, self.yellow) - self.print_message(message, color=color, fd=sys.stderr if self.info_stderr else sys.stdout, truncate=truncate) + self.print_message(message, color=color, truncate=truncate) def print_message( # pylint: disable=locally-disabled, invalid-name self, message, # type: str color=None, # type: t.Optional[str] - fd=sys.stdout, # type: t.IO[str] + stderr=False, # type: bool truncate=False, # type: bool ): # type: (...) -> None """Display a message.""" @@ -592,10 +836,18 @@ class Display: message = message.replace(self.clear, color) message = '%s%s%s' % (color, message, self.clear) + fd = sys.stderr if stderr else self.fd + print(message, file=fd) fd.flush() +class InternalError(Exception): + """An unhandled internal error indicating a bug in the code.""" + def __init__(self, message: str) -> None: + super().__init__(f'An internal error has occurred in ansible-test: {message}') + + class ApplicationError(Exception): """General application error.""" @@ -648,12 +900,15 @@ class MissingEnvironmentVariable(ApplicationError): self.name = name -def retry(func, ex_type=SubprocessError, sleep=10, attempts=10): +def retry(func, ex_type=SubprocessError, sleep=10, attempts=10, warn=True): """Retry the specified function on failure.""" for dummy in range(1, attempts): try: return func() - except ex_type: + except ex_type as ex: + if warn: + display.warning(str(ex)) + time.sleep(sleep) return func() diff --git a/test/lib/ansible_test/_internal/util_common.py b/test/lib/ansible_test/_internal/util_common.py index 99d22c2bca..86e0d776bb 100644 --- a/test/lib/ansible_test/_internal/util_common.py +++ b/test/lib/ansible_test/_internal/util_common.py @@ -126,6 +126,8 @@ class CommonConfig: """Configuration common to all commands.""" def __init__(self, args, command): # type: (t.Any, str) -> None self.command = command + self.interactive = False + self.check_layout = True self.success = None # type: t.Optional[bool] self.color = args.color # type: bool @@ -135,7 +137,7 @@ class CommonConfig: self.truncate = args.truncate # type: int self.redact = args.redact # type: bool - self.info_stderr = False # type: bool + self.display_stderr = False # type: bool self.session_name = generate_name() @@ -369,7 +371,7 @@ def intercept_python( python, # type: PythonConfig cmd, # type: t.List[str] env, # type: t.Dict[str, str] - capture=False, # type: bool + capture, # type: bool data=None, # type: t.Optional[str] cwd=None, # type: t.Optional[str] always=False, # type: bool @@ -399,21 +401,23 @@ def intercept_python( def run_command( args, # type: CommonConfig cmd, # type: t.Iterable[str] - capture=False, # type: bool + capture, # type: bool env=None, # type: t.Optional[t.Dict[str, str]] data=None, # type: t.Optional[str] cwd=None, # type: t.Optional[str] always=False, # type: bool stdin=None, # type: t.Optional[t.IO[bytes]] stdout=None, # type: t.Optional[t.IO[bytes]] + interactive=False, # type: bool + force_stdout=False, # type: bool cmd_verbosity=1, # type: int str_errors='strict', # type: str error_callback=None, # type: t.Optional[t.Callable[[SubprocessError], None]] ): # type: (...) -> t.Tuple[t.Optional[str], t.Optional[str]] """Run the specified command and return stdout and stderr as a tuple.""" explain = args.explain and not always - return raw_command(cmd, capture=capture, env=env, data=data, cwd=cwd, explain=explain, stdin=stdin, stdout=stdout, - cmd_verbosity=cmd_verbosity, str_errors=str_errors, error_callback=error_callback) + return raw_command(cmd, capture=capture, env=env, data=data, cwd=cwd, explain=explain, stdin=stdin, stdout=stdout, interactive=interactive, + force_stdout=force_stdout, cmd_verbosity=cmd_verbosity, str_errors=str_errors, error_callback=error_callback) def yamlcheck(python): diff --git a/test/lib/ansible_test/_internal/venv.py b/test/lib/ansible_test/_internal/venv.py index 64d8d04ce1..21dded627c 100644 --- a/test/lib/ansible_test/_internal/venv.py +++ b/test/lib/ansible_test/_internal/venv.py @@ -20,6 +20,7 @@ from .util import ( remove_tree, ApplicationError, str_to_version, + raw_command, ) from .util_common import ( @@ -92,7 +93,7 @@ def create_virtual_environment(args, # type: EnvironmentConfig # creating a virtual environment using 'venv' when running in a virtual environment created by 'virtualenv' results # in a copy of the original virtual environment instead of creation of a new one # avoid this issue by only using "real" python interpreters to invoke 'venv' - for real_python in iterate_real_pythons(args, python.version): + for real_python in iterate_real_pythons(python.version): if run_venv(args, real_python, system_site_packages, pip, path): display.info('Created Python %s virtual environment using "venv": %s' % (python.version, path), verbosity=1) return True @@ -128,7 +129,7 @@ def create_virtual_environment(args, # type: EnvironmentConfig return False -def iterate_real_pythons(args, version): # type: (EnvironmentConfig, str) -> t.Iterable[str] +def iterate_real_pythons(version): # type: (str) -> t.Iterable[str] """ Iterate through available real python interpreters of the requested version. The current interpreter will be checked and then the path will be searched. @@ -138,7 +139,7 @@ def iterate_real_pythons(args, version): # type: (EnvironmentConfig, str) -> t. if version_info == sys.version_info[:len(version_info)]: current_python = sys.executable - real_prefix = get_python_real_prefix(args, current_python) + real_prefix = get_python_real_prefix(current_python) if real_prefix: current_python = find_python(version, os.path.join(real_prefix, 'bin')) @@ -159,7 +160,7 @@ def iterate_real_pythons(args, version): # type: (EnvironmentConfig, str) -> t. if found_python == current_python: return - real_prefix = get_python_real_prefix(args, found_python) + real_prefix = get_python_real_prefix(found_python) if real_prefix: found_python = find_python(version, os.path.join(real_prefix, 'bin')) @@ -168,12 +169,12 @@ def iterate_real_pythons(args, version): # type: (EnvironmentConfig, str) -> t. yield found_python -def get_python_real_prefix(args, python_path): # type: (EnvironmentConfig, str) -> t.Optional[str] +def get_python_real_prefix(python_path): # type: (str) -> t.Optional[str] """ Return the real prefix of the specified interpreter or None if the interpreter is not a virtual environment created by 'virtualenv'. """ cmd = [python_path, os.path.join(os.path.join(ANSIBLE_TEST_TARGET_TOOLS_ROOT, 'virtualenvcheck.py'))] - check_result = json.loads(run_command(args, cmd, capture=True, always=True)[0]) + check_result = json.loads(raw_command(cmd, capture=True)[0]) real_prefix = check_result['real_prefix'] return real_prefix diff --git a/test/lib/ansible_test/_util/controller/sanity/code-smell/changelog.py b/test/lib/ansible_test/_util/controller/sanity/code-smell/changelog.py index fe2ba5e372..983eaeb426 100644 --- a/test/lib/ansible_test/_util/controller/sanity/code-smell/changelog.py +++ b/test/lib/ansible_test/_util/controller/sanity/code-smell/changelog.py @@ -47,7 +47,11 @@ def main(): env = os.environ.copy() env.update(PYTHONPATH='%s:%s' % (os.path.join(os.path.dirname(__file__), 'changelog'), env['PYTHONPATH'])) - subprocess.call(cmd, env=env) # ignore the return code, rely on the output instead + # ignore the return code, rely on the output instead + process = subprocess.run(cmd, stdin=subprocess.DEVNULL, capture_output=True, text=True, env=env, check=False) + + sys.stdout.write(process.stdout) + sys.stderr.write(process.stderr) if __name__ == '__main__': diff --git a/test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/main.py b/test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/main.py index 0bdd9dee21..f18477d529 100644 --- a/test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/main.py +++ b/test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/main.py @@ -436,14 +436,13 @@ class ModuleValidator(Validator): base_path = self._get_base_branch_module_path() command = ['git', 'show', '%s:%s' % (self.base_branch, base_path or self.path)] - p = subprocess.Popen(command, stdout=subprocess.PIPE, - stderr=subprocess.PIPE) - stdout, stderr = p.communicate() + p = subprocess.run(command, stdin=subprocess.DEVNULL, capture_output=True, check=False) + if int(p.returncode) != 0: return None t = tempfile.NamedTemporaryFile(delete=False) - t.write(stdout) + t.write(p.stdout) t.close() return t.name @@ -2456,11 +2455,12 @@ class GitCache: @staticmethod def _git(args): cmd = ['git'] + args - p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) - stdout, stderr = p.communicate() + p = subprocess.run(cmd, stdin=subprocess.DEVNULL, capture_output=True, text=True, check=False) + if p.returncode != 0: - raise GitError(stderr, p.returncode) - return stdout.decode('utf-8').splitlines() + raise GitError(p.stderr, p.returncode) + + return p.stdout.splitlines() class GitError(Exception): diff --git a/test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/module_args.py b/test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/module_args.py index ee938142ff..03a1401927 100644 --- a/test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/module_args.py +++ b/test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/module_args.py @@ -122,14 +122,12 @@ def get_ps_argument_spec(filename, collection): }) script_path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'ps_argspec.ps1') - proc = subprocess.Popen(['pwsh', script_path, util_manifest], stdout=subprocess.PIPE, stderr=subprocess.PIPE, - shell=False) - stdout, stderr = proc.communicate() + proc = subprocess.run(['pwsh', script_path, util_manifest], stdin=subprocess.DEVNULL, capture_output=True, text=True, check=False) if proc.returncode != 0: - raise AnsibleModuleImportError("STDOUT:\n%s\nSTDERR:\n%s" % (stdout.decode('utf-8'), stderr.decode('utf-8'))) + raise AnsibleModuleImportError("STDOUT:\n%s\nSTDERR:\n%s" % (proc.stdout, proc.stderr)) - kwargs = json.loads(stdout) + kwargs = json.loads(proc.stdout) # the validate-modules code expects the options spec to be under the argument_spec key not options as set in PS kwargs['argument_spec'] = kwargs.pop('options', {}) diff --git a/test/lib/ansible_test/_util/target/cli/ansible_test_cli_stub.py b/test/lib/ansible_test/_util/target/cli/ansible_test_cli_stub.py index 9520949308..930654fc1e 100755 --- a/test/lib/ansible_test/_util/target/cli/ansible_test_cli_stub.py +++ b/test/lib/ansible_test/_util/target/cli/ansible_test_cli_stub.py @@ -27,6 +27,9 @@ def main(args=None): raise SystemExit('This version of ansible-test cannot be executed with Python version %s. Supported Python versions are: %s' % ( version_to_str(sys.version_info[:3]), ', '.join(CONTROLLER_PYTHON_VERSIONS))) + if any(not os.get_blocking(handle.fileno()) for handle in (sys.stdin, sys.stdout, sys.stderr)): + raise SystemExit('Standard input, output and error file handles must be blocking to run ansible-test.') + # noinspection PyProtectedMember from ansible_test._internal import main as cli_main diff --git a/test/lib/ansible_test/_util/target/setup/bootstrap.sh b/test/lib/ansible_test/_util/target/setup/bootstrap.sh index 3eeac1dd03..80ad16c02a 100644 --- a/test/lib/ansible_test/_util/target/setup/bootstrap.sh +++ b/test/lib/ansible_test/_util/target/setup/bootstrap.sh @@ -281,6 +281,39 @@ bootstrap_remote_rhel_pinned_pip_packages() pip_install "${pip_packages}" } +bootstrap_remote_ubuntu() +{ + py_pkg_prefix="python3" + + packages=" + gcc + ${py_pkg_prefix}-dev + ${py_pkg_prefix}-pip + ${py_pkg_prefix}-venv + " + + if [ "${controller}" ]; then + # The resolvelib package is not listed here because the available version (0.8.1) is incompatible with ansible. + # Instead, ansible-test will install it using pip. + packages=" + ${packages} + ${py_pkg_prefix}-cryptography + ${py_pkg_prefix}-jinja2 + ${py_pkg_prefix}-packaging + ${py_pkg_prefix}-yaml + " + fi + + while true; do + # shellcheck disable=SC2086 + apt-get update -qq -y && \ + DEBIAN_FRONTEND=noninteractive apt-get install -qq -y --no-install-recommends ${packages} \ + && break + echo "Failed to install packages. Sleeping before trying again..." + sleep 10 + done +} + bootstrap_docker() { # Required for newer mysql-server packages to install/upgrade on Ubuntu 16.04. @@ -299,6 +332,7 @@ bootstrap_remote() "freebsd") bootstrap_remote_freebsd ;; "macos") bootstrap_remote_macos ;; "rhel") bootstrap_remote_rhel ;; + "ubuntu") bootstrap_remote_ubuntu ;; esac done } diff --git a/test/sanity/code-smell/docs-build.py b/test/sanity/code-smell/docs-build.py index 9461620a9a..aaa69378c7 100644 --- a/test/sanity/code-smell/docs-build.py +++ b/test/sanity/code-smell/docs-build.py @@ -29,13 +29,12 @@ def main(): try: cmd = ['make', 'core_singlehtmldocs'] - sphinx = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, cwd=docs_dir) - stdout, stderr = sphinx.communicate() + sphinx = subprocess.run(cmd, stdin=subprocess.DEVNULL, capture_output=True, cwd=docs_dir, check=False, text=True) finally: shutil.move(tmp, requirements_txt) - stdout = stdout.decode('utf-8') - stderr = stderr.decode('utf-8') + stdout = sphinx.stdout + stderr = sphinx.stderr if sphinx.returncode != 0: sys.stderr.write("Command '%s' failed with status code: %d\n" % (' '.join(cmd), sphinx.returncode)) diff --git a/test/sanity/code-smell/package-data.py b/test/sanity/code-smell/package-data.py index 8a7e4b5c68..5497b7cae8 100644 --- a/test/sanity/code-smell/package-data.py +++ b/test/sanity/code-smell/package-data.py @@ -172,14 +172,15 @@ def clean_repository(file_list): def create_sdist(tmp_dir): """Create an sdist in the repository""" - create = subprocess.Popen( + create = subprocess.run( ['make', 'snapshot', 'SDIST_DIR=%s' % tmp_dir], - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - universal_newlines=True, + stdin=subprocess.DEVNULL, + capture_output=True, + text=True, + check=False, ) - stderr = create.communicate()[1] + stderr = create.stderr if create.returncode != 0: raise Exception('make snapshot failed:\n%s' % stderr) @@ -220,15 +221,16 @@ def extract_sdist(sdist_path, tmp_dir): def install_sdist(tmp_dir, sdist_dir): """Install the extracted sdist into the temporary directory""" - install = subprocess.Popen( + install = subprocess.run( ['python', 'setup.py', 'install', '--root=%s' % tmp_dir], - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - universal_newlines=True, + stdin=subprocess.DEVNULL, + capture_output=True, + text=True, cwd=os.path.join(tmp_dir, sdist_dir), + check=False, ) - stdout, stderr = install.communicate() + stdout, stderr = install.stdout, install.stderr if install.returncode != 0: raise Exception('sdist install failed:\n%s' % stderr) diff --git a/test/units/test_no_tty.py b/test/units/test_no_tty.py new file mode 100644 index 0000000000..290c0b922a --- /dev/null +++ b/test/units/test_no_tty.py @@ -0,0 +1,7 @@ +import sys + + +def test_no_tty(): + assert not sys.stdin.isatty() + assert not sys.stdout.isatty() + assert not sys.stderr.isatty() |