[Freeipa-devel] [PATCH] 1027 add logging to ipa-upgradeconfig

[Petr Viktorin]

On 06/20/2012 04:17 PM, Rob Crittenden wrote:
> Petr Viktorin wrote:
>> On 06/18/2012 10:43 PM, Rob Crittenden wrote:
>>> Add some additional logging to ipa-upgradeconfig and have it update
>>> /var/log/ipaupgrade.log so we can see what an upgrade has already done.
>>>
>>> rob
>>>
>>
>> I don't think the indentation works too well:
>>
>> 2012-06-19T12:52:47Z INFO Verifying that root certificate is published
>> 2012-06-19T12:52:47Z DEBUG Certificate file exists
>> 2012-06-19T12:52:47Z INFO Verifying that CA proxy configuration is
>> correct
>> 2012-06-19T12:52:47Z DEBUG No CA detected in /etc/pki-ca
>> 2012-06-19T12:52:47Z INFO Verifying that KDC configuration is using
>> ipa-kdb backend
>> 2012-06-19T12:52:47Z DEBUG dbmodules already updated in /etc/krb5.conf
>>
>>
>> It would be nice to add a message right after logging setup saying that
>> ipa-upgradeconfig is being run.
>>
>>
>> The --debug flag only affects console output, standard_logging_setup
>> always sets the file handler to debug level. There's no need for the
>> flag with >/dev/null.
>>
>>
>> Otherwise the patch works great.
>>
>
> I had put the indentation so that when executed in a shell you can tell
> what the heck is going on :-) I'm open to suggestion but I found it hard
> to read when it was all left-justified.

I guess it works now, but we'll need a better approach. If a subsystem 
logs anything between the indented messages, it will be very confusing.

ipa-client-install uses ini-style sections in debug output, which isn't 
ideal either but could work here:

[Verify root cert]
Verifying that root certificate is published
Certificate file exists
[Verify CA proxy]
Verifying that CA proxy configuration is correct
No CA detected in /etc/pki-ca
[Verify ipa-kdb backend]
Verifying that KDC configuration is using ipa-kdb backend
dbmodules already updated in /etc/krb5.conf


Having the logger indent all messages properly would be a nicer solution 
but that would be a rather giant change.

> I'll remove the unnecessary --debug flag.
>
> rob

The indentation is a tiny issue and shouldn't hold the patch back, so 
ACK if you don't want to chase this further.

-- 
Petr³