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

[Rob Crittenden]

Petr Viktorin wrote:
> 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.
>

Ok, I changed added the braces, removed --debug and pushed to master.

rob