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
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to