On 14/03/16 14:01, Martin Basti wrote:



On 14.03.2016 13:46, Martin Babinsky wrote:
On 03/11/2016 09:16 AM, David Kupka wrote:
Current version (0.5.0) of python-augeas is missing copy() method. Use
dkupka/python-augeas copr repo before new version it's build and
available in the official repos.



Hi David,

TLDR: NACK :D.

Here are high-level remarks to discuss:

Maybe it would be a good idea to move ipaaugeas/changeconf and ntp to
ipaplatform since it is dealing with (sorta) platform specific
behavior (ntp vs. chrony vs. whatever we will have for timesync in the
future). CC'ing Jan for thoughts.

Also regarding patches 0096-0097, we could have platform specific
TimeDateService object that could wrap NTP/chrony management. for
example, the task namespace functions in Pathc 0096 could be
reimplemented as a methods of the service (RedhatTimeDateService,
FedoraTimeDateService etc.) and then called in a platform-agnostic
manner.

Here are some comments regarding code:

Patch 0095:

1.)
+    IPA_CUSTOM_AUGEAS_LENSES_DIR = '/usr/share/augeas/lenses/ipa/'

Do not forget to add this directory to %install and %files spection of
the spec file so that it is correctly added to RPM build.

2.)

please separate import of system-wide and IPA-specific modules by
blank line

+import collections
+from augeas import Augeas
+from ipaplatform.paths import paths
+from ipapython.ipa_log_manager import root_logger

3.) the call to parent's __new__ should have signature 'super(aug_obj,
cls).__new__(*args, **kwargs)'

+            cls._instance = super(aug_obj, cls).__new__(cls, *args,
**kwargs)

4.)

+            raise RuntimeError('Augeas lenses was loaded. Could not
add more'
+                               'lenses.')

Should be 'Augeas lenses _were_ loaded'

5.)

+        if lens in self.lenses:
+            raise RuntimeError('Lens %s already added.' % lens)
+        self.lenses.append(lens)
+        load_path = '/augeas/load/{0}'.format(lens


Shouldn't the following code use os.path,join to construct the load_path?

6.) I would prefer the following indentation style in the add_lens()
method

@@ -65,9 +65,9 @@ class aug_obj(object):
         for conf_file in conf_files:
             self._aug.set(os.path.join(load_path, 'incl[0]'), conf_file)
             self.tree['old'] = self.tree.get(conf_file, None)
-            self.tree[conf_file] = aug_node(self._aug,
- os.path.join('/files',
- conf_file[1:]))
+            self.tree[conf_file] = aug_node(
+                self._aug, os.path.join('/files', conf_file[1:])
+            )

7.) I would also prefer if the hardcoded paths like '/augeas/load',
'files', and '//error' would be made into either module variables or
class members.

8.)

+    def load(self):
+        if self._loaded:
+            raise RuntimeError('Augeas lenses was loaded. Could not
add more'
+                               'lenses.')

Fix the excpetion text in the same way as in 4.)

9.)

+        errors = self._aug.match(os.path.join('//error'))

is the os.path.join necessary here?

10.) I guess you can rewrite the error message in load() method using
list comprehension:

@@ -76,9 +76,9 @@ class aug_obj(object):
         self._aug.load()
         errors = self._aug.match(os.path.join('//error'))
         if errors:
-            err_msg = ""
-            for error in errors:
-                err_msg += ("{}: {}".format(error,
self._aug.get(error)))
+            err_msg = '\n'.join(
+                ["{}: {}".format(e, self._aug.get(e)) for e in errors]
+            )
             raise RuntimeError(err_msg)
         self._loaded = True

11.)

+class aug_node(collections.MutableMapping):
+    """ Single augeas node.
+    Can be handled as python dict().
+    """
+    def __init__(self, aug, path):
+        self._aug = aug
+        if path and os.path.isabs(path):
+            self._path = path
+        else:
+            self._tmp = _tmp_path(aug, path)
+            self._path = self._tmp.path

Isn't it better to change signature of __init__ to:

    def __init__(self, aug, path=None):

and then test whether path is None?

12.)

    def __setitem__(self, key, node):
+        target = os.path.join(self._path, key)
+        end = '{0}[0]'.format(os.path.join(self._path, key))
+        if self._aug.match(target):
+            self._aug.remove(target)
+        target_list = aug_list(self._aug, target)
+        for src_node in aug_list(node._aug, node._path):
+            target_list.append(src_node)

The 'end' variable is declared but not used.

13.)

+
+    def __len__(self):
+        return len(self._aug.match('{0}/*'.format(self._path)))
+
+    def __iter__(self):
+        for key in self._aug.match('{0}/*'.format(self._path)):
+            yield self._aug.label(key)
+        raise StopIteration()
+

Shouldn't we construct paths using os.path.join for the sake of
consistency?

14.)

+    def __bool__(self):
+        return (bool(len(self)) or bool(self.value))

The parentheses around the boolean expression are not needed

15.)

+class aug_list(collections.MutableSequence):
+    """Augeas NODESET.
+    Can be handled as a list().
+    """
+    def __init__(self, aug, path):
+        self._aug = aug
+        if path and os.path.isabs(path):
+            self._path = path
+        else:
+            self._tmp = _tmp_path(aug, path)
+            self._path = self._tmp.path

I would use 'path=None' int he signature and then test whether 'path
is not None'.

16.)

+        if not self._aug.match(target):
+            raise IndexError()

It would be nice if you could put some basic error message into the
raised exceptions, like "node index out of range" or something like that

17.)

+        elif isinstance(index, slice):
+            label = self._path.split('/')[-1]

you could use os.path.basename() to get the leaf node.


18.)

+            replace = range_target[:len(node)]
+            delete = create = []

Be careful here as you create two references to the same empty list
object, which is probably not what you wanted.

19.)
+                try:
+                    create_start = range_target[-1]+1
+                except IndexError:
+                    create_start = self._idx_pos(index.start)
+                create_stop = create_start+len(node)-len(replace)
+                create = list(range(create_start, create_stop))

Please respect PEP8 and put spaces around arithmetic operators in
assignments.

Also it would be nice to have at least a minimal test suite for this
module, but that may be a separate ticket.

patch 0096:

1.) please fix the commit message
2.) please use new-style license header in ipapython/ntp.py
3.)

+        return ("Conflicting Time&Date synchroniztion service '%s' is "
+                "currently enabled and/or running on the system."
+                % self.conflicting_service)

Please fix the typo in the error message.

4.)
+        service = services.service(self.service_name)
+        if sstore:
+            if sstore.get_state('ntp', 'enabled') is None:
+                sstore.backup_state('ntp', 'enabled',
service.is_enabled())
+
+        if fstore:
+            if not fstore.has_file(self.conf_file):
+                fstore.backup_file(self.conf_file)

the conditions in the 'if' statement can be merged into single AND
expression

5.)
+        self._store_service_state(sstore, fstore)
+        if sstore:
+            sstore.backup_state('ntp', "enabled", service.is_enabled())
+
+        if fstore:
+            fstore.backup_file(self.conf_file)

I think these checks are redundant here.

6.)
+        # In such case it is OK to fail
+        try:
+            restored = fstore.restore_file(self.conf_file)
+        except Exception:
+            pass

Instead of 'pass' it would be better to set restored to False so that
you don't hit NameError later.

7.)
+
+    def configure_client(self, ntp_servers=[], sstore=None,
fstore=None):
+        self.server_options['burst'] = None
+        self.server_options['iburst'] = None

I would rather set these instance variables in __init__() than here.

8.)

+    def configure_client(self, ntp_servers=[], sstore=None,
fstore=None):
+        self.conf_file = paths.CHRONY_CONF
self.conf_file is already defined in constructor.

9.)

+        self.server_options['iburst'] = None
this should be moved to __init__()

10.)
+        with ipaaugeas.aug_obj() as aug:
+            try:
+                aug.add_lens(self.lens, [self.conf_file])
+                aug.load()
+            except RuntimeError as e:
+                raise NTPConfigurationError(e)

This code is repeated quite a few times, maybe it would be a good idea
to factor it out to a method of NTPService object.


Patch 0097:

1.) please fix a typo here:

+        description="Disble any other Time synchronization services."

2.)

+    installutils, kra, krbinstance, memcacheinstance, ntpinstance,
you have 2 spaces before 'ntpinstance'


I'm adding my nitpicks too :)

1)
+#!/usr/bin/python

This should not be in modules, only in executable files

2)
Missing license in ipaaugeas.py

Martin^2

Hello,
after offline discussion with Honza I've rewritten the augeas wrapper and modified ipautil/ntp.py accordingly. The rest should be pretty much the same.
Patches attached.

--
David Kupka
From 9e324e0b4ea1016b0b83a0032899d3cb693a8ba7 Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Thu, 10 Mar 2016 22:33:15 +0100
Subject: [PATCH 1/4] augeas: add wrapper around python binding

python-augeas binding is mostly 1-1 mapping of C functions. Put a layer of
list- and dict-like classes to allow a slightly more comfortable work.

https://fedorahosted.org/freeipa/ticket/4920
---
 freeipa.spec.in         |   1 +
 ipapython/configfile.py | 131 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 132 insertions(+)
 create mode 100644 ipapython/configfile.py

diff --git a/freeipa.spec.in b/freeipa.spec.in
index aaa40cc9a2246ed1d244e160edf935da216c75c5..c0e98ca3657198d229f78294def5a72f07008817 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -173,6 +173,7 @@ Requires: systemd-python
 Requires: %{etc_systemd_dir}
 Requires: gzip
 Requires: oddjob
+Requires: python-augeas >= 0.5.0
 
 Provides: %{alt_name}-server = %{version}
 Conflicts: %{alt_name}-server
diff --git a/ipapython/configfile.py b/ipapython/configfile.py
new file mode 100644
index 0000000000000000000000000000000000000000..b48a9eae97dc4c1b19d6ae7e961ce701a4a36ed7
--- /dev/null
+++ b/ipapython/configfile.py
@@ -0,0 +1,131 @@
+#
+# Copyright (C) 2016  FreeIPA Contributors see COPYING for license
+#
+
+import os
+import augeas
+
+
+def copy(aug, src, dst):
+    """
+    Deep copy src -> dst replacing dst and all descendant
+
+    Replace call to this function with aug.copy(src, dst) when python-augeas
+    with copy method is released on all platforms we care about
+    """
+    try:
+        aug.copy(src, dst)
+    except AttributeError:
+        for d in aug.match("{}/*".format(dst)):
+            aug.remove(d)
+        source_tree = [src]
+        for source in source_tree:
+            source_tree.extend(aug.match("{}/*".format(source)))
+            destination = source.replace(src, dst)
+            aug.set(destination, aug.get(source))
+
+
+class Element(object):
+    """
+    Basic manipulation with augeas representation of configuration
+    """
+    path_template = ''
+
+    def __init__(self, pre, loc):
+        self.path = self.path_template.format(pre.path, loc)
+        self.aug = pre.aug
+
+    def __call__(self, *loc):
+        """
+        Returns reference to object representing part of configuration
+        """
+        if len(loc) == 1:
+            loc = loc[0]
+        else:
+            return self(loc[0])(*loc[1:])
+
+        if isinstance(loc, str):
+            return List(self, loc)
+
+        if isinstance(loc, int):
+            if loc < 0:
+                loc = "last()-{}".format(-loc-1)
+            else:
+                loc = loc + 1
+            return Node(self, loc)
+
+    def __bool__(self):
+        return bool(self.aug.match(self.path))
+
+    def __setitem__(self, loc, value):
+        if isinstance(value, Element):
+            # self.aug.copy(value.path, self(loc).path)
+            copy(self.aug, value.path, self(loc).path)
+        else:
+            target = self(loc).path
+            self.aug.set(target, value)
+
+    def __delitem__(self, loc):
+        self.aug.remove(self(loc).path)
+
+    def __getitem__(self, loc):
+        """
+        Returns value stored in configuration represented by the instance.
+        In case instance represents a list tuple of all values is returned.
+        """
+        if isinstance(loc, tuple):
+            target = self(*loc)
+        else:
+            target = self(loc)
+
+        if isinstance(target, List):
+            if not target:
+                raise KeyError(loc)
+            return tuple(map(self.aug.get, self.aug.match(target.path)))
+
+        if isinstance(target, Node):
+            if not target:
+                raise IndexError("list index out of range")
+            return self.aug.get(target.path)
+
+    __nonzero__ = __bool__  # for python2
+
+
+class Node(Element):
+    """
+    Always represents single configuration entry.
+    """
+    path_template = "{}[{}]"
+
+
+class List(Element):
+    """
+    Represents list of configuration entries.
+    """
+    path_template = "{}/{}"
+
+    def __iter__(self):
+        for i in range(len(self.aug.match(self.path))):
+            yield self(i)
+
+    def append(self, value):
+        self.aug.set(Node.path_template.format(self.path, 0), value)
+
+
+class Root(Element):
+    def __init__(self):
+        self.aug = augeas.Augeas()
+        self.path = ''
+
+
+class Config(object):
+    def __init__(self, path):
+        self.root = Root()
+        self.path = path
+
+    def __enter__(self):
+        return self.root('files')(*os.path.split(self.path))
+
+    def __exit__(self, ex_type, ex_value, ex_tb):
+        self.root.aug.save()
+        self.root.aug.close()
-- 
2.5.0

From a719c5f0d5e35e7f9ca2c363e223f80b27da06a7 Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Thu, 10 Mar 2016 22:36:06 +0100
Subject: [PATCH 2/4] ntp: Add module for NTP configuration

Use augeas to modify NTP services configuration. Also only add needed
directives instead of rewriting whole configuration file.

https://fedorahosted.org/freeipa/ticket/4920
https://fedorahosted.org/freeipa/ticket/4001
---
 ipaplatform/base/services.py   |   2 +
 ipaplatform/fedora/paths.py    |   4 +-
 ipaplatform/fedora/services.py |   2 +
 ipaplatform/redhat/services.py |   2 +
 ipaplatform/rhel/services.py   |   2 +
 ipapython/ntp.py               | 307 +++++++++++++++++++++++++++++++++++++++++
 6 files changed, 318 insertions(+), 1 deletion(-)
 create mode 100644 ipapython/ntp.py

diff --git a/ipaplatform/base/services.py b/ipaplatform/base/services.py
index 641a654183c52c0330cb4ece2a54c6bd0a96394c..0969eb165f0bd660d205d12255305a6836a4af59 100644
--- a/ipaplatform/base/services.py
+++ b/ipaplatform/base/services.py
@@ -493,3 +493,5 @@ knownservices = None
 # System may support more time&date services. FreeIPA supports ntpd only, other
 # services will be disabled during IPA installation
 timedate_services = ['ntpd', 'chronyd']
+ntp_service = 'ntpd'
+conflicting_ntp_services = ['chronyd', 'systemd-timesyncd']
diff --git a/ipaplatform/fedora/paths.py b/ipaplatform/fedora/paths.py
index 49a904f2f271097d75be235eb30d614dc1bb41ac..24a37820a93a250f0e76c22bd0b42761d7f07a74 100644
--- a/ipaplatform/fedora/paths.py
+++ b/ipaplatform/fedora/paths.py
@@ -27,7 +27,9 @@ from ipaplatform.redhat.paths import RedHatPathNamespace
 
 
 class FedoraPathNamespace(RedHatPathNamespace):
-    pass
+    CHRONYD = "/usr/sbin/chronyd"
+    CHRONY_CONF = "/etc/chrony.conf"
+    CHRONYD_PID_FILE = '/var/run/chronyd.pid'
 
 
 paths = FedoraPathNamespace()
diff --git a/ipaplatform/fedora/services.py b/ipaplatform/fedora/services.py
index 5b1dfc824b042b5d162fb23d9f6621b484099745..4bddc2b9d5b39f81c1c95b536df380a17421cd02 100644
--- a/ipaplatform/fedora/services.py
+++ b/ipaplatform/fedora/services.py
@@ -57,5 +57,7 @@ class FedoraServices(redhat_services.RedHatServices):
 # Objects below are expected to be exported by platform module
 
 timedate_services = redhat_services.timedate_services
+ntp_service = 'chronyd'
+conflicting_ntp_services = ['ntpd', 'systemd-timesyncd']
 service = fedora_service_class_factory
 knownservices = FedoraServices()
diff --git a/ipaplatform/redhat/services.py b/ipaplatform/redhat/services.py
index 92dae452a31a0b3680e9c407eccb120881cc9e25..7b4fc6fda6ec8632de89af4732432552636aa021 100644
--- a/ipaplatform/redhat/services.py
+++ b/ipaplatform/redhat/services.py
@@ -255,5 +255,7 @@ class RedHatServices(base_services.KnownServices):
 # Objects below are expected to be exported by platform module
 
 timedate_services = base_services.timedate_services
+ntp_service = base_services.ntp_service
+conflicting_ntp_services = base_services.conflicting_ntp_services
 service = redhat_service_class_factory
 knownservices = RedHatServices()
diff --git a/ipaplatform/rhel/services.py b/ipaplatform/rhel/services.py
index 980c84a9653187bdd26e26efe9d120a5637c4595..c36ea5b43b4ad56111ce41d8c6a9c6ce60a1f36a 100644
--- a/ipaplatform/rhel/services.py
+++ b/ipaplatform/rhel/services.py
@@ -57,5 +57,7 @@ class RHELServices(redhat_services.RedHatServices):
 # Objects below are expected to be exported by platform module
 
 timedate_services = redhat_services.timedate_services
+ntp_service = redhat_services.ntp_service
+conflicting_ntp_services = redhat_services.conflicting_ntp_services
 service = rhel_service_class_factory
 knownservices = RHELServices()
diff --git a/ipapython/ntp.py b/ipapython/ntp.py
new file mode 100644
index 0000000000000000000000000000000000000000..ff80018589cbf0628753ba8a0d9ba613fb6d7134
--- /dev/null
+++ b/ipapython/ntp.py
@@ -0,0 +1,307 @@
+#
+# Copyright (C) 2016  FreeIPA Contributors see COPYING for license
+#
+
+import os
+
+from ipapython import ipautil
+from ipapython.ipa_log_manager import root_logger
+from ipaplatform import services
+from ipaplatform.paths import paths
+from ipapython import configfile
+import ipautil
+
+
+class NTPConfigurationError(Exception):
+    pass
+
+
+class NTPConflictingService(NTPConfigurationError):
+    def __init__(self, message='', conflicting_service=None):
+        super(NTPConflictingService, self).__init__(self, message)
+        self.conflicting_service = conflicting_service
+
+    def __str__(self):
+        return ("Conflicting Time&Date synchronization service '%s' is "
+                "currently enabled and/or running on the system."
+                % self.conflicting_service)
+
+
+class NTPService(object):
+    """
+    Base Time&Date service.
+    """
+    def __init__(self):
+        self.service_name = ''
+        self.conf_file = ''
+        self.lens = ''
+        self.source_keywords = ['server']
+        self.server_options = {}
+        self.global_options = {}
+
+    def _store_service_state(self, sstore=None, fstore=None):
+        """
+        Backup state of service before first configuration.
+
+        Do not override state and file when there are already there.
+        We relay on a fact that before first installation state and
+        file store are clean and therefore presence such entries
+        means that service was configured.
+        """
+        service = services.service(self.service_name)
+        if sstore and sstore.get_state('ntp', 'enabled') is None:
+            sstore.backup_state('ntp', 'enabled', service.is_enabled())
+
+        if fstore and not fstore.has_file(self.conf_file):
+            fstore.backup_file(self.conf_file)
+
+    def configure_client(self, ntp_servers=[], sstore=None, fstore=None):
+        service = services.service(self.service_name)
+
+        self._store_service_state(sstore, fstore)
+
+        with configfile.Config(self.conf_file) as timesync:
+
+            # search for time source in configuration
+            for source_kw in self.source_keywords:
+                sources = timesync(source_kw)
+                if sources:
+                    break
+            else:
+                # check if source server is specified
+                if not ntp_servers:
+                    raise NTPConfigurationError(
+                        "No time source specified in configuration nor "
+                        "provided")
+
+            for ntp_server in ntp_servers:
+                timesync('server').append(ntp_server)
+
+            for server in timesync('server'):
+                for o in self.server_options:
+                    if not server(o):
+                        server[o] = self.server_options[o]
+
+            for o in self.global_options:
+                if not timesync(o):
+                    timesync[o] = self.server_options[o]
+
+        service.enable()
+        service.restart()
+
+    def synconce(self, servers):
+        """
+        Perform one-shot time synchronization with provided server.
+
+        !! This method should be implemented by all inherited classes. !!
+        """
+        raise NotImplementedError()
+
+    def check_services(self):
+        """
+        System may contain conflicting services used for time&date
+        synchronization.  As IPA server/client supports only ntpd, make sure
+        that other services are not enabled to prevent conflicts. For example
+        when both chronyd and ntpd are enabled, systemd would always start only
+        chronyd to manage system time&date which would make IPA configuration
+        of ntpd ineffective.
+
+        Reference links:
+          https://fedorahosted.org/freeipa/ticket/2974
+          http://fedoraproject.org/wiki/Features/ChronyDefaultNTP
+        """
+        for service in services.conflicting_ntp_services:
+            # Make sure that the service is not enabled
+            instance = services.service(service)
+            if instance.is_enabled() or instance.is_running():
+                raise NTPConflictingService(
+                    conflicting_service=instance.service_name)
+
+    def force(self, sstore):
+        """
+        Force ntpd configuration and disable and stop any other conflicting
+        time&date service
+        """
+        for service in services.conflicting_ntp_services:
+            instance = services.service(service)
+            enabled = instance.is_enabled()
+            running = instance.is_running()
+
+            if enabled or running:
+                sstore.backup_state(instance.service_name, 'enabled', enabled)
+                sstore.backup_state(instance.service_name, 'running', running)
+
+                if running:
+                    instance.stop()
+
+                if enabled:
+                    instance.disable()
+
+    def restore(self, sstore, fstore):
+        ntp = services.service(self.service_name)
+        ntp_enabled = sstore.restore_state('ntp', 'enabled')
+
+        # Restore might fail due to file missing in backup
+        # the reason for it might be that freeipa-client was updated
+        # to this version but not unenrolled/enrolled again
+        # In such case it is OK to fail
+        try:
+            restored = fstore.restore_file(self.conf_file)
+        except ValueError:
+            restored = False
+
+        if not ntp_enabled:
+            ntp.stop()
+            ntp.disable()
+        else:
+            if restored:
+                ntp.restart()
+
+    def restore_forced(self, sstore):
+        """
+        Restore from --force-ntpd installation and enable/start service that
+        were disabled/stopped during installation
+        """
+        for service in services.conflicting_ntp_services:
+            if sstore.has_state(service):
+                instance = services.service(service)
+                enabled = sstore.restore_state(instance.service_name,
+                                               'enabled')
+                running = sstore.restore_state(instance.service_name,
+                                               'running')
+                if enabled:
+                    instance.enable()
+                if running:
+                    try:
+                        instance.start()
+                    except ipautil.CalledProcessError as e:
+                        root_logger.error('Failed to start %s: %s'
+                                          % (service, e))
+
+    def configure_server(self):
+        raise NotImplementedError()
+
+
+class Ntp(NTPService):
+
+    def __init__(self):
+        super(Ntp, self).__init__()
+        self.service_name = 'ntpd'
+        self.conf_file = paths.NTP_CONF
+        self.lens = 'Ntp.lns'
+        self.server_options['burst'] = None
+        self.server_options['iburst'] = None
+
+    def configure_client(self, ntp_servers=[], sstore=None, fstore=None):
+        super(Ntp, self).configure_client(ntp_servers, sstore, fstore)
+
+    def synconce(self, servers):
+        """
+        Syncs time with specified server using ntpd.
+        Primarily designed to be used before Kerberos setup
+        to get time following the KDC time
+        """
+        ntpd = paths.NTPD
+        for server_fqdn in servers:
+            tmp_ntpd_conf = ipautil.write_tmp_file('server %s' % server_fqdn)
+            try:
+                ipautil.run([ntpd, '-qgc', tmp_ntpd_conf.name], timeout=60)
+            except (ipautil.CalledProcessError, OSError) as e:
+                root_logger.warning('Time synchronization with server %s'
+                                    'failed: %s ' % (server_fqdn, e))
+            break
+        else:
+            raise RuntimeError('Time synchronization failed')
+
+    def configure_server(self, sstore=None, fstore=None):
+        self._store_service_state(sstore, fstore)
+
+        with configfile.Config(self.conf_file) as timesync:
+
+            # add local server if not already in configuration
+            for server in timesync('server'):
+                if server[0] == '127.127.1.0':
+                    break
+            else:
+                timesync('server').append('127.127.1.0')
+
+            for f in timesync('fudge'):
+                if f[0] == '127.127.1.0':
+                    break
+            else:
+                timesync('fudge').append('127.127.1.0')
+                timesync('fudge', -1)['stratum'] = '8'
+
+            # allow clients only to sync
+            for restrict in timesync['restrict']:
+                if restrict[0] == 'default':
+                    break
+            else:
+                timesync['restrict'].append('default')
+
+            flags = ['nomodify', 'notrap', 'nopeer', 'noquery']
+            for restrict in timesync['restrict']:
+                if restrict[0] == 'default':
+                    for flag in flags:
+                        if not restrict(flag):
+                            restrict[flag] = None
+
+                    if restrict('ignore'):
+                        del restrict['ignore']
+                    break
+
+            # allow unrestricted access from localhost
+            timesync['restrict'].append('localhost')
+
+
+class Chrony(NTPService):
+
+    def __init__(self):
+        super(Chrony, self).__init__()
+        self.service_name = 'chronyd'
+        self.conf_file = paths.CHRONY_CONF
+        self.lens = 'chrony.lns'
+        self.source_keywords = ['server', 'pool', 'peer']
+        self.server_options['iburst'] = None
+
+    def synconce(self, servers):
+        """
+        Syncs time with specified server using ntpd.
+        Primarily designed to be used before Kerberos setup
+        to get time following the KDC time
+
+        Returns True if sync was successful
+        """
+        chronyd = paths.CHRONYD
+        for server_fqdn in servers:
+            try:
+                ipautil.run([chronyd, '-q', 'server %s iburst' % server_fqdn],
+                            timeout=60)
+            except (ipautil.CalledProcessError, OSError) as e:
+                root_logger.warning('Time synchronization with server %s '
+                                    'failed: %s ' % (server_fqdn, e))
+
+            # chronyd uses system capabilities a thus drops root privileges
+            # after creating pid file (among other initial actions). Therefore
+            # it is unable to remove the file at cleanup.
+            try:
+                os.remove(paths.CHRONYD_PID_FILE)
+            except OSError as e:
+                # not much we can do, log it and hope for the best
+                root_logger.warning("Failed to remove chronyd pid file.")
+
+            break  # sync successful
+        else:
+            raise RuntimeError('Time synchronization failed')
+
+    def configure_client(self, ntp_servers=[], sstore=None, fstore=None):
+        super(Chrony, self).configure_client(ntp_servers, sstore, fstore)
+
+    def configure_server(self, sstore=None, fstore=None):
+        self._store_service_state(sstore, fstore)
+
+        with configfile.Config(self.conf_file) as timesync:
+            if not timesync('local'):
+                timesync('local')['stratum'] = '8'
+            if not timesync['allow']:
+                timesync['allow'] = 'all'
-- 
2.5.0

From ca89ab2a6f549a7ce740f2c2c9fda5e72f545f1a Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Fri, 11 Mar 2016 07:58:52 +0100
Subject: [PATCH 3/4] ntp: Add platform specific tasks

Tasks configures NTP service that is default for given platform. User still can
configure service of his choice and IPA won't touch it unless forced.

https://fedorahosted.org/freeipa/ticket/4669
---
 ipaplatform/base/tasks.py   | 30 ++++++++++++++++++++++++++++++
 ipaplatform/fedora/tasks.py | 30 +++++++++++++++++++++++++++++-
 2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/ipaplatform/base/tasks.py b/ipaplatform/base/tasks.py
index f5fb2b155020c213769830dd48ccc3b36bbd9e64..685daf7afa8ba90db7efad23883de7a61bb6b5fe 100644
--- a/ipaplatform/base/tasks.py
+++ b/ipaplatform/base/tasks.py
@@ -27,6 +27,7 @@ import grp
 
 from pkg_resources import parse_version
 
+import ipapython
 from ipaplatform.paths import paths
 from ipapython.ipa_log_manager import log_mgr
 from ipapython import ipautil
@@ -244,3 +245,32 @@ class BaseTaskNamespace(object):
     def remove_httpd_service_ipa_conf(self):
         """Remove configuration of httpd service of IPA"""
         raise NotImplementedError()
+
+    def ntp_check_conflict(self, force=False):
+        if not force:
+            ntp = ipapython.ntp.Ntp()
+            ntp.check_services()
+
+    def ntp_sync_once(self, server):
+        ntp = ipapython.ntp.Ntp()
+        ntp.synconce(server)
+
+    def ntp_configure_client(self, sstore, fstore, ntp_servers=[],
+                             force=False):
+        ntp = ipapython.ntp.Ntp()
+        if force:
+            ntp.force(sstore)
+        ntp.configure_client(ntp_servers, sstore, fstore)
+
+    def ntp_configure_server(self, sstore, fstore, force=False):
+        ntp = ipapython.ntp.Ntp()
+        if force:
+            ntp.force(sstore)
+        ntp.configure_server(sstore, fstore)
+
+    def ntp_uninstall(self, sstore, fstore):
+        ntp_configured = sstore.has_state('ntp')
+        if ntp_configured:
+            ntp = ipapython.ntp.Ntp()
+            ntp.restore(sstore, fstore)
+            ntp.restore_forced(sstore=sstore)
diff --git a/ipaplatform/fedora/tasks.py b/ipaplatform/fedora/tasks.py
index 903bb9211a2b9f0f2435ad66023a2b443b3ce5f1..c6c960793fc8f4097f64ea0873e304f3e2365b5b 100644
--- a/ipaplatform/fedora/tasks.py
+++ b/ipaplatform/fedora/tasks.py
@@ -24,10 +24,38 @@ This module contains default Fedora-specific implementations of system tasks.
 '''
 
 from ipaplatform.redhat.tasks import RedHatTaskNamespace
+import ipapython
 
 
 class FedoraTaskNamespace(RedHatTaskNamespace):
-    pass
 
+    def ntp_check_conflict(self, force=False):
+        if not force:
+            ntp = ipapython.ntp.Chrony()
+            ntp.check_services()
+
+    def ntp_sync_once(self, server):
+        ntp = ipapython.ntp.Chrony()
+        ntp.synconce(server)
+
+    def ntp_configure_client(self, sstore, fstore, ntp_servers=[],
+                             force=False):
+        ntp = ipapython.ntp.Chrony()
+        if force:
+            ntp.force(sstore)
+        ntp.configure_client(ntp_servers, sstore, fstore)
+
+    def ntp_configure_server(self, sstore, fstore, force=False):
+        ntp = ipapython.ntp.Chrony()
+        if force:
+            ntp.force(sstore)
+        ntp.configure_server(sstore, fstore)
+
+    def ntp_uninstall(self, sstore, fstore):
+        ntp_configured = sstore.has_state('ntp')
+        if ntp_configured:
+            ntp = ipapython.ntp.Chrony()
+            ntp.restore(sstore, fstore)
+            ntp.restore_forced(sstore)
 
 tasks = FedoraTaskNamespace()
-- 
2.5.0

From f1aff16495aa32f778280b828dabaac13a7a5906 Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Thu, 10 Mar 2016 22:41:29 +0100
Subject: [PATCH 4/4] ntp: install: Use tasks to configure NTP daemon

https://fedorahosted.org/freeipa/ticket/4669
---
 client/ipa-client-install                  | 105 ++++---------
 ipaclient/ntpconf.py                       | 233 -----------------------------
 ipaplatform/base/services.py               |   3 -
 ipaplatform/fedora/services.py             |   1 -
 ipaplatform/redhat/services.py             |   1 -
 ipaplatform/rhel/services.py               |   1 -
 ipaserver/install/dns.py                   |   3 +-
 ipaserver/install/ntpinstance.py           | 164 ++------------------
 ipaserver/install/server/install.py        |  25 +---
 ipaserver/install/server/replicainstall.py |  37 ++---
 10 files changed, 64 insertions(+), 509 deletions(-)
 delete mode 100644 ipaclient/ntpconf.py

diff --git a/client/ipa-client-install b/client/ipa-client-install
index 0e6e65c4ad4ce01fe1257eee4bb2633a70c3de4e..7b1b306dc70386937d53b1a4673359181a9eccce 100755
--- a/client/ipa-client-install
+++ b/client/ipa-client-install
@@ -42,7 +42,6 @@ try:
     from ipapython.ipa_log_manager import standard_logging_setup, root_logger
     from ipaclient import ipadiscovery
     import ipaclient.ipachangeconf
-    import ipaclient.ntpconf
     from ipapython.ipautil import (
         run, user_input, CalledProcessError, file_exists, dir_exists,
         realm_to_suffix)
@@ -50,7 +49,7 @@ try:
     from ipaplatform import services
     from ipaplatform.paths import paths
     from ipapython import ipautil, sysrestore, version, certmonger, ipaldap
-    from ipapython import kernel_keyring, certdb
+    from ipapython import kernel_keyring, certdb, ntp
     from ipapython.config import IPAOptionParser
     from ipalib import api, errors
     from ipalib import x509, certstore
@@ -142,9 +141,10 @@ def parse_options():
                                 "multiple times")
     basic_group.add_option("-N", "--no-ntp", action="store_false",
                       help="do not configure ntp", default=True, dest="conf_ntp")
-    basic_group.add_option("", "--force-ntpd", dest="force_ntpd",
+    basic_group.add_option("", "--force-ntp", dest="force_ntp",
                       action="store_true", default=False,
-                      help="Stop and disable any time&date synchronization services besides ntpd")
+                      help="Stop and disable any NTP services besides %s" %
+                      services.ntp_service)
     basic_group.add_option("--nisdomain", dest="nisdomain",
                            help="NIS domain name")
     basic_group.add_option("--no-nisdomain", action="store_true", default=False,
@@ -230,8 +230,8 @@ def parse_options():
     if (options.server and not options.domain):
         parser.error("--server cannot be used without providing --domain")
 
-    if options.force_ntpd and not options.conf_ntp:
-        parser.error("--force-ntpd cannot be used together with --no-ntp")
+    if options.force_ntp and not options.conf_ntp:
+        parser.error("--force-ntp cannot be used together with --no-ntp")
 
     if options.firefox_dir and not options.configure_firefox:
         parser.error("--firefox-dir cannot be used without --configure-firefox option")
@@ -734,35 +734,7 @@ def uninstall(options, env):
                 service.service_name
             )
 
-    ntp_configured = statestore.has_state('ntp')
-    if ntp_configured:
-        ntp_enabled = statestore.restore_state('ntp', 'enabled')
-        ntp_step_tickers = statestore.restore_state('ntp', 'step-tickers')
-        restored = False
-
-        try:
-            # Restore might fail due to file missing in backup
-            # the reason for it might be that freeipa-client was updated
-            # to this version but not unenrolled/enrolled again
-            # In such case it is OK to fail
-            restored = fstore.restore_file(paths.NTP_CONF)
-            restored |= fstore.restore_file(paths.SYSCONFIG_NTPD)
-            if ntp_step_tickers:
-               restored |= fstore.restore_file(paths.NTP_STEP_TICKERS)
-        except Exception:
-            pass
-
-        if not ntp_enabled:
-           services.knownservices.ntpd.stop()
-           services.knownservices.ntpd.disable()
-        else:
-           if restored:
-               services.knownservices.ntpd.restart()
-
-    try:
-        ipaclient.ntpconf.restore_forced_ntpd(statestore)
-    except CalledProcessError as e:
-        root_logger.error('Failed to start chronyd: %s', e)
+    tasks.ntp_uninstall(statestore, fstore)
 
     if was_sshd_configured and services.knownservices.sshd.is_running():
         services.knownservices.sshd.restart()
@@ -2223,21 +2195,12 @@ def install(options, env, fstore, statestore):
     cli_domain_source = 'Unknown source'
     cli_server_source = 'Unknown source'
 
-    if options.conf_ntp and not options.on_master and not options.force_ntpd:
+    if options.conf_ntp and not options.on_master:
         try:
-            ipaclient.ntpconf.check_timedate_services()
-        except ipaclient.ntpconf.NTPConflictingService as e:
-            print("WARNING: ntpd time&date synchronization service will not" \
-                  " be configured as")
-            print("conflicting service (%s) is enabled" % e.conflicting_service)
-            print("Use --force-ntpd option to disable it and force configuration" \
-                  " of ntpd")
-            print("")
-
-            # configuration of ntpd is disabled in this case
+            tasks.ntp_check_conflict(options.force_ntp)
+        except ntp.NTPConflictingService as e:
+            print(e)
             options.conf_ntp = False
-        except ipaclient.ntpconf.NTPConfigurationError:
-            pass
 
     if options.unattended and (options.password is None and
                                options.principal is None and
@@ -2507,31 +2470,27 @@ def install(options, env, fstore, statestore):
         # hostname if different from system hostname
         tasks.backup_and_replace_hostname(fstore, statestore, options.hostname)
 
-    ntp_srv_servers = []
+    ntp_servers = []
     if not options.on_master and options.conf_ntp:
         # Attempt to sync time with IPA server.
         # If we're skipping NTP configuration, we also skip the time sync here.
         # We assume that NTP servers are discoverable through SRV records in the DNS
         # If that fails, we try to sync directly with IPA server, assuming it runs NTP
         root_logger.info('Synchronizing time with KDC...')
-        ntp_srv_servers = ds.ipadns_search_srv(cli_domain, '_ntp._udp',
+
+        if options.ntp_servers:
+            # use user specified NTP servers if there are any
+            ntp_servers = options.ntp_servers
+        else:
+            # resolve NTP servers
+            ntp_servers = ds.ipadns_search_srv(cli_domain, '_ntp._udp',
                                                None, break_on_first=False)
-        synced_ntp = False
-        ntp_servers = ntp_srv_servers
 
-        # use user specified NTP servers if there are any
-        if options.ntp_servers:
-            ntp_servers = options.ntp_servers
-
-        for s in ntp_servers:
-            synced_ntp = ipaclient.ntpconf.synconce_ntp(s, options.debug)
-            if synced_ntp:
-                break
-
-        if not synced_ntp and not options.ntp_servers:
-            synced_ntp = ipaclient.ntpconf.synconce_ntp(cli_server[0],
-                                                        options.debug)
-        if not synced_ntp:
+        # add freeipa server as a NTP server
+        ntp_servers.append(cli_server[0])
+        try:
+            tasks.ntp_sync_once(ntp_servers)
+        except RuntimeError as e:
             root_logger.warning("Unable to sync time with NTP " +
                 "server, assuming the time is in sync. Please check " +
                 "that 123 UDP port is opened.")
@@ -3017,20 +2976,8 @@ def install(options, env, fstore, statestore):
                         "/etc/ldap.conf failed: %s", str(e))
 
     if options.conf_ntp and not options.on_master:
-        # disable other time&date services first
-        if options.force_ntpd:
-            ipaclient.ntpconf.force_ntpd(statestore)
-
-        if options.ntp_servers:
-            ntp_servers = options.ntp_servers
-        elif ntp_srv_servers:
-            ntp_servers = ntp_srv_servers
-        else:
-            root_logger.warning("No SRV records of NTP servers found. IPA "
-                                "server address will be used")
-            ntp_servers = cli_server
-
-        ipaclient.ntpconf.config_ntp(ntp_servers, fstore, statestore)
+        tasks.ntp_configure_client(statestore, fstore, options.ntp_servers,
+                                   options.force_ntp)
         root_logger.info("NTP enabled")
 
     if options.conf_ssh:
diff --git a/ipaclient/ntpconf.py b/ipaclient/ntpconf.py
deleted file mode 100644
index 9a7db6544b54288569dc7699e67ddc865bb88db4..0000000000000000000000000000000000000000
--- a/ipaclient/ntpconf.py
+++ /dev/null
@@ -1,233 +0,0 @@
-# Authors: Karl MacMillan <kmacmil...@redhat.com>
-#
-# Copyright (C) 2007  Red Hat
-# see file 'COPYING' for use and warranty information
-#
-# This program is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation, either version 3 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program.  If not, see <http://www.gnu.org/licenses/>.
-#
-
-from ipapython import ipautil
-from ipapython.ipa_log_manager import root_logger
-import shutil
-import os
-from ipaplatform.tasks import tasks
-from ipaplatform import services
-from ipaplatform.paths import paths
-
-ntp_conf = """# Permit time synchronization with our time source, but do not
-# permit the source to query or modify the service on this system.
-restrict default kod nomodify notrap nopeer noquery
-restrict -6 default kod nomodify notrap nopeer noquery
-
-# Permit all access over the loopback interface.  This could
-# be tightened as well, but to do so would effect some of
-# the administrative functions.
-restrict 127.0.0.1
-restrict -6 ::1
-
-# Hosts on local network are less restricted.
-#restrict 192.168.1.0 mask 255.255.255.0 nomodify notrap
-
-# Use public servers from the pool.ntp.org project.
-# Please consider joining the pool (http://www.pool.ntp.org/join.html).
-$SERVERS_BLOCK
-
-#broadcast 192.168.1.255 key 42		# broadcast server
-#broadcastclient			# broadcast client
-#broadcast 224.0.1.1 key 42		# multicast server
-#multicastclient 224.0.1.1		# multicast client
-#manycastserver 239.255.254.254		# manycast server
-#manycastclient 239.255.254.254 key 42	# manycast client
-
-# Undisciplined Local Clock. This is a fake driver intended for backup
-# and when no outside source of synchronized time is available.
-server	127.127.1.0	# local clock
-#fudge	127.127.1.0 stratum 10
-
-# Drift file.  Put this in a directory which the daemon can write to.
-# No symbolic links allowed, either, since the daemon updates the file
-# by creating a temporary in the same directory and then rename()'ing
-# it to the file.
-driftfile /var/lib/ntp/drift
-
-# Key file containing the keys and key identifiers used when operating
-# with symmetric key cryptography.
-keys /etc/ntp/keys
-
-# Specify the key identifiers which are trusted.
-#trustedkey 4 8 42
-
-# Specify the key identifier to use with the ntpdc utility.
-#requestkey 8
-
-# Specify the key identifier to use with the ntpq utility.
-#controlkey 8
-"""
-
-ntp_sysconfig = """OPTIONS="-x -p /var/run/ntpd.pid"
-
-# Set to 'yes' to sync hw clock after successful ntpdate
-SYNC_HWCLOCK=yes
-
-# Additional options for ntpdate
-NTPDATE_OPTIONS=""
-"""
-ntp_step_tickers = """# Use IPA-provided NTP server for initial time
-$TICKER_SERVERS_BLOCK
-"""
-def __backup_config(path, fstore = None):
-    if fstore:
-        fstore.backup_file(path)
-    else:
-        shutil.copy(path, "%s.ipasave" % (path))
-
-def __write_config(path, content):
-    fd = open(path, "w")
-    fd.write(content)
-    fd.close()
-
-def config_ntp(ntp_servers, fstore = None, sysstore = None):
-    path_step_tickers = paths.NTP_STEP_TICKERS
-    path_ntp_conf = paths.NTP_CONF
-    path_ntp_sysconfig = paths.SYSCONFIG_NTPD
-    sub_dict = {}
-    sub_dict["SERVERS_BLOCK"] = "\n".join("server %s" % s for s in ntp_servers)
-    sub_dict["TICKER_SERVERS_BLOCK"] = "\n".join(ntp_servers)
-
-    nc = ipautil.template_str(ntp_conf, sub_dict)
-    config_step_tickers = False
-
-
-    if os.path.exists(path_step_tickers):
-        config_step_tickers = True
-        ns = ipautil.template_str(ntp_step_tickers, sub_dict)
-        __backup_config(path_step_tickers, fstore)
-        __write_config(path_step_tickers, ns)
-        tasks.restore_context(path_step_tickers)
-
-    if sysstore:
-        module = 'ntp'
-        sysstore.backup_state(module, "enabled", services.knownservices.ntpd.is_enabled())
-        if config_step_tickers:
-            sysstore.backup_state(module, "step-tickers", True)
-
-    __backup_config(path_ntp_conf, fstore)
-    __write_config(path_ntp_conf, nc)
-    tasks.restore_context(path_ntp_conf)
-
-    __backup_config(path_ntp_sysconfig, fstore)
-    __write_config(path_ntp_sysconfig, ntp_sysconfig)
-    tasks.restore_context(path_ntp_sysconfig)
-
-    # Set the ntpd to start on boot
-    services.knownservices.ntpd.enable()
-
-    # Restart ntpd
-    services.knownservices.ntpd.restart()
-
-
-def synconce_ntp(server_fqdn, debug=False):
-    """
-    Syncs time with specified server using ntpd.
-    Primarily designed to be used before Kerberos setup
-    to get time following the KDC time
-
-    Returns True if sync was successful
-    """
-    ntpd = paths.NTPD
-    if not os.path.exists(ntpd):
-        return False
-
-    tmp_ntp_conf = ipautil.write_tmp_file('server %s' % server_fqdn)
-    args = [ntpd, '-qgc', tmp_ntp_conf.name]
-    if debug:
-        args.append('-d')
-    try:
-        # The ntpd command will never exit if it is unable to reach the
-        # server, so timeout after 15 seconds.
-        timeout = 15
-        root_logger.info('Attempting to sync time using ntpd.  '
-                         'Will timeout after %d seconds' % timeout)
-        ipautil.run(args, timeout=timeout)
-        return True
-    except ipautil.CalledProcessError:
-        return False
-
-
-class NTPConfigurationError(Exception):
-    pass
-
-class NTPConflictingService(NTPConfigurationError):
-    def __init__(self, message='', conflicting_service=None):
-        super(NTPConflictingService, self).__init__(self, message)
-        self.conflicting_service = conflicting_service
-
-def check_timedate_services():
-    """
-    System may contain conflicting services used for time&date synchronization.
-    As IPA server/client supports only ntpd, make sure that other services are
-    not enabled to prevent conflicts. For example when both chronyd and ntpd
-    are enabled, systemd would always start only chronyd to manage system
-    time&date which would make IPA configuration of ntpd ineffective.
-
-    Reference links:
-      https://fedorahosted.org/freeipa/ticket/2974
-      http://fedoraproject.org/wiki/Features/ChronyDefaultNTP
-    """
-    for service in services.timedate_services:
-        if service == 'ntpd':
-            continue
-        # Make sure that the service is not enabled
-        instance = services.service(service)
-        if instance.is_enabled() or instance.is_running():
-            raise NTPConflictingService(conflicting_service=instance.service_name)
-
-def force_ntpd(statestore):
-    """
-    Force ntpd configuration and disable and stop any other conflicting
-    time&date service
-    """
-    for service in services.timedate_services:
-        if service == 'ntpd':
-            continue
-        instance = services.service(service)
-        enabled = instance.is_enabled()
-        running = instance.is_running()
-
-        if enabled or running:
-            statestore.backup_state(instance.service_name, 'enabled', enabled)
-            statestore.backup_state(instance.service_name, 'running', running)
-
-            if running:
-                instance.stop()
-
-            if enabled:
-                instance.disable()
-
-def restore_forced_ntpd(statestore):
-    """
-    Restore from --force-ntpd installation and enable/start service that were
-    disabled/stopped during installation
-    """
-    for service in services.timedate_services:
-        if service == 'ntpd':
-            continue
-        if statestore.has_state(service):
-            instance = services.service(service)
-            enabled = statestore.restore_state(instance.service_name, 'enabled')
-            running = statestore.restore_state(instance.service_name, 'running')
-            if enabled:
-                instance.enable()
-            if running:
-                instance.start()
diff --git a/ipaplatform/base/services.py b/ipaplatform/base/services.py
index 0969eb165f0bd660d205d12255305a6836a4af59..98896312d5f3ed651d0fbcc5c99a94d2ac22e07f 100644
--- a/ipaplatform/base/services.py
+++ b/ipaplatform/base/services.py
@@ -490,8 +490,5 @@ class SystemdService(PlatformService):
 service = None
 knownservices = None
 
-# System may support more time&date services. FreeIPA supports ntpd only, other
-# services will be disabled during IPA installation
-timedate_services = ['ntpd', 'chronyd']
 ntp_service = 'ntpd'
 conflicting_ntp_services = ['chronyd', 'systemd-timesyncd']
diff --git a/ipaplatform/fedora/services.py b/ipaplatform/fedora/services.py
index 4bddc2b9d5b39f81c1c95b536df380a17421cd02..ba67b219b8c643f5486cdce34c730714557025a3 100644
--- a/ipaplatform/fedora/services.py
+++ b/ipaplatform/fedora/services.py
@@ -56,7 +56,6 @@ class FedoraServices(redhat_services.RedHatServices):
 
 # Objects below are expected to be exported by platform module
 
-timedate_services = redhat_services.timedate_services
 ntp_service = 'chronyd'
 conflicting_ntp_services = ['ntpd', 'systemd-timesyncd']
 service = fedora_service_class_factory
diff --git a/ipaplatform/redhat/services.py b/ipaplatform/redhat/services.py
index 7b4fc6fda6ec8632de89af4732432552636aa021..078dbe86a849152568bba7bae37f8d1c99bc14e9 100644
--- a/ipaplatform/redhat/services.py
+++ b/ipaplatform/redhat/services.py
@@ -254,7 +254,6 @@ class RedHatServices(base_services.KnownServices):
 
 # Objects below are expected to be exported by platform module
 
-timedate_services = base_services.timedate_services
 ntp_service = base_services.ntp_service
 conflicting_ntp_services = base_services.conflicting_ntp_services
 service = redhat_service_class_factory
diff --git a/ipaplatform/rhel/services.py b/ipaplatform/rhel/services.py
index c36ea5b43b4ad56111ce41d8c6a9c6ce60a1f36a..5cd07a2e4243fddd4f3a27fb3b90157ab94fb702 100644
--- a/ipaplatform/rhel/services.py
+++ b/ipaplatform/rhel/services.py
@@ -56,7 +56,6 @@ class RHELServices(redhat_services.RedHatServices):
 
 # Objects below are expected to be exported by platform module
 
-timedate_services = redhat_services.timedate_services
 ntp_service = redhat_services.ntp_service
 conflicting_ntp_services = redhat_services.conflicting_ntp_services
 service = rhel_service_class_factory
diff --git a/ipaserver/install/dns.py b/ipaserver/install/dns.py
index dbeacaee804dacd102d47aa28e9600adedead884..648834c598fe5a1add4ef706f6a9d9f33f32e5bb 100644
--- a/ipaserver/install/dns.py
+++ b/ipaserver/install/dns.py
@@ -307,9 +307,10 @@ def install(standalone, replica, options, api=api):
                               api.env.basedn)
     conn = api.Backend.ldap2
 
+    sstore = sysrestore.StateFile(paths.SYSRESTORE)
     fstore = sysrestore.FileStore(paths.SYSRESTORE)
 
-    conf_ntp = ntpinstance.NTPInstance(fstore).is_enabled()
+    conf_ntp = ntpinstance.NTPInstance(sstore, fstore).is_enabled()
 
     if standalone:
         # otherwise this is done by server/replica installer
diff --git a/ipaserver/install/ntpinstance.py b/ipaserver/install/ntpinstance.py
index 8b0f0e5395dae3c058fc31bd8914741e4d158830..bb4acf163dac60b5f7c31235aaca8b599e8828d2 100644
--- a/ipaserver/install/ntpinstance.py
+++ b/ipaserver/install/ntpinstance.py
@@ -19,168 +19,34 @@
 #
 
 from ipaserver.install import service
-from ipapython import sysrestore
-from ipapython import ipautil
-from ipaplatform.constants import constants
-from ipaplatform.paths import paths
-from ipapython.ipa_log_manager import root_logger
+from ipaplatform import services
+from ipaplatform.tasks import tasks
 
-NTPD_OPTS_VAR = constants.NTPD_OPTS_VAR
-NTPD_OPTS_QUOTE = constants.NTPD_OPTS_QUOTE
 
 class NTPInstance(service.Service):
-    def __init__(self, fstore=None):
-        service.Service.__init__(self, "ntpd", service_desc="NTP daemon")
+    def __init__(self, sstore, fstore, force=False):
+        service.Service.__init__(self, services.ntp_service,
+                                 service_desc="NTP daemon")
 
-        if fstore:
-            self.fstore = fstore
-        else:
-            self.fstore = sysrestore.FileStore(paths.SYSRESTORE)
+        self.sstore = sstore
+        self.fstore = fstore
+        self.force = force
 
-    def __write_config(self):
+    def __configure_client(self):
+        tasks.ntp_configure_client(self.sstore, self.fstore, force=self.force)
 
-        self.fstore.backup_file(paths.NTP_CONF)
-        self.fstore.backup_file(paths.SYSCONFIG_NTPD)
-
-        # We use the OS variable to point it towards either the rhel
-        # or fedora pools. Other distros should be added in the future
-        # or we can get our own pool.
-        os = ""
-        if ipautil.file_exists(paths.ETC_FEDORA_RELEASE):
-            os = "fedora"
-        elif ipautil.file_exists(paths.ETC_REDHAT_RELEASE):
-            os = "rhel"
-
-        srv_vals = []
-        srv_vals.append("0.%s.pool.ntp.org" % os)
-        srv_vals.append("1.%s.pool.ntp.org" % os)
-        srv_vals.append("2.%s.pool.ntp.org" % os)
-        srv_vals.append("3.%s.pool.ntp.org" % os)
-        srv_vals.append("127.127.1.0")
-        fudge = ["fudge", "127.127.1.0", "stratum", "10"]
-
-        #read in memory, change it, then overwrite file
-        file_changed = False
-        fudge_present = False
-        ntpconf = []
-        fd = open(paths.NTP_CONF, "r")
-        for line in fd:
-            opt = line.split()
-            if len(opt) < 1:
-                ntpconf.append(line)
-                continue
-
-            if opt[0] == "server":
-                match = False
-                for srv in srv_vals:
-                    if opt[1] == srv:
-                        match = True
-                        break
-                if match:
-                    srv_vals.remove(srv)
-                else:
-                    file_changed = True
-                    line = ""
-            elif opt[0] == "fudge":
-                if opt[0:4] == fudge[0:4]:
-                    fudge_present = True
-                else:
-                    file_changed = True
-                    line = ""
-
-            ntpconf.append(line)
-
-        if file_changed or len(srv_vals) != 0 or not fudge_present:
-            fd = open(paths.NTP_CONF, "w")
-            for line in ntpconf:
-                fd.write(line)
-            fd.write("\n### Added by IPA Installer ###\n")
-            if len(srv_vals) != 0:
-                for srv in srv_vals:
-                    fd.write("server "+srv+" iburst\n")
-            if not fudge_present:
-                fd.write("fudge 127.127.1.0 stratum 10\n")
-            fd.close()
-
-        #read in memory, find OPTIONS, check/change it, then overwrite file
-        needopts = [ {'val':'-x', 'need':True},
-                     {'val':'-g', 'need':True} ]
-        fd = open(paths.SYSCONFIG_NTPD, "r")
-        lines = fd.readlines()
-        fd.close()
-        for line in lines:
-            sline = line.strip()
-            if not sline.startswith(NTPD_OPTS_VAR):
-                continue
-            sline = sline.replace(NTPD_OPTS_QUOTE, '')
-            for opt in needopts:
-                if sline.find(opt['val']) != -1:
-                    opt['need'] = False
-
-        newopts = []
-        for opt in needopts:
-            if opt['need']:
-                newopts.append(opt['val'])
-
-        done = False
-        if newopts:
-            fd = open(paths.SYSCONFIG_NTPD, "w")
-            for line in lines:
-                if not done:
-                    sline = line.strip()
-                    if not sline.startswith(NTPD_OPTS_VAR):
-                        fd.write(line)
-                        continue
-                    sline = sline.replace(NTPD_OPTS_QUOTE, '')
-                    (variable, opts) = sline.split('=', 1)
-                    fd.write(NTPD_OPTS_VAR + '="%s %s"\n' % (opts, ' '.join(newopts)))
-                    done = True
-                else:
-                    fd.write(line)
-            fd.close()
-
-    def __stop(self):
-        self.backup_state("running", self.is_running())
-        self.stop()
-
-    def __start(self):
-        self.start()
-
-    def __enable(self):
-        self.backup_state("enabled", self.is_enabled())
-        self.enable()
+    def __configure_server(self):
+        tasks.ntp_configure_server(self.sstore, self.fstore, self.force)
 
     def create_instance(self):
 
         # we might consider setting the date manually using ntpd -qg in case
         # the current time is very far off.
 
-        self.step("stopping ntpd", self.__stop)
-        self.step("writing configuration", self.__write_config)
-        self.step("configuring ntpd to start on boot", self.__enable)
-        self.step("starting ntpd", self.__start)
+        self.step("configuring NTP client", self.__configure_client)
+        self.step("configuring NTP server", self.__configure_server)
 
         self.start_creation()
 
     def uninstall(self):
-        if self.is_configured():
-            self.print_msg("Unconfiguring %s" % self.service_name)
-
-        running = self.restore_state("running")
-        enabled = self.restore_state("enabled")
-
-        # service is not in LDAP, stop and disable service
-        # before restoring configuration
-        self.stop()
-        self.disable()
-
-        try:
-            self.fstore.restore_file(paths.NTP_CONF)
-        except ValueError as error:
-            root_logger.debug(error)
-
-        if enabled:
-            self.enable()
-
-        if running:
-            self.restart()
+        tasks.ntp_uninstall(self.sstore, self.fstore)
diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py
index f01022c4c3a056513db47f70727aa48157a8c6f2..106f077de6e4a29f74c4e8464180b4eff6fbcaf4 100644
--- a/ipaserver/install/server/install.py
+++ b/ipaserver/install/server/install.py
@@ -16,7 +16,7 @@ import textwrap
 
 import six
 
-from ipapython import certmonger, ipaldap, ipautil, sysrestore
+from ipapython import certmonger, ipaldap, ipautil, sysrestore, ntp
 from ipapython.dn import DN
 from ipapython.install import core
 from ipapython.install.common import step
@@ -31,7 +31,6 @@ from ipalib import api, create_api, constants, errors, x509
 from ipalib.krb_utils import KRB5KDC_ERR_C_PRINCIPAL_UNKNOWN
 from ipalib.constants import CACERT
 from ipalib.util import validate_domain_name
-import ipaclient.ntpconf
 from ipaserver.install import (
     bindinstance, ca, cainstance, certs, dns, dsinstance, httpinstance,
     installutils, kra, krbinstance, memcacheinstance, ntpinstance,
@@ -526,14 +525,10 @@ def install_check(installer):
 
     if not options.no_ntp:
         try:
-            ipaclient.ntpconf.check_timedate_services()
-        except ipaclient.ntpconf.NTPConflictingService as e:
-            print(("WARNING: conflicting time&date synchronization service '%s'"
-                  " will be disabled" % e.conflicting_service))
-            print("in favor of ntpd")
-            print("")
-        except ipaclient.ntpconf.NTPConfigurationError:
-            pass
+            tasks.ntp_check_conflict()
+        except ntp.NTPConflictingService as e:
+            print(e)
+            options.no_ntp = True
 
     # Check to see if httpd is already configured to listen on 443
     if httpinstance.httpd_443_configured():
@@ -842,10 +837,8 @@ def install(installer):
     if not options.external_cert_files:
         # Configure ntpd
         if not options.no_ntp:
-            ipaclient.ntpconf.force_ntpd(sstore)
-            ntp = ntpinstance.NTPInstance(fstore)
-            if not ntp.is_configured():
-                ntp.create_instance()
+            ntp_instance = ntpinstance.NTPInstance(sstore, fstore)
+            ntp_instance.create_instance()
 
         if options.dirsrv_cert_files:
             ds = dsinstance.DsInstance(fstore=fstore,
@@ -1196,7 +1189,7 @@ def uninstall(installer):
     except Exception as e:
         pass
 
-    ntpinstance.NTPInstance(fstore).uninstall()
+    tasks.ntp_uninstall(sstore, fstore)
 
     kra.uninstall(False)
 
@@ -1227,8 +1220,6 @@ def uninstall(installer):
 
     sstore._load()
 
-    ipaclient.ntpconf.restore_forced_ntpd(sstore)
-
     # Clean up group_exists (unused since IPA 2.2, not being set since 4.1)
     sstore.restore_state("install", "group_exists")
 
diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index e3052c129d9b6dcdef43b4fd554b13adfff27fdf..2b5f187f405970af1c645182c17dbc3aa85f0c1c 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -17,7 +17,7 @@ import tempfile
 
 import six
 
-from ipapython import ipaldap, ipautil, sysrestore
+from ipapython import ipaldap, ipautil, sysrestore, ntp
 from ipapython.dn import DN
 from ipapython.install.common import step
 from ipapython.install.core import Knob
@@ -27,7 +27,6 @@ from ipaplatform.tasks import tasks
 from ipaplatform.paths import paths
 from ipalib import api, certstore, constants, create_api, errors, x509
 import ipaclient.ipachangeconf
-import ipaclient.ntpconf
 from ipaserver.install import (
     bindinstance, ca, cainstance, certs, dns, dsinstance, httpinstance,
     installutils, kra, krainstance, krbinstance, memcacheinstance,
@@ -521,14 +520,10 @@ def install_check(installer):
 
     if not options.no_ntp:
         try:
-            ipaclient.ntpconf.check_timedate_services()
-        except ipaclient.ntpconf.NTPConflictingService as e:
-            print(("WARNING: conflicting time&date synchronization service '%s'"
-                  " will" % e.conflicting_service))
-            print("be disabled in favor of ntpd")
-            print("")
-        except ipaclient.ntpconf.NTPConfigurationError:
-            pass
+            tasks.ntp_check_conflict()
+        except ntp.NTPConflictingService as e:
+            print(e)
+            options.no_ntp = True
 
     # get the directory manager password
     dirman_password = options.password
@@ -784,9 +779,8 @@ def install(installer):
 
         # Configure ntpd
         if not options.no_ntp:
-            ipaclient.ntpconf.force_ntpd(sstore)
-            ntp = ntpinstance.NTPInstance()
-            ntp.create_instance()
+            ntp_instance = ntpinstance.NTPInstance(sstore, fstore)
+            ntp_instance.create_instance()
 
         # Configure dirsrv
         ds = install_replica_ds(config, options, ca_enabled)
@@ -975,14 +969,10 @@ def promote_check(installer):
 
     if not options.no_ntp:
         try:
-            ipaclient.ntpconf.check_timedate_services()
-        except ipaclient.ntpconf.NTPConflictingService as e:
-            print("WARNING: conflicting time&date synchronization service '%s'"
-                  " will" % e.conflicting_service)
-            print("be disabled in favor of ntpd")
-            print("")
-        except ipaclient.ntpconf.NTPConfigurationError:
-            pass
+            tasks.ntp_check_conflict()
+        except ntp.NTPConflictingService as e:
+            print(e)
+            options.no_ntp = True
 
     api.bootstrap(context='installer')
     api.finalize()
@@ -1349,9 +1339,8 @@ def promote(installer):
 
     # Configure ntpd
     if not options.no_ntp:
-        ipaclient.ntpconf.force_ntpd(sstore)
-        ntp = ntpinstance.NTPInstance()
-        ntp.create_instance()
+        ntp_instance = ntpinstance.NTPInstance(sstore, fstore)
+        ntp_instance.create_instance()
 
     try:
         # Configure dirsrv
-- 
2.5.0

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