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 <mbabi...@redhat.com>
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
         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, api.env.basedn, api.env.realm)
+
     # ldap2 connection is not valid after DS restart, close connection otherwise
     # it will cause network errors
     if api.Backend.ldap2.isconnected():
diff --git a/ipaserver/install/service.py b/ipaserver/install/service.py
index 40767acd57d5e1fa8126144ca64f6951848ce214..cdd4354dfe9bedc807c404aeca7662c1920d0116 100644
--- a/ipaserver/install/service.py
+++ b/ipaserver/install/service.py
@@ -41,6 +41,7 @@ SERVICE_LIST = {
     'MEMCACHE': ('ipa_memcached', 39),
     'HTTP': ('httpd', 40),
     'KEYS': ('ipa-custodia', 41),
+    'NTP': ('ntpd', 45),
     'CA': ('pki-tomcatd', 50),
     'KRA': ('pki-tomcatd', 51),
     'ADTRUST': ('smb', 60),
-- 
2.5.5

From a0b7009293b6619f8606289eec9930757da87bc3 Mon Sep 17 00:00:00 2001
From: Martin Babinsky <mbabi...@redhat.com>
Date: Sun, 12 Jun 2016 17:03:10 +0200
Subject: [PATCH 2/2] Introduce "NTP server" role

This makes IPA servers that publish their NTP services in LDAP searchable by
`server-role-find` and `server-find` command.

The list of active IPA NTP servers will be displayed in to output of `ipa
config-show` command.

https://fedorahosted.org/freeipa/ticket/5815
---
 ipaserver/plugins/config.py | 14 +++++++++-----
 ipaserver/servroles.py      |  5 +++++
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/ipaserver/plugins/config.py b/ipaserver/plugins/config.py
index 94a48a27de273baf0906e6c75ab0bc18ded8ea6d..a9e811ecd5d6c42fc7de1433b1f1ef10fc25e4aa 100644
--- a/ipaserver/plugins/config.py
+++ b/ipaserver/plugins/config.py
@@ -240,6 +240,12 @@ class config(LDAPObject):
             flags={'virtual_attribute', 'no_create', 'no_update'}
         ),
         Str(
+            'ntp_server_server*',
+            label=_('IPA NTP servers'),
+            doc=_('IPA servers with enabled NTP'),
+            flags={'virtual_attribute', 'no_create', 'no_update'}
+        ),
+        Str(
             'ca_renewal_master_server?',
             label=_('IPA CA renewal master'),
             doc=_('Renewal master for IPA certificate authority'),
@@ -256,11 +262,9 @@ class config(LDAPObject):
 
         backend = self.api.Backend.serverroles
 
-        ca_config = backend.config_retrieve("CA server")
-        master_config = backend.config_retrieve("IPA master")
-
-        entry_attrs.update(ca_config)
-        entry_attrs.update(master_config)
+        for role in ("CA server", "IPA master", "NTP server"):
+            config = backend.config_retrieve(role)
+            entry_attrs.update(config)
 
 
 @register()
diff --git a/ipaserver/servroles.py b/ipaserver/servroles.py
index 8628cd625f897da5c1a8539ef860ae70a44de2d8..cf4599995118c589a5b51236c68e13f14ac1257b 100644
--- a/ipaserver/servroles.py
+++ b/ipaserver/servroles.py
@@ -566,6 +566,11 @@ role_instances = (
         u"KRA server",
         component_services=['KRA']
     ),
+    ServiceBasedRole(
+        u"ntp_server_server",
+        u"NTP server",
+        component_services=['NTP']
+    )
 )
 
 attribute_instances = (
-- 
2.5.5

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