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 (community) Approve
Canonical Server packageset reviewers Pending
git-ubuntu developers 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

e685f84... by Christian Ehrhardt 

Update sources in response to systemd-networkd events (LP: #1718227).

This is reusing the already provided script for NetworkManager.
This script is not hard-tied to NetworkManager as it does not use
and of its provided environment variables.

OTOH the NM scripts are general scripts that are called on any change
and then differentiate between invocation reasons via the environment provided.
But the script for chrony is agnostic to the event types - it just checks if
servers are reachable and calls chronyc online/offline accordingly.

Therefore we just link this script into networkd-dispatcher events to work the
same way. Even if not setting servers offline via loosing an interface, but
instead e.g. using the auto_offline function of chrony and then configuring a
new device it will detect the servers as no-reachable and try to connect the
sources again.

Signed-off-by: Christian Ehrhardt <email address hidden>

3ee0889... by Christian Ehrhardt 

changelog: Update sources in response to systemd-networkd events (LP: #1718227).

Signed-off-by: Christian Ehrhardt <email address hidden>

6b7b54b... by Christian Ehrhardt 

- d/control: set Recommends to networkd-dispatcher

Signed-off-by: Christian Ehrhardt <email address hidden>

0d4466e... by Christian Ehrhardt 

add patches to nm-dispatcher script for networkd-dispatcher

Signed-off-by: Christian Ehrhardt <email address hidden>

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!

e14e575... by Christian Ehrhardt 

d/control: make networkd-dispatcher dependency versioned for the new path by LP: #1765152

Signed-off-by: Christian Ehrhardt <email address hidden>

659b8a3... by Christian Ehrhardt 

d/links: adapt paths for changes by LP: #1765152

Signed-off-by: Christian Ehrhardt <email address hidden>

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

Fixups according to Julians change for bug 1765152 pushed.

a43489f... by Christian Ehrhardt 

link targets should be non-absolute

Signed-off-by: Christian Ehrhardt <email address hidden>

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