Re: [Freeipa-devel] [PATCH 25] Improve error logging for Dogtag subsystem installation

[Christian Heimes]

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 
>>> 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

Re: [Freeipa-devel] [PATCH 25] Improve error logging for Dogtag subsystem installation

[Jan Cholasta]

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 
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

Re: [Freeipa-devel] [PATCH 25] Improve error logging for Dogtag subsystem installation

[Christian Heimes]

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 
>> 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...

In case of Dogtag stderr contains the relevant error message. In order
to understand the events, that lead to the particular error, a user has
to read the log file anyway -- unless you run pkispawn with '-vv' for
extra verbosity. But then you get pages over pages of debug output on
*stderr*. It's not helpful either.

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

Re: [Freeipa-devel] [PATCH 25] Improve error logging for Dogtag subsystem installation

[Jan Cholasta]

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 
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.


IMO Christian's approach is OK, because it lets the caller decide how to 
log (or not) stderr on failure.





In case of Dogtag stderr contains the relevant error message. In order
to understand the events, that lead to the particular error, a user has
to read the log file anyway -- unless you run pkispawn with '-vv' for
extra verbosity. But then you get pages over pages of debug output on
*stderr*. It's not helpful either.


Sure, I was not talking about that :-)




--
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

Re: [Freeipa-devel] [PATCH 25] Improve error logging for Dogtag subsystem installation

[Petr Spacek]

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 
>>> 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)

> In case of Dogtag stderr contains the relevant error message. In order
> to understand the events, that lead to the particular error, a user has
> to read the log file anyway -- unless you run pkispawn with '-vv' for
> extra verbosity. But then you get pages over pages of debug output on
> *stderr*. It's not helpful either.

Sure, I was not talking about that :-)

-- 
Petr^2 Spacek

-- 
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

Re: [Freeipa-devel] [PATCH 25] Improve error logging for Dogtag subsystem installation

[Petr Spacek]

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 
> 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.

Petr^2 Spacek

> IMO Christian's approach is OK, because it lets the caller decide how to log
> (or not) stderr on failure.
> 
>>
>>> In case of Dogtag stderr contains the relevant error message. In order
>>> to understand the events, that lead to the particular error, a user has
>>> to read the log file anyway -- unless you run pkispawn with '-vv' for
>>> extra verbosity. But then you get pages over pages of debug output on
>>> *stderr*. It's not helpful either.
>>
>> Sure, I was not talking about that :-)

-- 
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

Re: [Freeipa-devel] [PATCH 25] Improve error logging for Dogtag subsystem installation

[Christian Heimes]

Now the correct patch file instead of a vim swap file...
From 33be1f56a64e53d261a1058c4606a7e48c0aac52 Mon Sep 17 00:00:00 2001
From: Christian Heimes 
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.
---
 ipaplatform/redhat/tasks.py|  3 +--
 ipapython/dnssec/bindmgr.py|  1 -
 ipapython/dnssec/odsmgr.py |  1 -
 ipapython/ipautil.py   | 24 +---
 ipaserver/install/dns.py   |  4 +---
 ipaserver/install/dogtaginstance.py| 28 ++--
 ipaserver/install/opendnssecinstance.py|  3 +--
 ipaserver/install/server/replicainstall.py |  3 +--
 8 files changed, 31 insertions(+), 36 deletions(-)

diff --git a/ipaplatform/redhat/tasks.py b/ipaplatform/redhat/tasks.py
index 94d2cb4e906965a20bcfdd55f38854005091c26f..1c502a2c859b23851d3b6101fca31e6cbb75b1eb 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -31,7 +31,6 @@ import socket
 import sys
 import base64
 
-from subprocess import CalledProcessError
 from nss.error import NSPRError
 from pyasn1.error import PyAsn1Error
 from six.moves import urllib
@@ -173,7 +172,7 @@ class RedHatTaskNamespace(BaseTaskNamespace):
 def reload_systemwide_ca_store(self):
 try:
 ipautil.run([paths.UPDATE_CA_TRUST])
-except CalledProcessError as e:
+except ipautil.CalledProcessError as e:
 root_logger.error(
 "Could not update systemwide CA trust database: %s", e)
 return False
diff --git a/ipapython/dnssec/bindmgr.py b/ipapython/dnssec/bindmgr.py
index 1822dacf2535e7c37062e4d639e01289edcf5074..5b1d34135e8e5bd5c135b3d204c8de76531ecd07 100644
--- a/ipapython/dnssec/bindmgr.py
+++ b/ipapython/dnssec/bindmgr.py
@@ -9,7 +9,6 @@ import os
 import logging
 import shutil
 import stat
-import subprocess
 
 from ipalib import api
 import ipalib.constants
diff --git a/ipapython/dnssec/odsmgr.py b/ipapython/dnssec/odsmgr.py
index efbe16cc6ebf050d9cf347ed97b2b2e4b37c8a6e..a36ed7224a5abeb8c1ee91cc7eb60c048c05d2ed 100644
--- a/ipapython/dnssec/odsmgr.py
+++ b/ipapython/dnssec/odsmgr.py
@@ -6,7 +6,6 @@
 import logging
 from lxml import etree
 import dns.name
-import subprocess
 
 from ipapython import ipa_log_manager, ipautil
 
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 4551ea5c4025223dcff5cdc8998fedeccd14c3c2..ac85cb7b90ebde6f895dc09cae485a95c1c4a28d 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -63,20 +63,14 @@ KRB5_KDC_UNREACH = 2529639068 # Cannot contact any KDC for requested realm
 KRB5KDC_ERR_SVC_UNAVAILABLE = 2529638941 # A service is not available that is
  # required to process the request
 
-try:
-from subprocess import CalledProcessError
-except ImportError:
-# Python 2.4 doesn't implement CalledProcessError
-class CalledProcessError(Exception):
-"""This exception is raised when a process run by check_call() returns
-a non-zero exit status. The exit status will be stored in the
-returncode attribute."""
-def __init__(self, returncode, cmd, output=None):
-self.returncode = returncode
-self.cmd = cmd
-self.output = output
-def __str__(self):
-return "Command '%s' returned non-zero exit status %d" % (self.cmd, self.returncode)
+
+class CalledProcessError(subprocess.CalledProcessError):
+"""Custom CalledProcessError with error output
+"""
+def __init__(self, returncode, cmd, output=None, erroutput=None):
+super(CalledProcessError, self).__init__(returncode, cmd, output)
+self.erroutput = erroutput
+
 
 def get_domain_name():
 try:
@@ -379,7 +373,7 @@ def run(args, stdin=None, raiseonerr=True,
 

Re: [Freeipa-devel] [PATCH 25] Improve error logging for Dogtag subsystem installation

[Petr Spacek]

On 1.12.2015 18:42, Christian Heimes wrote:
> From 33be1f56a64e53d261a1058c4606a7e48c0aac52 Mon Sep 17 00:00:00 2001
> From: Christian Heimes 
> 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().

What do you think?

-- 
Petr^2 Spacek

-- 
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