Merge ~ankushpathak/ubuntu/+source/chrony:feat/split_out_pool_sources_oracular into ubuntu/+source/chrony:ubuntu/devel
- Git
- lp:~ankushpathak/ubuntu/+source/chrony
- feat/split_out_pool_sources_oracular
- Merge into ubuntu/devel
Status: | Merged |
---|---|
Approved by: | Andreas Hasenack |
Approved revision: | d1e15f1763a4831238dcc1f605f733810bd36417 |
Merged at revision: | d1e15f1763a4831238dcc1f605f733810bd36417 |
Proposed branch: | ~ankushpathak/ubuntu/+source/chrony:feat/split_out_pool_sources_oracular |
Merge into: | ubuntu/+source/chrony:ubuntu/devel |
Diff against target: |
253 lines (+115/-20) 10 files modified
debian/NEWS (+13/-0) debian/changelog (+20/-0) debian/chrony.conf (+1/-17) debian/chrony.config (+10/-0) debian/control (+1/-0) debian/install (+1/-0) debian/postinst (+26/-3) debian/postrm (+14/-0) debian/templates (+12/-0) debian/ubuntu-ntp-pools.sources (+17/-0) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Andreas Hasenack | Approve | ||
git-ubuntu import | Pending | ||
Review via email:
|
Commit message
Move default NTP pool sources config out of chrony.conf
Description of the change
Teststing details (tested on a noble instance on GCE):
1. Upgrade chrony when `sources.
2. Upgrade chrony when `sources.
3. Upgrade chrony when `sources.
4. Upgrade chrony when `sources.
5. Upgrade chrony when `sources.
6. Built a GCE base oracular image with debconf config `chrony/
7. Purged chrony after upgrading to ensure debconf selections are cleared.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote : | # |
First pass
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ankush Pathak (ankushpathak) wrote : | # |
@ahasenack I have updated the MP to address your comments. Verified all the tests listed in the description.
> This change is altering the ordering of pools. Before, we had the ntp.ubuntu.com and the pool.ntp.org pools first, and then dhcp, and then sources.d. Now we have dhcp first, and then sources.d. In other words, dhcp is coming before the ubuntu ntp configuration. Any thoughts on that?
I don't think the order of pool configuration makes a difference. I have seen chrony synchronize with servers that were not listed first in the configuration.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote : | # |
I had to add debconf support to a package from scratch once, maybe this can help. It was in isc-kea[1].
1. https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote : | # |
Ok, I'm starting to understand this better. So the whole point of the debconf questions is not to have the user answer them, ever, but just to be something we can preseed in the cloud images.
With these questions as they are, the cloud image would preseed them as follows, if I understood this correctly:
- configure_
- preserve_
Presumably you were thinking of having the cloud's NTP pools be also in a file called sources.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote : | # |
Now imagining the default ubuntu installation, and the debconf default answers are taken (i.e., no preseeding has happened to change them, like in the cloud case).
- fresh install: chrony.conf has no pools, and postinst will create sources.
- upgrade: if sources.
The problem in the upgrade case above is when the user made changes, and the package made changes too. In this scenario, the user won't be told that there have been packaging changes. Which is what we wanted for the cloud case, but not the ubuntu case. We are effectively treating this as if --force-confhold had been passed to dpkg, right?
I'm thinking we should only have a debconf prompt for the first case, where the cloud can preseed it in such a way that sources.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote : | # |
How would you feel if we shipped a selection of ntp pools in /usr/share/chrony, one for each cloud, and one for ubuntu, and at install time used debconf seeding to pick one? The default would be the ubuntu pools.
The drawback is that these pools would be in the package, and if you had to change them, that would require an SRU each time.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote : | # |
I'm trying something around that idea.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ankush Pathak (ankushpathak) wrote : | # |
@ahasenack I was out for several days, but I am working on addressing your comments.
I have discussed shipping cloud specific configuration with the chrony package within my squad. We don't think that's a good idea. Different clouds have different approaches to setup chrony time sources. The flexibility of being able to manage that through the CPC build hooks is the preferred approach.
I have elaborated further on this under point number 9 in this comment[0] on the bug.
[0] https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ankush Pathak (ankushpathak) wrote : | # |
> Presumably you were thinking of having the cloud's NTP pools be also in a file called sources.
This would depend on the cloud. For instance, GCE relies on cloud-init to configure chrony time sources. So the drop-in file on GCE instances would depend on how cloud-init decides to configure time sources once this change lands. Another example, Azure uses a hw clock device which would ideally be configured through a file in `/etc/chrony/
> I'm thinking we should only have a debconf prompt for the first case, where the cloud can preseed it in such a way that sources.
This makes sense. To handle this I propose
* Remove the `chrony/
* And the following code change
```
db_get chrony/
if [ "${RET}" = "true" ];
then
ucf --debconf-ok --three-way "${packaged_
fi
```
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote : | # |
Right, I did something like this in my branch. On top of your 3ff8196d943bd98
diff --git a/debian/NEWS b/debian/NEWS
index e335d28..260dcf5 100644
--- a/debian/NEWS
+++ b/debian/NEWS
@@ -3,9 +3,9 @@ chrony (4.5-3ubuntu2) oracular; urgency=medium
Starting with chrony version 4.5-3ubuntu2 the default time sources are
configured by default in the /etc/chrony/
file.
- default-
- sources and adding custom ones while avoiding a potential ucf prompt during
- chrony upgrade is possible.
+ This can be omitted by pre-seeding the
+ chrony/
+ case the package will be installed without any time sources configured.
-- Ankush Pathak <email address hidden> Tue, 16 Jul 2024 17:57:41 -0600
diff --git a/debian/
new file mode 100644
index 0000000..446b883
--- /dev/null
+++ b/debian/
@@ -0,0 +1,21 @@
+#!/bin/sh
+
+set -e
+
+. /usr/share/
+
+reconfigure=""
+
+if [ "${1}" = "configure" ] || [ "${1}" = "reconfigure" ]; then
+ if [ "${1}" = "reconfigure" ] || [ -n "${DEBCONF_
+ reconfigure="yes"
+ fi
+ # ask questions on:
+ # - reconfigure
+ # - fresh install
+ if [ -n "${reconfigure}" ] || dpkg --compare-versions "$2" lt "4.5-3ubuntu2"; then
+ db_input low chrony/
+ db_go || true
+ db_get chrony/
+ fi
+fi
diff --git a/debian/postinst b/debian/postinst
index 1dc5ee5..4d6e0ef 100644
--- a/debian/postinst
+++ b/debian/postinst
@@ -9,6 +9,7 @@ set -e
# targets: configure|
+ucf_managed_
case "$1" in
configure)
@@ -21,35 +22,23 @@ case "$1" in
-
- if [ ! -f "${default_
- then
- db_input low chrony/
- db_go
- db_get chrony/
- if [ "${RET}" = "true" ];
- then
- cp --preserve "${packaged_
- fi
- # Handle the case where the system already has default-
- elif ! cmp -s "${packaged_
- then
- db_input low chrony/
- db_go
- db_get chrony/
- if [ "${RET}" = "false" ];
- then
- cp --preserve "${packaged_
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote : | # |
My branch is at https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote : | # |
I'm using this for preseeding tests:
#!/bin/bash
debconf-
chrony chrony/
EOF
and
#!/bin/bash
debconf-
chrony chrony/
EOF
We can probably change the name of that key to something shorter, but let's leave that to the end.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote : | # |
I removed some brain farts, polished things up a bit, and pushed to https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote : | # |
There is at least one remaining thing: what to do if the user has changed their chrony.conf, and opt to keep that config file in place in an upgrade. By default, the way things are now, we would install the ubuntu ntp sources in sources.d/, without removing them from chrony.conf (because the user opted to keep the config file instead of installing the new one from the package).
Could we collate chrony.conf plus all snippets from sources.d/*.sources and grep for ntp pools or servers? Ir there are any, then we don't install ours, and act as if chrony/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote : | # |
I have a ppa here with a build from my branch: https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ankush Pathak (ankushpathak) wrote : | # |
Please let me know what you think should be the next steps here, and I can start working through those.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote : | # |
Could you perhaps please test the package I have in the PPA, see if it addresses what CPC needs? Then I think the only remaining question is https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ankush Pathak (ankushpathak) wrote : | # |
As discussed in our sync-up:
* I'll update the MP with the changes from your branch.
* Test the changes, leaning on doing more CPC centric tests
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ankush Pathak (ankushpathak) wrote : | # |
> There is at least one remaining thing: what to do if the user has changed their chrony.conf, and opt to keep that config file in place in an upgrade. By default, the way things are now, we would install the ubuntu ntp sources in sources.d/, without removing them from chrony.conf (because the user opted to keep the config file instead of installing the new one from the package).
> Could we collate chrony.conf plus all snippets from sources.d/*.sources and grep for ntp pools or servers? Ir there are any, then we don't install ours, and act as if chrony/
Capturing that, we've decided to continue with this specific scenario as being an acceptable side effect, for now.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote : | # |
I was just reminded that all these changes need coordination with how cloud-init handles ntp configuration in its module. In the bug that spawned this MP there were some comments about driving changes in cloud-init according to the plan here, what has been discussed with cloud-init so far, if anything?
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ankush Pathak (ankushpathak) wrote : | # |
I will discuss this today again with @catred and see what she thinks about the status of the complementary changes required to cloud-init.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ankush Pathak (ankushpathak) wrote : | # |
Testing results on commit e19f54e
* Upgraded chrony on a GCE noble instance. `/etc/chrony/
* Upgraded chrony on a GCE noble instance. `/etc/chrony/
* Upgraded chrony on a GCE noble instance. `/etc/chrony/
* Upgraded chrony on a GCE noble instance. `/etc/chrony/
* Upgraded chrony on a GCE noble instance with `chrony/
* Upgraded chrony on a GCE noble instance with `chrony/
* Upgraded chrony on a GCE noble instance with `chrony/
* Upgraded chrony on a GCE noble instance with `chrony/
* Upgraded chrony on a GCE noble instance with `chrony/
* Created `/etc/chrony/
* Created `/etc/chrony/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote (last edit ): | # |
Question inline about e19f54ec9fe768d
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ankush Pathak (ankushpathak) wrote : | # |
Responded in-line.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ankush Pathak (ankushpathak) : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ankush Pathak (ankushpathak) : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ankush Pathak (ankushpathak) wrote : | # |
Updated with the name change.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote : | # |
I uploaded to the ppa
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote : | # |
The ppas are built, could you take it for another round of testing please?
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ankush Pathak (ankushpathak) wrote : | # |
I'll start testing, but will most likely not finish today. Are we still good if I need until tomorrow to complete testing?
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote : | # |
What's the story with cloud-init? That is important, we can't break them just out of the blue. How did the conversations go?
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ankush Pathak (ankushpathak) wrote : | # |
I checked with @catred about this. She said that the current approach of this bug fix should be safe to land and not break cloud-init since cloud-init overwrites chrony.conf. The build hooks for any cloud images that rely on cloud-init to configure chrony will need to be updated to set `chrony/
I will request her to chime in here in case she has more insights.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote : | # |
> since cloud-init overwrites chrony.conf
Let's say you have an image that has pre-seeded that question with false, i.e., there is no default ubuntu ntp sources in sources.d, and chrony.conf has no default ntp sources, and you injected a new sources.
Then someone boots that image and tells cloud-init to use a pool of myntp.example.com.
When that system boots, won't it now have two pools? One for myntp.example.com defined in chrony.conf, and another one coming from sources.
Or does cloud-init *remove* the sourcedir option from chrony.conf?
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ankush Pathak (ankushpathak) wrote : | # |
Based on my testing on GCE, chrony.conf written by cloud-init does not have the sourcedir directive.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ankush Pathak (ankushpathak) wrote : | # |
Added some responses inline.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ankush Pathak (ankushpathak) wrote : | # |
Testing result on commit 0f78c08
TLDR: Tests look good.
* Upgraded chrony on a GCE noble instance. `/etc/chrony/
* Upgraded chrony on a GCE noble instance. `/etc/chrony/
* Upgraded chrony on a GCE noble instance. `/etc/chrony/
* Upgraded chrony on a GCE noble instance. `/etc/chrony/
* Upgraded chrony on a GCE noble instance. `/etc/chrony/
* Upgraded chrony on a GCE noble instance. `/etc/chrony/
* Upgraded chrony on a GCE noble instance with `chrony/
* Upgraded chrony on a GCE noble instance with `chrony/
* Upgraded chrony on a GCE noble instance with `chrony/
* Upgraded chrony on a GCE noble instance with `chrony/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote : | # |
inline
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote : | # |
d/changelog
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ankush Pathak (ankushpathak) wrote : | # |
Inline response.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote : | # |
replied
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ankush Pathak (ankushpathak) wrote : | # |
Inline response
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote : | # |
Then please commit the changes and push (don't forget the d/changelog update), thanks!
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ankush Pathak (ankushpathak) wrote : | # |
A couple of inline questions.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote : | # |
Replied, let's see what it looks like committed.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote : | # |
Only d/changelog fixes.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ankush Pathak (ankushpathak) wrote : | # |
Updated changelog
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ankush Pathak (ankushpathak) wrote : | # |
Update changelog
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote : | # |
That's it, +1!
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Andreas Hasenack (ahasenack) wrote : | # |
Sponsored:
Uploading chrony_
Uploading chrony_
Uploading chrony_
Uploading chrony_
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ankush Pathak (ankushpathak) wrote : | # |
Thanks!
Should I update the MP status to "Merged"?
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Ankush Pathak (ankushpathak) wrote : | # |
Ah! Saw the update.
Preview Diff
1 | diff --git a/debian/NEWS b/debian/NEWS |
2 | index fe7e4b9..73ce486 100644 |
3 | --- a/debian/NEWS |
4 | +++ b/debian/NEWS |
5 | @@ -1,3 +1,16 @@ |
6 | +chrony (4.5-3ubuntu2) oracular; urgency=medium |
7 | + |
8 | + Starting with chrony version 4.5-3ubuntu2 the default time sources are |
9 | + configured by default in the /etc/chrony/sources.d/ubuntu-ntp-pools.sources |
10 | + file. |
11 | + Setting the chrony/configure_ubuntu_pools_in_sourcesd debconf key to "false", |
12 | + either via pre-seeding or dpkg-reconfigure, will result in the package being |
13 | + installed without any time sources configured. With this setting, in the case |
14 | + of an upgrade, if /etc/chrony/sources.d/ubuntu-ntp-pools.sources exists, it |
15 | + will be removed and subsequent upgrades will not recreate it. |
16 | + |
17 | + -- Ankush Pathak <ankush.pathak@canonical.com> Tue, 16 Jul 2024 17:57:41 -0600 |
18 | + |
19 | chrony (4.1~pre1-1) experimental; urgency=medium |
20 | |
21 | Starting with chrony 4.0, it is possible to specify NTP sources in files |
22 | diff --git a/debian/changelog b/debian/changelog |
23 | index 9bf1f34..6bc6d8a 100644 |
24 | --- a/debian/changelog |
25 | +++ b/debian/changelog |
26 | @@ -1,3 +1,23 @@ |
27 | +chrony (4.5-3ubuntu2) oracular; urgency=medium |
28 | + * Move Ubuntu NTP sources to /etc/chrony/sources.d/ubuntu-ntp-pools.sources, |
29 | + gated on a low priority (default yes) debconf question (LP: #2048876): |
30 | + - d/NEWS: Add entry with information about default time sources moving out |
31 | + from chrony.conf to /etc/chrony/sources.d/ubuntu-ntp-pools.sources. |
32 | + - d/chrony.conf, d/ubuntu-ntp-pools.sources: Move Ubuntu NTP pool sources |
33 | + from chrony.conf to ubuntu-ntp-pools.sources |
34 | + - d/chrony.config: Ask chrony/configure_ubuntu_pools_in_sourcesd debconf |
35 | + question. |
36 | + - d/control: Add debconf dependency. |
37 | + - d/install: Install ubuntu-ntp-pools.sources in /usr/share/chrony |
38 | + - d/postinst: Handle ubuntu-ntp-pools.sources installation, removal and |
39 | + ucf tracking based on debconf question. |
40 | + - d/postrm: Handle /etc/chrony/sources.d/ubuntu-ntp-pools.sources removal |
41 | + from ucf tracking based on debconf question on uninstall. |
42 | + - d/templates: Add debconf question to customize installation of |
43 | + /etc/chrony/sources.d/ubuntu-ntp-pools.sources. |
44 | + |
45 | + -- Ankush Pathak <ankush.pathak@canonical.com> Thu, 15 Aug 2024 00:03:08 +0530 |
46 | + |
47 | chrony (4.5-3ubuntu1) oracular; urgency=medium |
48 | |
49 | * Merge with Debian unstable (LP: #2064393, LP: #2068526). Remaining |
50 | diff --git a/debian/chrony.conf b/debian/chrony.conf |
51 | index b4a1ca9..d9d9434 100644 |
52 | --- a/debian/chrony.conf |
53 | +++ b/debian/chrony.conf |
54 | @@ -4,23 +4,7 @@ |
55 | # Include configuration files found in /etc/chrony/conf.d. |
56 | confdir /etc/chrony/conf.d |
57 | |
58 | -# This will use (up to): |
59 | -# - 4 sources from ntp.ubuntu.com which some are ipv6 enabled |
60 | -# - 2 sources from 2.ubuntu.pool.ntp.org which is ipv6 enabled as well |
61 | -# - 1 source from [01].ubuntu.pool.ntp.org each (ipv4 only atm) |
62 | -# This means by default, up to 6 dual-stack and up to 2 additional IPv4-only |
63 | -# sources will be used. |
64 | -# At the same time it retains some protection against one of the entries being |
65 | -# down (compare to just using one of the lines). See (LP: #1754358) for the |
66 | -# discussion. |
67 | -# |
68 | -# About using servers from the NTP Pool Project in general see (LP: #104525). |
69 | -# Approved by Ubuntu Technical Board on 2011-02-08. |
70 | -# See http://www.pool.ntp.org/join.html for more information. |
71 | -pool ntp.ubuntu.com iburst maxsources 4 |
72 | -pool 0.ubuntu.pool.ntp.org iburst maxsources 1 |
73 | -pool 1.ubuntu.pool.ntp.org iburst maxsources 1 |
74 | -pool 2.ubuntu.pool.ntp.org iburst maxsources 2 |
75 | +# The Ubuntu NTP pool servers configuration was moved to /etc/chrony/sources.d/ubuntu-ntp-pools.sources |
76 | |
77 | # Use time sources from DHCP. |
78 | sourcedir /run/chrony-dhcp |
79 | diff --git a/debian/chrony.config b/debian/chrony.config |
80 | new file mode 100644 |
81 | index 0000000..c272d60 |
82 | --- /dev/null |
83 | +++ b/debian/chrony.config |
84 | @@ -0,0 +1,10 @@ |
85 | +#!/bin/sh |
86 | + |
87 | +set -e |
88 | + |
89 | +. /usr/share/debconf/confmodule |
90 | + |
91 | +if [ "${1}" = "configure" ] || [ "${1}" = "reconfigure" ]; then |
92 | + db_input low chrony/configure_ubuntu_pools_in_sourcesd || true |
93 | + db_go || true |
94 | +fi |
95 | diff --git a/debian/control b/debian/control |
96 | index 4a27c4e..6f0fadf 100644 |
97 | --- a/debian/control |
98 | +++ b/debian/control |
99 | @@ -27,6 +27,7 @@ Package: chrony |
100 | Architecture: linux-any |
101 | Pre-Depends: ${misc:Pre-Depends} |
102 | Depends: adduser (>= 3.130), |
103 | + debconf (>= 0.5), |
104 | iproute2 [linux-any], |
105 | tzdata-legacy, |
106 | libcap2-bin (>= 1:2.32-1), |
107 | diff --git a/debian/install b/debian/install |
108 | index 36cff68..3d3a6e9 100644 |
109 | --- a/debian/install |
110 | +++ b/debian/install |
111 | @@ -1,5 +1,6 @@ |
112 | debian/chrony-helper usr/libexec/chrony |
113 | debian/chrony.conf usr/share/chrony |
114 | +debian/ubuntu-ntp-pools.sources usr/share/chrony |
115 | debian/conf.d etc/chrony |
116 | debian/ntp-units.d/50-chrony.list usr/lib/systemd/ntp-units.d |
117 | debian/sources.d etc/chrony |
118 | diff --git a/debian/postinst b/debian/postinst |
119 | index 1379453..33aa8fa 100644 |
120 | --- a/debian/postinst |
121 | +++ b/debian/postinst |
122 | @@ -3,13 +3,15 @@ |
123 | # |
124 | # see: dh_installdeb(1) |
125 | |
126 | +. /usr/share/debconf/confmodule |
127 | set -e |
128 | |
129 | |
130 | # targets: configure|abort-upgrade|abort-remove|abort-deconfigure |
131 | |
132 | +ucf_managed_sources="true" |
133 | case "$1" in |
134 | - configure) |
135 | + configure|reconfigure) |
136 | |
137 | adduser --system \ |
138 | --group \ |
139 | @@ -18,13 +20,34 @@ case "$1" in |
140 | --home /var/lib/chrony \ |
141 | --no-create-home _chrony |
142 | |
143 | + ubuntu_ntp_pools_sources_filepath="/etc/chrony/sources.d/ubuntu-ntp-pools.sources" |
144 | + packaged_ubuntu_ntp_pools_sources_filepath="/usr/share/chrony/ubuntu-ntp-pools.sources" |
145 | + |
146 | + db_get chrony/configure_ubuntu_pools_in_sourcesd |
147 | + ucf_managed_sources="${RET}" |
148 | + |
149 | if command -v ucf >/dev/null |
150 | then |
151 | - ucf --three-way /usr/share/chrony/chrony.conf /etc/chrony/chrony.conf |
152 | - ucf --three-way /usr/share/chrony/chrony.keys /etc/chrony/chrony.keys |
153 | + ucf --debconf-ok --three-way /usr/share/chrony/chrony.conf /etc/chrony/chrony.conf |
154 | + ucf --debconf-ok --three-way /usr/share/chrony/chrony.keys /etc/chrony/chrony.keys |
155 | + if [ "${ucf_managed_sources}" = "true" ]; then |
156 | + ucf --debconf-ok --three-way "${packaged_ubuntu_ntp_pools_sources_filepath}" "${ubuntu_ntp_pools_sources_filepath}" |
157 | + else |
158 | + # If this was under ucf before, purge it. |
159 | + # If it wasn't under ucf before, this does not fail |
160 | + ucf --debconf-ok --purge "${ubuntu_ntp_pools_sources_filepath}" |
161 | + rm -f "${ubuntu_ntp_pools_sources_filepath}" |
162 | + fi |
163 | if [ -x "$(command -v ucfr)" ]; then |
164 | ucfr chrony /etc/chrony/chrony.conf |
165 | ucfr chrony /etc/chrony/chrony.keys |
166 | + if [ "${ucf_managed_sources}" = "true" ]; then |
167 | + ucfr chrony "${ubuntu_ntp_pools_sources_filepath}" |
168 | + else |
169 | + # If this was under ucf before, purge it. |
170 | + # If it wasn't under ucf before, this does not fail |
171 | + ucfr --purge chrony "${ubuntu_ntp_pools_sources_filepath}" |
172 | + fi |
173 | fi |
174 | fi |
175 | |
176 | diff --git a/debian/postrm b/debian/postrm |
177 | index 79713e3..9a80d36 100644 |
178 | --- a/debian/postrm |
179 | +++ b/debian/postrm |
180 | @@ -5,8 +5,16 @@ |
181 | |
182 | set -e |
183 | |
184 | +. /usr/share/debconf/confmodule |
185 | + |
186 | # targets: purge|remove|upgrade|failed-upgrade|abort-install|abort-upgrade|disappear |
187 | |
188 | +ubuntu_ntp_pools_sources_filepath="/etc/chrony/sources.d/ubuntu-ntp-pools.sources" |
189 | +ucf_managed_sources="true" |
190 | + |
191 | +db_get chrony/configure_ubuntu_pools_in_sourcesd |
192 | +ucf_managed_sources="${RET}" |
193 | + |
194 | case "$1" in |
195 | purge) |
196 | rm -f /var/lib/chrony/* |
197 | @@ -16,9 +24,15 @@ case "$1" in |
198 | then |
199 | ucf --purge /etc/chrony/chrony.conf |
200 | ucf --purge /etc/chrony/chrony.keys |
201 | + if [ "${ucf_managed_sources}" = "true" ]; then |
202 | + ucf --purge "${ubuntu_ntp_pools_sources_filepath}" |
203 | + fi |
204 | if [ -x "$(command -v ucfr)" ]; then |
205 | ucfr --purge chrony /etc/chrony/chrony.conf |
206 | ucfr --purge chrony /etc/chrony/chrony.keys |
207 | + if [ "${ucf_managed_sources}" = "true" ]; then |
208 | + ucfr --purge chrony "${ubuntu_ntp_pools_sources_filepath}" |
209 | + fi |
210 | fi |
211 | fi |
212 | rm -rf /etc/chrony |
213 | diff --git a/debian/templates b/debian/templates |
214 | new file mode 100644 |
215 | index 0000000..e93ccd8 |
216 | --- /dev/null |
217 | +++ b/debian/templates |
218 | @@ -0,0 +1,12 @@ |
219 | +Template: chrony/configure_ubuntu_pools_in_sourcesd |
220 | +Type: boolean |
221 | +Default: true |
222 | +Description: Configure Ubuntu NTP pools in /etc/chrony/sources.d/ubuntu-ntp-pools.sources |
223 | + The default Ubuntu NTP pools were moved from /etc/chrony/chrony.conf to an |
224 | + included file in /etc/chrony/sources.d/ubuntu-ntp-pools.sources. The main |
225 | + chrony.conf file shipped by the package no longer contains any NTP pools. |
226 | + If you answer "yes", what was described above will be applied. |
227 | + If you answer "no", then /etc/chrony/sources.d/ubuntu-ntp-pools.sources will |
228 | + NOT be created. If it exists already, it will be REMOVED, and subsequent |
229 | + upgrades will not recreate it. |
230 | + If in doubt, it's recommended that you answer "yes". |
231 | diff --git a/debian/ubuntu-ntp-pools.sources b/debian/ubuntu-ntp-pools.sources |
232 | new file mode 100644 |
233 | index 0000000..fbcdec7 |
234 | --- /dev/null |
235 | +++ b/debian/ubuntu-ntp-pools.sources |
236 | @@ -0,0 +1,17 @@ |
237 | +# This will use (up to): |
238 | +# - 4 sources from ntp.ubuntu.com which some are ipv6 enabled |
239 | +# - 2 sources from 2.ubuntu.pool.ntp.org which is ipv6 enabled as well |
240 | +# - 1 source from [01].ubuntu.pool.ntp.org each (ipv4 only atm) |
241 | +# This means by default, up to 6 dual-stack and up to 2 additional IPv4-only |
242 | +# sources will be used. |
243 | +# At the same time it retains some protection against one of the entries being |
244 | +# down (compare to just using one of the lines). See (LP: #1754358) for the |
245 | +# discussion. |
246 | +# |
247 | +# About using servers from the NTP Pool Project in general see (LP: #104525). |
248 | +# Approved by Ubuntu Technical Board on 2011-02-08. |
249 | +# See http://www.pool.ntp.org/join.html for more information. |
250 | +pool ntp.ubuntu.com iburst maxsources 4 |
251 | +pool 0.ubuntu.pool.ntp.org iburst maxsources 1 |
252 | +pool 1.ubuntu.pool.ntp.org iburst maxsources 1 |
253 | +pool 2.ubuntu.pool.ntp.org iburst maxsources 2 |
This is not a blocker for review, but since you are using git, it would help to have the commits split into smaller chunks, and a small description of what is being done. Then you can also transport that into d/changelog with "git-ubuntu. reconstruct pkg/ubuntu/devel". That will put all commits since "pkg/ubuntu/devel" in one d/changelog entry. If the commit message is already in the format of d/changelog, then you don't need to do any other massaging in d/changelog other than version number. See the previous commits in pkg/ubuntu/devel for examples.