[Freeipa-devel] [PATCH 0023] enable debugging of spawned ntpd command during client install

[Martin Babinsky]

On 03/26/2015 01:14 PM, Martin Kosek wrote:
> On 03/25/2015 04:18 PM, Jan Cholasta wrote:
>> Hi,
>>
>> Dne 25.3.2015 v 15:26 Martin Babinsky napsal(a):
>>> The attached patch related to https://fedorahosted.org/freeipa/ticket/4931
>>
>> Please make sure <https://fedorahosted.org/freeipa/ticket/3048> stays fixed.
This should be ok as we do not use ntpdate for timesync anymore (I have 
tested client-install with this patch in VM with quite large clock skews 
between client and server and it did sync just fine also with -d flag).
>>
>>>
>>> It is certainly not a final solution, more of an initial "hack" of sorts
>>> just to gather some suggestions, since I am not even sure if this is the
>>> right thing to do.
>>>
>>> The reporter from bugzilla suggests to enable debugging of ALL commands
>>> called through ipautil.run(), but I think that fixing all cca 157 found
>>> usages of run() is too much work with a quite small benefit.
>>>
>>> Anyway I would welcome some opinions about this: should the external
>>> commands really inherit the debug settings of ipa-* utilities, and if
>>> so, is the method showed in this patch the right way to do it?
>>
>> I am not a fan of this method, ipautil.run does not know anything about the
>> command it runs and I think it should stay that way.
>>
>> I would prefer to have an ipautil.run wrapper with debug flag using appropriate
>> debugging option for each command where we need to conditionally enable
>> debugging. Or just add the debugging option unconditionally to every command
>> where it could be useful.
>
> +1, I do not like this change to ipautil.run either. It should be sole
> responsibility of the caller to specify the right combinations of options,
> including debug option, where applicable.
>

How should we attack this issue then?

If we really would do "all comands inherit the debug option", then it 
would be a good idea to avoid things like

     if options.debug:
         args.append('-d')

littered all over the install/upgrade/etc. code. Jan's idea of a wrapper 
around run() then sounds like a good move. But I suppose we would then 
need to store somewhere (ipaplatform?) an additional data saying which 
command uses which option for debugging.

(Or should we just slap on an optional '-d' passed to ntpd to quickly 
resolve the main issue in the ticket and worry about all of this later? ;))

-- 
Martin^3 Babinsky