On 2.12.2015 13:44, Petr Spacek wrote:
On 2.12.2015 13:23, Jan Cholasta wrote:
On 2.12.2015 12:54, Petr Spacek wrote:
On 2.12.2015 12:51, Christian Heimes wrote:
On 2015-12-02 08:37, Petr Spacek wrote:
On 1.12.2015 18:42, Christian Heimes wrote:
From 33be1f56a64e53d261a1058c4606a7e48c0aac52 Mon Sep 17 00:00:00 2001
From: Christian Heimes <chei...@redhat.com>
Date: Tue, 1 Dec 2015 15:49:53 +0100
Subject: [PATCH 25] Improve error logging for Dogtag subsystem installation
In the case of a failed installation or uninstallation of a Dogtag
subsystem, the error output of pkispawn / pkidestroyed are now shown to
the user. It makes it more obvious what went wrong and makes it easier
to debug a problem.
The error handler also attempts to get the full name of the installation
/ uninstallation log file from stdout. pkispawn and pkidestroy print the
absolute name as 'Log file: /path/to/file.log'. The user no longer has
to guess the right log file.
Example:
[1/8]: configuring KRA instance
Failed to configure KRA instance: Command ''/usr/sbin/pkispawn' '-s'
'KRA' '-f' '/tmp/tmp1UpbwF'' returned non-zero exit status 1
pkispawn : ERROR ....... PKI subsystem 'KRA' for instance
'pki-tomcat' already exists!
See the installation logs and the following files/directories for more
information:
/var/log/pki/pki-tomcat
/var/log/pki/pki-kra-spawn.20151201151735.log
[error] RuntimeError: KRA configuration failed.
The patch also changes a couple of modules that were using
the CalledProcessError exception object from subprocess instead of
ipautil.
I'm wondering if ipautil.run() can log stdout and stderr on log level ERROR
when return code is non-zero (and log on level DEBUG as usual when return
code
is zero).
IMHO it would be nicer, universal, and does not require any changes in places
calling ipautil.run().
I think it's a bit confusing to print out stdout and stderr, because
both streams are captured separately. The output is missing its
chronological order. subprocess can capture stdout and stderr in the
same stream, but then we can't distinguish between output and error
output...
I do not think it is a problem if these two are clearly marked as such:
standard output: %s (if non-empty)
stanrard error output: %s (if non-empty)
We do not want to log with level ERROR by default when rc != 0, because some
commands generate a *lot* of output.
I do not agree, but whatever. Somebody needs to review the original
Christian's patch.
We had a short discussion about this with Petr offline and we agreed
that a reasonable compromise would be to log the last few lines of
stderr with ERROR level when a command fails.
This would mean adding custom __str__() to CalledProcessError, so that
the stderr tail is logged when the CalledProcessError is not handled,
and also logging it from ipautil.run() if raiseonerr == False.
--
Jan Cholasta
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
