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.


Attachment: signature.asc
Description: OpenPGP digital signature

Manage your subscription for the Freeipa-devel mailing list:
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to