cloud-init-dev team mailing list archive

[Ryan Harper <ryan.harper@xxxxxxxxxxxxx>]

Ryan Harper has proposed merging ~raharper/cloud-init:feature/update-ntp-spec into cloud-init:master.

Requested reviews:
  cloud-init commiters (cloud-init-dev)

For more details, see:
https://code.launchpad.net/~raharper/cloud-init/+git/cloud-init/+merge/339438

Implement ntp client spec with auto support for distro selection
    
Add a common distro handler for determining which ntp client to
use, preferring installed clients over clients which need to be
installed, allow distributions to override the cloud-init defaults

LP: #1749722
-- 
Your team cloud-init commiters is requested to review the proposed merge of ~raharper/cloud-init:feature/update-ntp-spec into cloud-init:master.

diff --git a/cloudinit/config/cc_ntp.py b/cloudinit/config/cc_ntp.py
index cbd0237..099b8d8 100644
--- a/cloudinit/config/cc_ntp.py
+++ b/cloudinit/config/cc_ntp.py
@@ -15,16 +15,14 @@ from cloudinit import type_utils
 from cloudinit import util
 
 import os
+import tempfile
 from textwrap import dedent
 
 LOG = logging.getLogger(__name__)
 
 frequency = PER_INSTANCE
-NTP_CONF = '/etc/ntp.conf'
-TIMESYNCD_CONF = '/etc/systemd/timesyncd.conf.d/cloud-init.conf'
 NR_POOL_SERVERS = 4
-distros = ['centos', 'debian', 'fedora', 'opensuse', 'sles', 'ubuntu']
-
+distros = ['centos', 'debian', 'fedora', 'opensuse', 'rhel', 'sles', 'ubuntu']
 
 # The schema definition for each cloud-config module is a strict contract for
 # describing supported configuration parameters for each cloud-config section.
@@ -49,6 +47,27 @@ schema = {
     'examples': [
         dedent("""\
         ntp:
+          enabled: true
+          ntp_client: myntpclient
+          config:
+             confpath: /etc/myntpclient/myntpclient.conf
+             check_exe: myntpclientd
+             packages:
+               - myntpclient
+             service_name: myntpclient
+             template_name: myntpclient.conf
+             template: |
+                 ## template:jinja
+                 # My NTP Client config
+                 {% if pools -%}# pools{% endif %}
+                 {% for pool in pools -%}
+                 pool {{pool}} iburst
+                 {% endfor %}
+                 {%- if servers %}# servers
+                 {% endif %}
+                 {% for server in servers -%}
+                 server {{server}} iburst
+                 {% endfor %}
           pools: [0.int.pool.ntp.org, 1.int.pool.ntp.org, ntp.myorg.org]
           servers:
             - ntp.server.local
@@ -83,7 +102,56 @@ schema = {
                         List of ntp servers. If both pools and servers are
                          empty, 4 default pool servers will be provided with
                          the format ``{0-3}.{distro}.pool.ntp.org``.""")
-                }
+                },
+                'ntp_client': {
+                    'type': 'string',
+                    'description': dedent("""\
+                        Name of an NTP client to use to configure system NTP
+                        """),
+                },
+                'enabled': {
+                    'type': 'boolean',
+                    'description': "",
+                },
+                'config': {
+                    'type': ['object', 'null'],
+                    'properties': {
+                        'confpath': {
+                            'type': 'string',
+                            'description': "",
+                        },
+                        'check_exe': {
+                            'type': 'string',
+                            'description': "",
+                        },
+                        'name': {
+                            'type': 'string',
+                            'description': "",
+                        },
+                        'packages': {
+                            'type': 'array',
+                            'items': {
+                                'type': 'string',
+                            },
+                            'uniqueItems': True,
+                            'description': dedent("""\
+                                List of packages needed to be installed for the
+                                selected ``ntp_client``."""),
+                        },
+                        'service_name': {
+                            'type': 'string',
+                            'description': "",
+                        },
+                        'template_name': {
+                            'type': 'string',
+                            'description': "",
+                        },
+                        'template': {
+                            'type': 'string',
+                            'description': "",
+                        },
+                    },
+                },
             },
             'required': [],
             'additionalProperties': False
@@ -103,6 +171,9 @@ def handle(name, cfg, cloud, log, _args):
     ntp_cfg = cfg['ntp']
     if ntp_cfg is None:
         ntp_cfg = {}  # Allow empty config which will install the package
+    else:
+        # do not allow handle updates to modify the cfg object
+        ntp_cfg = cfg['ntp'].copy()
 
     # TODO drop this when validate_cloudconfig_schema is strict=True
     if not isinstance(ntp_cfg, (dict)):
@@ -111,51 +182,37 @@ def handle(name, cfg, cloud, log, _args):
             " is a {_type} instead".format(_type=type_utils.obj_name(ntp_cfg)))
 
     validate_cloudconfig_schema(cfg, schema)
-    if ntp_installable():
-        service_name = 'ntp'
-        confpath = NTP_CONF
-        template_name = None
-        packages = ['ntp']
-        check_exe = 'ntpd'
-    else:
-        service_name = 'systemd-timesyncd'
-        confpath = TIMESYNCD_CONF
-        template_name = 'timesyncd.conf'
-        packages = []
-        check_exe = '/lib/systemd/systemd-timesyncd'
 
-    rename_ntp_conf()
+    # Allow users to explicitly enable/disable
+    enabled = ntp_cfg.get('enabled', True)
+    if util.is_false(enabled):
+        LOG.debug(
+            "Skipping module named %s, disabled by cfg", name)
+        return
+
+    ntp_client_config = ntp_cfg.get('config')
+    if not ntp_client_config:
+        ntp_client_config = cloud.distro.get_ntp_client_info(ntp_cfg)
+        ntp_cfg['config'] = ntp_client_config
+
+    rename_ntp_conf(config=ntp_client_config.get('confpath'))
+
     # ensure when ntp is installed it has a configuration file
     # to use instead of starting up with packaged defaults
-    write_ntp_config_template(ntp_cfg, cloud, confpath, template=template_name)
-    install_ntp(cloud.distro.install_packages, packages=packages,
-                check_exe=check_exe)
+    write_ntp_config_template(ntp_cfg, cloud)
+    install_ntp_client(cloud.distro.install_packages,
+                       packages=ntp_client_config['packages'],
+                       check_exe=ntp_client_config['check_exe'])
 
     try:
-        reload_ntp(service_name, systemd=cloud.distro.uses_systemd())
+        reload_ntp(ntp_client_config['service_name'],
+                   systemd=cloud.distro.uses_systemd())
     except util.ProcessExecutionError as e:
         LOG.exception("Failed to reload/start ntp service: %s", e)
         raise
 
 
-def ntp_installable():
-    """Check if we can install ntp package
-
-    Ubuntu-Core systems do not have an ntp package available, so
-    we always return False.  Other systems require package managers to install
-    the ntp package If we fail to find one of the package managers, then we
-    cannot install ntp.
-    """
-    if util.system_is_snappy():
-        return False
-
-    if any(map(util.which, ['apt-get', 'dnf', 'yum', 'zypper'])):
-        return True
-
-    return False
-
-
-def install_ntp(install_func, packages=None, check_exe="ntpd"):
+def install_ntp_client(install_func, packages=None, check_exe="ntpd"):
     if util.which(check_exe):
         return
     if packages is None:
@@ -165,32 +222,29 @@ def install_ntp(install_func, packages=None, check_exe="ntpd"):
 
 
 def rename_ntp_conf(config=None):
-    """Rename any existing ntp.conf file"""
-    if config is None:  # For testing
-        config = NTP_CONF
+    """Rename any existing ntp client config file"""
     if os.path.exists(config):
         util.rename(config, config + ".dist")
 
 
 def generate_server_names(distro):
     names = []
-    pool_distro = distro
-    # For legal reasons x.pool.sles.ntp.org does not exist,
-    # use the opensuse pool
-    if distro == 'sles':
-        pool_distro = 'opensuse'
+    pool_distro = distro.get_ntp_server_name()
     for x in range(0, NR_POOL_SERVERS):
-        name = "%d.%s.pool.ntp.org" % (x, pool_distro)
+        name = "%d.%s" % (x, pool_distro)
         names.append(name)
     return names
 
 
-def write_ntp_config_template(cfg, cloud, path, template=None):
+def write_ntp_config_template(cfg, cloud):
     servers = cfg.get('servers', [])
     pools = cfg.get('pools', [])
+    clientcfg = cfg.get('config')
+    if not clientcfg:
+        clientcfg = cloud.distro.get_ntp_client_info(cfg)
 
     if len(servers) == 0 and len(pools) == 0:
-        pools = generate_server_names(cloud.distro.name)
+        pools = generate_server_names(cloud.distro)
         LOG.debug(
             'Adding distro default ntp pool servers: %s', ','.join(pools))
 
@@ -199,17 +253,27 @@ def write_ntp_config_template(cfg, cloud, path, template=None):
         'pools': pools,
     }
 
-    if template is None:
-        template = 'ntp.conf.%s' % cloud.distro.name
+    path = clientcfg.get('confpath')
+    template_name = clientcfg.get('template_name').replace('{distro}',
+                                                           cloud.distro.name)
+    template = clientcfg.get('template')
+    if template:
+        tfile = tempfile.NamedTemporaryFile(delete=False,
+                                            prefix='template_name',
+                                            suffix=".tmpl")
+        template_fn = tfile.name
+        util.write_file(template_fn, content=template)
+    else:
+        template_fn = cloud.get_template_filename(template_name)
 
-    template_fn = cloud.get_template_filename(template)
     if not template_fn:
-        template_fn = cloud.get_template_filename('ntp.conf')
-        if not template_fn:
-            raise RuntimeError(
-                'No template found, not rendering {path}'.format(path=path))
+        raise RuntimeError(
+            'No template found, not rendering {}'.format(path))
 
     templater.render_to_file(template_fn, path, params)
+    # clean up temporary template
+    if template:
+        util.del_file(template_fn)
 
 
 def reload_ntp(service, systemd=False):
diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py
index 55260ea..1d0b363 100755
--- a/cloudinit/distros/__init__.py
+++ b/cloudinit/distros/__init__.py
@@ -49,6 +49,48 @@ LOG = logging.getLogger(__name__)
 # It could break when Amazon adds new regions and new AZs.
 _EC2_AZ_RE = re.compile('^[a-z][a-z]-(?:[a-z]+-)+[0-9][a-z]$')
 
+# Default NTP Client Configurations
+PREFERRED_NTP_CLIENTS = ['systemd-timesyncd', 'ntp', 'ntpdate']
+NTP_CONF = '/etc/ntp.conf'
+NTP_CLIENT_CONFIG = {
+    'chrony': {
+        'check_exe': 'chronyd',
+        'confpath': '/etc/chrony.conf',
+        'name': 'chrony',
+        'packages': ['chrony'],
+        'service_name': 'chrony',
+        'template_name': 'chrony.conf.{distro}',
+        'template': None,
+    },
+    'ntp': {
+        'check_exe': 'ntpd',
+        'confpath': NTP_CONF,
+        'name': 'ntp',
+        'packages': ['ntp'],
+        'service_name': 'ntp',
+        'template_name': 'ntp.conf.{distro}',
+        'template': None,
+    },
+    'ntpdate': {
+        'check_exe': 'ntpdate',
+        'confpath': NTP_CONF,
+        'name': 'ntpdate',
+        'packages': ['ntpdate'],
+        'service_name': 'ntpdate',
+        'template_name': 'ntp.conf.{distro}',
+        'template': None,
+    },
+    'systemd-timesyncd': {
+        'check_exe': '/lib/systemd/systemd-timesyncd',
+        'confpath': '/etc/systemd/timesyncd.conf.d/cloud-init.conf',
+        'name': 'systemd-timesyncd',
+        'packages': [],
+        'service_name': 'systemd-timesyncd',
+        'template_name': 'timesyncd.conf',
+        'template': None,
+    },
+}
+
 
 @six.add_metaclass(abc.ABCMeta)
 class Distro(object):
@@ -60,6 +102,8 @@ class Distro(object):
     tz_zone_dir = "/usr/share/zoneinfo"
     init_cmd = ['service']  # systemctl, service etc
     renderer_configs = {}
+    preferred_ntp_clients = PREFERRED_NTP_CLIENTS
+    ntp_client_config = NTP_CLIENT_CONFIG
 
     def __init__(self, name, cfg, paths):
         self._paths = paths
@@ -109,6 +153,17 @@ class Distro(object):
         """Wrapper to report whether this distro uses systemd or sysvinit."""
         return uses_systemd()
 
+    def package_installer(self):
+        """Wrapper to report whether this distro has a package installer."""
+        return package_installer()
+
+    def find_programs(self, programs=None, search=None, target=None):
+        """Wrapper to find binaries on system, search for each element
+           in programs, testing each path in search, under directory
+           target.
+        """
+        return find_programs(programs=programs, search=search, target=target)
+
     @abc.abstractmethod
     def package_command(self, cmd, args=None, pkgs=None):
         raise NotImplementedError()
@@ -196,6 +251,65 @@ class Distro(object):
     def get_locale(self):
         raise NotImplementedError()
 
+    def get_ntp_server_name(self):
+        return '%s.pool.ntp.org' % self.name
+
+    def get_ntp_client_info(self, usercfg=None):
+        """Search for installed clients from distro preferred list
+        if client is set to 'auto' (default for unset) we search the
+        list of clients from distro, if a client name is provied in
+        config, then we return that.
+
+        returns a dictionary of the selected ntp client configuration"""
+        if not usercfg:
+            usercfg = {}
+
+        ntp_client = self.get_option("ntp_client", "auto")
+        if 'ntp_client' in usercfg:
+            ntp_client = usercfg.get('ntp_client')
+
+        if ntp_client == "auto":
+            # - Preference will be given to clients that are already installed.
+            # - If multiple ntp client packages are installed, the behavior is
+            #   not defined other than that one will be selected and configured
+            # - If no ntp client packages are installed behavior is again
+            #   undefined.
+            clients = self.preferred_ntp_clients
+        else:
+            # user provided client name
+            clients = [ntp_client]
+            # TODO: allow usercfg to include a config dict, merge with system
+
+        # for each client, extract default config, use 'check_exe' option
+        # to determine if client is already installed.
+        for client in clients:
+            # allow user to define config for a client
+            if 'config' in usercfg:
+                cfg = usercfg.get('config')
+            else:
+                cfg = self.ntp_client_config.get(client, {})
+            if not cfg:
+                LOG.warning(
+                    'Skipping ntp client %s, no configuration available',
+                    client)
+                continue
+            check = cfg.get('check_exe')
+            if not check:
+                LOG.warning(
+                    'Skipping ntp client %s, missing "check_exe" option',
+                    client)
+                continue
+            if self.find_programs(programs=check):
+                LOG.debug('Found ntp client installed: %s', client)
+                return cfg
+
+        # if the system has a package installer, then return the configuration
+        # of the first (possibly only) ntp client.
+        if self.package_installer():
+            return self.ntp_client_config.get(clients[0], {})
+
+        raise RuntimeError('No ntp client installed or available')
+
     @abc.abstractmethod
     def _read_hostname(self, filename, default=None):
         raise NotImplementedError()
@@ -758,6 +872,39 @@ def set_etc_timezone(tz, tz_file=None, tz_conf="/etc/timezone",
     return
 
 
+def find_programs(programs=None, search=None, target=None):
+    """Look for each program in path search under target"""
+    if not programs:
+        return []
+
+    if not search:
+        search = os.environ.get('PATH')
+
+    found = []
+    for program in programs:
+        if not util.which(program, search=search, target=target):
+            continue
+        found.append(program)
+
+    return found
+
+
+def package_installer():
+    """Check if we can install packages
+
+    Ubuntu-Core systems do not have an package installer available for the core
+    system, so we always return False.  Other systems have package managers to
+    install additional packages.
+    """
+    if util.system_is_snappy():
+        return False
+
+    if any(map(util.which, ['apt-get', 'dnf', 'yum', 'zypper'])):
+        return True
+
+    return False
+
+
 def uses_systemd():
     try:
         res = os.lstat('/run/systemd/system')
diff --git a/cloudinit/distros/debian.py b/cloudinit/distros/debian.py
index 33cc0bf..d1bff13 100644
--- a/cloudinit/distros/debian.py
+++ b/cloudinit/distros/debian.py
@@ -39,6 +39,18 @@ ENI_HEADER = """# This file is generated from information provided by
 NETWORK_CONF_FN = "/etc/network/interfaces.d/50-cloud-init.cfg"
 LOCALE_CONF_FN = "/etc/default/locale"
 
+NTP_CLIENT_CONFIG = {
+    'chrony': {
+        'check_exe': 'chronyd',
+        'confpath': '/etc/chrony/chrony.conf',
+        'name': 'chrony',
+        'packages': ['chrony'],
+        'service_name': 'chronyd',
+        'template_name': 'chrony.conf.{distro}',
+        'template': None,
+    },
+}
+
 
 class Distro(distros.Distro):
     hostname_conf_fn = "/etc/hostname"
@@ -63,6 +75,7 @@ class Distro(distros.Distro):
         self.osfamily = 'debian'
         self.default_locale = 'en_US.UTF-8'
         self.system_locale = None
+        self.ntp_client_config.update(NTP_CLIENT_CONFIG)
 
     def get_locale(self):
         """Return the default locale if set, else use default locale"""
diff --git a/cloudinit/distros/opensuse.py b/cloudinit/distros/opensuse.py
index a219e9f..92dfbc9 100644
--- a/cloudinit/distros/opensuse.py
+++ b/cloudinit/distros/opensuse.py
@@ -22,6 +22,36 @@ from cloudinit.settings import PER_INSTANCE
 
 LOG = logging.getLogger(__name__)
 
+NTP_CLIENT_CONFIG = {
+    'chrony': {
+        'check_exe': 'chronyd',
+        'confpath': '/etc/chrony.conf',
+        'name': 'chrony',
+        'packages': ['chrony'],
+        'service_name': 'chronyd',
+        'template_name': 'chrony.conf.{distro}',
+        'template': None,
+    },
+    'ntp': {
+        'check_exe': 'ntpd',
+        'confpath': '/etc/ntp.conf',
+        'name': 'ntp',
+        'packages': ['ntp'],
+        'service_name': 'ntpd',
+        'template_name': 'ntp.conf.{distro}',
+        'template': None,
+    },
+    'systemd-timesyncd': {
+        'check_exe': '/usr/lib/systemd/systemd-timesyncd',
+        'confpath': '/etc/systemd/timesyncd.conf.d/cloud-init.conf',
+        'name': 'systemd-timesyncd',
+        'packages': [],
+        'service_name': 'systemd-timesyncd',
+        'template_name': 'timesyncd.conf',
+        'template': None,
+    },
+}
+
 
 class Distro(distros.Distro):
     clock_conf_fn = '/etc/sysconfig/clock'
@@ -35,6 +65,7 @@ class Distro(distros.Distro):
     systemd_hostname_conf_fn = '/etc/hostname'
     systemd_locale_conf_fn = '/etc/locale.conf'
     tz_local_fn = '/etc/localtime'
+    preferred_ntp_clients = ['ntp', 'systemd-timesyncd', 'chrony']
 
     def __init__(self, name, cfg, paths):
         distros.Distro.__init__(self, name, cfg, paths)
@@ -44,6 +75,7 @@ class Distro(distros.Distro):
         if self.uses_systemd():
             self.init_cmd = ['systemctl']
             cfg['ssh_svcname'] = 'sshd.service'
+        self.ntp_client_config.update(NTP_CLIENT_CONFIG)
 
     def apply_locale(self, locale, out_fn=None):
         if self.uses_systemd():
@@ -209,4 +241,11 @@ class Distro(distros.Distro):
                                             nameservers, searchservers)
         return dev_names
 
+    def get_ntp_server_name(self):
+        """Return distro ntp pool server name"""
+
+        # For legal reasons SLES must use 'opensuse' pool.
+        return '%s.pool.ntp.org' % ('opensuse'
+                                    if self.name == 'sles' else self.name)
+
 # vi: ts=4 expandtab
diff --git a/cloudinit/distros/ubuntu.py b/cloudinit/distros/ubuntu.py
index 82ca34f..8cd0b84 100644
--- a/cloudinit/distros/ubuntu.py
+++ b/cloudinit/distros/ubuntu.py
@@ -14,8 +14,14 @@ from cloudinit import log as logging
 
 LOG = logging.getLogger(__name__)
 
+PREFERRED_NTP_CLIENTS = ['chrony', 'systemd-timesyncd', 'ntp', 'ntpdate']
+
 
 class Distro(debian.Distro):
+
+    # Ubuntu ntp client priority
+    preferred_ntp_clients = PREFERRED_NTP_CLIENTS
+
     pass
 
 # vi: ts=4 expandtab
diff --git a/config/cloud.cfg.tmpl b/config/cloud.cfg.tmpl
index fad1184..0149fc0 100644
--- a/config/cloud.cfg.tmpl
+++ b/config/cloud.cfg.tmpl
@@ -147,6 +147,8 @@ system_info:
      groups: [adm, audio, cdrom, dialout, dip, floppy, lxd, netdev, plugdev, sudo, video]
      sudo: ["ALL=(ALL) NOPASSWD:ALL"]
      shell: /bin/bash
+   # Automatically discover the best ntp_client
+   ntp_client: auto
    # Other config here will be given to the distro class and/or path classes
    paths:
       cloud_dir: /var/lib/cloud/
diff --git a/templates/chrony.conf.debian.tmpl b/templates/chrony.conf.debian.tmpl
new file mode 100644
index 0000000..e544af3
--- /dev/null
+++ b/templates/chrony.conf.debian.tmpl
@@ -0,0 +1,33 @@
+# Welcome to the chrony configuration file. See chrony.conf(5) for more
+# information about usuable directives.
+{% if pools %}# pools
+{% endif %}
+{% for pool in pools -%}
+pool {{pool}} iburst
+{% endfor %}
+
+# This directive specify the location of the file containing ID/key pairs for
+# NTP authentication.
+keyfile /etc/chrony/chrony.keys
+
+# This directive specify the file into which chronyd will store the rate
+# information.
+driftfile /var/lib/chrony/chrony.drift
+
+# Uncomment the following line to turn logging on.
+#log tracking measurements statistics
+
+# Log files location.
+logdir /var/log/chrony
+
+# Stop bad estimates upsetting machine clock.
+maxupdateskew 100.0
+
+# This directive enables kernel synchronisation (every 11 minutes) of the
+# real-time clock. Note that it can’t be used along with the 'rtcfile' directive.
+rtcsync
+
+# Step the system clock instead of slewing it if the adjustment is larger than
+# one second, but only in the first three clock updates.
+makestep 1 3
+
diff --git a/templates/chrony.conf.fedora.tmpl b/templates/chrony.conf.fedora.tmpl
new file mode 100644
index 0000000..b40c27b
--- /dev/null
+++ b/templates/chrony.conf.fedora.tmpl
@@ -0,0 +1,42 @@
+# Use public servers from the pool.ntp.org project.
+# Please consider joining the pool (http://www.pool.ntp.org/join.html).
+{% if pools %}# pools
+{% endif %}
+{% for pool in pools -%}
+pool {{pool}} iburst
+{% endfor %}
+
+# Record the rate at which the system clock gains/losses time.
+driftfile /var/lib/chrony/drift
+
+# Allow the system clock to be stepped in the first three updates
+# if its offset is larger than 1 second.
+makestep 1.0 3
+
+# Enable kernel synchronization of the real-time clock (RTC).
+rtcsync
+
+# Enable hardware timestamping on all interfaces that support it.
+#hwtimestamp *
+
+# Increase the minimum number of selectable sources required to adjust
+# the system clock.
+#minsources 2
+
+# Allow NTP client access from local network.
+#allow 192.168.0.0/16
+
+# Serve time even if not synchronized to a time source.
+#local stratum 10
+
+# Specify file containing keys for NTP authentication.
+#keyfile /etc/chrony.keys
+
+# Get TAI-UTC offset and leap seconds from the system tz database.
+leapsectz right/UTC
+
+# Specify directory for log files.
+logdir /var/log/chrony
+
+# Select which information is logged.
+#log measurements statistics tracking
diff --git a/templates/chrony.conf.opensuse.tmpl b/templates/chrony.conf.opensuse.tmpl
new file mode 100644
index 0000000..aaa6952
--- /dev/null
+++ b/templates/chrony.conf.opensuse.tmpl
@@ -0,0 +1,32 @@
+# Use public servers from the pool.ntp.org project.
+# Please consider joining the pool (http://www.pool.ntp.org/join.html).
+{% if pools %}# pools
+{% endif %}
+{% for pool in pools -%}
+pool {{pool}} iburst
+{% endfor %}
+
+# Record the rate at which the system clock gains/losses time.
+driftfile /var/lib/chrony/drift
+
+# In first three updates step the system clock instead of slew
+# if the adjustment is larger than 1 second.
+makestep 1.0 3
+
+# Enable kernel synchronization of the real-time clock (RTC).
+rtcsync
+
+# Allow NTP client access from local network.
+#allow 192.168/16
+
+# Serve time even if not synchronized to any NTP server.
+#local stratum 10
+
+# Specify file containing keys for NTP authentication.
+#keyfile /etc/chrony.keys
+
+# Specify directory for log files.
+logdir /var/log/chrony
+
+# Select which information is logged.
+#log measurements statistics tracking
diff --git a/templates/chrony.conf.rhel.tmpl b/templates/chrony.conf.rhel.tmpl
new file mode 100644
index 0000000..e500144
--- /dev/null
+++ b/templates/chrony.conf.rhel.tmpl
@@ -0,0 +1,39 @@
+# Use public servers from the pool.ntp.org project.
+# Please consider joining the pool (http://www.pool.ntp.org/join.html).
+{% if pools %}# pools
+{% endif %}
+{% for pool in pools -%}
+pool {{pool}} iburst
+{% endfor %}
+
+# Record the rate at which the system clock gains/losses time.
+driftfile /var/lib/chrony/drift
+
+# Allow the system clock to be stepped in the first three updates
+# if its offset is larger than 1 second.
+makestep 1.0 3
+
+# Enable kernel synchronization of the real-time clock (RTC).
+rtcsync
+
+# Enable hardware timestamping on all interfaces that support it.
+#hwtimestamp *
+
+# Increase the minimum number of selectable sources required to adjust
+# the system clock.
+#minsources 2
+
+# Allow NTP client access from local network.
+#allow 192.168.0.0/16
+
+# Serve time even if not synchronized to a time source.
+#local stratum 10
+
+# Specify file containing keys for NTP authentication.
+#keyfile /etc/chrony.keys
+
+# Specify directory for log files.
+logdir /var/log/chrony
+
+# Select which information is logged.
+#log measurements statistics tracking
diff --git a/templates/chrony.conf.sles.tmpl b/templates/chrony.conf.sles.tmpl
new file mode 100644
index 0000000..aaa6952
--- /dev/null
+++ b/templates/chrony.conf.sles.tmpl
@@ -0,0 +1,32 @@
+# Use public servers from the pool.ntp.org project.
+# Please consider joining the pool (http://www.pool.ntp.org/join.html).
+{% if pools %}# pools
+{% endif %}
+{% for pool in pools -%}
+pool {{pool}} iburst
+{% endfor %}
+
+# Record the rate at which the system clock gains/losses time.
+driftfile /var/lib/chrony/drift
+
+# In first three updates step the system clock instead of slew
+# if the adjustment is larger than 1 second.
+makestep 1.0 3
+
+# Enable kernel synchronization of the real-time clock (RTC).
+rtcsync
+
+# Allow NTP client access from local network.
+#allow 192.168/16
+
+# Serve time even if not synchronized to any NTP server.
+#local stratum 10
+
+# Specify file containing keys for NTP authentication.
+#keyfile /etc/chrony.keys
+
+# Specify directory for log files.
+logdir /var/log/chrony
+
+# Select which information is logged.
+#log measurements statistics tracking
diff --git a/templates/chrony.conf.ubuntu.tmpl b/templates/chrony.conf.ubuntu.tmpl
new file mode 100644
index 0000000..e163f94
--- /dev/null
+++ b/templates/chrony.conf.ubuntu.tmpl
@@ -0,0 +1,37 @@
+## template:jinja
+# Welcome to the chrony configuration file. See chrony.conf(5) for more
+# information about usuable directives.
+
+# Use servers from the NTP Pool Project. Approved by Ubuntu Technical Board
+# on 2011-02-08 (LP: #104525). See http://www.pool.ntp.org/join.html for
+# more information.
+{% if pools %}# pools
+{% endif %}
+{% for pool in pools -%}
+pool {{pool}} iburst
+{% endfor %}
+
+# This directive specify the location of the file containing ID/key pairs for
+# NTP authentication.
+keyfile /etc/chrony/chrony.keys
+
+# This directive specify the file into which chronyd will store the rate
+# information.
+driftfile /var/lib/chrony/chrony.drift
+
+# Uncomment the following line to turn logging on.
+#log tracking measurements statistics
+
+# Log files location.
+logdir /var/log/chrony
+
+# Stop bad estimates upsetting machine clock.
+maxupdateskew 100.0
+
+# This directive enables kernel synchronisation (every 11 minutes) of the
+# real-time clock. Note that it can’t be used along with the 'rtcfile' directive.
+rtcsync
+
+# Step the system clock instead of slewing it if the adjustment is larger than
+# one second, but only in the first three clock updates.
+makestep 1 3
diff --git a/tests/unittests/test_handler/test_handler_ntp.py b/tests/unittests/test_handler/test_handler_ntp.py
index 28a8455..f968f3e 100644
--- a/tests/unittests/test_handler/test_handler_ntp.py
+++ b/tests/unittests/test_handler/test_handler_ntp.py
@@ -9,6 +9,7 @@ from cloudinit.tests.helpers import FilesystemMockingTestCase, mock, skipIf
 import os
 from os.path import dirname
 import shutil
+import unittest
 
 NTP_TEMPLATE = b"""\
 ## template: jinja
@@ -40,50 +41,95 @@ class TestNtp(FilesystemMockingTestCase):
         super(TestNtp, self).setUp()
         self.subp = util.subp
         self.new_root = self.tmp_dir()
+        self.add_patch("cloudinit.distros.find_programs", 'm_find_programs')
 
-    def _get_cloud(self, distro):
-        self.patchUtils(self.new_root)
+    def _patch_ntp_config(self, distro):
+        """reroot ntp client confpath under new_root
+
+            e.g.:
+            confpath = /etc/ntp.conf -> /tmp/ci-testCcNtp/etc/ntp.conf
+
+            set the perferred client as the side-effect for find programs
+        """
+        for (client, cfg) in distro.ntp_client_config.items():
+            confpath = cfg.get('confpath')
+            if not confpath.startswith(self.new_root):
+                newpath = os.path.join(self.new_root, confpath[1:])
+                distro.ntp_client_config[client]['confpath'] = newpath
+
+    def _patch_preferred_client(self, distro, client=None):
+        if client:
+            distro.preferred_ntp_clients = [client]
+        else:
+            client = distro.preferred_ntp_clients[0]
+        check_exe = distro.ntp_client_config[client].get('check_exe')
+        self.m_find_programs.return_value = check_exe
+
+    def _get_cloud(self, distro, sys_cfg=None):
+        self.reRoot(self.new_root)
         paths = helpers.Paths({'templates_dir': self.new_root})
         cls = distros.fetch(distro)
-        mydist = cls(distro, {}, paths)
-        myds = DataSourceNone.DataSourceNone({}, mydist, paths)
-        return cloud.Cloud(myds, paths, {}, mydist, None)
+        if not sys_cfg:
+            sys_cfg = {}
+        mydist = cls(distro, sys_cfg, paths)
+        self._patch_ntp_config(mydist)
+        myds = DataSourceNone.DataSourceNone(sys_cfg, mydist, paths)
+        return cloud.Cloud(myds, paths, sys_cfg, mydist, None)
+
+    def _get_template_path(self, template_name, distro, basepath=None):
+        # ntp.conf.{distro} -> ntp.conf.debian.tmpl
+        template_fn = '{0}.tmpl'.format(
+            template_name.replace('{distro}', distro.name))
+        if not basepath:
+            basepath = self.new_root
+        path = os.path.join(basepath, template_fn)
+        return path
+
+    def _write_ntp_template(self, content, distro, ntpclient=None):
+        # ask distro to select an ntp client
+        self._patch_preferred_client(distro, client=ntpclient)
+        ntpclient = distro.get_ntp_client_info()
+        confpath = ntpclient.get('confpath')
+
+        # write out a template for the distro preferred client
+        template = ntpclient.get('template_name')
+        template_fn = self._get_template_path(template, distro)
+        # NOTE, we write a generic template rather than a client
+        # specific; we're not testing the template format here.
+        util.write_file(template_fn, content=content)
+
+        return confpath
 
     @mock.patch("cloudinit.config.cc_ntp.util")
     def test_ntp_install(self, mock_util):
-        """ntp_install installs via install_func when check_exe is absent."""
+        """ntp_install_client runs install_func when check_exe is absent."""
         mock_util.which.return_value = None  # check_exe not found.
         install_func = mock.MagicMock()
-        cc_ntp.install_ntp(install_func, packages=['ntpx'], check_exe='ntpdx')
+        cc_ntp.install_ntp_client(install_func,
+                                  packages=['ntpx'], check_exe='ntpdx')
 
         mock_util.which.assert_called_with('ntpdx')
         install_func.assert_called_once_with(['ntpx'])
 
     @mock.patch("cloudinit.config.cc_ntp.util")
     def test_ntp_install_not_needed(self, mock_util):
-        """ntp_install doesn't attempt install when check_exe is found."""
-        mock_util.which.return_value = ["/usr/sbin/ntpd"]  # check_exe found.
+        """ntp_install_client doesn't install when check_exe is found."""
+        client = 'chrony'
+        mock_util.which.return_value = [client]  # check_exe found.
         install_func = mock.MagicMock()
-        cc_ntp.install_ntp(install_func, packages=['ntp'], check_exe='ntpd')
+        cc_ntp.install_ntp_client(install_func, packages=[client],
+                                  check_exe=client)
         install_func.assert_not_called()
 
     @mock.patch("cloudinit.config.cc_ntp.util")
     def test_ntp_install_no_op_with_empty_pkg_list(self, mock_util):
-        """ntp_install calls install_func with empty list"""
+        """ntp_install_client runs install_func with empty list"""
         mock_util.which.return_value = None  # check_exe not found
         install_func = mock.MagicMock()
-        cc_ntp.install_ntp(install_func, packages=[], check_exe='timesyncd')
+        cc_ntp.install_ntp_client(install_func, packages=[],
+                                  check_exe='timesyncd')
         install_func.assert_called_once_with([])
 
-    def test_ntp_rename_ntp_conf(self):
-        """When NTP_CONF exists, rename_ntp moves it."""
-        ntpconf = self.tmp_path("ntp.conf", self.new_root)
-        util.write_file(ntpconf, "")
-        with mock.patch("cloudinit.config.cc_ntp.NTP_CONF", ntpconf):
-            cc_ntp.rename_ntp_conf()
-        self.assertFalse(os.path.exists(ntpconf))
-        self.assertTrue(os.path.exists("{0}.dist".format(ntpconf)))
-
     @mock.patch("cloudinit.config.cc_ntp.util")
     def test_reload_ntp_defaults(self, mock_util):
         """Test service is restarted/reloaded (defaults)"""
@@ -95,51 +141,31 @@ class TestNtp(FilesystemMockingTestCase):
     @mock.patch("cloudinit.config.cc_ntp.util")
     def test_reload_ntp_systemd(self, mock_util):
         """Test service is restarted/reloaded (systemd)"""
-        service = 'ntp'
-        cmd = ['systemctl', 'reload-or-restart', service]
-        cc_ntp.reload_ntp(service, systemd=True)
-        mock_util.subp.assert_called_with(cmd, capture=True)
+        for distro in cc_ntp.distros:
+            mycloud = self._get_cloud(distro)
+            ntpcfg = mycloud.distro.ntp_client_config
+            for client, c_cfg in ntpcfg.items():
+                service = c_cfg.get('service_name')
+                cmd = ['systemctl', 'reload-or-restart', service]
+                cc_ntp.reload_ntp(service, systemd=True)
+                mock_util.subp.assert_called_with(cmd, capture=True)
 
-    @mock.patch("cloudinit.config.cc_ntp.util")
-    def test_reload_ntp_systemd_timesycnd(self, mock_util):
-        """Test service is restarted/reloaded (systemd/timesyncd)"""
-        service = 'systemd-timesycnd'
-        cmd = ['systemctl', 'reload-or-restart', service]
-        cc_ntp.reload_ntp(service, systemd=True)
-        mock_util.subp.assert_called_with(cmd, capture=True)
+    def test_ntp_rename_ntp_conf(self):
+        """When NTP_CONF exists, rename_ntp moves it."""
+        ntpconf = self.tmp_path("ntp.conf", self.new_root)
+        util.write_file(ntpconf, "")
+        cc_ntp.rename_ntp_conf(config=ntpconf)
+        self.assertFalse(os.path.exists(ntpconf))
+        self.assertTrue(os.path.exists("{0}.dist".format(ntpconf)))
 
     def test_ntp_rename_ntp_conf_skip_missing(self):
         """When NTP_CONF doesn't exist rename_ntp doesn't create a file."""
         ntpconf = self.tmp_path("ntp.conf", self.new_root)
         self.assertFalse(os.path.exists(ntpconf))
-        with mock.patch("cloudinit.config.cc_ntp.NTP_CONF", ntpconf):
-            cc_ntp.rename_ntp_conf()
+        cc_ntp.rename_ntp_conf(config=ntpconf)
         self.assertFalse(os.path.exists("{0}.dist".format(ntpconf)))
         self.assertFalse(os.path.exists(ntpconf))
 
-    def test_write_ntp_config_template_from_ntp_conf_tmpl_with_servers(self):
-        """write_ntp_config_template reads content from ntp.conf.tmpl.
-
-        It reads ntp.conf.tmpl if present and renders the value from servers
-        key. When no pools key is defined, template is rendered using an empty
-        list for pools.
-        """
-        distro = 'ubuntu'
-        cfg = {
-            'servers': ['192.168.2.1', '192.168.2.2']
-        }
-        mycloud = self._get_cloud(distro)
-        ntp_conf = self.tmp_path("ntp.conf", self.new_root)  # Doesn't exist
-        # Create ntp.conf.tmpl
-        with open('{0}.tmpl'.format(ntp_conf), 'wb') as stream:
-            stream.write(NTP_TEMPLATE)
-        with mock.patch('cloudinit.config.cc_ntp.NTP_CONF', ntp_conf):
-            cc_ntp.write_ntp_config_template(cfg, mycloud, ntp_conf)
-        content = util.read_file_or_url('file://' + ntp_conf).contents
-        self.assertEqual(
-            "servers ['192.168.2.1', '192.168.2.2']\npools []\n",
-            content.decode())
-
     def test_write_ntp_config_template_uses_ntp_conf_distro_no_servers(self):
         """write_ntp_config_template reads content from ntp.conf.distro.tmpl.
 
@@ -147,43 +173,61 @@ class TestNtp(FilesystemMockingTestCase):
         renders the value from the keys servers and pools. When no
         servers value is present, template is rendered using an empty list.
         """
-        distro = 'ubuntu'
         cfg = {
-            'pools': ['10.0.0.1', '10.0.0.2']
+            'pools': ['10.0.0.1', '10.0.0.2'],
         }
-        mycloud = self._get_cloud(distro)
-        ntp_conf = self.tmp_path('ntp.conf', self.new_root)  # Doesn't exist
-        # Create ntp.conf.tmpl which isn't read
-        with open('{0}.tmpl'.format(ntp_conf), 'wb') as stream:
-            stream.write(b'NOT READ: ntp.conf.<distro>.tmpl is primary')
-        # Create ntp.conf.tmpl.<distro>
-        with open('{0}.{1}.tmpl'.format(ntp_conf, distro), 'wb') as stream:
-            stream.write(NTP_TEMPLATE)
-        with mock.patch('cloudinit.config.cc_ntp.NTP_CONF', ntp_conf):
-            cc_ntp.write_ntp_config_template(cfg, mycloud, ntp_conf)
-        content = util.read_file_or_url('file://' + ntp_conf).contents
-        self.assertEqual(
-            "servers []\npools ['10.0.0.1', '10.0.0.2']\n",
-            content.decode())
 
-    def test_write_ntp_config_template_defaults_pools_when_empty_lists(self):
+        for distro in cc_ntp.distros:
+            mycloud = self._get_cloud(distro)
+            confpath = self._write_ntp_template(NTP_TEMPLATE, mycloud.distro)
+
+            cc_ntp.write_ntp_config_template(cfg, mycloud)
+            content = util.read_file_or_url('file://' + confpath).contents
+            self.assertEqual(
+                "servers []\npools ['10.0.0.1', '10.0.0.2']\n",
+                content.decode())
+
+    def test_write_ntp_config_template_defaults_pools_w_empty_lists(self):
         """write_ntp_config_template defaults pools servers upon empty config.
 
         When both pools and servers are empty, default NR_POOL_SERVERS get
         configured.
         """
-        distro = 'ubuntu'
+        cfg = {
+            'pools': [],
+            'servers': [],
+        }
+
+        for distro in cc_ntp.distros:
+            mycloud = self._get_cloud(distro)
+            pools = cc_ntp.generate_server_names(mycloud.distro)
+            cfg.update({'pools': pools})
+
+            confpath = self._write_ntp_template(NTP_TEMPLATE, mycloud.distro)
+
+            cc_ntp.write_ntp_config_template(cfg, mycloud)
+            content = util.read_file_or_url('file://' + confpath).contents
+            self.assertEqual(
+                "servers []\npools {}\n".format(pools),
+                content.decode())
+
+    def test_defaults_pools_empty_lists_sles(self):
+        """write_ntp_config_template defaults opensuse pools upon empty config.
+
+        When both pools and servers are empty, default NR_POOL_SERVERS get
+        configured.
+        """
+        distro = 'sles'
         mycloud = self._get_cloud(distro)
-        ntp_conf = self.tmp_path('ntp.conf', self.new_root)  # Doesn't exist
-        # Create ntp.conf.tmpl
-        with open('{0}.tmpl'.format(ntp_conf), 'wb') as stream:
-            stream.write(NTP_TEMPLATE)
-        with mock.patch('cloudinit.config.cc_ntp.NTP_CONF', ntp_conf):
-            cc_ntp.write_ntp_config_template({}, mycloud, ntp_conf)
-        content = util.read_file_or_url('file://' + ntp_conf).contents
-        default_pools = [
-            "{0}.{1}.pool.ntp.org".format(x, distro)
-            for x in range(0, cc_ntp.NR_POOL_SERVERS)]
+        default_pools = cc_ntp.generate_server_names(mycloud.distro)
+        confpath = self._write_ntp_template(NTP_TEMPLATE, mycloud.distro)
+
+        cc_ntp.write_ntp_config_template({}, mycloud)
+
+        content = util.read_file_or_url('file://' + confpath).contents
+
+        for pool in default_pools:
+            self.assertIn('opensuse', pool)
         self.assertEqual(
             "servers []\npools {0}\n".format(default_pools),
             content.decode())
@@ -192,35 +236,7 @@ class TestNtp(FilesystemMockingTestCase):
                 ",".join(default_pools)),
             self.logs.getvalue())
 
-    @mock.patch("cloudinit.config.cc_ntp.ntp_installable")
-    def test_ntp_handler_mocked_template(self, m_ntp_install):
-        """Test ntp handler renders ubuntu ntp.conf template."""
-        pools = ['0.mycompany.pool.ntp.org', '3.mycompany.pool.ntp.org']
-        servers = ['192.168.23.3', '192.168.23.4']
-        cfg = {
-            'ntp': {
-                'pools': pools,
-                'servers': servers
-            }
-        }
-        mycloud = self._get_cloud('ubuntu')
-        ntp_conf = self.tmp_path('ntp.conf', self.new_root)  # Doesn't exist
-        m_ntp_install.return_value = True
-
-        # Create ntp.conf.tmpl
-        with open('{0}.tmpl'.format(ntp_conf), 'wb') as stream:
-            stream.write(NTP_TEMPLATE)
-        with mock.patch('cloudinit.config.cc_ntp.NTP_CONF', ntp_conf):
-            with mock.patch.object(util, 'which', return_value=None):
-                cc_ntp.handle('notimportant', cfg, mycloud, None, None)
-
-        content = util.read_file_or_url('file://' + ntp_conf).contents
-        self.assertEqual(
-            'servers {0}\npools {1}\n'.format(servers, pools),
-            content.decode())
-
-    @mock.patch("cloudinit.config.cc_ntp.util")
-    def test_ntp_handler_mocked_template_snappy(self, m_util):
+    def test_ntp_handler_mocked_template_timesyncd_only(self):
         """Test ntp handler renders timesycnd.conf template on snappy."""
         pools = ['0.mycompany.pool.ntp.org', '3.mycompany.pool.ntp.org']
         servers = ['192.168.23.3', '192.168.23.4']
@@ -230,25 +246,22 @@ class TestNtp(FilesystemMockingTestCase):
                 'servers': servers
             }
         }
-        mycloud = self._get_cloud('ubuntu')
-        m_util.system_is_snappy.return_value = True
 
-        # Create timesyncd.conf.tmpl
-        tsyncd_conf = self.tmp_path("timesyncd.conf", self.new_root)
-        template = '{0}.tmpl'.format(tsyncd_conf)
-        with open(template, 'wb') as stream:
-            stream.write(TIMESYNCD_TEMPLATE)
+        for distro in cc_ntp.distros:
+            mycloud = self._get_cloud(distro)
+            # specify systemd-timesyncd as found client
+            confpath = self._write_ntp_template(TIMESYNCD_TEMPLATE,
+                                                mycloud.distro,
+                                                ntpclient='systemd-timesyncd')
 
-        with mock.patch('cloudinit.config.cc_ntp.TIMESYNCD_CONF', tsyncd_conf):
             cc_ntp.handle('notimportant', cfg, mycloud, None, None)
+            content = util.read_file_or_url('file://' + confpath).contents
+            self.assertEqual(
+                "[Time]\nNTP=%s %s \n" % (" ".join(servers), " ".join(pools)),
+                content.decode())
 
-        content = util.read_file_or_url('file://' + tsyncd_conf).contents
-        self.assertEqual(
-            "[Time]\nNTP=%s %s \n" % (" ".join(servers), " ".join(pools)),
-            content.decode())
-
-    def test_ntp_handler_real_distro_templates(self):
-        """Test ntp handler renders the shipped distro ntp.conf templates."""
+    def test_ntp_handler_real_distro_ntp_templates(self):
+        """Test ntp handler renders the shipped distro ntp client templates."""
         pools = ['0.mycompany.pool.ntp.org', '3.mycompany.pool.ntp.org']
         servers = ['192.168.23.3', '192.168.23.4']
         cfg = {
@@ -257,31 +270,56 @@ class TestNtp(FilesystemMockingTestCase):
                 'servers': servers
             }
         }
-        ntp_conf = self.tmp_path('ntp.conf', self.new_root)  # Doesn't exist
-        for distro in ('debian', 'ubuntu', 'fedora', 'rhel', 'sles'):
-            mycloud = self._get_cloud(distro)
-            root_dir = dirname(dirname(os.path.realpath(util.__file__)))
-            tmpl_file = os.path.join(
-                '{0}/templates/ntp.conf.{1}.tmpl'.format(root_dir, distro))
-            # Create a copy in our tmp_dir
-            shutil.copy(
-                tmpl_file,
-                os.path.join(self.new_root, 'ntp.conf.%s.tmpl' % distro))
-            with mock.patch('cloudinit.config.cc_ntp.NTP_CONF', ntp_conf):
-                with mock.patch.object(util, 'which', return_value=[True]):
-                    cc_ntp.handle('notimportant', cfg, mycloud, None, None)
-
-            content = util.read_file_or_url('file://' + ntp_conf).contents
-            expected_servers = '\n'.join([
-                'server {0} iburst'.format(server) for server in servers])
-            self.assertIn(
-                expected_servers, content.decode(),
-                'failed to render ntp.conf for distro:{0}'.format(distro))
-            expected_pools = '\n'.join([
-                'pool {0} iburst'.format(pool) for pool in pools])
-            self.assertIn(
-                expected_pools, content.decode(),
-                'failed to render ntp.conf for distro:{0}'.format(distro))
+
+        for client in ['ntp', 'systemd-timesyncd']:
+            for distro in cc_ntp.distros:
+                mycloud = self._get_cloud(distro)
+
+                # FIXME: use _write_ntp_template here
+                self._patch_preferred_client(mycloud.distro, client=client)
+                ntpclient = mycloud.distro.get_ntp_client_info()
+                confpath = ntpclient.get('confpath')
+
+                template = ntpclient.get('template_name')
+                # find sourcetree template file
+                root_dir = (
+                    dirname(dirname(os.path.realpath(util.__file__))) +
+                    '/templates')
+                source_fn = self._get_template_path(template, mycloud.distro,
+                                                    basepath=root_dir)
+                template_fn = self._get_template_path(template, mycloud.distro)
+
+                # don't fail if cloud-init doesn't have a template
+                if not os.path.exists(source_fn):
+                    reason = ("Distro '%s' does not have template"
+                              " for ntp client '%s'" % (distro, client))
+                    raise unittest.SkipTest(reason)
+
+                # Create a copy in our tmp_dir
+                shutil.copy(source_fn, template_fn)
+
+                cc_ntp.handle('notimportant', cfg, mycloud, None, None)
+
+                content = util.read_file_or_url('file://' + confpath).contents
+                if client == 'ntp':
+                    expected_servers = '\n'.join([
+                        'server {0} iburst'.format(server)
+                        for server in servers])
+                    self.assertIn(expected_servers, content.decode(),
+                                  ('failed to render ntp.conf'
+                                   ' for distro:{0}'.format(distro)))
+                    expected_pools = '\n'.join([
+                        'pool {0} iburst'.format(pool) for pool in pools])
+                    self.assertIn(expected_pools, content.decode(),
+                                  ('failed to render ntp.conf'
+                                   ' for distro:{0}'.format(distro)))
+                elif client == 'systemd-timesyncd':
+                    expected_content = (
+                        "# cloud-init generated file\n" +
+                        "# See timesyncd.conf(5) for details.\n\n" +
+                        "[Time]\nNTP=%s %s \n" % (" ".join(servers),
+                                                  " ".join(pools)))
+                    self.assertEqual(expected_content, content.decode())
 
     def test_no_ntpcfg_does_nothing(self):
         """When no ntp section is defined handler logs a warning and noops."""
@@ -294,23 +332,20 @@ class TestNtp(FilesystemMockingTestCase):
     def test_ntp_handler_schema_validation_allows_empty_ntp_config(self):
         """Ntp schema validation allows for an empty ntp: configuration."""
         valid_empty_configs = [{'ntp': {}}, {'ntp': None}]
-        distro = 'ubuntu'
-        cc = self._get_cloud(distro)
-        ntp_conf = os.path.join(self.new_root, 'ntp.conf')
-        with open('{0}.tmpl'.format(ntp_conf), 'wb') as stream:
-            stream.write(NTP_TEMPLATE)
+
         for valid_empty_config in valid_empty_configs:
-            with mock.patch('cloudinit.config.cc_ntp.NTP_CONF', ntp_conf):
-                cc_ntp.handle('cc_ntp', valid_empty_config, cc, None, [])
-            with open(ntp_conf) as stream:
-                content = stream.read()
-            default_pools = [
-                "{0}.{1}.pool.ntp.org".format(x, distro)
-                for x in range(0, cc_ntp.NR_POOL_SERVERS)]
-            self.assertEqual(
-                "servers []\npools {0}\n".format(default_pools),
-                content)
-        self.assertNotIn('Invalid config:', self.logs.getvalue())
+            for distro in cc_ntp.distros:
+                mycloud = self._get_cloud(distro)
+                confpath = self._write_ntp_template(NTP_TEMPLATE,
+                                                    mycloud.distro)
+                cc_ntp.handle('cc_ntp', valid_empty_config, mycloud, None, [])
+
+                content = util.read_file_or_url('file://' + confpath).contents
+                pools = cc_ntp.generate_server_names(mycloud.distro)
+                self.assertEqual(
+                    "servers []\npools {0}\n".format(pools),
+                    content.decode())
+            self.assertNotIn('Invalid config:', self.logs.getvalue())
 
     @skipIf(_missing_jsonschema_dep, "No python-jsonschema dependency")
     def test_ntp_handler_schema_validation_warns_non_string_item_type(self):
@@ -319,19 +354,20 @@ class TestNtp(FilesystemMockingTestCase):
         Schema validation is not strict, so ntp config is still be rendered.
         """
         invalid_config = {'ntp': {'pools': [123], 'servers': ['valid', None]}}
-        cc = self._get_cloud('ubuntu')
-        ntp_conf = os.path.join(self.new_root, 'ntp.conf')
-        with open('{0}.tmpl'.format(ntp_conf), 'wb') as stream:
-            stream.write(NTP_TEMPLATE)
-        with mock.patch('cloudinit.config.cc_ntp.NTP_CONF', ntp_conf):
-            cc_ntp.handle('cc_ntp', invalid_config, cc, None, [])
-        self.assertIn(
-            "Invalid config:\nntp.pools.0: 123 is not of type 'string'\n"
-            "ntp.servers.1: None is not of type 'string'",
-            self.logs.getvalue())
-        with open(ntp_conf) as stream:
-            content = stream.read()
-        self.assertEqual("servers ['valid', None]\npools [123]\n", content)
+
+        for distro in cc_ntp.distros:
+            mycloud = self._get_cloud(distro)
+            confpath = self._write_ntp_template(NTP_TEMPLATE, mycloud.distro)
+
+            cc_ntp.handle('cc_ntp', invalid_config, mycloud, None, [])
+
+            self.assertIn(
+                "Invalid config:\nntp.pools.0: 123 is not of type 'string'\n"
+                "ntp.servers.1: None is not of type 'string'",
+                self.logs.getvalue())
+            content = util.read_file_or_url('file://' + confpath).contents
+            self.assertEqual("servers ['valid', None]\npools [123]\n",
+                             content.decode())
 
     @skipIf(_missing_jsonschema_dep, "No python-jsonschema dependency")
     def test_ntp_handler_schema_validation_warns_of_non_array_type(self):
@@ -340,19 +376,19 @@ class TestNtp(FilesystemMockingTestCase):
         Schema validation is not strict, so ntp config is still be rendered.
         """
         invalid_config = {'ntp': {'pools': 123, 'servers': 'non-array'}}
-        cc = self._get_cloud('ubuntu')
-        ntp_conf = os.path.join(self.new_root, 'ntp.conf')
-        with open('{0}.tmpl'.format(ntp_conf), 'wb') as stream:
-            stream.write(NTP_TEMPLATE)
-        with mock.patch('cloudinit.config.cc_ntp.NTP_CONF', ntp_conf):
-            cc_ntp.handle('cc_ntp', invalid_config, cc, None, [])
-        self.assertIn(
-            "Invalid config:\nntp.pools: 123 is not of type 'array'\n"
-            "ntp.servers: 'non-array' is not of type 'array'",
-            self.logs.getvalue())
-        with open(ntp_conf) as stream:
-            content = stream.read()
-        self.assertEqual("servers non-array\npools 123\n", content)
+
+        for distro in cc_ntp.distros:
+            mycloud = self._get_cloud(distro)
+            confpath = self._write_ntp_template(NTP_TEMPLATE, mycloud.distro)
+
+            cc_ntp.handle('cc_ntp', invalid_config, mycloud, None, [])
+            self.assertIn(
+                "Invalid config:\nntp.pools: 123 is not of type 'array'\n"
+                "ntp.servers: 'non-array' is not of type 'array'",
+                self.logs.getvalue())
+            content = util.read_file_or_url('file://' + confpath).contents
+            self.assertEqual("servers non-array\npools 123\n",
+                             content.decode())
 
     @skipIf(_missing_jsonschema_dep, "No python-jsonschema dependency")
     def test_ntp_handler_schema_validation_warns_invalid_key_present(self):
@@ -362,21 +398,20 @@ class TestNtp(FilesystemMockingTestCase):
         """
         invalid_config = {
             'ntp': {'invalidkey': 1, 'pools': ['0.mycompany.pool.ntp.org']}}
-        cc = self._get_cloud('ubuntu')
-        ntp_conf = os.path.join(self.new_root, 'ntp.conf')
-        with open('{0}.tmpl'.format(ntp_conf), 'wb') as stream:
-            stream.write(NTP_TEMPLATE)
-        with mock.patch('cloudinit.config.cc_ntp.NTP_CONF', ntp_conf):
-            cc_ntp.handle('cc_ntp', invalid_config, cc, None, [])
-        self.assertIn(
-            "Invalid config:\nntp: Additional properties are not allowed "
-            "('invalidkey' was unexpected)",
-            self.logs.getvalue())
-        with open(ntp_conf) as stream:
-            content = stream.read()
-        self.assertEqual(
-            "servers []\npools ['0.mycompany.pool.ntp.org']\n",
-            content)
+
+        for distro in cc_ntp.distros:
+            mycloud = self._get_cloud(distro)
+            confpath = self._write_ntp_template(NTP_TEMPLATE, mycloud.distro)
+
+            cc_ntp.handle('cc_ntp', invalid_config, mycloud, None, [])
+            self.assertIn(
+                "Invalid config:\nntp: Additional properties are not allowed "
+                "('invalidkey' was unexpected)",
+                self.logs.getvalue())
+            content = util.read_file_or_url('file://' + confpath).contents
+            self.assertEqual(
+                "servers []\npools ['0.mycompany.pool.ntp.org']\n",
+                content.decode())
 
     @skipIf(_missing_jsonschema_dep, "No python-jsonschema dependency")
     def test_ntp_handler_schema_validation_warns_of_duplicates(self):
@@ -387,26 +422,26 @@ class TestNtp(FilesystemMockingTestCase):
         invalid_config = {
             'ntp': {'pools': ['0.mypool.org', '0.mypool.org'],
                     'servers': ['10.0.0.1', '10.0.0.1']}}
-        cc = self._get_cloud('ubuntu')
-        ntp_conf = os.path.join(self.new_root, 'ntp.conf')
-        with open('{0}.tmpl'.format(ntp_conf), 'wb') as stream:
-            stream.write(NTP_TEMPLATE)
-        with mock.patch('cloudinit.config.cc_ntp.NTP_CONF', ntp_conf):
-            cc_ntp.handle('cc_ntp', invalid_config, cc, None, [])
-        self.assertIn(
-            "Invalid config:\nntp.pools: ['0.mypool.org', '0.mypool.org'] has "
-            "non-unique elements\nntp.servers: ['10.0.0.1', '10.0.0.1'] has "
-            "non-unique elements",
-            self.logs.getvalue())
-        with open(ntp_conf) as stream:
-            content = stream.read()
-        self.assertEqual(
-            "servers ['10.0.0.1', '10.0.0.1']\n"
-            "pools ['0.mypool.org', '0.mypool.org']\n",
-            content)
 
-    @mock.patch("cloudinit.config.cc_ntp.ntp_installable")
-    def test_ntp_handler_timesyncd(self, m_ntp_install):
+        for distro in cc_ntp.distros:
+            mycloud = self._get_cloud(distro)
+            confpath = self._write_ntp_template(NTP_TEMPLATE, mycloud.distro)
+
+            cc_ntp.handle('cc_ntp', invalid_config, mycloud, None, [])
+
+            self.assertIn(
+                "Invalid config:\nntp.pools: ['0.mypool.org', '0.mypool.org']"
+                " has non-unique elements\nntp.servers: "
+                "['10.0.0.1', '10.0.0.1'] has non-unique elements",
+                self.logs.getvalue())
+            content = util.read_file_or_url('file://' + confpath).contents
+            self.assertEqual(
+                "servers ['10.0.0.1', '10.0.0.1']\n"
+                "pools ['0.mypool.org', '0.mypool.org']\n",
+                content.decode())
+
+    # FIXME
+    def ntp_handler_timesyncd(self, m_ntp_install):
         """Test ntp handler configures timesyncd"""
         m_ntp_install.return_value = False
         distro = 'ubuntu'
@@ -418,43 +453,199 @@ class TestNtp(FilesystemMockingTestCase):
         tsyncd_conf = self.tmp_path("timesyncd.conf", self.new_root)
         # Create timesyncd.conf.tmpl
         template = '{0}.tmpl'.format(tsyncd_conf)
-        print(template)
         with open(template, 'wb') as stream:
             stream.write(TIMESYNCD_TEMPLATE)
-        with mock.patch('cloudinit.config.cc_ntp.TIMESYNCD_CONF', tsyncd_conf):
-            cc_ntp.write_ntp_config_template(cfg, mycloud, tsyncd_conf,
-                                             template='timesyncd.conf')
+        cfg['config'] = {'confpath': tsyncd_conf,
+                         'template': None,
+                         'template_name': 'timesyncd.conf'}
+        cc_ntp.write_ntp_config_template(cfg, mycloud)
 
         content = util.read_file_or_url('file://' + tsyncd_conf).contents
         self.assertEqual(
             "[Time]\nNTP=192.168.2.1 192.168.2.2 0.mypool.org \n",
             content.decode())
 
-    def test_write_ntp_config_template_defaults_pools_empty_lists_sles(self):
-        """write_ntp_config_template defaults pools servers upon empty config.
+    def test_ntp_handler_enabled_false(self):
+        """Test ntp handler does not run if enabled: false """
+        cfg = {'ntp': {'enabled': False}}
 
-        When both pools and servers are empty, default NR_POOL_SERVERS get
-        configured.
-        """
-        distro = 'sles'
-        mycloud = self._get_cloud(distro)
-        ntp_conf = self.tmp_path('ntp.conf', self.new_root)  # Doesn't exist
-        # Create ntp.conf.tmpl
-        with open('{0}.tmpl'.format(ntp_conf), 'wb') as stream:
-            stream.write(NTP_TEMPLATE)
-        with mock.patch('cloudinit.config.cc_ntp.NTP_CONF', ntp_conf):
-            cc_ntp.write_ntp_config_template({}, mycloud, ntp_conf)
-        content = util.read_file_or_url('file://' + ntp_conf).contents
-        default_pools = [
-            "{0}.opensuse.pool.ntp.org".format(x)
-            for x in range(0, cc_ntp.NR_POOL_SERVERS)]
-        self.assertEqual(
-            "servers []\npools {0}\n".format(default_pools),
-            content.decode())
-        self.assertIn(
-            "Adding distro default ntp pool servers: {0}".format(
-                ",".join(default_pools)),
-            self.logs.getvalue())
+        for distro in cc_ntp.distros:
+            mycloud = self._get_cloud(distro)
+            cc_ntp.handle('notimportant', cfg, mycloud, None, None)
+            self.assertEqual(0, self.m_find_programs.call_count)
+
+    @mock.patch("cloudinit.distros.Distro.package_installer")
+    @mock.patch("cloudinit.config.cc_ntp.util.which")
+    def test_ntp_custom_client_overrides_installed_clients(self,
+                                                           m_which,
+                                                           m_pkg_installer):
+        """Test user client is installed despite other clients present """
 
+        cfg = {'ntp': {'ntp_client': 'ntpdate'}}
+
+        for distro in cc_ntp.distros:
+            mycloud = self._get_cloud(distro)
+            self._write_ntp_template(NTP_TEMPLATE, mycloud.distro,
+                                     ntpclient='ntpdate')
+            # no ntpdate installed
+            self.m_find_programs.return_value = []
+            # system has package installer
+            m_pkg_installer.return_value = True
+
+            # cc_ntp.install_ntp_client checks for binary presence
+            m_which.return_value = None
+
+            with mock.patch.object(mycloud.distro,
+                                   'install_packages') as m_install:
+                cc_ntp.handle('notimportant', cfg, mycloud, None, None)
+
+            m_which.assert_any_call('ntpdate')
+            m_install.assert_called_with(['ntpdate'])
+
+    @mock.patch("cloudinit.distros.Distro.package_installer")
+    def test_ntp_requires_install_but_no_installer(self, m_pkg_installer):
+        """Test install client path raises RuntimeError if no installer """
+
+        cfg = {'ntp': {'ntp_client': 'ntpdate'}}
+
+        for distro in cc_ntp.distros:
+            mycloud = self._get_cloud(distro)
+            # no ntpdate installed
+            self.m_find_programs.return_value = []
+            # system does NOT have package installer
+            m_pkg_installer.return_value = False
+
+            with self.assertRaises(RuntimeError):
+                cc_ntp.handle('notimportant', cfg, mycloud, None, None)
+
+    @mock.patch("cloudinit.distros.Distro.uses_systemd")
+    def test_ntp_the_whole_package(self, m_sysd):
+        """Test enabled config renders template, and restarts service """
+
+        cfg = {'ntp': {'enabled': True}}
+
+        for distro in cc_ntp.distros:
+            mycloud = self._get_cloud(distro)
+            confpath = self._write_ntp_template(NTP_TEMPLATE, mycloud.distro)
+            # extract the selected ntp client info
+            ntpinfo = mycloud.distro.get_ntp_client_info()
+            service_name = ntpinfo['service_name']
+
+            pools = cc_ntp.generate_server_names(mycloud.distro)
+
+            # force uses systemd path
+            m_sysd.return_value = True
+
+            with mock.patch('cloudinit.config.cc_ntp.util') as m_util:
+                # default client is present
+                m_util.which.return_value = True
+                # use the config 'enabled' value
+                m_util.is_false.return_value = util.is_false(
+                    cfg['ntp']['enabled'])
+
+                cc_ntp.handle('notimportant', cfg, mycloud, None, None)
+
+                m_util.subp.assert_called_with(
+                    ['systemctl', 'reload-or-restart',
+                     service_name], capture=True)
+            content = util.read_file_or_url('file://' + confpath).contents
+            self.assertEqual(
+                "servers []\npools {}\n".format(pools),
+                content.decode())
+
+    @mock.patch("cloudinit.distros.Distro.package_installer")
+    def test_ntp_distro_searches_all_preferred_clients(self, m_installer):
+        """Test get_ntp_client_info search all distro perferred clients """
+        # nothing is installed
+        self.m_find_programs.return_value = []
+        # but we have an installer
+        m_installer.return_value = True
+        for distro in cc_ntp.distros:
+            mycloud = self._get_cloud(distro)
+
+            expected_client = mycloud.distro.preferred_ntp_clients[0]
+            expected_cfg = mycloud.distro.ntp_client_config[expected_client]
+            expected_calls = []
+            for client in mycloud.distro.preferred_ntp_clients:
+                cfg = mycloud.distro.ntp_client_config[client]
+                expected_calls.append(
+                    mock.call(programs=cfg['check_exe'],
+                              search=None, target=None))
+
+            mycloud.distro.get_ntp_client_info()
+
+            self.m_find_programs.assert_has_calls(expected_calls)
+            self.assertEqual(sorted(expected_cfg), sorted(cfg))
+
+    @mock.patch("cloudinit.distros.Distro.package_installer")
+    def test_ntp_system_config_overrides_distro_builtin_clients(self,
+                                                                m_installer):
+        """Test distro system_config overrides builtin preferred ntp clients"""
+        system_client = 'chrony'
+        sys_cfg = {'ntp_client': system_client}
+
+        # nothing is installed
+        self.m_find_programs.return_value = []
+        # but we have an installer
+        m_installer.return_value = True
+        for distro in cc_ntp.distros:
+            mycloud = self._get_cloud(distro, sys_cfg=sys_cfg)
+
+            expected_cfg = mycloud.distro.ntp_client_config[system_client]
+            result = mycloud.distro.get_ntp_client_info()
+
+            self.assertEqual(sorted(expected_cfg), sorted(result))
+            self.m_find_programs.assert_has_calls([])
+
+    @mock.patch("cloudinit.distros.Distro.package_installer")
+    def test_ntp_user_config_overrides_system_cfg(self, m_installer):
+        """Test user-data overrides system_config ntp_client"""
+        system_client = 'chrony'
+        sys_cfg = {'ntp_client': system_client}
+        user_client = 'systemd-timesyncd'
+        # cc_ntp pulls 'ntp' config dictionary and passes contents
+        user_ntp_cfg = {'ntp_client': user_client}
+
+        # nothing is installed
+        self.m_find_programs.return_value = []
+        # but we have an installer
+        m_installer.return_value = True
+        for distro in cc_ntp.distros:
+            mycloud = self._get_cloud(distro, sys_cfg=sys_cfg)
+
+            expected_cfg = mycloud.distro.ntp_client_config[user_client]
+            result = mycloud.distro.get_ntp_client_info(usercfg=user_ntp_cfg)
+
+            self.assertEqual(sorted(expected_cfg), sorted(result))
+            self.m_find_programs.assert_has_calls([])
+
+    def test_ntp_user_provided_config_with_template(self):
+        custom = b'\n#MyCustomTemplate'
+        user_template = NTP_TEMPLATE + custom
+        cfg = {
+            'pools': ['mypool.org'],
+            'ntp_client': 'myntpd',
+            'config': {
+                'check_exe': 'myntpd',
+                'confpath': os.path.join(self.new_root,
+                                         'etc/myntp/myntp.conf'),
+                'name': 'myntp',
+                'packages': ['myntp'],
+                'service_name': 'myntp',
+                'template_name': 'myntp.conf',
+                'template': user_template,
+            }
+        }
+
+        for distro in cc_ntp.distros:
+            mycloud = self._get_cloud(distro)
+            ntpclient = mycloud.distro.get_ntp_client_info(usercfg=cfg)
+            confpath = ntpclient.get('confpath')
+            cc_ntp.write_ntp_config_template(cfg, mycloud)
+
+            content = util.read_file_or_url('file://' + confpath).contents
+            self.assertEqual(
+                "servers []\npools ['mypool.org']\n%s" % custom.decode(),
+                content.decode())
 
 # vi: ts=4 expandtab