Petr Viktorin wrote:
On 03/30/2012 11:00 PM, Rob Crittenden wrote:
Petr Viktorin wrote:
On 03/26/2012 05:35 PM, Petr Viktorin wrote:
On 03/26/2012 04:54 PM, Rob Crittenden wrote:

Some minor compliants.

Ideally, there would be a routine that sets up the logging and handles
command-line arguments in some uniform way (which is also needed before
logging setup to detect ipa-server-install --uninstall).
The original patch did the common logging setup, and I hacked around
install/uninstall problem too.
I guess I overdid it when I simplified the patch.
I'm somewhat confused about the scope, so bear with me as I clarify
you mean.

If you abort the installation you get this somewhat unnerving error:

Continue to configure the system with these values? [no]:
ipa : ERROR ipa-server-install failed, SystemExit: Installation
Installation aborted

ipa-ldap-updater is the same:

# ipa-ldap-updater
[2012-03-26T14:53:41Z ipa] <ERROR>: ipa-ldap-updater failed,
IPA is not configured on this system.
IPA is not configured on this system.

and ipa-upgradeconfig

$ ipa-upgradeconfig
[2012-03-26T14:54:05Z ipa] <ERROR>: ipa-upgradeconfig failed,
You must be root to run this script.

You must be root to run this script.

I'm guessing that the issue is that the log file isn't opened yet.
It would be nice if the logging would be confined to just the log.

If I understand you correctly, the code should check if logging has
configured already, and if not, skip displaying the message?

When uninstalling you get the message 'ipa-server-install successful'.
This is a little odd as well.

ipa-server-install is the name of the command. Wontfix for now, unless
you disagree strongly.

Updated patch: only log if logging has been configured (detected by
looking at the root logger's handlers), and changed the message to “The
ipa-server-install command has succeeded/failed”.

Works much better thanks. Just one request. When you created final_log()
you show less information than you did in earlier patches. It is nice
seeing the SystemExit failure. Can you do something like this (basically
cut-n-pasted from v05)?

diff --git a/ipaserver/install/
index 851b58d..ca82a1b 100644
--- a/ipaserver/install/
+++ b/ipaserver/install/
@@ -721,15 +721,15 @@ def script_context(operation_name):
# Only log if logging was already configured
# TODO: Do this properly (e.g. configure logging before the try/except)
if log_mgr.handlers.keys() != ['console']:
-, operation_name)
except BaseException, e:
if isinstance(e, SystemExit) and (e.code is None or e.code == 0):
# Not an error after all
- final_log('The %s command was successful')
+ final_log('The %s command was successful' % operation_name)
- final_log('The %s command failed')
+ final_log('%s failed, %s: %s' % (operation_name, type(e).__name__,
final_log('The %s command was successful')

This looks like:

2012-03-30T20:56:53Z INFO ipa-dns-install failed, SystemExit:
DNS is already configured in this IPA server.



Hate to do this to you but I've found a few more issues. I basically went down the list and ran all the commands in various conditions.

Some don't open any logs at all so the output gets written twice, like ipa-replica-prepare and ipa-replica-manage:

# ipa-replica-manage del foo
Directory Manager password:

ipa: INFO: The ipa-replica-manage command failed, SystemExit: '' has no replication agreement for 'foo'
'' has no replication agreement for 'foo'

Same with ipa-csreplica-manage.

# ipa-replica-prepare foo
Directory Manager (existing master) password:

ipa: INFO: The ipa-replica-prepare command failed, SystemExit:
The password provided is incorrect for LDAP server

The password provided is incorrect for LDAP server


Freeipa-devel mailing list

Reply via email to