Surface ES startup errors to the Rally CLI console (#1476)

This change uses the subprocess.run function introduced in Python 3.5 to check the subprocess and print its output to the console on failure. There is some background to why we do not do this currently in #879, but I believe the output is worthwhile, and at the time I think 3.5 was very recently our minimum Python version so this option may have been overlooked... or there may be a more technical reason I am missing.

esrally/mechanic/launcher.py

@@ -18,12 +18,13 @@
 import os
 import shlex
 import subprocess
+from textwrap import indent
 
 import psutil
 
 from esrally import exceptions, telemetry, time
 from esrally.mechanic import cluster, java_resolver
-from esrally.utils import io, opts, process
+from esrally.utils import console, io, opts, process
 
 
 class DockerLauncher:
@@ -199,13 +200,9 @@ def _set_env(self, env, k, v, separator=" ", prepend=False):
     @staticmethod
     def _run_subprocess(command_line, env):
         command_line_args = shlex.split(command_line)
-
-        with subprocess.Popen(
-            command_line_args, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, env=env, start_new_session=True
-        ) as command_line_process:
-            # wait for it to finish
-            command_line_process.wait()
-        return command_line_process.returncode
+        subprocess.run(
+            command_line_args, stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=env, start_new_session=True, check=True, text=True
+        )
 
     @staticmethod
     def _start_process(binary_path, env):
@@ -215,11 +212,11 @@ def _start_process(binary_path, env):
         cmd = [io.escape_path(os.path.join(".", "bin", "elasticsearch"))]
         pid_path = io.escape_path(os.path.join(".", "pid"))
         cmd.extend(["-d", "-p", pid_path])
-        ret = ProcessLauncher._run_subprocess(command_line=" ".join(cmd), env=env)
-        if ret != 0:
-            msg = "Daemon startup failed with exit code [{}]".format(ret)
-            logging.error(msg)
-            raise exceptions.LaunchError(msg)
+        try:
+            ProcessLauncher._run_subprocess(command_line=" ".join(cmd), env=env)
+        except subprocess.CalledProcessError as e:
+            console.error("Daemon startup failed with exit code [%s]. STDERR:\n\n%s\n" % (e.returncode, indent(e.stderr, "\t")))
+            raise e
 
         return wait_for_pidfile(pid_path)
 

tests/driver/runner_test.py

@@ -6215,7 +6215,7 @@ async def test_adds_request_timings(self):
         assert len(timings) == 3
 
         assert timings[0]["operation"] == "initial-call"
-        assert timings[0]["service_time"] == pytest.approx(0.1, abs=0.1)
+        assert timings[0]["service_time"] == pytest.approx(0.1, abs=0.15)
 
         assert timings[1]["operation"] == "stream-a"
         assert timings[1]["service_time"] == pytest.approx(0.2, abs=0.1)

tests/mechanic/launcher_test.py

@@ -97,6 +97,7 @@ def __init__(self, *args, **kwargs):
         # mocking them as their optional functionality is disabled.
         self.returncode = 1
         self.stdout = io.StringIO()
+        self.args = None
 
     def communicate(self, input=None, timeout=None):
         return [b"", b""]