Skip to content

Commit

Permalink
Display stderr and stdout when run_command fails
Browse files Browse the repository at this point in the history
Throwing a `CalledProcessError` would hide the
messages when converted into a `Xenapi.Failure`
use a generic `Exception` with a properly formed
detail instead.

Signed-off-by: BenjiReis <[email protected]>
  • Loading branch information
benjamreis committed Oct 3, 2023
1 parent e9415e0 commit 0a2d4df
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 4 deletions.
4 changes: 2 additions & 2 deletions SOURCES/etc/xapi.d/plugins/raid.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import XenAPIPlugin

sys.path.append('.')
from xcpngutils import configure_logging, run_command, error_wrapped
from xcpngutils import configure_logging, run_command, error_wrapped, ProcessException
from xcpngutils.operationlocker import OperationLocker

_LOGGER = configure_logging('raid')
Expand All @@ -21,7 +21,7 @@ def check_raid_pool(session, args):
with OperationLocker():
try:
result = run_command(['mdadm', '--detail', device])
except subprocess.CalledProcessError:
except ProcessException:
# No RAID
return json.dumps({})

Expand Down
17 changes: 16 additions & 1 deletion SOURCES/etc/xapi.d/plugins/xcpngutils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,22 @@ def log_unhandled_exception(origin, exception_type, exception_value,
return logger


class ProcessException(Exception):
def __init__(self, code, command, stderr, stdout):
self.returncode = code
self.command = command
self.stderr = stderr
self.stdout = stdout

def __str__(self):
return f"Command '{self.command}' failed with code: {self.returncode}"

def run_command(command, check=True):
process = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
stdout, stderr = process.communicate()
code = process.returncode
if check and code != 0:
raise subprocess.CalledProcessError(code, command, None)
raise ProcessException(code, command, stderr, stdout)
result = {'stdout': stdout, 'stderr': stderr, 'command': command, 'returncode': code}
return result

Expand Down Expand Up @@ -123,6 +133,11 @@ def wrapper(*args, **kwds):
except EnvironmentError as e:
message = e.strerror if e.strerror is not None else str(e.args)
raise XenAPIPlugin.Failure(str(e.errno), [message, str(e.filename), traceback.format_exc()])
except ProcessException as e:
raise_plugin_error(
e.returncode, str(e),
detail=f"stdout: '{e.stdout}', stderr: '{e.stderr}'", backtrace=traceback.format_exc()
)
except Exception as e:
raise_plugin_error('-1', str(e), backtrace=traceback.format_exc())

Expand Down
3 changes: 2 additions & 1 deletion tests/test_raid.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import XenAPIPlugin

from raid import check_raid_pool
from xcpngutils import ProcessException

MDADM_DETAIL_CMD = ['mdadm', '--detail', '/dev/md127']

Expand Down Expand Up @@ -61,7 +62,7 @@ def test_raid_error(self, run_command, fs):
assert e.value.params[1] == 'Error!'

def test_raid_missing(self, run_command, fs):
run_command.side_effect = subprocess.CalledProcessError(1, MDADM_DETAIL_CMD, None)
run_command.side_effect = ProcessException(1, MDADM_DETAIL_CMD, "stderr", "stdout")
res = check_raid_pool(None, None)
assert json.loads(res) == json.loads("{}")
run_command.assert_called_once_with(MDADM_DETAIL_CMD)
Expand Down

0 comments on commit 0a2d4df

Please sign in to comment.