Implement ntp client spec with auto support for distro selection

Add a base NTP client configuration dictionary and allow Distro
specific changes to be merged.  Add a select client function which
implements logic to preferr installed clients over clients which
need to be installed.  Also allow distributions to override the
cloud-init defaults.

LP: #1749722

4 files changed, 462 insertions, 78 deletions

diff --git a/cloudinit/config/cc_ntp.py b/cloudinit/config/cc_ntp.py
index cbd0237d..9e074bda 100644
--- a/cloudinit/config/cc_ntp.py
+++ b/cloudinit/config/cc_ntp.py
@@ -10,20 +10,95 @@ from cloudinit.config.schema import (
     get_schema_doc, validate_cloudconfig_schema)
 from cloudinit import log as logging
 from cloudinit.settings import PER_INSTANCE
+from cloudinit import temp_utils
 from cloudinit import templater
 from cloudinit import type_utils
 from cloudinit import util
 
+import copy
 import os
+import six
 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']
+
+NTP_CLIENT_CONFIG = {
+    'chrony': {
+        'check_exe': 'chronyd',
+        'confpath': '/etc/chrony.conf',
+        'packages': ['chrony'],
+        'service_name': 'chrony',
+        'template_name': 'chrony.conf.{distro}',
+        'template': None,
+    },
+    'ntp': {
+        'check_exe': 'ntpd',
+        'confpath': NTP_CONF,
+        'packages': ['ntp'],
+        'service_name': 'ntp',
+        'template_name': 'ntp.conf.{distro}',
+        'template': None,
+    },
+    'ntpdate': {
+        'check_exe': 'ntpdate',
+        'confpath': NTP_CONF,
+        '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',
+        'packages': [],
+        'service_name': 'systemd-timesyncd',
+        'template_name': 'timesyncd.conf',
+        'template': None,
+    },
+}
+
+# This is Distro-specific configuration overrides of the base config
+DISTRO_CLIENT_CONFIG = {
+    'debian': {
+        'chrony': {
+            'confpath': '/etc/chrony/chrony.conf',
+        },
+    },
+    'opensuse': {
+        'chrony': {
+            'service_name': 'chronyd',
+        },
+        'ntp': {
+            'confpath': '/etc/ntp.conf',
+            'service_name': 'ntpd',
+        },
+        'systemd-timesyncd': {
+            'check_exe': '/usr/lib/systemd/systemd-timesyncd',
+        },
+    },
+    'sles': {
+        'chrony': {
+            'service_name': 'chronyd',
+        },
+        'ntp': {
+            'confpath': '/etc/ntp.conf',
+            'service_name': 'ntpd',
+        },
+        'systemd-timesyncd': {
+            'check_exe': '/usr/lib/systemd/systemd-timesyncd',
+        },
+    },
+    'ubuntu': {
+        'chrony': {
+            'confpath': '/etc/chrony/chrony.conf',
+        },
+    },
+}
 
 
 # The schema definition for each cloud-config module is a strict contract for
@@ -48,7 +123,34 @@ schema = {
     'distros': distros,
     'examples': [
         dedent("""\
+        # Override ntp with chrony configuration on Ubuntu
+        ntp:
+          enabled: true
+          ntp_client: chrony  # Uses cloud-init default chrony configuration
+        """),
+        dedent("""\
+        # Provide a custom ntp client configuration
         ntp:
+          enabled: true
+          ntp_client: myntpclient
+          config:
+             confpath: /etc/myntpclient/myntpclient.conf
+             check_exe: myntpclientd
+             packages:
+               - myntpclient
+             service_name: myntpclient
+             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,79 +185,159 @@ 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',
+                    'default': 'auto',
+                    'description': dedent("""\
+                        Name of an NTP client to use to configure system NTP.
+                         When unprovided or 'auto' the default client preferred
+                         by the distribution will be used. The following
+                         built-in client names can be used to override existing
+                         configuration defaults: chrony, ntp, ntpdate,
+                         systemd-timesyncd."""),
+                },
+                'enabled': {
+                    'type': 'boolean',
+                    'default': True,
+                    'description': dedent("""\
+                        Attempt to enable ntp clients if set to True.  If set
+                         to False, ntp client will not be configured or
+                         installed"""),
+                },
+                'config': {
+                    'description': dedent("""\
+                        Configuration settings or overrides for the
+                         ``ntp_client`` specified."""),
+                    'type': ['object'],
+                    'properties': {
+                        'confpath': {
+                            'type': 'string',
+                            'description': dedent("""\
+                                The path to where the ``ntp_client``
+                                 configuration is written."""),
+                        },
+                        'check_exe': {
+                            'type': 'string',
+                            'description': dedent("""\
+                                The executable name for the ``ntp_client``.
+                                 For example, ntp service ``check_exe`` is
+                                 'ntpd' because it runs the ntpd binary."""),
+                        },
+                        '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': dedent("""\
+                                The systemd or sysvinit service name used to
+                                 start and stop the ``ntp_client``
+                                 service."""),
+                        },
+                        'template': {
+                            'type': 'string',
+                            'description': dedent("""\
+                                Inline template allowing users to define their
+                                 own ``ntp_client`` configuration template.
+                                 The value must start with '## template:jinja'
+                                 to enable use of templating support.
+                                """),
+                        },
+                    },
+                    # Don't use REQUIRED_NTP_CONFIG_KEYS to allow for override
+                    # of builtin client values.
+                    'required': [],
+                    'minProperties': 1,  # If we have config, define something
+                    'additionalProperties': False
+                },
             },
             'required': [],
             'additionalProperties': False
         }
     }
 }
-
-__doc__ = get_schema_doc(schema)  # Supplement python help()
+REQUIRED_NTP_CONFIG_KEYS = frozenset([
+    'check_exe', 'confpath', 'packages', 'service_name'])
 
 
-def handle(name, cfg, cloud, log, _args):
-    """Enable and configure ntp."""
-    if 'ntp' not in cfg:
-        LOG.debug(
-            "Skipping module named %s, not present or disabled by cfg", name)
-        return
-    ntp_cfg = cfg['ntp']
-    if ntp_cfg is None:
-        ntp_cfg = {}  # Allow empty config which will install the package
+__doc__ = get_schema_doc(schema)  # Supplement python help()
 
-    # TODO drop this when validate_cloudconfig_schema is strict=True
-    if not isinstance(ntp_cfg, (dict)):
-        raise RuntimeError(
-            "'ntp' key existed in config, but not a dictionary type,"
-            " 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()
-    # 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)
+def distro_ntp_client_configs(distro):
+    """Construct a distro-specific ntp client config dictionary by merging
+       distro specific changes into base config.
 
-    try:
-        reload_ntp(service_name, systemd=cloud.distro.uses_systemd())
-    except util.ProcessExecutionError as e:
-        LOG.exception("Failed to reload/start ntp service: %s", e)
-        raise
+    @param distro: String providing the distro class name.
+    @returns: Dict of distro configurations for ntp clients.
+    """
+    dcfg = DISTRO_CLIENT_CONFIG
+    cfg = copy.copy(NTP_CLIENT_CONFIG)
+    if distro in dcfg:
+        cfg = util.mergemanydict([cfg, dcfg[distro]], reverse=True)
+    return cfg
 
 
-def ntp_installable():
-    """Check if we can install ntp package
+def select_ntp_client(ntp_client, distro):
+    """Determine which ntp client is to be used, consulting the distro
+       for its preference.
 
-    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.
+    @param ntp_client: String name of the ntp client to use.
+    @param distro: Distro class instance.
+    @returns: Dict of the selected ntp client or {} if none selected.
     """
-    if util.system_is_snappy():
-        return False
 
-    if any(map(util.which, ['apt-get', 'dnf', 'yum', 'zypper'])):
-        return True
+    # construct distro-specific ntp_client_config dict
+    distro_cfg = distro_ntp_client_configs(distro.name)
+
+    # user specified client, return its config
+    if ntp_client and ntp_client != 'auto':
+        LOG.debug('Selected NTP client "%s" via user-data configuration',
+                  ntp_client)
+        return distro_cfg.get(ntp_client, {})
+
+    # default to auto if unset in distro
+    distro_ntp_client = distro.get_option('ntp_client', 'auto')
+
+    clientcfg = {}
+    if distro_ntp_client == "auto":
+        for client in distro.preferred_ntp_clients:
+            cfg = distro_cfg.get(client)
+            if util.which(cfg.get('check_exe')):
+                LOG.debug('Selected NTP client "%s", already installed',
+                          client)
+                clientcfg = cfg
+                break
+
+        if not clientcfg:
+            client = distro.preferred_ntp_clients[0]
+            LOG.debug(
+                'Selected distro preferred NTP client "%s", not yet installed',
+                client)
+            clientcfg = distro_cfg.get(client)
+    else:
+        LOG.debug('Selected NTP client "%s" via distro system config',
+                  distro_ntp_client)
+        clientcfg = distro_cfg.get(distro_ntp_client, {})
+
+    return clientcfg
 
-    return False
 
+def install_ntp_client(install_func, packages=None, check_exe="ntpd"):
+    """Install ntp client package if not already installed.
 
-def install_ntp(install_func, packages=None, check_exe="ntpd"):
+    @param install_func: function.  This parameter is invoked with the contents
+    of the packages parameter.
+    @param packages: list.  This parameter defaults to ['ntp'].
+    @param check_exe: string.  The name of a binary that indicates the package
+    the specified package is already installed.
+    """
     if util.which(check_exe):
         return
     if packages is None:
@@ -164,15 +346,23 @@ def install_ntp(install_func, packages=None, check_exe="ntpd"):
     install_func(packages)
 
 
-def rename_ntp_conf(config=None):
-    """Rename any existing ntp.conf file"""
-    if config is None:  # For testing
-        config = NTP_CONF
-    if os.path.exists(config):
-        util.rename(config, config + ".dist")
+def rename_ntp_conf(confpath=None):
+    """Rename any existing ntp client config file
+
+    @param confpath: string. Specify a path to an existing ntp client
+    configuration file.
+    """
+    if os.path.exists(confpath):
+        util.rename(confpath, confpath + ".dist")
 
 
 def generate_server_names(distro):
+    """Generate a list of server names to populate an ntp client configuration
+    file.
+
+    @param distro: string.  Specify the distro name
+    @returns: list: A list of strings representing ntp servers for this distro.
+    """
     names = []
     pool_distro = distro
     # For legal reasons x.pool.sles.ntp.org does not exist,
@@ -185,34 +375,60 @@ def generate_server_names(distro):
     return names
 
 
-def write_ntp_config_template(cfg, cloud, path, template=None):
-    servers = cfg.get('servers', [])
-    pools = cfg.get('pools', [])
+def write_ntp_config_template(distro_name, servers=None, pools=None,
+                              path=None, template_fn=None, template=None):
+    """Render a ntp client configuration for the specified client.
+
+    @param distro_name: string.  The distro class name.
+    @param servers: A list of strings specifying ntp servers. Defaults to empty
+    list.
+    @param pools: A list of strings specifying ntp pools. Defaults to empty
+    list.
+    @param path: A string to specify where to write the rendered template.
+    @param template_fn: A string to specify the template source file.
+    @param template: A string specifying the contents of the template. This
+    content will be written to a temporary file before being used to render
+    the configuration file.
+
+    @raises: ValueError when path is None.
+    @raises: ValueError when template_fn is None and template is None.
+    """
+    if not servers:
+        servers = []
+    if not pools:
+        pools = []
 
     if len(servers) == 0 and len(pools) == 0:
-        pools = generate_server_names(cloud.distro.name)
+        pools = generate_server_names(distro_name)
         LOG.debug(
             'Adding distro default ntp pool servers: %s', ','.join(pools))
 
-    params = {
-        'servers': servers,
-        'pools': pools,
-    }
+    if not path:
+        raise ValueError('Invalid value for path parameter')
 
-    if template is None:
-        template = 'ntp.conf.%s' % cloud.distro.name
+    if not template_fn and not template:
+        raise ValueError('Not template_fn or template provided')
 
-    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))
+    params = {'servers': servers, 'pools': pools}
+    if template:
+        tfile = temp_utils.mkstemp(prefix='template_name-', suffix=".tmpl")
+        template_fn = tfile[1]  # filepath is second item in tuple
+        util.write_file(template_fn, content=template)
 
     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):
+    """Restart or reload an ntp system service.
+
+    @param service: A string specifying the name of the service to be affected.
+    @param systemd: A boolean indicating if the distro uses systemd, defaults
+    to False.
+    @returns: A tuple of stdout, stderr results from executing the action.
+    """
     if systemd:
         cmd = ['systemctl', 'reload-or-restart', service]
     else:
@@ -220,4 +436,117 @@ def reload_ntp(service, systemd=False):
     util.subp(cmd, capture=True)
 
 
+def supplemental_schema_validation(ntp_config):
+    """Validate user-provided ntp:config option values.
+
+    This function supplements flexible jsonschema validation with specific
+    value checks to aid in triage of invalid user-provided configuration.
+
+    @param ntp_config: Dictionary of configuration value under 'ntp'.
+
+    @raises: ValueError describing invalid values provided.
+    """
+    errors = []
+    missing = REQUIRED_NTP_CONFIG_KEYS.difference(set(ntp_config.keys()))
+    if missing:
+        keys = ', '.join(sorted(missing))
+        errors.append(
+            'Missing required ntp:config keys: {keys}'.format(keys=keys))
+    elif not any([ntp_config.get('template'),
+                  ntp_config.get('template_name')]):
+        errors.append(
+            'Either ntp:config:template or ntp:config:template_name values'
+            ' are required')
+    for key, value in sorted(ntp_config.items()):
+        keypath = 'ntp:config:' + key
+        if key == 'confpath':
+            if not all([value, isinstance(value, six.string_types)]):
+                errors.append(
+                    'Expected a config file path {keypath}.'
+                    ' Found ({value})'.format(keypath=keypath, value=value))
+        elif key == 'packages':
+            if not isinstance(value, list):
+                errors.append(
+                    'Expected a list of required package names for {keypath}.'
+                    ' Found ({value})'.format(keypath=keypath, value=value))
+        elif key in ('template', 'template_name'):
+            if value is None:  # Either template or template_name can be none
+                continue
+            if not isinstance(value, six.string_types):
+                errors.append(
+                    'Expected a string type for {keypath}.'
+                    ' Found ({value})'.format(keypath=keypath, value=value))
+        elif not isinstance(value, six.string_types):
+            errors.append(
+                'Expected a string type for {keypath}.'
+                ' Found ({value})'.format(keypath=keypath, value=value))
+
+    if errors:
+        raise ValueError(r'Invalid ntp configuration:\n{errors}'.format(
+            errors='\n'.join(errors)))
+
+
+def handle(name, cfg, cloud, log, _args):
+    """Enable and configure ntp."""
+    if 'ntp' not in cfg:
+        LOG.debug(
+            "Skipping module named %s, not present or disabled by cfg", name)
+        return
+    ntp_cfg = cfg['ntp']
+    if ntp_cfg is None:
+        ntp_cfg = {}  # Allow empty config which will install the package
+
+    # TODO drop this when validate_cloudconfig_schema is strict=True
+    if not isinstance(ntp_cfg, (dict)):
+        raise RuntimeError(
+            "'ntp' key existed in config, but not a dictionary type,"
+            " is a {_type} instead".format(_type=type_utils.obj_name(ntp_cfg)))
+
+    validate_cloudconfig_schema(cfg, schema)
+
+    # 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
+
+    # Select which client is going to be used and get the configuration
+    ntp_client_config = select_ntp_client(ntp_cfg.get('ntp_client'),
+                                          cloud.distro)
+
+    # Allow user ntp config to override distro configurations
+    ntp_client_config = util.mergemanydict(
+        [ntp_client_config, ntp_cfg.get('config', {})], reverse=True)
+
+    supplemental_schema_validation(ntp_client_config)
+    rename_ntp_conf(confpath=ntp_client_config.get('confpath'))
+
+    template_fn = None
+    if not ntp_client_config.get('template'):
+        template_name = (
+            ntp_client_config.get('template_name').replace('{distro}',
+                                                           cloud.distro.name))
+        template_fn = cloud.get_template_filename(template_name)
+        if not template_fn:
+            msg = ('No template found, not rendering %s' %
+                   ntp_client_config.get('template_name'))
+            raise RuntimeError(msg)
+
+    write_ntp_config_template(cloud.distro.name,
+                              servers=ntp_cfg.get('servers', []),
+                              pools=ntp_cfg.get('pools', []),
+                              path=ntp_client_config.get('confpath'),
+                              template_fn=template_fn,
+                              template=ntp_client_config.get('template'))
+
+    install_ntp_client(cloud.distro.install_packages,
+                       packages=ntp_client_config['packages'],
+                       check_exe=ntp_client_config['check_exe'])
+    try:
+        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
+
 # vi: ts=4 expandtab
diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py
index 55260eae..6c22b07f 100755
--- a/cloudinit/distros/__init__.py
+++ b/cloudinit/distros/__init__.py
@@ -49,6 +49,9 @@ 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 = ['chrony', 'systemd-timesyncd', 'ntp', 'ntpdate']
+
 
 @six.add_metaclass(abc.ABCMeta)
 class Distro(object):
@@ -60,6 +63,7 @@ class Distro(object):
     tz_zone_dir = "/usr/share/zoneinfo"
     init_cmd = ['service']  # systemctl, service etc
     renderer_configs = {}
+    _preferred_ntp_clients = None
 
     def __init__(self, name, cfg, paths):
         self._paths = paths
@@ -339,6 +343,14 @@ class Distro(object):
             contents.write("%s\n" % (eh))
             util.write_file(self.hosts_fn, contents.getvalue(), mode=0o644)
 
+    @property
+    def preferred_ntp_clients(self):
+        """Allow distro to determine the preferred ntp client list"""
+        if not self._preferred_ntp_clients:
+            self._preferred_ntp_clients = list(PREFERRED_NTP_CLIENTS)
+
+        return self._preferred_ntp_clients
+
     def _bring_up_interface(self, device_name):
         cmd = ['ifup', device_name]
         LOG.debug("Attempting to run bring up interface %s using command %s",
diff --git a/cloudinit/distros/opensuse.py b/cloudinit/distros/opensuse.py
index 162dfa05..9f90e95e 100644
--- a/cloudinit/distros/opensuse.py
+++ b/cloudinit/distros/opensuse.py
@@ -208,4 +208,28 @@ class Distro(distros.Distro):
                                             nameservers, searchservers)
         return dev_names
 
+    @property
+    def preferred_ntp_clients(self):
+        """The preferred ntp client is dependent on the version."""
+
+        """Allow distro to determine the preferred ntp client list"""
+        if not self._preferred_ntp_clients:
+            distro_info = util.system_info()['dist']
+            name = distro_info[0]
+            major_ver = int(distro_info[1].split('.')[0])
+
+            # This is horribly complicated because of a case of
+            # "we do not care if versions should be increasing syndrome"
+            if (
+                (major_ver >= 15 and 'openSUSE' not in name) or
+                (major_ver >= 15 and 'openSUSE' in name and major_ver != 42)
+            ):
+                self._preferred_ntp_clients = ['chrony',
+                                               'systemd-timesyncd', 'ntp']
+            else:
+                self._preferred_ntp_clients = ['ntp',
+                                               'systemd-timesyncd', 'chrony']
+
+        return self._preferred_ntp_clients
+
 # vi: ts=4 expandtab
diff --git a/cloudinit/distros/ubuntu.py b/cloudinit/distros/ubuntu.py
index 82ca34f5..fdc1f622 100644
--- a/cloudinit/distros/ubuntu.py
+++ b/cloudinit/distros/ubuntu.py
@@ -10,12 +10,31 @@
 # This file is part of cloud-init. See LICENSE file for license information.
 
 from cloudinit.distros import debian
+from cloudinit.distros import PREFERRED_NTP_CLIENTS
 from cloudinit import log as logging
+from cloudinit import util
+
+import copy
 
 LOG = logging.getLogger(__name__)
 
 
 class Distro(debian.Distro):
+
+    @property
+    def preferred_ntp_clients(self):
+        """The preferred ntp client is dependent on the version."""
+        if not self._preferred_ntp_clients:
+            (name, version, codename) = util.system_info()['dist']
+            # Xenial cloud-init only installed ntp, UbuntuCore has timesyncd.
+            if codename == "xenial" and not util.system_is_snappy():
+                self._preferred_ntp_clients = ['ntp']
+            else:
+                self._preferred_ntp_clients = (
+                    copy.deepcopy(PREFERRED_NTP_CLIENTS))
+        return self._preferred_ntp_clients
+
     pass
 
+
 # vi: ts=4 expandtab