On 2015-12-03 11:04, Jan Cholasta wrote: > 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.
Yes, that sounds like a reasonable idea. In the default case (raiseonerr == True) ipautil.run() returns a custom CalledProcessError exception that prints the command and the last two or three non-empty lines from stderr. Callers can either log the exception directly or format the out as they see fit. With raiseonerr == False and exit code != 0 the same information is logged with log level ERROR. I can just create the exception object and log its string representation without raising the exception. Christian
signature.asc
Description: OpenPGP digital signature
-- 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
