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.

changelogs/fragments/ansible-test-shell-features.yml

@@ -0,0 +1,7 @@
+minor_changes:
+  - ansible-test - Add support for running non-interactive commands with ``ansible-test shell``.
+  - ansible-test - Add support for exporting inventory with ``ansible-test shell --export {path}``.
+  - ansible-test - The ``shell`` command can be used outside a collection if no controller delegation is required.
+  - ansible-test - Improve consistency of output messages by using stdout or stderr for most output, but not both.
+bugfixes:
+  - ansible-test - Sanity test output with the ``--lint`` option is no longer mixed in with bootstrapping output.

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)

test/lib/ansible_test/_internal/cli/commands/shell.py

@@ -38,10 +38,22 @@ 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

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

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]

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:

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

@@ -39,13 +39,18 @@ 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 sys.stdin.isatty():
+    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
@@ -61,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):

test/lib/ansible_test/_internal/config.py

@@ -214,8 +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):
@@ -231,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
@@ -270,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."""

test/lib/ansible_test/_internal/connections.py

@@ -50,6 +50,7 @@ class Connection(metaclass=abc.ABCMeta):
             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."""
 
@@ -97,6 +98,7 @@ class LocalConnection(Connection):
             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(
@@ -107,6 +109,7 @@ class LocalConnection(Connection):
             stdin=stdin,
             stdout=stdout,
             interactive=interactive,
+            force_stdout=force_stdout,
         )
 
 
@@ -137,6 +140,7 @@ class SshConnection(Connection):
             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)
@@ -170,6 +174,7 @@ class SshConnection(Connection):
                 stdin=stdin,
                 stdout=stdout,
                 interactive=interactive,
+                force_stdout=force_stdout,
                 error_callback=error_callback,
             )
 
@@ -217,6 +222,7 @@ class DockerConnection(Connection):
             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 = []
@@ -237,6 +243,7 @@ class DockerConnection(Connection):
             stdin=stdin,
             stdout=stdout,
             interactive=interactive,
+            force_stdout=force_stdout,
         )
 
     def inspect(self):  # type: () -> DockerInspect

test/lib/ansible_test/_internal/docker_util.py

@@ -515,6 +515,7 @@ def docker_exec(
         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."""
@@ -524,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, interactive=interactive, 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]
@@ -546,15 +548,19 @@ def docker_command(
         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, interactive=interactive, 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]

test/lib/ansible_test/_internal/host_profiles.py

@@ -150,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)
 

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."""

test/lib/ansible_test/_internal/util.py

@@ -330,6 +330,7 @@ def raw_command(
         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]]
@@ -353,6 +354,12 @@ def raw_command(
     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()
 
@@ -440,7 +447,8 @@ def raw_command(
 
         if communicate:
             data_bytes = to_optional_bytes(data)
-            stdout_bytes, stderr_bytes = communicate_with_process(process, data_bytes, stdout == subprocess.PIPE, stderr == subprocess.PIPE, capture=capture)
+            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:
@@ -463,7 +471,14 @@ 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) -> t.Tuple[bytes, bytes]:
+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]
@@ -477,13 +492,13 @@ def communicate_with_process(process: subprocess.Popen, stdin: t.Optional[bytes]
         threads.append(WriterThread(process.stdin, stdin))
 
     if stdout:
-        stdout_reader = reader(process.stdout)
+        stdout_reader = reader(process.stdout, force_stdout)
         threads.append(stdout_reader)
     else:
         stdout_reader = None
 
     if stderr:
-        stderr_reader = reader(process.stderr)
+        stderr_reader = reader(process.stderr, force_stdout)
         threads.append(stderr_reader)
     else:
         stderr_reader = None
@@ -531,10 +546,11 @@ class WriterThread(WrappedThread):
 
 class ReaderThread(WrappedThread, metaclass=abc.ABCMeta):
     """Thread to read stdout from a subprocess."""
-    def __init__(self, handle: t.IO[bytes]) -> None:
+    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
@@ -561,7 +577,7 @@ class OutputThread(ReaderThread):
     def _run(self) -> None:
         """Workload to run on a thread."""
         src = self.handle
-        dst = sys.stdout.buffer
+        dst = sys.stdout.buffer if self.force_stdout else display.fd.buffer
 
         try:
             for line in src:
@@ -740,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
@@ -752,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."""
@@ -780,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."""
@@ -816,6 +836,8 @@ 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()
 

test/lib/ansible_test/_internal/util_common.py

@@ -127,6 +127,7 @@ class CommonConfig:
     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
@@ -136,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()
 
@@ -408,6 +409,7 @@ def run_command(
         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]]
@@ -415,7 +417,7 @@ def run_command(
     """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, interactive=interactive,
-                       cmd_verbosity=cmd_verbosity, str_errors=str_errors, error_callback=error_callback)
+                       force_stdout=force_stdout, cmd_verbosity=cmd_verbosity, str_errors=str_errors, error_callback=error_callback)
 
 
 def yamlcheck(python):