Merge ~paelzer/ubuntu/+source/chrony:merge-cosmic-3.3-1 into ubuntu/+source/chrony:debian/sid
- Git
- lp:~paelzer/ubuntu/+source/chrony
- merge-cosmic-3.3-1
- Merge into debian/sid
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) |
||||||||||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Robie Basak | Approve | ||
Canonical Server | Pending | ||
git-ubuntu developers | Pending | ||
Review via email: mp+345498@code.launchpad.net |
Commit message
Description of the change
Christian Ehrhardt (paelzer) wrote : | # |
Simon Déziel (sdeziel) : | # |
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>
Christian Ehrhardt (paelzer) wrote : | # |
Thanks Simon, I added fixes for the typos to the proposal.
Paul Gear (paulgear) wrote : | # |
Couple of minor style/grammar suggestions inline.
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-
But for now I'm refusing to make the changes part of this merge.
Still waiting for a +1 on the merge in general.
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
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.
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.
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-
- 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).
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.
Robie Basak (racb) : | # |
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
1 | diff --git a/debian/README.container b/debian/README.container |
2 | new file mode 100644 |
3 | index 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 |
67 | diff --git a/debian/changelog b/debian/changelog |
68 | index 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 |
232 | diff --git a/debian/chrony.conf b/debian/chrony.conf |
233 | index 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. |
261 | diff --git a/debian/chrony.default b/debian/chrony.default |
262 | index 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" |
273 | diff --git a/debian/chrony.service b/debian/chrony.service |
274 | index 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 |
293 | diff --git a/debian/chronyd-starter.sh b/debian/chronyd-starter.sh |
294 | new file mode 100755 |
295 | index 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} |
369 | diff --git a/debian/control b/debian/control |
370 | index 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 |
393 | diff --git a/debian/docs b/debian/docs |
394 | index 5b97370..1741ab0 100644 |
395 | --- a/debian/docs |
396 | +++ b/debian/docs |
397 | @@ -1,2 +1,3 @@ |
398 | README |
399 | FAQ |
400 | +debian/README.container |
401 | diff --git a/debian/install b/debian/install |
402 | index 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/ |
410 | diff --git a/debian/links b/debian/links |
411 | new file mode 100644 |
412 | index 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 |
421 | diff --git a/debian/patches/lp-1718227-nm-dispatcher-for-networkd.patch b/debian/patches/lp-1718227-nm-dispatcher-for-networkd.patch |
422 | new file mode 100644 |
423 | index 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 | + |
493 | diff --git a/debian/patches/series b/debian/patches/series |
494 | index 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 |
499 | diff --git a/debian/postrm b/debian/postrm |
500 | index 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 |
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