summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMatt Clay <matt@mystile.com>2022-04-25 21:39:09 +0200
committerGitHub <noreply@github.com>2022-04-25 21:39:09 +0200
commit62d03c8e752ee35057031a91d7028e0a2e5d43e4 (patch)
treeb03ed52d38dfabb54be656acc10ced6254ae36db
parentFix use of deprecated antsibull-docs option. (diff)
downloadansible-62d03c8e752ee35057031a91d7028e0a2e5d43e4.tar.xz
ansible-62d03c8e752ee35057031a91d7028e0a2e5d43e4.zip
ansible-test - Fix subprocess management. (#77638)
* 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.
Diffstat (limited to '')
-rw-r--r--changelogs/fragments/ansible-test-subprocess-isolation.yml10
-rw-r--r--test/integration/targets/ansible-test-no-tty/aliases2
-rwxr-xr-xtest/integration/targets/ansible-test-no-tty/runme.py7
-rwxr-xr-xtest/integration/targets/ansible-test-no-tty/runme.sh5
-rw-r--r--test/lib/ansible_test/_internal/ansible_util.py6
-rw-r--r--test/lib/ansible_test/_internal/commands/coverage/__init__.py11
-rw-r--r--test/lib/ansible_test/_internal/commands/coverage/combine.py4
-rw-r--r--test/lib/ansible_test/_internal/commands/integration/cloud/cs.py2
-rw-r--r--test/lib/ansible_test/_internal/commands/sanity/__init__.py1
-rw-r--r--test/lib/ansible_test/_internal/commands/sanity/pylint.py2
-rw-r--r--test/lib/ansible_test/_internal/commands/sanity/validate_modules.py2
-rw-r--r--test/lib/ansible_test/_internal/commands/shell/__init__.py6
-rw-r--r--test/lib/ansible_test/_internal/config.py1
-rw-r--r--test/lib/ansible_test/_internal/connections.py16
-rw-r--r--test/lib/ansible_test/_internal/core_ci.py2
-rw-r--r--test/lib/ansible_test/_internal/delegation.py13
-rw-r--r--test/lib/ansible_test/_internal/docker_util.py8
-rw-r--r--test/lib/ansible_test/_internal/host_profiles.py10
-rw-r--r--test/lib/ansible_test/_internal/util.py20
-rw-r--r--test/lib/ansible_test/_internal/util_common.py4
-rw-r--r--test/lib/ansible_test/_internal/venv.py13
-rw-r--r--test/lib/ansible_test/_util/controller/sanity/code-smell/changelog.py6
-rw-r--r--test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/main.py16
-rw-r--r--test/lib/ansible_test/_util/controller/sanity/validate-modules/validate_modules/module_args.py8
-rwxr-xr-xtest/lib/ansible_test/_util/target/cli/ansible_test_cli_stub.py3
-rw-r--r--test/sanity/code-smell/docs-build.py7
-rw-r--r--test/sanity/code-smell/package-data.py22
-rw-r--r--test/units/test_no_tty.py7
28 files changed, 148 insertions, 66 deletions
diff --git a/changelogs/fragments/ansible-test-subprocess-isolation.yml b/changelogs/fragments/ansible-test-subprocess-isolation.yml
new file mode 100644
index 0000000000..3be259d608
--- /dev/null
+++ b/changelogs/fragments/ansible-test-subprocess-isolation.yml
@@ -0,0 +1,10 @@
+bugfixes:
+ - ansible-test - Subprocesses are now isolated from the stdin, stdout and stderr of ansible-test.
+ This avoids issues with subprocesses tampering with the file descriptors, such as SSH making them non-blocking.
+ As a result of this change, subprocess output from unit and integration tests on stderr now go to stdout.
+ - ansible-test - Subprocesses no longer have access to the TTY ansible-test is connected to, if any.
+ This maintains consistent behavior between local testing and CI systems, which typically do not provide a TTY.
+ Tests which require a TTY should use pexpect or another mechanism to create a PTY.
+minor_changes:
+ - ansible-test - Blocking mode is now enforced for stdin, stdout and stderr.
+ If any of these are non-blocking then ansible-test will exit during startup with an error.
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/_internal/ansible_util.py b/test/lib/ansible_test/_internal/ansible_util.py
index a3582dc89f..c6f231bc7d 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')
diff --git a/test/lib/ansible_test/_internal/commands/coverage/__init__.py b/test/lib/ansible_test/_internal/commands/coverage/__init__.py
index 8cae31298c..3d27b64bb6 100644
--- a/test/lib/ansible_test/_internal/commands/coverage/__init__.py
+++ b/test/lib/ansible_test/_internal/commands/coverage/__init__.py
@@ -100,7 +100,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/combine.py b/test/lib/ansible_test/_internal/commands/coverage/combine.py
index 3356cc857c..ea1851a1c9 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,
)
@@ -196,7 +196,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/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/sanity/__init__.py b/test/lib/ansible_test/_internal/commands/sanity/__init__.py
index d819c37e33..7b7e6196fe 100644
--- a/test/lib/ansible_test/_internal/commands/sanity/__init__.py
+++ b/test/lib/ansible_test/_internal/commands/sanity/__init__.py
@@ -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..c518af94c9 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 (
@@ -44,6 +45,9 @@ def command_shell(args): # type: (ShellConfig) -> None
if args.raw and isinstance(args.targets[0], ControllerConfig):
raise ApplicationError('The --raw option has no effect on the controller.')
+ if 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:
@@ -87,4 +91,4 @@ def command_shell(args): # type: (ShellConfig) -> None
else:
cmd = []
- con.run(cmd)
+ con.run(cmd, interactive=True)
diff --git a/test/lib/ansible_test/_internal/config.py b/test/lib/ansible_test/_internal/config.py
index 0a14a806ca..5da5fc00e4 100644
--- a/test/lib/ansible_test/_internal/config.py
+++ b/test/lib/ansible_test/_internal/config.py
@@ -238,6 +238,7 @@ class ShellConfig(EnvironmentConfig):
super().__init__(args, 'shell')
self.raw = args.raw # type: bool
+ self.interactive = True
class SanityConfig(TestConfig):
diff --git a/test/lib/ansible_test/_internal/connections.py b/test/lib/ansible_test/_internal/connections.py
index 14234b2d93..b0c7d948ad 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
@@ -47,6 +46,7 @@ class Connection(metaclass=abc.ABCMeta):
def run(self,
command, # type: t.List[str]
capture=False, # 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]]
@@ -60,7 +60,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 +82,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):
@@ -93,6 +93,7 @@ class LocalConnection(Connection):
def run(self,
command, # type: t.List[str]
capture=False, # 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]]
@@ -105,6 +106,7 @@ class LocalConnection(Connection):
data=data,
stdin=stdin,
stdout=stdout,
+ interactive=interactive,
)
@@ -131,6 +133,7 @@ class SshConnection(Connection):
def run(self,
command, # type: t.List[str]
capture=False, # 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]]
@@ -143,7 +146,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 +169,7 @@ class SshConnection(Connection):
data=data,
stdin=stdin,
stdout=stdout,
+ interactive=interactive,
error_callback=error_callback,
)
@@ -209,6 +213,7 @@ class DockerConnection(Connection):
def run(self,
command, # type: t.List[str]
capture=False, # 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]]
@@ -219,7 +224,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 +236,7 @@ class DockerConnection(Connection):
data=data,
stdin=stdin,
stdout=stdout,
+ interactive=interactive,
)
def inspect(self): # type: () -> DockerInspect
diff --git a/test/lib/ansible_test/_internal/core_ci.py b/test/lib/ansible_test/_internal/core_ci.py
index dbb428aeeb..37e4ac061f 100644
--- a/test/lib/ansible_test/_internal/core_ci.py
+++ b/test/lib/ansible_test/_internal/core_ci.py
@@ -469,7 +469,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
diff --git a/test/lib/ansible_test/_internal/delegation.py b/test/lib/ansible_test/_internal/delegation.py
index 6298bf2405..7ee5bd971d 100644
--- a/test/lib/ansible_test/_internal/delegation.py
+++ b/test/lib/ansible_test/_internal/delegation.py
@@ -160,11 +160,12 @@ 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(['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']))
container = con.inspect()
@@ -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), 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..b8bc838c4b 100644
--- a/test/lib/ansible_test/_internal/docker_util.py
+++ b/test/lib/ansible_test/_internal/docker_util.py
@@ -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(
@@ -514,6 +514,7 @@ def docker_exec(
capture=False, # type: bool
stdin=None, # type: t.Optional[t.IO[bytes]]
stdout=None, # type: t.Optional[t.IO[bytes]]
+ interactive=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 +524,7 @@ 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, data=data)
def docker_info(args): # type: (CommonConfig) -> t.Dict[str, t.Any]
@@ -544,6 +545,7 @@ def docker_command(
capture=False, # type: bool
stdin=None, # type: t.Optional[t.IO[bytes]]
stdout=None, # type: t.Optional[t.IO[bytes]]
+ interactive=False, # type: bool
always=False, # type: bool
data=None, # type: t.Optional[str]
): # type: (...) -> t.Tuple[t.Optional[str], t.Optional[str]]
@@ -552,7 +554,7 @@ def docker_command(
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, data=data)
def docker_environment(): # type: () -> t.Dict[str, str]
diff --git a/test/lib/ansible_test/_internal/host_profiles.py b/test/lib/ansible_test/_internal/host_profiles.py
index 9079c7e924..e5a3dbd825 100644
--- a/test/lib/ansible_test/_internal/host_profiles.py
+++ b/test/lib/ansible_test/_internal/host_profiles.py
@@ -484,8 +484,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
@@ -717,8 +718,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/util.py b/test/lib/ansible_test/_internal/util.py
index 9f1b7a4fa7..a0fd6fa70d 100644
--- a/test/lib/ansible_test/_internal/util.py
+++ b/test/lib/ansible_test/_internal/util.py
@@ -261,6 +261,7 @@ def raw_command(
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
cmd_verbosity=1, # type: int
str_errors='strict', # type: str
error_callback=None, # type: t.Optional[t.Callable[[SubprocessError], None]]
@@ -274,6 +275,14 @@ def raw_command(
cmd = list(cmd)
+ if not capture and not interactive:
+ # When not capturing stdout/stderr and not running interactively, send subprocess stdout/stderr through a shell to the current stdout.
+ # Additionally send output through a pipe to `tee` to block access to the current TTY, if any.
+ # This prevents subprocesses from sharing stdin/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.
+ cmd = ['/bin/sh', '-c', f'{shlex.join(cmd)} 2>&1 | tee']
+
escaped_cmd = ' '.join(shlex.quote(c) for c in cmd)
display.info('Run command: %s' % escaped_cmd, verbosity=cmd_verbosity, truncate=True)
@@ -298,6 +307,10 @@ def raw_command(
elif data is not None:
stdin = subprocess.PIPE
communicate = True
+ elif interactive:
+ pass # allow the subprocess access to our stdin
+ else:
+ stdin = subprocess.DEVNULL
if stdout:
communicate = True
@@ -654,12 +667,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..2f68a6dfe5 100644
--- a/test/lib/ansible_test/_internal/util_common.py
+++ b/test/lib/ansible_test/_internal/util_common.py
@@ -126,6 +126,7 @@ class CommonConfig:
"""Configuration common to all commands."""
def __init__(self, args, command): # type: (t.Any, str) -> None
self.command = command
+ self.interactive = False
self.success = None # type: t.Optional[bool]
self.color = args.color # type: bool
@@ -406,13 +407,14 @@ def run_command(
always=False, # type: bool
stdin=None, # type: t.Optional[t.IO[bytes]]
stdout=None, # type: t.Optional[t.IO[bytes]]
+ interactive=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,
+ return raw_command(cmd, capture=capture, env=env, data=data, cwd=cwd, explain=explain, stdin=stdin, stdout=stdout, interactive=interactive,
cmd_verbosity=cmd_verbosity, str_errors=str_errors, error_callback=error_callback)
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/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 110b163562..e707b0eb4f 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()