Merge ~paulgear/ntp-charm/+git/ntp-charm:chrony-support into ntp-charm:master

Proposed by Paul Gear on 2018-03-06
Status: Merged
Approved by: Paul Gear on 2018-03-23
Approved revision: 9e7a3c32a64a7697834bfaf7fc07871fe7dc9a06
Merged at revision: ff753b4081d20cbf0ddc2fabf3a6b933a3006d23
Proposed branch: ~paulgear/ntp-charm/+git/ntp-charm:chrony-support
Merge into: ntp-charm:master
Diff against target: 488 lines (+288/-58)
7 files modified
config.yaml (+16/-14)
hooks/ntp_hooks.py (+38/-33)
hooks/ntp_implementation.py (+132/-0)
templates/chrony.conf (+58/-0)
templates/chrony.default (+13/-0)
templates/ntp.conf (+29/-11)
templates/ntp.default (+2/-0)
Reviewer Review Type Date Requested Status
Stuart Bishop 2018-03-06 Approve on 2018-03-19
Review via email: mp+340780@code.launchpad.net

Commit message

Add basic chrony support to ntp charm in preparation for bionic release. The Nagios check does not currently work with chrony.

Description of the change

First cut at adding chrony support to ntp charm in preparation for bionic release.

To post a comment you must log in.
Seth Arnold (seth-arnold) wrote :

Nice changes, thanks

Stuart Bishop (stub) wrote :

Looks good.

I do strongly believe that the ntp_implementation config option should be dropped, as its only use case is to create broken or unsupported deployments. The function in ntp_implementation.py can keep its auto mode, since it is really external/shared code rather than part of the charm, but the charm should always call it with 'auto'. Not only does a foot-gun option not encode best practice like the charm should, but it will also cause problems in a few years when we drop Xenial support and the NTP code along with it.

review: Approve
Paul Gear (paulgear) wrote :

That all makes good sense to me; will push a new rev shortly.

Stuart Bishop (stub) wrote :

+1

review: Approve

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

Unable to determine commit message from repository - please click "Set commit message" and enter the commit message manually.

Change successfully merged at revision ff753b4081d20cbf0ddc2fabf3a6b933a3006d23

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/config.yaml b/config.yaml
2index 1bbc1ed..993673d 100644
3--- a/config.yaml
4+++ b/config.yaml
5@@ -3,13 +3,13 @@ options:
6 default: ""
7 type: string
8 description: >
9- Space-separated list of NTP servers to use as sources for time.
10+ Space-separated list of NTP servers to use as time sources.
11 peers:
12 default: ""
13 type: string
14 description: >
15- Space-separated list of NTP servers to use as peers for time.
16- Peers are allowed to query the local NTP server via ntpq.
17+ Space-separated list of NTP servers to use as peers. Under ntpd,
18+ peers are allowed to query the local NTP server via ntpq.
19 pools:
20 default: >
21 0.ubuntu.pool.ntp.org
22@@ -19,25 +19,27 @@ options:
23 ntp.ubuntu.com
24 type: string
25 description: >
26- Space-separated list of NTP servers to use as pool sources for
27- time. Pool sources are recommended over normal sources for their
28- self-healing capabilities. Leave empty to disable pool sources.
29+ Space-separated list of NTP servers to use as pool sources. These
30+ are recommended over normal sources for their self-healing
31+ capabilities. Leave empty to disable pool sources.
32 orphan_stratum:
33- default: 6
34+ default: 0
35 type: int
36 description: >
37- The NTP stratum at which ntp most lose connectivity to before it
38- considers itself orphaned, and starts communicating with local peers.
39- The default value of 6 will enable orphaned operation when there are
40- no stratum 6 servers or servers of a higher stratum available, which
41- is two strata below most Internet NTP hosts.
42- Set to 0 to disable orphan mode entirely.
43+ The stratum at which NTP must lose connectivity to before it considers
44+ itself orphaned, and starts determining the reference time with local
45+ peers. A typical value is 6, which will enable orphaned operation
46+ when there are no stratum 6 servers or servers of a higher stratum
47+ available, which is two strata below most Internet NTP hosts. Set to
48+ 0 to disable orphan mode entirely. You must enable at least one peer
49+ in order to use orphan mode, but four or more is recommended for best
50+ results.
51 nagios_context:
52 default: "juju"
53 type: string
54 description: |
55 Used by the nrpe-external-master subordinate charm.
56- A string that will be prepended to instance name to set the host name
57+ A string which will be prepended to instance name to set the host name
58 in nagios. So for instance the hostname would be something like:
59 juju-myservice-0
60 If you're running multiple environments with the same services in them
61diff --git a/hooks/ntp_hooks.py b/hooks/ntp_hooks.py
62index efde7ad..e9f537d 100755
63--- a/hooks/ntp_hooks.py
64+++ b/hooks/ntp_hooks.py
65@@ -1,21 +1,18 @@
66 #!/usr/bin/python3
67
68 from charmhelpers.contrib.charmsupport import nrpe
69-from charmhelpers.contrib.templating.jinja import render
70 from charmhelpers.core import hookenv, host, unitdata
71 import charmhelpers.fetch as fetch
72 import os
73-import shutil
74 import sys
75
76+import ntp_implementation
77 import ntp_scoring
78
79 NAGIOS_PLUGINS = '/usr/local/lib/nagios/plugins'
80
81-NTP_CONF = '/etc/ntp.conf'
82-NTP_CONF_ORIG = '{}.orig'.format(NTP_CONF)
83-
84 hooks = hookenv.Hooks()
85+implementation = ntp_implementation.get_implementation()
86
87
88 def get_peer_sources(topN=6):
89@@ -65,9 +62,8 @@ def get_peer_sources(topN=6):
90 def install():
91 fetch.apt_update(fatal=True)
92 ntp_scoring.install_packages()
93- if ntp_scoring.get_virt_type() != 'container':
94- fetch.apt_install(["ntp"], fatal=True)
95- shutil.copy(NTP_CONF, NTP_CONF_ORIG)
96+ fetch.apt_install(implementation.packages_to_install(), fatal=True)
97+ implementation.save_config()
98
99
100 def get_sources(sources, iburst=True, source_list=None):
101@@ -92,16 +88,20 @@ def get_sources(sources, iburst=True, source_list=None):
102 @hooks.hook('master-relation-departed')
103 @hooks.hook('ntp-peers-relation-joined',
104 'ntp-peers-relation-changed')
105-@host.restart_on_change({NTP_CONF: ['ntp']})
106+@host.restart_on_change({implementation.config_file(): [implementation.service_name()]})
107 def write_config():
108 ntp_scoring.install_packages()
109+
110 if ntp_scoring.get_virt_type() == 'container':
111- host.service_stop('ntp')
112- host.service_pause('ntp')
113- hookenv.close_port(123, protocol="UDP")
114- return
115+ is_container = 1
116+ else:
117+ is_container = 0
118
119- host.service_resume('ntp')
120+ implementation.set_startup_options({
121+ 'is_container': is_container,
122+ })
123+
124+ host.service_resume(implementation.service_name())
125 hookenv.open_port(123, protocol="UDP")
126
127 use_iburst = hookenv.config('use_iburst')
128@@ -142,17 +142,17 @@ def write_config():
129 kv.unset('auto_peer')
130
131 if len(remote_sources) == 0 and len(remote_peers) == 0 and len(remote_pools) == 0:
132- # we have no peers/pools/servers; restore default ntp.conf provided by OS
133- shutil.copy(NTP_CONF_ORIG, NTP_CONF)
134+ # we have no peers/pools/servers; restore default config
135+ implementation.restore_config()
136 else:
137 # otherwise, write our own configuration
138- with open(NTP_CONF, "w") as ntpconf:
139- ntpconf.write(render(os.path.basename(NTP_CONF), {
140- 'orphan_stratum': orphan_stratum,
141- 'peers': remote_peers,
142- 'pools': remote_pools,
143- 'servers': remote_sources,
144- }))
145+ implementation.set_config({
146+ 'is_container': is_container,
147+ 'orphan_stratum': orphan_stratum,
148+ 'peers': remote_peers,
149+ 'pools': remote_pools,
150+ 'servers': remote_sources,
151+ })
152
153 if hookenv.relation_ids('nrpe-external-master'):
154 update_nrpe_config()
155@@ -263,27 +263,32 @@ def hyperv_sync_status():
156
157 @hooks.hook('update-status')
158 def assess_status():
159- version = fetch.get_upstream_version('ntp')
160+ package = implementation.package_name()
161+ version = fetch.get_upstream_version(package)
162 if version is not None:
163 hookenv.application_version_set(version)
164
165- # create base status
166- if ntp_scoring.get_virt_type() == 'container':
167- state = 'blocked'
168- status = 'NTP not supported in containers: please configure on host'
169- elif host.service_running('ntp'):
170+ # base status
171+ status = package + ': '
172+
173+ # append service status
174+ if host.service_running(implementation.service_name()):
175 state = 'active'
176- status = 'Ready'
177+ status += 'Ready'
178 else:
179 state = 'blocked'
180- status = 'Not running'
181+ status += 'Not running'
182+
183+ # append container status
184+ if ntp_scoring.get_virt_type() == 'container':
185+ status += ', time sync disabled in container'
186
187- # append Hyper-V status, if any
188+ # append Hyper-V status
189 hyperv_status = hyperv_sync_status()
190 if hyperv_status is not None:
191 status += ', ' + hyperv_status
192
193- # append scoring status, if any
194+ # append scoring status
195 # (don't force update of the score from update-status more than once a month)
196 max_age = 31 * 86400
197 scorestr = ntp_scoring.get_score_string(max_seconds=max_age)
198diff --git a/hooks/ntp_implementation.py b/hooks/ntp_implementation.py
199new file mode 100644
200index 0000000..e565b0e
201--- /dev/null
202+++ b/hooks/ntp_implementation.py
203@@ -0,0 +1,132 @@
204+
205+# Copyright (c) 2018 Canonical Ltd
206+# License: GPLv3
207+# Author: Paul Gear
208+
209+# NTP implementation details
210+
211+import charmhelpers.contrib.templating.jinja as templating
212+import charmhelpers.core.host
213+import charmhelpers.osplatform
214+import os.path
215+import shutil
216+
217+
218+class NTPImplementation:
219+ """Base class for NTP implementations."""
220+
221+ def config_file(self):
222+ raise NotImplementedError
223+
224+ def _config_file_backup(self):
225+ return self.config_file() + ".bak"
226+
227+ def _config_file_orig(self):
228+ return self.config_file() + ".orig"
229+
230+ def _config_file_template(self):
231+ raise NotImplementedError
232+
233+ def package_name(self):
234+ raise NotImplementedError
235+
236+ def packages_to_install(self):
237+ return [self.package_name()]
238+
239+ def restore_config(self):
240+ for path in [self._config_file_orig(), self._config_file_backup()]:
241+ if os.path.exists(path):
242+ shutil.copy(path, self.config_file())
243+
244+ def save_config(self):
245+ backup = self._config_file_backup()
246+ if not os.path.exists(backup):
247+ shutil.copy(self.config_file(), backup)
248+
249+ def service_name(self):
250+ raise NotImplementedError
251+
252+ def set_config(self, config):
253+ with open(self.config_file(), "w") as conffile:
254+ conffile.write(templating.render(self._config_file_template(), config))
255+
256+ def set_startup_options(self, config):
257+ with open(self._startup_config_file(), "w") as startup:
258+ startup.write(templating.render(self._startup_template_file(), config))
259+
260+ def _startup_config_file(self):
261+ raise NotImplementedError
262+
263+ def _startup_template_file(self):
264+ raise NotImplementedError
265+
266+
267+class Chronyd(NTPImplementation):
268+
269+ def config_file(self):
270+ return "/etc/chrony/chrony.conf"
271+
272+ def _config_file_orig(self):
273+ return "/usr/share/chrony/chrony.conf"
274+
275+ def _config_file_template(self):
276+ return "chrony.conf"
277+
278+ def package_name(self):
279+ return "chrony"
280+
281+ def service_name(self):
282+ return "chrony"
283+
284+ def _startup_config_file(self):
285+ return "/etc/default/chrony"
286+
287+ def _startup_template_file(self):
288+ return "chrony.default"
289+
290+
291+class NTPd(NTPImplementation):
292+
293+ def config_file(self):
294+ return "/etc/ntp.conf"
295+
296+ def _config_file_template(self):
297+ return "ntp.conf"
298+
299+ def package_name(self):
300+ return "ntp"
301+
302+ def service_name(self):
303+ return "ntp"
304+
305+ def _startup_config_file(self):
306+ return "/etc/default/ntp"
307+
308+ def _startup_template_file(self):
309+ return "ntp.default"
310+
311+
312+def get_implementation(implementation_name=None):
313+ """Select the appropriate NTP implementation for this platform."""
314+ if implementation_name is not None and implementation_name.lower() == 'chrony':
315+ return Chronyd()
316+ if implementation_name is not None and implementation_name.lower() == 'ntpd':
317+ return NTPd()
318+
319+ # anything else: auto mode
320+ platform = charmhelpers.osplatform.get_platform()
321+ version = float(charmhelpers.core.host.lsb_release()['DISTRIB_RELEASE'])
322+
323+ if platform == 'ubuntu':
324+ if version > 18:
325+ # Ubuntu 18.04 or later: use chronyd
326+ return Chronyd()
327+ else:
328+ # Ubuntu 17.10 or earlier: use ntpd
329+ return NTPd()
330+ elif platform == 'centos':
331+ # CentOS: use chronyd
332+ return Chronyd()
333+ else:
334+ # something else: use ntpd
335+ return NTPd()
336diff --git a/templates/chrony.conf b/templates/chrony.conf
337new file mode 100644
338index 0000000..6e559cc
339--- /dev/null
340+++ b/templates/chrony.conf
341@@ -0,0 +1,58 @@
342+# juju-generated chrony configuration
343+
344+# This directive specify the location of the file containing ID/key pairs for
345+# NTP authentication.
346+keyfile /etc/chrony/chrony.keys
347+
348+# This directive specify the file into which chronyd will store the rate
349+# information.
350+driftfile /var/lib/chrony/chrony.drift
351+
352+# Enable logging
353+log tracking measurements statistics rtc refclocks
354+
355+# Log files location.
356+logdir /var/log/chrony
357+
358+# Step the system clock instead of slewing it if the adjustment is larger than
359+# one second, but only in the first three clock updates.
360+makestep 1 3
361+
362+# Stop bad estimates upsetting machine clock.
363+maxupdateskew 100.0
364+
365+{%- if is_container == 0 %}
366+
367+# This directive enables kernel synchronisation (every 11 minutes) of the
368+# real-time clock. Note that it can't be used along with the 'rtcfile' directive.
369+rtcsync
370+{%- endif %}
371+
372+# Save state about peers & servers
373+dumpdir /var/lib/chrony
374+
375+{%- if orphan_stratum > 0 %}
376+
377+# Orphan mode enabled
378+local stratum {{ orphan_stratum }} orphan
379+{%- endif %}
380+
381+# allow any host to use us as an NTP server
382+# (use juju unexpose to prevent access)
383+allow
384+
385+# PEERS
386+{%- for peer in peers %}
387+peer {{ peer.name }} {{ peer.iburst }}
388+{%- endfor %}
389+
390+# POOLS
391+{%- for pool in pools %}
392+pool {{ pool.name }} {{ pool.iburst }}
393+{%- endfor %}
394+
395+# SERVERS
396+{%- for server in servers %}
397+server {{ server.name }} {{ server.iburst }}
398+{%- endfor %}
399+
400diff --git a/templates/chrony.default b/templates/chrony.default
401new file mode 100644
402index 0000000..aaaf90e
403--- /dev/null
404+++ b/templates/chrony.default
405@@ -0,0 +1,13 @@
406+# juju-generated chrony startup config
407+
408+# This is a configuration file for /etc/init.d/chrony and
409+# /lib/systemd/system/chrony.service; it allows you to pass various options to
410+# the chrony daemon without editing the init script or service file.
411+
412+# Options to pass to chrony.
413+{%- if is_container == 1 %}
414+DAEMON_OPTS="-x"
415+{%- else %}
416+DAEMON_OPTS=""
417+{%- endif %}
418+
419diff --git a/templates/ntp.conf b/templates/ntp.conf
420index d5674c7..1a38ecf 100644
421--- a/templates/ntp.conf
422+++ b/templates/ntp.conf
423@@ -1,28 +1,46 @@
424 # juju-generated ntp configuration
425 tinker panic 0
426 driftfile /var/lib/ntp/ntp.drift
427+
428 statsdir /var/log/ntpstats
429 statistics loopstats peerstats clockstats sysstats
430 filegen loopstats file loopstats type day enable
431 filegen peerstats file peerstats type day enable
432 filegen clockstats file clockstats type day enable
433 filegen sysstats file sysstats type day enable
434+
435 restrict -4 default kod notrap nomodify nopeer noquery limited
436 restrict -6 default kod notrap nomodify nopeer noquery limited
437 restrict source notrap nomodify noquery
438 restrict 127.0.0.1
439 restrict ::1
440-{% if orphan_stratum > 0 %}
441+
442+{%- if is_container == 1 %}
443+# running in a container - time adjustments disabled
444+disable kernel
445+{%- endif %}
446+
447+{%- if orphan_stratum > 0 %}
448+
449+# orphan mode enabled
450 tos orphan {{ orphan_stratum }}
451-{% endif %}
452-{% for peer in peers %}restrict {{ peer.name }} kod notrap nomodify
453-{% endfor %}
454-# SERVERS
455-{% for server in servers %}server {{ server.name }} {{ server.iburst }}
456-{% endfor %}
457+{%- endif %}
458+
459 # PEERS
460-{% for peer in peers %}peer {{ peer.name }} {{ peer.iburst }}
461-{% endfor %}
462+{%- for peer in peers %}
463+restrict {{ peer.name }} kod notrap nomodify
464+{%- endfor %}
465+{%- for peer in peers %}
466+peer {{ peer.name }} {{ peer.iburst }}
467+{%- endfor %}
468+
469 # POOLS
470-{% for pool in pools %}pool {{ pool.name }} {{ pool.iburst }}
471-{% endfor %}
472+{%- for pool in pools %}
473+pool {{ pool.name }} {{ pool.iburst }}
474+{%- endfor %}
475+
476+# SERVERS
477+{%- for server in servers %}
478+server {{ server.name }} {{ server.iburst }}
479+{%- endfor %}
480+
481diff --git a/templates/ntp.default b/templates/ntp.default
482new file mode 100644
483index 0000000..f84c389
484--- /dev/null
485+++ b/templates/ntp.default
486@@ -0,0 +1,2 @@
487+# juju-generated ntp startup configuration
488+NTPD_OPTS='-g'

Subscribers

People subscribed via source and target branches