Merge ~mpontillo/maas:chrony-xleave-hwtimestamp--bug-1789872 into maas:master

Proposed by Mike Pontillo
Status: Merged
Approved by: Mike Pontillo
Approved revision: a2c946f8491a88c961184e297eee0dd80910ab7f
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~mpontillo/maas:chrony-xleave-hwtimestamp--bug-1789872
Merge into: maas:master
Diff against target: 63 lines (+17/-3)
2 files modified
src/provisioningserver/ntp/config.py (+9/-2)
src/provisioningserver/ntp/tests/test_config.py (+8/-1)
Reviewer Review Type Date Requested Status
MAAS Lander Approve
Andres Rodriguez (community) Approve
Review via email: mp+354272@code.launchpad.net

Commit message

LP: #1789872 - Configure chrony with xleave and hwtimestamp.

Description of the change

This change is relatively straightforward and simple, but before we consider landing this I'll be running some end-to-end tests to ensure it behaves correctly, especially when interacting with non-interleaved-mode servers, and on setups that have no hope of ever supporting hardware timestamps, such as MAAS running in VMs or containers.

To post a comment you must log in.
Revision history for this message
Andres Rodriguez (andreserl) wrote :

lgtm!

review: Approve
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b chrony-xleave-hwtimestamp--bug-1789872 lp:~mpontillo/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/3836/console
COMMIT: 020b8c508da788736ff3d5f6a286e016f8da31f5

review: Needs Fixing
86fed2b... by Mike Pontillo

Format imports.

a2c946f... by Mike Pontillo

Add comments.

Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b chrony-xleave-hwtimestamp--bug-1789872 lp:~mpontillo/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: a2c946f8491a88c961184e297eee0dd80910ab7f

review: Approve
Revision history for this message
Mike Pontillo (mpontillo) wrote :

I tested this change locally using a combination of configurations including chrony running on metal with hardware timestamp support, and interoperability with a version of chrony on Ubuntu 16.04 (which doesn't support hardware timestamps or interleaved mode), and chrony running in LXD containers. It doesn't appear that this configuration will cause problems in any of the above situations, so I'm comfortable landing this.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/src/provisioningserver/ntp/config.py b/src/provisioningserver/ntp/config.py
index d55d2fc..d3af172 100644
--- a/src/provisioningserver/ntp/config.py
+++ b/src/provisioningserver/ntp/config.py
@@ -110,14 +110,21 @@ def _render_ntp_maas_conf(servers, peers, offset):
110 :param offset: A relative stratum used when calculating the stratum for110 :param offset: A relative stratum used when calculating the stratum for
111 orphan mode (https://chrony.tuxfamily.org/doc/3.2/chrony.conf.html).111 orphan mode (https://chrony.tuxfamily.org/doc/3.2/chrony.conf.html).
112 """112 """
113 lines = ["# MAAS NTP configuration."]113 lines = [
114 "# MAAS NTP configuration.",
115 # LP: #1789872 - Use hardware timestamps (on interfaces where it is
116 # available).
117 "hwtimestamp *",
118 ]
114 servers = map(normalise_address, servers)119 servers = map(normalise_address, servers)
115 lines.extend(120 lines.extend(
116 "%s %s iburst" % (121 "%s %s iburst" % (
117 ("server" if isinstance(server, IPAddress) else "pool"), server)122 ("server" if isinstance(server, IPAddress) else "pool"), server)
118 for server in servers)123 for server in servers)
119 peers = map(normalise_address, peers)124 peers = map(normalise_address, peers)
120 lines.extend("peer %s" % peer for peer in peers)125 # Note: `xleave` is needed here in order to take advantage of the increased
126 # accuracy that comes with enabling hardware timestamps.
127 lines.extend("peer %s xleave" % peer for peer in peers)
121 # Chrony provides a special 'orphan' mode that is compatible128 # Chrony provides a special 'orphan' mode that is compatible
122 # with ntpd's 'tos orphan' mode. (see129 # with ntpd's 'tos orphan' mode. (see
123 # https://chrony.tuxfamily.org/doc/devel/chrony.conf.html)130 # https://chrony.tuxfamily.org/doc/devel/chrony.conf.html)
diff --git a/src/provisioningserver/ntp/tests/test_config.py b/src/provisioningserver/ntp/tests/test_config.py
index 89ba737..5499b63 100644
--- a/src/provisioningserver/ntp/tests/test_config.py
+++ b/src/provisioningserver/ntp/tests/test_config.py
@@ -17,6 +17,7 @@ from netaddr import IPAddress
17from provisioningserver.ntp import config17from provisioningserver.ntp import config
18from provisioningserver.path import get_data_path18from provisioningserver.path import get_data_path
19from testtools.matchers import (19from testtools.matchers import (
20 Contains,
20 Equals,21 Equals,
21 Is,22 Is,
22 IsInstance,23 IsInstance,
@@ -51,8 +52,10 @@ def extract_peers(configuration):
5152
5253
53def extract_peers_full(configuration):54def extract_peers_full(configuration):
55 # We expect each peer to be annotated with `xleave`, so include that
56 # in the regular expression as well.
54 return re.findall(57 return re.findall(
55 r"^ *(peer) +(\S+)(?: +([^\r\n]+))?$",58 r"^ *(peer) +(\S+) xleave(?: +([^\r\n]+))?$",
56 configuration, re.MULTILINE)59 configuration, re.MULTILINE)
5760
5861
@@ -269,6 +272,10 @@ class TestRenderNTPMAASConf(MAASTestCase):
269 extract_tos_options(ntp_maas_conf),272 extract_tos_options(ntp_maas_conf),
270 Equals([str(offset + 8), "orphan"]))273 Equals([str(offset + 8), "orphan"]))
271274
275 def test_configures_hwtimestamp_mode(self):
276 ntp_maas_conf = config._render_ntp_maas_conf([], [], 0)
277 self.assertThat(ntp_maas_conf, Contains("\nhwtimestamp *\n"))
278
272279
273example_ntp_conf = """\280example_ntp_conf = """\
274# Welcome to the chrony configuration file. See chrony.conf(5) for more281# Welcome to the chrony configuration file. See chrony.conf(5) for more

Subscribers

People subscribed via source and target branches