[Freeipa-devel] [PATCH 0159-0160] emancipate IPA NTP service into role

[Petr Spacek]

On 15.6.2016 09:57, Martin Basti wrote:
> 
> 
> On 15.06.2016 09:55, Petr Vobornik wrote:
>> On 06/14/2016 07:28 PM, Martin Basti wrote:
>>>
>>> On 14.06.2016 18:58, Martin Babinsky wrote:
>>>> On 06/14/2016 05:06 PM, Martin Basti wrote:
>>>>>
>>>>> On 12.06.2016 17:37, Martin Babinsky wrote:
>>>>>> These two patches turn oft-neglected ntp service into a full fledged
>>>>>> role whose status can be queried centrally. They should also enable
>>>>>> generation of location-specific _ntp._udp records.
>>>>>>
>>>>>> Please note that NTP is LDAP-enabled by additional call after DS
>>>>>> instance is configured. I was not feeling confident by swapping NTP
>>>>>> and DS configuration steps as I was afraid it will break things. If
>>>>>> not, I will happily update the patch accordingly.
>>>>>>
>>>>>> https://fedorahosted.org/freeipa/ticket/5815
>>>>>> https://fedorahosted.org/freeipa/ticket/5826
>>>>>>
>>>>>>
>>>>>>
>>>>> Hello, I have a few comments:
>>>>>
>>>>> Patch: 159
>>>>> 1)
>>>>> +    if ntp.is_configured():
>>>>> +        ntp.ldap_enable('NTP', fqdn, None, base_dn)
>>>>> +        ntp.enable()
>>>>>
>>>>> All ipa services are in disabled state, ipactl starts them according
>>>>> configuration in LDAP
>>>>> IMO it should be something like:
>>>>> ntp.disable()
>>>>> if running:
>>>>>      ntp.start()
>>>>>
>>>>> 2)
>>>>> could you upgrade NTP only once in upgrade.py? Use sysupgrade state
>>>>>
>>>>> 3)
>>>>> +    'NTP': ('ntpd', 42),
>>>>> I prefer 45, it is easier to put any service before NTP if needed
>>>>> without huge renumbering
>>>>>
>>>>>
>>>>> Patch 160: LGTM
>>>>>
>>>>> Martin^2
>>>>>
>>>>>
>>>> Right, attaching updated patches.
>>>>
>>> Patches are good, but I'm curious if there is any chance for NTP to be
>>> able synchronize time before replication on replica install. If no, IMO
>>> better is to move NTP service configuration after dirserver to be able
>>> to configure LDAP entry directly.
>>> But if there is not time for this, I'm fine with opening ticket and
>>> fixing it later.
>>>
>>> Martin^2
>> Isn't it already done during Client part of replica installation?
>>
> With domain level 1 yes, with domain level 0, no.

ACK to current version of the patch.

Please keep the order as it is (NTP first) because it ensures proper time sync
even on domain level 0.

-- 
Petr^2 Spacek