Merge ~paelzer/ubuntu/+source/chrony:merge-cosmic-3.3-1 into ubuntu/+source/chrony:debian/sid

Proposed by Christian Ehrhardt 
Status: Merged
Merge reported by: Christian Ehrhardt 
Merged at revision: 876571cbac39a410618fd728505f0c5a47357c48
Proposed branch: ~paelzer/ubuntu/+source/chrony:merge-cosmic-3.3-1
Merge into: ubuntu/+source/chrony:debian/sid
Diff against target: 534 lines (+391/-5)
13 files modified
debian/README.container (+60/-0)
debian/changelog (+143/-0)
debian/chrony.conf (+18/-1)
debian/chrony.default (+4/-0)
debian/chrony.service (+2/-2)
debian/chronyd-starter.sh (+70/-0)
debian/control (+4/-1)
debian/docs (+1/-0)
debian/install (+1/-0)
debian/links (+5/-0)
debian/patches/lp-1718227-nm-dispatcher-for-networkd.patch (+66/-0)
debian/patches/series (+1/-0)
debian/postrm (+16/-1)
Reviewer Review Type Date Requested Status
Robie Basak Approve
Canonical Server Pending
git-ubuntu developers Pending
Review via email: mp+345498@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

This merge for Chrony drops a few things Debian accepted from us already.
Furthermore two changes are upstream now.

The new upstream release was too close to 18.04 Release, but now we can bring it into Ubuntu.

Furthermore we fix bug 1771028 with that upload.

Test builds in PPA: https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/3259

Revision history for this message
Simon Déziel (sdeziel) :
Revision history for this message
Simon Déziel (sdeziel) wrote :

Found 2 typos in the proposal (diff associated to the previous comment). The rest LGTM.

6831de7... by Christian Ehrhardt 

- debian/README.container: fix typos

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

876571c... by Christian Ehrhardt 

changelog: README.container: fix typos

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

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

Thanks Simon, I added fixes for the typos to the proposal.

Revision history for this message
Paul Gear (paulgear) wrote :

Couple of minor style/grammar suggestions inline.

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

As discussed on IRC, now seeing the changes they are IMHO too small to justify a conffile prompt.
We should all try to remember, and on the day we make functional changes anyway (so we can't avoid the prompt) should add them there.
I pushed a branch default-conf-wording to hopefully remember some day.
But for now I'm refusing to make the changes part of this merge.

Still waiting for a +1 on the merge in general.

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

Note: did some more tests and so far all worked fine

Note (to myself): there is an interesting discussion in regard to RNG usage conflicting with newer kernels (4.17). The discussion is atm ongoing on the kernel side and might sort out there. If not there will be a 3.3.1 release with a fallback to urandom and being non-blocking.
TL;DR: no change due to that ... yet

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

Debian already included my apparmor changes - so we can have one less addition on this merge
Also this upload had switched tomcrypt to nettle (which is in main), so we can drop our modification of that as well (runtime dep is on libnettle6)

I now force pushed this branch to be based on this new 3.3-2 version as it simplifies the merge.

Revision history for this message
Robie Basak (racb) wrote :

Looks good!

One question from an upload perspective:

2d3ebbe: is a policy-rc.d interaction required? I'm not sure so I'll defer to you or someone like xnox.

Minor typo, while you're fixing typos in README.container described inline.

From a workflow perspective:

9eec4ae->43b8bf9 seems to be transferred correctly but I can't find how it is or ever has been documented in debian/changelog. Should it perhaps now be squashed into cb49079? If yes, then I wouldn't expect it to be documented separately in the changelog for this upload (since it was part of the logical delta previously), but I would expect it to have been documented when the change was made in the past, unless it was done at the same time as the introduction of the original change to d/links.

Commit message for d4ebf92 appears inaccurate now, but the changelog message is correct. The upload is correct, but from a workflow POV it should be fixed by rebasing this branch to avoid carrying the inaccuracy forwards.

IMHO, instead of:

Forwarded: no (backport)
Origin: upstream, ...

The correct form in dep3 is:

Origin: backport, ...

"The field is really required only if the patch is vendor specific..." so I don't think a "Forwarded" entry is needed at all in the case of a backport.

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

> Looks good!
>
> One question from an upload perspective:
>
> 2d3ebbe: is a policy-rc.d interaction required? I'm not sure so I'll defer to
> you or someone like xnox.

Interesting - This is no new change, I can kick a discussion with xnox separate to this but would not block the upload here for now. If it turns out we need a modification later on that can be done individually.

To kick that off properly could you elaborate a few sentences on the policy-rc.d interaction you'd have in mind outside of this MP e.g. in a mail to me?

> Minor typo, while you're fixing typos in README.container described inline.

Yeah while changing it anyway let change that as well. Commit added.

> From a workflow perspective:
>
> 9eec4ae->43b8bf9 seems to be transferred correctly but I can't find how it is
> or ever has been documented in debian/changelog. Should it perhaps now be
> squashed into cb49079? If yes, then I wouldn't expect it to be documented
> separately in the changelog for this upload (since it was part of the logical
> delta previously), but I would expect it to have been documented when the
> change was made in the past, unless it was done at the same time as the
> introduction of the original change to d/links.

It was done at the same time when introducing the d/links for this at all.
So I had a commit adding and a commit improving this since the other package changed
But it was released just with the combined content - therefore no "fixup" changelog entry.
The combined changelog entry was a subsection of the networkd-dispatcher:
       - d/links: link dispatcher script to networkd-dispatcher events routable
         and off
I missed the squashing when making the Delta logical.
I did that now (force push) to next time have it combined the right way in the old history.

> Commit message for d4ebf92 appears inaccurate now, but the changelog message
> is correct. The upload is correct, but from a workflow POV it should be fixed
> by rebasing this branch to avoid carrying the inaccuracy forwards.

Ack, the update I did to changelog is now integrated in the commit message.

> IMHO, instead of:
>
> Forwarded: no (backport)
> Origin: upstream, ...
>
> The correct form in dep3 is:
>
> Origin: backport, ...
>
> "The field is really required only if the patch is vendor specific..." so I
> don't think a "Forwarded" entry is needed at all in the case of a backport.

Yes, my dep3 templates are ever improving - I have 3 now and adapted my template as I agree.
I also modified the commit to match.
I'm not going to rewrite all my old patches I've done last year, but from now on I make a difference between backport from upstream git and backport from upstream discussion (which was how the forwarded came in - to carry a link or such).

Revision history for this message
Robie Basak (racb) wrote :

On Fri, May 18, 2018 at 05:59:20AM -0000,  Christian Ehrhardt  wrote:
> > Looks good!
> >
> > One question from an upload perspective:
> >
> > 2d3ebbe: is a policy-rc.d interaction required? I'm not sure so I'll defer to
> > you or someone like xnox.
>
> Interesting - This is no new change, I can kick a discussion with xnox separate to this but would not block the upload here for now. If it turns out we need a modification later on that can be done individually.
>
> To kick that off properly could you elaborate a few sentences on the policy-rc.d interaction you'd have in mind outside of this MP e.g. in a mail to me?

Agreed that it isn't introduced in this merge so shouldn't block the
merge. I've filed a bug so it doesn't get forgotten - and you can
prioritise (or deprioritise) it appropriately.

[...]

> I'm not going to rewrite all my old patches I've done last year, but from now on I make a difference between backport from upstream git and backport from upstream discussion (which was how the forwarded came in - to carry a link or such).

Agreed. Not worth going over old stuff.

Revision history for this message
Robie Basak (racb) :
review: Approve
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Thanks, yeah we can discuss/reconsider the policy handling outside of this.
It is almost a general (not chrony related) systemctl-vs-policy discussion (before every package fixes on its own or even differently).

Tagged and uploaded to git
Also pushed the package

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/README.container b/debian/README.container
2new file mode 100644
3index 0000000..16f2618
4--- /dev/null
5+++ b/debian/README.container
6@@ -0,0 +1,60 @@
7+Chrony in Containers
8+--------------------
9+
10+Currently in in 99.9+% of the cases syncing the local clock in a container
11+is wrong. Most of the time it will be unable to do so, because it is lacking
12+CAP_SYS_TIME. Or worse, if the CAP_SYS_TIME privilege is granted, multiple
13+containers could fight over the system's time, because the Linux kernel does
14+not provide time namespaces (yet).
15+
16+There are two things a user installing chrony usually wants:
17+1. synchronize my time (NTP client)
18+2. serve NTP (NTP server)
19+
20+In a container the first makes (usually) no sense, so by default we enable -x
21+there (as it would only crash otherwise).
22+This will disable the control of the system clock.
23+See `man chronyd` for more details on the -x option.
24+
25+Formerly, the check for Condition=CAP_SYS_TIME in the systemd service avoided
26+the crash of the NTP client portion, but that means the server use case will
27+not work by default in containers. It is still not recommended to use a
28+container as an NTP server, but if the host clock is synchronised via NTP,
29+adding the -x option to chronyd instances running in containers will allow
30+them to function as NTP servers which do not adjust the system clock.
31+The Condition=CAP_SYS_TIME check was a silent, no-log-entry stealing away
32+leaving users often unclear what happened - especially if they were more after
33+the NTP server than the NTP client.
34+
35+One could argue that someone who installs chrony expects the system time to be
36+synchronised, so it should fail if it is not able to do so. On the other hand
37+it could be argued that someone who installs chrony expects time to be served
38+over the network via NTP.
39+We can't know which expectation is applicable, so we assume that time should
40+be synchronised unless chronyd is running in a container (or is without
41+CAP_SYS_TIME in any other environment).
42+
43+To make things worse recent container implementations will offer CAP_SYS_TIME
44+to the container. Since from the container's point of view, this capability is
45+available for the container's user namespace. Just later on adjtimex and similar
46+are actually evaluated against the host kernel where they will fail. Due to
47+that without further precaution running chrony in Ubuntu in the future will
48+likely have the service start (as Condition=CAP_SYS_TIME will be true) but
49+then immediately fail.
50+This will depend on the environment e.g. versions and types of containers and
51+thereby feel just 'unreliable' from users point of view.
52+Furthermore it will affect upgrades as the service has to be restarted for a
53+package upgrade to be considered complete.
54+
55+Due to all of that Ubuntu decided (LP: #1589780) to default to -x (do not
56+set the system clock) in containers.
57+
58+If one really wants to (try to) sync time in a container or CAP_SYS_TIME-less
59+environment set SYNC_IN_CONTAINER="yes" in /etc/default/chrony to disable
60+this special handling.
61+
62+It is important to mention that as soon as upstream provides a way to provide
63+a default config working in those cases Ubuntu intends to use that and drop
64+the current workaround.
65+
66+ -- Christian Ehrhardt <christian.ehrhardt@canonical.com> Fri, 16 Mar 2018 12:25:44 +0100
67diff --git a/debian/changelog b/debian/changelog
68index 7d014bf..46a8195 100644
69--- a/debian/changelog
70+++ b/debian/changelog
71@@ -1,3 +1,55 @@
72+chrony (3.3-2ubuntu1) cosmic; urgency=medium
73+
74+ * Merge with Debian unstable (LP: #1771061). Remaining changes:
75+ - d/chrony.conf: use ubuntu ntp pool and server (LP 1744664)
76+ - Set -x as default if unable to set time (e.g. in containers) (LP: 1589780)
77+ Chrony is a single service which acts as both NTP client (i.e. syncing the
78+ local clock) and NTP server (i.e. providing NTP services to the network),
79+ and that is both desired and expected in the vast majority of cases.
80+ But in containers syncing the local clock is usually impossible, but this
81+ shall not break the providing of NTP services to the network.
82+ To some extent this makes chrony's default config more similar to 'ntpd',
83+ which complained in syslog but still provided NTP server service in those
84+ cases.
85+ - debian/chrony.service: allow the service to run without CAP_SYS_TIME
86+ - debian/control: add new dependency libcap2-bin for capsh (usually
87+ installed anyway, but make them explicit to be sure).
88+ - debian/chrony.default: new option SYNC_IN_CONTAINER to not fall back
89+ (Default off).
90+ - debian/chronyd-starter.sh: wrapper to handle special cases in containers
91+ and if CAP_SYS_TIME is missing. Effectively allows to run NTP server in
92+ containers on a default installation and avoid failing to sync time (or
93+ if allowed to sync, avoid multiple containers to fight over it by
94+ accident).
95+ - debian/install: make chronyd-starter.sh available on install.
96+ - debian/docs, debian/README.container: provide documentation about the
97+ handling of this case.
98+ - d/postrm: re-establish systemd-timesyncd on removal (LP: 1764357)
99+ - Notify chrony to update sources in response to systemd-networkd
100+ events (LP: 1718227)
101+ - d/links: link dispatcher script to networkd-dispatcher events routable
102+ and off
103+ - d/control: set Recommends to networkd-dispatcher
104+ - d/p/lp-1718227-nm-dispatcher-for-networkd.patch
105+ * Dropped changes
106+ - debian/usr.sbin.chronyd: ensure RTC/GPS usage isn't blocked by apparmor
107+ (LP: 1751241) (in Debian now)
108+ - debian/usr.sbin.chronyd: add cap net_admin for hwtimestamp (LP: 1761327)
109+ (in Debian now)
110+ - d/p/lp1589780-sys_linux-don-t-keep-CAP_SYS_TIME-with-x-option.patch:
111+ When dropping the root privileges, don't try to keep the CAP_SYS_TIME
112+ capability if the -x option was enabled. This allows chronyd to be
113+ started without the capability (e.g. in containers) and also drop the
114+ root privileges (This is upstream now).
115+ - d/p/lp-1718227-ignore-non-up-down-events-in-nm-dispatcher.patch (This is
116+ upstream now).
117+ - d/control: switch to nss instead of tomcrypt (Debian switched to nettle
118+ which is in main, so we can drop this)
119+ * Added changes
120+ - debian/README.container: fix typos
121+
122+ -- Christian Ehrhardt <christian.ehrhardt@canonical.com> Mon, 14 May 2018 09:06:01 +0200
123+
124 chrony (3.3-2) unstable; urgency=medium
125
126 * debian/chrony.service:
127@@ -53,6 +105,76 @@ chrony (3.2-5) unstable; urgency=medium
128
129 -- Vincent Blut <vincent.debian@free.fr> Wed, 28 Feb 2018 17:31:08 +0100
130
131+chrony (3.2-4ubuntu4) bionic; urgency=medium
132+
133+ * d/postrm: re-establish systemd-timesyncd on removal (LP: #1764357)
134+ * Notify chrony to update sources in response to systemd-networkd
135+ events (LP: #1718227)
136+ - d/links: link dispatcher script to networkd-dispatcher events routable
137+ and off
138+ - d/control: set Recommends to networkd-dispatcher
139+ - d/p/lp-1718227-ignore-non-up-down-events-in-nm-dispatcher.patch
140+ - d/p/lp-1718227-nm-dispatcher-for-networkd.patch
141+
142+ -- Christian Ehrhardt <christian.ehrhardt@canonical.com> Mon, 16 Apr 2018 17:04:06 +0200
143+
144+chrony (3.2-4ubuntu3) bionic; urgency=medium
145+
146+ * debian/usr.sbin.chronyd: add cap net_admin for hwtimestamp (LP: #1761327)
147+
148+ -- Christian Ehrhardt <christian.ehrhardt@canonical.com> Thu, 05 Apr 2018 09:38:10 +0200
149+
150+chrony (3.2-4ubuntu2) bionic; urgency=medium
151+
152+ * Set -x as default if unable to set time (e.g. in containers) (LP: #1589780)
153+ Chrony is a single service which acts as both NTP client (i.e. syncing the
154+ local clock) and NTP server (i.e. providing NTP services to the network),
155+ and that is both desired and expected in the vast majority of cases.
156+ But in containers syncing the local clock is usually impossible, but this
157+ shall not break the providing of NTP services to the network.
158+ To some extent this makes chrony's default config more similar to 'ntpd',
159+ which complained in syslog but still provided NTP server service in those
160+ cases.
161+ - d/p/lp1589780-sys_linux-don-t-keep-CAP_SYS_TIME-with-x-option.patch:
162+ When dropping the root privileges, don't try to keep the CAP_SYS_TIME
163+ capability if the -x option was enabled. This allows chronyd to be
164+ started without the capability (e.g. in containers) and also drop the
165+ root privileges.
166+ - debian/chrony.service: allow the service to run without CAP_SYS_TIME
167+ - debian/control: add new dependency libcap2-bin for capsh (usually
168+ installed anyway, but make them explicit to be sure).
169+ - debian/chrony.default: new option SYNC_IN_CONTAINER to not fall back
170+ (Default off).
171+ - debian/chronyd-starter.sh: wrapper to handle special cases in containers
172+ and if CAP_SYS_TIME is missing. Effectively allows to run NTP server in
173+ containers on a default installation and avoid failing to sync time (or
174+ if allowed to sync, avoid multiple containers to fight over it by
175+ accident).
176+ - debian/install: make chronyd-starter.sh available on install.
177+ - debian/docs, debian/README.container: provide documentation about the
178+ handling of this case.
179+ * debian/chrony.conf: update default chrony.conf to not violate the policy
180+ of pool.ntp.org (to use no more than four of their servers) and to provide
181+ more ipv6 capable sources by default (LP: #1754358)
182+
183+ -- Christian Ehrhardt <christian.ehrhardt@canonical.com> Fri, 16 Mar 2018 12:25:44 +0100
184+
185+chrony (3.2-4ubuntu1) bionic; urgency=medium
186+
187+ * Merge with Debian unstable. Remaining changes:
188+ - d/control: switch to nss instead of tomcrypt (nss is in main)
189+ - d/chrony.conf: use ubuntu ntp pool and server (LP 1744664)
190+ * Dropped changes (in Debian)
191+ - d/chrony.default, d/chrony.service: support /etc/default/chrony
192+ DAEMON_OPTS in systemd environment (LP: 1746081)
193+ - d/chrony.service: properly start after networking (LP: 1746458)
194+ - d/usr.sbin.chronyd: allow to create /run/chrony on demand (LP: 1746444)
195+ * Added Changes:
196+ - debian/usr.sbin.chronyd: ensure RTC/GPS usage isn't blocked by apparmor
197+ (LP: #1751241, Closes: #891201)
198+
199+ -- Christian Ehrhardt <christian.ehrhardt@canonical.com> Mon, 26 Feb 2018 14:44:54 +0100
200+
201 chrony (3.2-4) unstable; urgency=medium
202
203 * debian/changelog:
204@@ -119,6 +241,27 @@ chrony (3.2-3) unstable; urgency=medium
205
206 -- Vincent Blut <vincent.debian@free.fr> Wed, 07 Feb 2018 21:27:09 +0100
207
208+chrony (3.2-2ubuntu3) bionic; urgency=medium
209+
210+ * Revert the changes of (LP 1746458) as in the follow on discussion
211+ it became clear that we want it to start early (for example for an
212+ early offset from drift file). iIf needed chrony will later on pick
213+ up that servers are online via retries (augmented by hooks on network
214+ events).
215+
216+ -- Christian Ehrhardt <christian.ehrhardt@canonical.com> Thu, 08 Feb 2018 10:52:30 +0100
217+
218+chrony (3.2-2ubuntu2) bionic; urgency=medium
219+
220+ * d/control: use to nss instead of tomcrypt (in main) (LP: #1744072)
221+ * d/chrony.conf: use ubuntu ntp pool and server (LP: #1744664)
222+ * d/chrony.default, d/chrony.service: support /etc/default/chrony
223+ DAEMON_OPTS in systemd environment (LP: #1746081)
224+ * d/chrony.service: properly start after networking (LP: #1746458)
225+ * d/usr.sbin.chronyd: allow to create /run/chrony on demand (LP: #1746444)
226+
227+ -- Christian Ehrhardt <christian.ehrhardt@canonical.com> Fri, 19 Jan 2018 09:45:38 +0100
228+
229 chrony (3.2-2) unstable; urgency=medium
230
231 * Initial AppArmor profile for chronyd. Thanks to Jamie
232diff --git a/debian/chrony.conf b/debian/chrony.conf
233index 6c19767..d5a0b37 100644
234--- a/debian/chrony.conf
235+++ b/debian/chrony.conf
236@@ -1,6 +1,23 @@
237 # Welcome to the chrony configuration file. See chrony.conf(5) for more
238 # information about usuable directives.
239-pool 2.debian.pool.ntp.org iburst
240+
241+# This will use (up to):
242+# - 4 sources from ntp.ubuntu.com which some are ipv6 enabled
243+# - 2 sources from 2.ubuntu.pool.ntp.org which is ipv6 enabled as well
244+# - 1 source from [01].ubuntu.pool.ntp.org each (ipv4 only atm)
245+# This means by default, up to 6 dual-stack and up to 2 additional IPv4-only
246+# sources will be used.
247+# At the same time it retains some protection against one of the entries being
248+# down (compare to just using one of the lines). See (LP: #1754358) for the
249+# discussion.
250+#
251+# About using servers from the NTP Pool Project in general see (LP: #104525).
252+# Approved by Ubuntu Technical Board on 2011-02-08.
253+# See http://www.pool.ntp.org/join.html for more information.
254+pool ntp.ubuntu.com iburst maxsources 4
255+pool 0.ubuntu.pool.ntp.org iburst maxsources 1
256+pool 1.ubuntu.pool.ntp.org iburst maxsources 1
257+pool 2.ubuntu.pool.ntp.org iburst maxsources 2
258
259 # This directive specify the location of the file containing ID/key pairs for
260 # NTP authentication.
261diff --git a/debian/chrony.default b/debian/chrony.default
262index ae79e8a..b523f60 100644
263--- a/debian/chrony.default
264+++ b/debian/chrony.default
265@@ -4,3 +4,7 @@
266
267 # Options to pass to chrony.
268 DAEMON_OPTS=""
269+
270+# Sync systecm clock in containers or without CAP_SYS_TIME (likely to fail)
271+# See /usr/share/doc/chrony/README.container for details.
272+SYNC_IN_CONTAINER="no"
273diff --git a/debian/chrony.service b/debian/chrony.service
274index add851e..6e2c74f 100644
275--- a/debian/chrony.service
276+++ b/debian/chrony.service
277@@ -3,13 +3,13 @@ Description=chrony, an NTP client/server
278 Documentation=man:chronyd(8) man:chronyc(1) man:chrony.conf(5)
279 Conflicts=systemd-timesyncd.service openntpd.service ntp.service
280 After=network.target
281-ConditionCapability=CAP_SYS_TIME
282
283 [Service]
284 Type=forking
285 PIDFile=/run/chronyd.pid
286 EnvironmentFile=-/etc/default/chrony
287-ExecStart=/usr/sbin/chronyd $DAEMON_OPTS
288+# Starter takes care of special cases mostly for containers
289+ExecStart=/usr/lib/systemd/scripts/chronyd-starter.sh $DAEMON_OPTS
290 ExecStartPost=-/usr/lib/chrony/chrony-helper update-daemon
291 PrivateTmp=yes
292 ProtectHome=yes
293diff --git a/debian/chronyd-starter.sh b/debian/chronyd-starter.sh
294new file mode 100755
295index 0000000..c175db5
296--- /dev/null
297+++ b/debian/chronyd-starter.sh
298@@ -0,0 +1,70 @@
299+#!/bin/sh
300+set -ue
301+
302+CONF="/etc/default/chrony"
303+DOC="/usr/share/doc/chrony/README.container"
304+CAP="cap_sys_time"
305+CMD="/usr/sbin/chronyd"
306+# Take any args passed, use none if nothing was specified
307+EFFECTIVE_DAEMON_OPTS=${@:-""}
308+
309+if [ -f "${CONF}" ]; then
310+ . "${CONF}"
311+else
312+ echo "<4>Warning: ${CONF} is missing"
313+fi
314+# take from conffile if available, default to no otherwise
315+EFFECTIVE_SYNC_IN_CONTAINER=${SYNC_IN_CONTAINER:-"no"}
316+
317+if [ ! -x "${CMD}" ]; then
318+ echo "<3>Error: ${CMD} not executable"
319+ # ugly, but works around https://github.com/systemd/systemd/issues/2913
320+ sleep 0.1
321+ exit 1
322+fi
323+
324+# Check if -x is already set manually, don't process further if that is the case
325+X_SET=0
326+while getopts ":x" opt; do
327+ case $opt in
328+ x)
329+ X_SET=1
330+ ;;
331+ esac
332+done
333+
334+if [ ${X_SET} -ne 1 ]; then
335+ # Assume it is not in a container
336+ IS_CONTAINER=0
337+ if [ -x /usr/bin/systemd-detect-virt ]; then
338+ if /usr/bin/systemd-detect-virt --quiet --container; then
339+ IS_CONTAINER=1
340+ fi
341+ fi
342+
343+
344+ # Assume it has the cap
345+ HAS_CAP=1
346+ CAPSH="/sbin/capsh"
347+ if [ -x "${CAPSH}" ]; then
348+ ${CAPSH} --print | grep -q "^Current.*${CAP}" || HAS_CAP=0
349+ fi
350+
351+ if [ ${HAS_CAP} -eq 0 ]; then
352+ echo "<4>Warning: Missing ${CAP}, syncing the system clock will fail"
353+ fi
354+ if [ ${IS_CONTAINER} -eq 1 ]; then
355+ echo "<4>Warning: Running in a container, likely impossible and unintended to sync system clock"
356+ fi
357+
358+ if [ ${HAS_CAP} -eq 0 -o ${IS_CONTAINER} -eq 1 ]; then
359+ if [ "${EFFECTIVE_SYNC_IN_CONTAINER}" != "yes" ]; then
360+ echo "<5>Adding -x as fallback disabling control of the system clock, see ${DOC} to override this behavior"
361+ EFFECTIVE_DAEMON_OPTS="${EFFECTIVE_DAEMON_OPTS} -x"
362+ else
363+ echo "<5>Not falling back to disable control of the system clock, see ${DOC} to change this behavior"
364+ fi
365+ fi
366+fi
367+
368+${CMD} ${EFFECTIVE_DAEMON_OPTS}
369diff --git a/debian/control b/debian/control
370index 6750af8..9edfa53 100644
371--- a/debian/control
372+++ b/debian/control
373@@ -1,7 +1,8 @@
374 Source: chrony
375 Section: net
376 Priority: optional
377-Maintainer: Vincent Blut <vincent.debian@free.fr>
378+Maintainer: Ubuntu Developers <ubuntu-devel-discuss@lists.ubuntu.com>
379+XSBC-Original-Maintainer: Vincent Blut <vincent.debian@free.fr>
380 Uploaders: Joachim Wiedorn <joodebian@joonet.de>
381 Standards-Version: 4.1.4
382 Build-Depends: debhelper (>= 11),
383@@ -26,7 +27,9 @@ Depends: ${misc:Depends},
384 ucf,
385 iproute2 [linux-any],
386 lsb-base,
387+ libcap2-bin,
388 adduser
389+Recommends: networkd-dispatcher (>= 1.7-0ubuntu3)
390 Suggests: dnsutils
391 Conflicts: time-daemon, ntp
392 Provides: time-daemon
393diff --git a/debian/docs b/debian/docs
394index 5b97370..1741ab0 100644
395--- a/debian/docs
396+++ b/debian/docs
397@@ -1,2 +1,3 @@
398 README
399 FAQ
400+debian/README.container
401diff --git a/debian/install b/debian/install
402index e3cc902..259db0c 100644
403--- a/debian/install
404+++ b/debian/install
405@@ -2,3 +2,4 @@ debian/chrony.conf usr/share/chrony
406 debian/chrony-dnssrv@.* lib/systemd/system
407 debian/chrony-helper usr/lib/chrony
408 debian/usr.sbin.chronyd etc/apparmor.d
409+debian/chronyd-starter.sh usr/lib/systemd/scripts/
410diff --git a/debian/links b/debian/links
411new file mode 100644
412index 0000000..71e2c52
413--- /dev/null
414+++ b/debian/links
415@@ -0,0 +1,5 @@
416+# Update sources in response to systemd-networkd events (LP: #1718227).
417+# This is reusing the NetworkManager dispatch script which has no hard
418+# dependency to NetworkManager (not using any of its arguments)
419+etc/NetworkManager/dispatcher.d/20-chrony usr/lib/networkd-dispatcher/routable.d/chrony
420+etc/NetworkManager/dispatcher.d/20-chrony usr/lib/networkd-dispatcher/off.d/chrony
421diff --git a/debian/patches/lp-1718227-nm-dispatcher-for-networkd.patch b/debian/patches/lp-1718227-nm-dispatcher-for-networkd.patch
422new file mode 100644
423index 0000000..91c8147
424--- /dev/null
425+++ b/debian/patches/lp-1718227-nm-dispatcher-for-networkd.patch
426@@ -0,0 +1,66 @@
427+From 8cbc68f28f96d48a7ee128fa91731ca02a598913 Mon Sep 17 00:00:00 2001
428+From: Christian Ehrhardt <christian.ehrhardt@canonical.com>
429+Date: Wed, 18 Apr 2018 15:44:21 +0200
430+Subject: [PATCH 2/2] examples: make nm-dispatcher script usable for
431+ networkd-dispatcher
432+
433+Historically there were plenty of callback based implementations around
434+ifupdown via /etc/network/if-up and similar. NetworkManager added the
435+dispatcher [1] feature for such a kind of functionality.
436+
437+But so far a systemd-networkd (only) systemd had no means to handle those
438+cases. This is solved by networkd-dispatcher which is currently available
439+at least in ArchLinux and Ubuntu.
440+It takes away the responsibility to listen on netlink events in each
441+application and provides a more classic script-drop-in interface to respond
442+to networkd events [3].
443+
444+This commit makes the NM example compatible to be used by NetworkManager
445+dispatcher as well as by networkd-dispatcher. That way we avoid too much
446+code duplication and can from now on handle special cases in the
447+beginning so that the tail can stay commonly used.
448+
449+After discussion on IRC the current check differs by checking the
450+argument count (only in NetworkManager), if ever needed we could extend
451+that to check for known custom environment vars (NetworkManager =>
452+CONNECTION_UUID; networkd-dispatcher => OperationalState).
453+
454+[1]: https://developer.gnome.org/NetworkManager/stable/NetworkManager.html
455+[2]: https://github.com/craftyguy/networkd-dispatcher
456+[3]: https://github.com/systemd/systemd/blob/master/src/systemd/sd-network.h#L86
457+
458+Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
459+
460+Author: Christian Ehrhardt <christian.ehrhardt@canonical.com>
461+Origin: backport, https://git.tuxfamily.org/chrony/chrony.git/commit/?id=8cbc68f28f96d48a7ee128fa91731ca02a598913
462+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1718227
463+Last-Update: 2018-04-18
464+---
465+ examples/chrony.nm-dispatcher | 10 +++++++---
466+ 1 file changed, 7 insertions(+), 3 deletions(-)
467+
468+diff --git a/examples/chrony.nm-dispatcher b/examples/chrony.nm-dispatcher
469+index a609a66..8bd7df0 100644
470+--- a/examples/chrony.nm-dispatcher
471++++ b/examples/chrony.nm-dispatcher
472+@@ -1,10 +1,14 @@
473+ #!/bin/sh
474+-# This is a NetworkManager dispatcher script for chronyd to set its NTP sources
475+-# online or offline when a network interface is configured or removed
476++# This is a NetworkManager dispatcher / networkd-dispatcher script for
477++# chronyd to set its NTP sources online or offline when a network interface
478++# is configured or removed
479+
480+ export LC_ALL=C
481+
482+-[ "$2" != "up" ] && [ "$2" != "down" ] && exit 0
483++# For NetworkManager consider only up/down events
484++[ $# -ge 2 ] && [ "$2" != "up" ] && [ "$2" != "down" ] && exit 0
485++
486++# Note: for networkd-dispatcher routable.d ~= on and off.d ~= off
487+
488+ # Check if there is a default route
489+
490+--
491+2.7.4
492+
493diff --git a/debian/patches/series b/debian/patches/series
494index e69de29..21d48ce 100644
495--- a/debian/patches/series
496+++ b/debian/patches/series
497@@ -0,0 +1 @@
498+lp-1718227-nm-dispatcher-for-networkd.patch
499diff --git a/debian/postrm b/debian/postrm
500index 42f48bc..6fba6b5 100644
501--- a/debian/postrm
502+++ b/debian/postrm
503@@ -7,6 +7,15 @@ set -e
504
505 # targets: purge|remove|upgrade|failed-upgrade|abort-install|abort-upgrade|disappear
506
507+restore_timesyncd() {
508+ # on next reboot it would start, but that would leave time
509+ # unsynchronized until then. So as the Conflicts in the service file kill
510+ # systemd-timesyncd re-establish it if it is enabled
511+ if [ "$(systemctl is-enabled systemd-timesyncd 2>/dev/null)" = "enabled" ] ; then
512+ systemctl start systemd-timesyncd
513+ fi
514+}
515+
516 case "$1" in
517 purge)
518 rm -f /var/lib/chrony/*
519@@ -30,9 +39,15 @@ case "$1" in
520 then
521 deluser --quiet --system _chrony > /dev/null 2>&1 || true
522 fi
523+
524+ restore_timesyncd
525+ ;;
526+
527+ remove)
528+ restore_timesyncd
529 ;;
530
531- remove|upgrade|failed-upgrade|abort-install|abort-upgrade|disappear)
532+ upgrade|failed-upgrade|abort-install|abort-upgrade|disappear)
533
534 ;;
535

Subscribers

People subscribed via source and target branches