Better errors for missing executable

When trying to run a missing executable (e.g. developer forgot to
installed a required tool) we logged a generic error that makes it
harder to understand and the debug the issue:

    $ addons/velero/start velero
    Deploying velero
    Traceback (most recent call last):
      File "/home/nsoffer/src/ramen/test/addons/velero/start", line 38, in <module>
        deploy(cluster)
      File "/home/nsoffer/src/ramen/test/addons/velero/start", line 16, in deploy
        for line in commands.watch(
      File "/home/nsoffer/src/ramen/test/drenv/commands.py", line 96, in watch
        with subprocess.Popen(
             ^^^^^^^^^^^^^^^^^
      File "/usr/lib64/python3.11/subprocess.py", line 1026, in __init__
        self._execute_child(args, executable, preexec_fn, close_fds,
      File "/usr/lib64/python3.11/subprocess.py", line 1950, in _execute_child
        raise child_exception_type(errno_num, err_msg, err_filename)
    FileNotFoundError: [Errno 2] No such file or directory: 'velero'

There is no information on the code code that failed inside the script,
and in particular, the command we tried to run.

This is caused by not handling the error when subprocess.Popen() fail to
execute the child process. Now we raise the standard `commands.Error`,
preserving the traceback from the underlying error.

Example error with this change:

    $ addons/velero/start velero
    Deploying velero
    Traceback (most recent call last):
      File "/home/nsoffer/src/ramen/test/addons/velero/start", line 38, in <module>
        deploy(cluster)
      File "/home/nsoffer/src/ramen/test/addons/velero/start", line 16, in deploy
        for line in commands.watch(
      File "/home/nsoffer/src/ramen/test/drenv/commands.py", line 119, in watch
        raise Error(args, f"Could not execute: {e}").with_exception(e)
      File "/home/nsoffer/src/ramen/test/drenv/commands.py", line 111, in watch
        p = subprocess.Popen(
            ^^^^^^^^^^^^^^^^^
      File "/usr/lib64/python3.11/subprocess.py", line 1026, in __init__
        self._execute_child(args, executable, preexec_fn, close_fds,
      File "/usr/lib64/python3.11/subprocess.py", line 1950, in _execute_child
        raise child_exception_type(errno_num, err_msg, err_filename)
    drenv.commands.Error: Command failed:
       command: ('velero', 'install', '--provider=aws',
           '--plugins=velero/velero-plugin-for-aws:v1.2.1',
           '--bucket=bucket', '--secret-file=credentials.conf',
           '--use-volume-snapshots=false',
           '--backup-location-config=region=minio,s3ForcePathStyle=true,s3Url=http://192.168.39.8:30000',
           '--kubecontext=velero', '--wait')
       error:
          Could not execute: [Errno 2] No such file or directory: 'velero'

Signed-off-by: Nir Soffer <nsoffer@redhat.com>

test/drenv/commands.py

@@ -20,22 +20,31 @@
 class Error(Exception):
     INDENT = 3 * " "
 
-    def __init__(self, command, exitcode, error, output=None):
+    def __init__(self, command, error, exitcode=None, output=None):
         self.command = command
-        self.exitcode = exitcode
         self.error = error
+        self.exitcode = exitcode
         self.output = output
 
+    def with_exception(self, exc):
+        """
+        Return a new error preserving the traceback from another excpetion.
+        """
+        self.__cause__ = None
+        return self.with_traceback(exc.__traceback__)
+
     def _indent(self, s):
         return textwrap.indent(s, self.INDENT)
 
     def __str__(self):
         lines = [
             "Command failed:\n",
             self._indent(f"command: {self.command}\n"),
-            self._indent(f"exitcode: {self.exitcode}\n"),
         ]
 
+        if self.exitcode is not None:
+            lines.append(self._indent(f"exitcode: {self.exitcode}\n"))
+
         if self.output:
             output = self._indent(self.output.rstrip())
             lines.append(self._indent(f"output:\n{output}\n"))
@@ -58,19 +67,25 @@ def run(*args, input=None):
     fail to raise an error about the failing command. Invalid characters will
     be replaced with unicode replacement character (U+FFFD).
 
-    Raises Error if the child process terminated with non-zero exit code. The
-    error includes all data read from the child process stdout and stderr.
+    Raises Error if creating a child process failed or the child process
+    terminated with non-zero exit code. The error includes all data read from
+    the child process stdout and stderr.
     """
-    cp = subprocess.run(
-        args,
-        input=input.encode() if input else None,
-        stdout=subprocess.PIPE,
-        stderr=subprocess.PIPE,
-    )
+    try:
+        cp = subprocess.run(
+            args,
+            input=input.encode() if input else None,
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE,
+        )
+    except OSError as e:
+        raise Error(args, f"Could not execute: {e}").with_exception(e)
+
     output = cp.stdout.decode()
     if cp.returncode != 0:
         error = cp.stderr.decode(errors="replace")
-        raise Error(args, cp.returncode, error, output=output)
+        raise Error(args, error, exitcode=cp.returncode, output=output)
+
     return output
 
 
@@ -86,20 +101,26 @@ def watch(*args, input=None):
     fail to raise an error about the failing command. Invalid characters will
     be replaced with unicode replacement character (U+FFFD).
 
-    Raises Error if the child process terminated with non-zero exit code. The
-    error includes all data read from the child process stderr.
+    Raises Error if creating a child process failed or the child process
+    terminated with non-zero exit code. The error includes all data read from
+    the child process stderr.
     """
     # Avoid delays in python child process logs.
     env = dict(os.environ)
     env["PYTHONUNBUFFERED"] = "1"
 
-    with subprocess.Popen(
-        args,
-        stdin=subprocess.PIPE if input else None,
-        stdout=subprocess.PIPE,
-        stderr=subprocess.PIPE,
-        env=env,
-    ) as p:
+    try:
+        p = subprocess.Popen(
+            args,
+            stdin=subprocess.PIPE if input else None,
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE,
+            env=env,
+        )
+    except OSError as e:
+        raise Error(args, f"Could not execute: {e}").with_exception(e)
+
+    with p:
         error = bytearray()
         partial = bytearray()
 
@@ -124,7 +145,7 @@ def watch(*args, input=None):
 
     if p.returncode != 0:
         error = error.decode(errors="replace")
-        raise Error(args, p.returncode, error)
+        raise Error(args, error, exitcode=p.returncode)
 
 
 def stream(proc, input=None, bufsize=32 << 10):

test/drenv/commands_test.py

@@ -1,6 +1,7 @@
 # SPDX-FileCopyrightText: The RamenDR authors
 # SPDX-License-Identifier: Apache-2.0
 
+import re
 import subprocess
 from contextlib import contextmanager
 
@@ -183,6 +184,25 @@ def test_watch_no_newline():
     assert output == ["first second last"]
 
 
+def test_watch_error_missing_executable():
+    cmd = ("no-such-executable-in-path",)
+    output = []
+
+    with pytest.raises(commands.Error) as e:
+        for line in commands.watch(*cmd):
+            output.append(line)
+
+    assert output == []
+
+    assert e.value.command == cmd
+    assert e.value.exitcode is None
+    assert e.value.output is None
+    assert re.match(
+        r"Could not execute: .*'no-such-executable-in-path'.*",
+        e.value.error,
+    )
+
+
 def test_watch_error_empty():
     cmd = ("false",)
     output = []
@@ -248,6 +268,20 @@ def test_run_input_non_ascii():
     assert output == "\u05d0"
 
 
+def test_run_error_missing_executable():
+    cmd = ("no-such-executable-in-path",)
+    with pytest.raises(commands.Error) as e:
+        commands.run(*cmd)
+
+    assert e.value.command == cmd
+    assert e.value.exitcode is None
+    assert e.value.output is None
+    assert re.match(
+        r"Could not execute: .*'no-such-executable-in-path'.*",
+        e.value.error,
+    )
+
+
 def test_run_error_empty():
     cmd = ("false",)
     with pytest.raises(commands.Error) as e: