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

2016-06-15 Thread Martin Basti 



On 15.06.2016 13:29, Petr Spacek wrote:

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.


master:
* 567f00a59c53aca760336aea95423368ac621032 Add NTP to the list of 
services stored in IPA masters LDAP subtree

* 3e6af238bb695572e462ff49a3096ab0e2e85bc5 Introduce "NTP server" role

--
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

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

2016-06-15 Thread 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

-- 
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

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

2016-06-15 Thread Martin Basti 



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.

--
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

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

2016-06-15 Thread Petr Vobornik 
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?

-- 
Petr Vobornik

-- 
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

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

2016-06-14 Thread Martin Babinsky 

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.

--
Martin^3 Babinsky
From f62e8fc696b6c5f2b9fe4fd226c54a0511f64d17 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Sun, 12 Jun 2016 17:02:09 +0200
Subject: [PATCH 1/2] Add NTP to the list of services stored in IPA masters
 LDAP subtree

IPA masters can be configured as NTP servers but the status of this service
can not be determined centrally from querying relevant LDAP subtree. This
patch makes IPA master and replica publish the newly configured NTP service in
their service container during installation.

If the master was configured as NTP server, the NTP service entry will be
created upon upgrade.

https://fedorahosted.org/freeipa/ticket/5815
https://fedorahosted.org/freeipa/ticket/5826
---
 ipaserver/install/ntpinstance.py   | 22 +-
 ipaserver/install/server/install.py|  3 +++
 ipaserver/install/server/replicainstall.py |  5 +
 ipaserver/install/server/upgrade.py|  3 +++
 ipaserver/install/service.py   |  1 +
 5 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/ipaserver/install/ntpinstance.py b/ipaserver/install/ntpinstance.py
index 8b0f0e5395dae3c058fc31bd8914741e4d158830..2cac7baf136cabd38ee7d5fda6ff8e917988b340 100644
--- a/ipaserver/install/ntpinstance.py
+++ b/ipaserver/install/ntpinstance.py
@@ -19,6 +19,7 @@
 #
 
 from ipaserver.install import service
+from ipaserver.install import sysupgrade
 from ipapython import sysrestore
 from ipapython import ipautil
 from ipaplatform.constants import constants
@@ -28,9 +29,28 @@ from ipapython.ipa_log_manager import root_logger
 NTPD_OPTS_VAR = constants.NTPD_OPTS_VAR
 NTPD_OPTS_QUOTE = constants.NTPD_OPTS_QUOTE
 
+NTP_EXPOSED_IN_LDAP = 'exposed_in_ldap'
+
+
+def ntp_ldap_enable(fqdn, base_dn, realm):
+ntp = NTPInstance(realm=realm)
+is_exposed_in_ldap = sysupgrade.get_upgrade_state(
+'ntp', NTP_EXPOSED_IN_LDAP)
+
+was_running = ntp.is_running()
+
+if ntp.is_configured() and not is_exposed_in_ldap:
+ntp.ldap_enable('NTP', fqdn, None, base_dn)
+sysupgrade.set_upgrade_state('ntp', NTP_EXPOSED_IN_LDAP, True)
+
+if was_running:
+ntp.start()
+
+
 class NTPInstance(service.Service):
-def __init__(self, fstore=None):
+def __init__(self, realm=None, fstore=None):
 service.Service.__init__(self, "ntpd", service_desc="NTP daemon")
+self.realm = realm
 
 if fstore:
 self.fstore = fstore
diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py
index c36421fbd9e7ab9046ede57fef3dfa3ed2045b1d..930cca7b31ca06c04ab92deff49b6a4f198c2b6e 100644
--- a/ipaserver/install/server/install.py
+++ b/ipaserver/install/server/install.py
@@ -740,6 +740,9 @@ def install(installer):
idstart=options.idstart, idmax=options.idmax,
subject_base=options.subject,
hbac_allow=not options.no_hbac_allow)
+
+ntpinstance.ntp_ldap_enable(host_name, ds.suffix, realm_name)
+
 else:
 ds = dsinstance.DsInstance(fstore=fstore,
domainlevel=options.domainlevel)
diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 6c0ad6939111317e7510662d2953d89ee34cd8f9..f597880471eb3710ebc7163f771d4e6dc9f1e3d6 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -780,6 +780,8 @@ def install(installer):
 # Configure dirsrv
 ds = install_replica_ds(config, options, ca_enabled, remote_api)
 
+ntpinstance.ntp_ldap_enable(config.host_name, ds.suffix, api.env.realm)
+
 # Always try to install DNS records

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

2016-06-14 Thread Martin Basti 



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

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

2016-06-12 Thread Martin Babinsky 
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

--
Martin^3 Babinsky
From f44d98a042e3678ebd00bd7ac30c94683b823a5e Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Sun, 12 Jun 2016 17:02:09 +0200
Subject: [PATCH 01/02] Add NTP to the list of services stored in IPA masters
 LDAP subtree

IPA masters can be configured as NTP servers but the status of this service
can not be determined centrally from querying relevant LDAP subtree. This
patch makes IPA master and replica publish the newly configured NTP service in
their service container during installation.

If the master was configured as NTP server, the NTP service entry will be
created upon upgrade.

https://fedorahosted.org/freeipa/ticket/5815
https://fedorahosted.org/freeipa/ticket/5826
---
 ipaserver/install/ntpinstance.py   | 12 +++-
 ipaserver/install/server/install.py|  3 +++
 ipaserver/install/server/replicainstall.py |  5 +
 ipaserver/install/server/upgrade.py|  3 +++
 ipaserver/install/service.py   |  1 +
 5 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/ipaserver/install/ntpinstance.py b/ipaserver/install/ntpinstance.py
index 8b0f0e5395dae3c058fc31bd8914741e4d158830..047644cdd837c90fea1eb86c1d758e9b1508ae49 100644
--- a/ipaserver/install/ntpinstance.py
+++ b/ipaserver/install/ntpinstance.py
@@ -28,9 +28,19 @@ from ipapython.ipa_log_manager import root_logger
 NTPD_OPTS_VAR = constants.NTPD_OPTS_VAR
 NTPD_OPTS_QUOTE = constants.NTPD_OPTS_QUOTE
 
+
+def ntp_ldap_enable(fqdn, base_dn, realm):
+ntp = NTPInstance(realm=realm)
+
+if ntp.is_configured():
+ntp.ldap_enable('NTP', fqdn, None, base_dn)
+ntp.enable()
+
+
 class NTPInstance(service.Service):
-def __init__(self, fstore=None):
+def __init__(self, realm=None, fstore=None):
 service.Service.__init__(self, "ntpd", service_desc="NTP daemon")
+self.realm = realm
 
 if fstore:
 self.fstore = fstore
diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py
index c36421fbd9e7ab9046ede57fef3dfa3ed2045b1d..930cca7b31ca06c04ab92deff49b6a4f198c2b6e 100644
--- a/ipaserver/install/server/install.py
+++ b/ipaserver/install/server/install.py
@@ -740,6 +740,9 @@ def install(installer):
idstart=options.idstart, idmax=options.idmax,
subject_base=options.subject,
hbac_allow=not options.no_hbac_allow)
+
+ntpinstance.ntp_ldap_enable(host_name, ds.suffix, realm_name)
+
 else:
 ds = dsinstance.DsInstance(fstore=fstore,
domainlevel=options.domainlevel)
diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 6c0ad6939111317e7510662d2953d89ee34cd8f9..f597880471eb3710ebc7163f771d4e6dc9f1e3d6 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -780,6 +780,8 @@ def install(installer):
 # Configure dirsrv
 ds = install_replica_ds(config, options, ca_enabled, remote_api)
 
+ntpinstance.ntp_ldap_enable(config.host_name, ds.suffix, api.env.realm)
+
 # Always try to install DNS records
 install_dns_records(config, options, remote_api)
 finally:
@@ -1350,6 +1352,9 @@ def promote(installer):
 # or certmonger will fail to contact the peer master
 install_http_certs(config, fstore, remote_api)
 
+ntpinstance.ntp_ldap_enable(config.host_name, ds.suffix,
+remote_api.env.realm)
+
 finally:
 if conn.isconnected():
 conn.disconnect()
diff --git a/ipaserver/install/server/upgrade.py b/ipaserver/install/server/upgrade.py
index cd2ad2e112fde7e13b584cb550af4bcf65e781ad..e706e22d24a9b1028a6d8b10ae4f64a84cbac81b 100644
--- a/ipaserver/install/server/upgrade.py
+++ b/ipaserver/install/server/upgrade.py
@@ -32,6 +32,7 @@ from ipaserver.install import installutils
 from ipaserver.install import dsinstance
 from ipaserver.install import httpinstance
 from ipaserver.install import memcacheinstance
+from ipaserver.install import ntpinstance
 from ipaserver.install import bindinstance
 from ipaserver.install import service
 from ipaserver.install import cainstance
@@ -1571,6 +1572,8 @@ def upgrade_configuration():
 
 ds.configure_dirsrv_ccache()
 
+ntpinstance.ntp_ldap_enable(api.env.host,