cloud-init-dev team mailing list archive
-
cloud-init-dev team
-
Mailing list archive
-
Message #04557
Re: [Merge] ~raharper/cloud-init:feature/update-ntp-spec into cloud-init:master
Thanks for the review, Ill fix what you pointed out, check what docs look like, drop the zfs fix and rebase to master.
Diff comments:
> diff --git a/cloudinit/config/cc_ntp.py b/cloudinit/config/cc_ntp.py
> index cbd0237..a6f8a1d 100644
> --- a/cloudinit/config/cc_ntp.py
> +++ b/cloudinit/config/cc_ntp.py
> @@ -21,9 +23,80 @@ 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,
> + },
> +}
> +
> +DISTRO_CLIENT_CONFIG = {
+1
> + '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
> @@ -83,7 +183,75 @@ 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
+1
> + 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
Thanks, I see it now
> + installed"""),
> + },
> + 'config': {
> + 'description': dedent("""\
> + Configuration settings or overrides for the
> + ``ntp_client`` specified."""),
> + 'type': ['object', 'null'],
> + '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 exampe, ntp service ``check_exec`` is
Found a typo.
> + '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 should start with '## template:jinja'
> + to enable use of cloud-init templating support.
> + """),
> + },
> + },
> + 'required': [],
> + 'minProperties': 1, # If we have config, define something
> + 'additionalProperties': False
> + },
> },
> 'required': [],
> 'additionalProperties': False
> diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py
> index 8685b8e..9fadfb3 100644
> --- a/tests/unittests/test_util.py
> +++ b/tests/unittests/test_util.py
> @@ -366,8 +366,11 @@ class TestMountinfoParsing(helpers.ResourceUsingTestCase):
> expected = ('none', 'tmpfs', '/run/lock')
> self.assertEqual(expected, util.parse_mount_info('/run/lock', lines))
>
> + @mock.patch('cloudinit.util.os')
> @mock.patch('cloudinit.util.subp')
> - def test_get_device_info_from_zpool(self, zpool_output):
> + def test_get_device_info_from_zpool(self, zpool_output, m_os):
Note, I'm dropping this now that it's landed in master
> + # mock /dev/zfs exists
> + m_os.path.exists.return_value = True
> # mock subp command from util.get_mount_info_fs_on_zpool
> zpool_output.return_value = (
> self.readResource('zpool_status_simple.txt'), ''
--
https://code.launchpad.net/~raharper/cloud-init/+git/cloud-init/+merge/339438
Your team cloud-init commiters is requested to review the proposed merge of ~raharper/cloud-init:feature/update-ntp-spec into cloud-init:master.
References
