Merge ~paelzer/ubuntu/+source/chrony:bionic-lp1764357 into ubuntu/+source/chrony:ubuntu/devel

Proposed by Christian Ehrhardt 
Status: Merged
Merge reported by: Christian Ehrhardt 
Merged at revision: a43489f69cca92440ec4b93c609951409c73fd27
Proposed branch: ~paelzer/ubuntu/+source/chrony:bionic-lp1764357
Merge into: ubuntu/+source/chrony:ubuntu/devel
Diff against target: 199 lines (+136/-1)
7 files modified
debian/changelog (+13/-0)
debian/control (+1/-0)
debian/links (+5/-0)
debian/patches/lp-1718227-ignore-non-up-down-events-in-nm-dispatcher.patch (+32/-0)
debian/patches/lp-1718227-nm-dispatcher-for-networkd.patch (+67/-0)
debian/patches/series (+2/-0)
debian/postrm (+16/-1)
Reviewer Review Type Date Requested Status
Steve Langasek (community) Needs Fixing
Nish Aravamudan Approve
Canonical Server packageset reviewers Pending
Ubuntu Server Dev import team Pending
Review via email: mp+343328@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Tested from ppa [1] and working in my VM.
That said ready for review.

[1]: https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/3242

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Added the changes that I brought upstream and the related packaging changes for bug 1718227.

I also updated the version in the PPA.

Revision history for this message
Nish Aravamudan (nacc) wrote :

Agree with the changes, please upload tag & upload.

review: Approve
Revision history for this message
Steve Langasek (vorlon) :
review: Needs Fixing
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

The base file is a conffile, the links are not.
Not in /etc nor in /usr/lib - I think that is ok for now as any changes needed could be done in the one conffile that it is.

I'll add a versioned dependency for the new path and wait until 1765152 is resolved with my upload.
Thanks Steve for letting me know about this!

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Fixups according to Julians change for bug 1765152 pushed.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Tested with my ppa (as I'll upload) and the networkd-dispatcher from -proposed.
All good.

Tags pushed and uploading to bionic(-unapproved)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index 773796a..78c6ecc 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,16 @@
6+chrony (3.2-4ubuntu4) bionic; urgency=medium
7+
8+ * d/postrm: re-establish systemd-timesyncd on removal (LP: #1764357)
9+ * Notify chrony to update sources in response to systemd-networkd
10+ events (LP: #1718227)
11+ - d/links: link dispatcher script to networkd-dispatcher events routable
12+ and off
13+ - d/control: set Recommends to networkd-dispatcher
14+ - d/p/lp-1718227-ignore-non-up-down-events-in-nm-dispatcher.patch
15+ - d/p/lp-1718227-nm-dispatcher-for-networkd.patch
16+
17+ -- Christian Ehrhardt <christian.ehrhardt@canonical.com> Mon, 16 Apr 2018 17:04:06 +0200
18+
19 chrony (3.2-4ubuntu3) bionic; urgency=medium
20
21 * debian/usr.sbin.chronyd: add cap net_admin for hwtimestamp (LP: #1761327)
22diff --git a/debian/control b/debian/control
23index 4cb4022..2e2c71f 100644
24--- a/debian/control
25+++ b/debian/control
26@@ -29,6 +29,7 @@ Depends: ${misc:Depends},
27 lsb-base,
28 libcap2-bin,
29 adduser
30+Recommends: networkd-dispatcher (>= 1.7-0ubuntu3)
31 Suggests: dnsutils
32 Conflicts: time-daemon, ntp
33 Provides: time-daemon
34diff --git a/debian/links b/debian/links
35new file mode 100644
36index 0000000..71e2c52
37--- /dev/null
38+++ b/debian/links
39@@ -0,0 +1,5 @@
40+# Update sources in response to systemd-networkd events (LP: #1718227).
41+# This is reusing the NetworkManager dispatch script which has no hard
42+# dependency to NetworkManager (not using any of its arguments)
43+etc/NetworkManager/dispatcher.d/20-chrony usr/lib/networkd-dispatcher/routable.d/chrony
44+etc/NetworkManager/dispatcher.d/20-chrony usr/lib/networkd-dispatcher/off.d/chrony
45diff --git a/debian/patches/lp-1718227-ignore-non-up-down-events-in-nm-dispatcher.patch b/debian/patches/lp-1718227-ignore-non-up-down-events-in-nm-dispatcher.patch
46new file mode 100644
47index 0000000..cf12711
48--- /dev/null
49+++ b/debian/patches/lp-1718227-ignore-non-up-down-events-in-nm-dispatcher.patch
50@@ -0,0 +1,32 @@
51+From b563048ee28d121142a663ac6a598e4c71e2c21d Mon Sep 17 00:00:00 2001
52+From: Miroslav Lichvar <mlichvar@redhat.com>
53+Date: Tue, 13 Feb 2018 11:44:24 +0100
54+Subject: [PATCH 1/2] examples: ignore non-up/down events in nm-dispatcher
55+ script
56+
57+Forwarded: no (backport)
58+Author: Christian Ehrhardt <christian.ehrhardt@canonical.com>
59+Original-Author: Miroslav Lichvar <mlichvar@redhat.com>
60+Origin: upstream, https://git.tuxfamily.org/chrony/chrony.git/commit/?id=b563048ee28d121142a663ac6a598e4c71e2c21d
61+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1718227
62+Last-Update: 2018-04-18
63+---
64+ examples/chrony.nm-dispatcher | 2 ++
65+ 1 file changed, 2 insertions(+)
66+
67+diff --git a/examples/chrony.nm-dispatcher b/examples/chrony.nm-dispatcher
68+index 51d7fa2..a609a66 100644
69+--- a/examples/chrony.nm-dispatcher
70++++ b/examples/chrony.nm-dispatcher
71+@@ -4,6 +4,8 @@
72+
73+ export LC_ALL=C
74+
75++[ "$2" != "up" ] && [ "$2" != "down" ] && exit 0
76++
77+ # Check if there is a default route
78+
79+ if /sbin/ip route list 2> /dev/null | grep -q '^default'; then
80+--
81+2.7.4
82+
83diff --git a/debian/patches/lp-1718227-nm-dispatcher-for-networkd.patch b/debian/patches/lp-1718227-nm-dispatcher-for-networkd.patch
84new file mode 100644
85index 0000000..37dc411
86--- /dev/null
87+++ b/debian/patches/lp-1718227-nm-dispatcher-for-networkd.patch
88@@ -0,0 +1,67 @@
89+From 8cbc68f28f96d48a7ee128fa91731ca02a598913 Mon Sep 17 00:00:00 2001
90+From: Christian Ehrhardt <christian.ehrhardt@canonical.com>
91+Date: Wed, 18 Apr 2018 15:44:21 +0200
92+Subject: [PATCH 2/2] examples: make nm-dispatcher script usable for
93+ networkd-dispatcher
94+
95+Historically there were plenty of callback based implementations around
96+ifupdown via /etc/network/if-up and similar. NetworkManager added the
97+dispatcher [1] feature for such a kind of functionality.
98+
99+But so far a systemd-networkd (only) systemd had no means to handle those
100+cases. This is solved by networkd-dispatcher which is currently available
101+at least in ArchLinux and Ubuntu.
102+It takes away the responsibility to listen on netlink events in each
103+application and provides a more classic script-drop-in interface to respond
104+to networkd events [3].
105+
106+This commit makes the NM example compatible to be used by NetworkManager
107+dispatcher as well as by networkd-dispatcher. That way we avoid too much
108+code duplication and can from now on handle special cases in the
109+beginning so that the tail can stay commonly used.
110+
111+After discussion on IRC the current check differs by checking the
112+argument count (only in NetworkManager), if ever needed we could extend
113+that to check for known custom environment vars (NetworkManager =>
114+CONNECTION_UUID; networkd-dispatcher => OperationalState).
115+
116+[1]: https://developer.gnome.org/NetworkManager/stable/NetworkManager.html
117+[2]: https://github.com/craftyguy/networkd-dispatcher
118+[3]: https://github.com/systemd/systemd/blob/master/src/systemd/sd-network.h#L86
119+
120+Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
121+
122+Forwarded: no (backport)
123+Author: Christian Ehrhardt <christian.ehrhardt@canonical.com>
124+Origin: upstream, https://git.tuxfamily.org/chrony/chrony.git/commit/?id=8cbc68f28f96d48a7ee128fa91731ca02a598913
125+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1718227
126+Last-Update: 2018-04-18
127+---
128+ examples/chrony.nm-dispatcher | 10 +++++++---
129+ 1 file changed, 7 insertions(+), 3 deletions(-)
130+
131+diff --git a/examples/chrony.nm-dispatcher b/examples/chrony.nm-dispatcher
132+index a609a66..8bd7df0 100644
133+--- a/examples/chrony.nm-dispatcher
134++++ b/examples/chrony.nm-dispatcher
135+@@ -1,10 +1,14 @@
136+ #!/bin/sh
137+-# This is a NetworkManager dispatcher script for chronyd to set its NTP sources
138+-# online or offline when a network interface is configured or removed
139++# This is a NetworkManager dispatcher / networkd-dispatcher script for
140++# chronyd to set its NTP sources online or offline when a network interface
141++# is configured or removed
142+
143+ export LC_ALL=C
144+
145+-[ "$2" != "up" ] && [ "$2" != "down" ] && exit 0
146++# For NetworkManager consider only up/down events
147++[ $# -ge 2 ] && [ "$2" != "up" ] && [ "$2" != "down" ] && exit 0
148++
149++# Note: for networkd-dispatcher routable.d ~= on and off.d ~= off
150+
151+ # Check if there is a default route
152+
153+--
154+2.7.4
155+
156diff --git a/debian/patches/series b/debian/patches/series
157index 3b2acc6..947a7d5 100644
158--- a/debian/patches/series
159+++ b/debian/patches/series
160@@ -1 +1,3 @@
161 lp1589780-sys_linux-don-t-keep-CAP_SYS_TIME-with-x-option.patch
162+lp-1718227-ignore-non-up-down-events-in-nm-dispatcher.patch
163+lp-1718227-nm-dispatcher-for-networkd.patch
164diff --git a/debian/postrm b/debian/postrm
165index 42f48bc..6fba6b5 100644
166--- a/debian/postrm
167+++ b/debian/postrm
168@@ -7,6 +7,15 @@ set -e
169
170 # targets: purge|remove|upgrade|failed-upgrade|abort-install|abort-upgrade|disappear
171
172+restore_timesyncd() {
173+ # on next reboot it would start, but that would leave time
174+ # unsynchronized until then. So as the Conflicts in the service file kill
175+ # systemd-timesyncd re-establish it if it is enabled
176+ if [ "$(systemctl is-enabled systemd-timesyncd 2>/dev/null)" = "enabled" ] ; then
177+ systemctl start systemd-timesyncd
178+ fi
179+}
180+
181 case "$1" in
182 purge)
183 rm -f /var/lib/chrony/*
184@@ -30,9 +39,15 @@ case "$1" in
185 then
186 deluser --quiet --system _chrony > /dev/null 2>&1 || true
187 fi
188+
189+ restore_timesyncd
190+ ;;
191+
192+ remove)
193+ restore_timesyncd
194 ;;
195
196- remove|upgrade|failed-upgrade|abort-install|abort-upgrade|disappear)
197+ upgrade|failed-upgrade|abort-install|abort-upgrade|disappear)
198
199 ;;
200

Subscribers

People subscribed via source and target branches