[Freeipa-devel] [PATCH] 0014 Add final debug message in installers

[Rob Crittenden]

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
>>>> the
>>>> 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
>>>> what
>>>> 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
>>>>> aborted
>>>>> Installation aborted
>>>>>
>>>>> ipa-ldap-updater is the same:
>>>>>
>>>>> # ipa-ldap-updater
>>>>> [2012-03-26T14:53:41Z ipa] <ERROR>: ipa-ldap-updater failed,
>>>>> SystemExit:
>>>>> 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,
>>>>> SystemExit:
>>>>> 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
>>>> been
>>>> 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/installutils.py
>> b/ipaserver/install/installutils.
>> py
>> index 851b58d..ca82a1b 100644
>> --- a/ipaserver/install/installutils.py
>> +++ b/ipaserver/install/installutils.py
>> @@ -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']:
>> - root_logger.info(template, operation_name)
>> + root_logger.info(template)
>> try:
>> yield
>> 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)
>> else:
>> - final_log('The %s command failed')
>> + final_log('%s failed, %s: %s' % (operation_name, type(e).__name__,
>> e))
>> raise
>> else:
>> 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.
>>
>> rob
>
> Fixed.
>

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: 
'pony.greyoak.com' has no replication agreement for 'foo'
'pony.greyoak.com' 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 pony.greyoak.com

The password provided is incorrect for LDAP server pony.greyoak.com

rob