[Bug 2048876] Re: Allow server and pool sources to be overridden through a conf.d or sources.d configuration
Hi Bryce, Apologies for the delay in responding. Thank you for your review comments I'll try to address them in my next fix proposal. With respect to the suggesstion you shared about handling well known cases I'll give it some thought myself , discuss it within my squad, gather feedback and report here as appropriate. You are right in that cloud-init might also need changes to work in a compatible manner with this bug's fix. We anticipated this and plan to drive any required cloud-init changes with @catred's help. I plan to resume working on this bug next week. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2048876 Title: Allow server and pool sources to be overridden through a conf.d or sources.d configuration To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/chrony/+bug/2048876/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 2048876] Re: Allow server and pool sources to be overridden through a conf.d or sources.d configuration
Hi Andrew and Ankush, just wanted to check in about status of your plans for this bug? -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2048876 Title: Allow server and pool sources to be overridden through a conf.d or sources.d configuration To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/chrony/+bug/2048876/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 2048876] Re: Allow server and pool sources to be overridden through a conf.d or sources.d configuration
It's going to have to land in Ubuntu directly regardless of what's done. The package already has delta so even if this is done in Debian it'd have to get patched in for Ubuntu. This fix probably needs to be thought of in several discrete chunks. The moving of the config snippet out of chrony.conf to a file in sources.d/ is one component; this one is what I was referring to in that the sources.d/ directory is already provided for in the Debian directory, and our config snippet isn't upstreamable to them, so at least this particular component is not something they'd be interested in. Another component here is the addition of debconf questions, which came up in discussion with my teammates today. The essence of the prompt desiring to be solved here, as I understand it, is to enable configuration customization to work more seamlessly and in particular to reduce interactive prompting. Adding interactive debconf questions feels not in that spirit, maybe just shifting the prompting from one use case to others or moving it from upgrade time to installation time. If that's true, wouldn't it be better to look for ways to avoid prompts at all, at least for use cases we understand well? Robie shared one idea, which I hope he won't mind me quoting verbatim: > I would add a debconf + ucf mechanism to maintain a file in there called > /etc/chrony/sources.d/from-debconf. By debconf, this could either be disabled > (don't attempt to create the file), Ubuntu's default (make it the same as our > defaults), or cloud-specific (use some cloud-specific file). Then ucf would > maintain the same file but with different contents depending on debconf > configuration, defaulting to Ubuntu's default. > If a user customises the file using debconf, then great that'll work. If the > user customises the file by hand, or deletes it, then ucf would do the right > thing. > Especially as ucf is used already, this shouldn't be burdensome. > In this case, the debconf would be a multi-select against things like "do > nothing", "ubuntu", "gce", "aws region a defaults" or whatever, and there > would only need to be a single setting. > However, we'd need to figure out how that would interact with what cloud-init > might be doing already. And separately, if cloud-init does support > customisation, then surely clouds should be using that via vendordata rather > than doing something custom at image generation time? > Here's cloud-init's implementation documentation: > https://cloudinit.readthedocs.io/en/latest/reference/modules.html#ntp > Looks like that has chrony support, too, and it expects to overwrite > chrony.conf. Unfortunately, we don't know of an existing package following this ucf + debconf style; most rely on straight ucf only. Also, it sounds like if cloud-init relies on modifying chrony.conf for it's settings, then your fix for this bug will also break them. So for example cloud-init's ntp module configuration would need to be considered in this. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2048876 Title: Allow server and pool sources to be overridden through a conf.d or sources.d configuration To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/chrony/+bug/2048876/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 2048876] Re: Allow server and pool sources to be overridden through a conf.d or sources.d configuration
Many thanks for that feedback and guidance. In parallel with working on the suggested changes, I wanted to understand the right approach we should take to fixing this bug. From your comment in #1 "It can't hurt to discuss with Debian but it's highly doubtful they'll take our delta", would it be reasonable to assume that, as Ubuntu has already branched the chrony config associated with NTP pools away from Debian, we should attempt to land this bug fix to the NTP pools config directly into Ubuntu? Thanks again for your help, Andy. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2048876 Title: Allow server and pool sources to be overridden through a conf.d or sources.d configuration To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/chrony/+bug/2048876/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 2048876] Re: Allow server and pool sources to be overridden through a conf.d or sources.d configuration
Some review comments:
0. I'm ok if you prefer to handle this as debdiff, but the Server team
traditionally uses git-ubuntu for MP reviews. If you're familiar with
git you'll likely also find it more comfortable, however I understand if
due to the time pressure you'd rather do it an already known way. But
put git-ubuntu on your todo list to master, if it's not already.
1. As I'm studying more closely I see that the config for the NTP pool
servers is part of the package delta that Christian added, so that is
going to be less relevant to Debian, and also I see that Debian provides
the d/sources.d/ directory that you're using already, so while I did
initially think like the others that this should be done in Debian, I'm
seeing now it's more relevant to us only. It can't hurt to discuss with
Debian but it's highly doubtful they'll take our delta. Also, since
they already provide the d/sources.d/ directory I'm less worried about
divergence with them.
2. In the postinst, if you reverse the order of the if clauses you can
drop the additional filecheck, i.e.:
+if [ ! -f /etc/chrony/sources.d/default-ntp-pools.sources ];
+then
+db_input low chrony/configure_default_pools_in_sourcesd || true
+db_go
+db_get chrony/configure_default_pools_in_sourcesd
+if [ "${RET}" = "true" ];
+then
+cp --preserve /usr/share/chrony/default-ntp-pools.sources
/etc/chrony/sources.d/default-ntp-pools.sources
+fi
+elif ! cmp -s /usr/share/chrony/default-ntp-pools.sources
/etc/chrony/sources.d/default-ntp-pools.sources;
+then
+db_input low chrony/preserve_user_configured_pools_in_sourcesd ||
true
+db_go
+db_get chrony/preserve_user_configured_pools_in_sourcesd
+if [ "${RET}" = "false" ];
+then
+cp --preserve /usr/share/chrony/default-ntp-pools.sources
/etc/chrony/sources.d/default-ntp-pools.sources
+fi
+fi
I also think that presents your change a bit clearer since on initial
install the file doesn't exist in sources.d and will need copied in
place, so that first stanza is what happens first.
3. "The maintainer's version and current version of
/etc/chrony/default-ntp-pools.sources are different."
I'm not certain, but this verbage seems a bit off. Can you doublecheck
how other packages doing this same operation word the statement? (For
example, as this is addressed to an end user, I'm not sure they would
understand "current version" means the version currently installed on
their system, and might not know what we mean by 'maintainer'.) If
other packages are using this same wording style it's fine, but if
there's a more conventional phrasing I'd go with something closer to
that.
4. You might check whether the postrm should also do `rm -rf
/etc/chrony/sources.d/` when purging.
5. Typically something like this should also be accompanied with some
documentation regarding the change, such as in README.*,
sources.d/README, and/or NEWS.
6. Does chrony-helper require any alterations?
7. There are references to `man chrony.conf(5)` in the service file.
Probably worth checking the man page to make sure this change doesn't
make any of its text outdated, and if it does include a patch to adjust
it. May potentially be something to include in the Test Case.
8. Why is --debconf-ok added for these two lines?
+ucf --debconf-ok --three-way /usr/share/chrony/chrony.conf
/etc/chrony/chrony.conf
+ucf --debconf-ok --three-way /usr/share/chrony/chrony.keys
/etc/chrony/chrony.keys
9. The changelog entry could probably benefit from some wordsmithing.
But I'll leave that to the next iteration.
--
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/2048876
Title:
Allow server and pool sources to be overridden through a conf.d or
sources.d configuration
To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/chrony/+bug/2048876/+subscriptions
--
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 2048876] Re: Allow server and pool sources to be overridden through a conf.d or sources.d configuration
** Tags added: server-todo ** Changed in: chrony (Ubuntu) Assignee: (unassigned) => Ankush Pathak (ankushpathak) ** Changed in: chrony (Ubuntu) Importance: Undecided => High ** Also affects: chrony (Ubuntu Mantic) Importance: Undecided Status: New ** Also affects: chrony (Ubuntu Focal) Importance: Undecided Status: New ** Also affects: chrony (Ubuntu Bionic) Importance: Undecided Status: New ** Also affects: chrony (Ubuntu Jammy) Importance: Undecided Status: New ** Also affects: chrony (Ubuntu Noble) Importance: High Assignee: Ankush Pathak (ankushpathak) Status: Triaged ** Description changed: - Currently, the default chrony.conf configures a set of pools. Confirmed this on a focal and jammy instance on GCP. If one wishes to use only a specific server/server pool or not use a server at all they will need to modify /etc/chrony/chrony.conf. This will possibly lead to a prompt during an Ubuntu release upgrade and during an unattended chrony security upgrade. + [Impact] + * An explanation of the effects of the bug on users and + justification for backporting the fix to the stable release. + + * In addition, it is helpful, but not required, to include an + explanation of how the upload fixes this bug. + + [Workaround] + * If available, steps users can take to avoid the issue while waiting + for a fix. Emphasize whether the workaround sometimes or always + works, and any side effects or other caveats that may exist. + + [Test Case] + * Detailed instructions how to reproduce the bug + + * These should allow someone who is not familiar with the affected + package to reproduce the bug and verify that the updated package fixes + the problem. + + [Where Problems Could Occur] + * Think about what the upload changes in the software. Imagine the change is + wrong or breaks something else: how would this show up? + + * It is assumed that any SRU candidate patch is well-tested before + upload and has a low overall risk of regression, but it's important + to make the effort to think about what ''could'' happen in the + event of a regression. + + * This must '''never''' be "None" or "Low", or entirely an argument as to why + your upload is low risk. + + * This both shows the SRU team that the risks have been considered, + and provides guidance to testers in regression-testing the SRU. + + [Other Info] + + * Anything else you think is useful to include + * Anticipate questions from users, SRU, +1 maintenance, security teams and the Technical Board + and address these questions in advance + + [Original Report] + + Currently, the default chrony.conf configures a set of pools. Confirmed this on a focal and jammy instance on GCP. If one wishes to use only a specific server/server pool or not use a server at all they will need to modify /etc/chrony/chrony.conf. This will possibly lead to a prompt during an Ubuntu release upgrade and during an unattended chrony security upgrade. We are trying to move all configuration changes to their respective *.d directories. See: https://bugs.launchpad.net/livecd-rootfs/+bug/1968873 We test for modified chrony config file by invoking `sudo md5sum --quiet --check /var/lib/ucf/hashfile`. - Listing the cases that I know where we are not able to move chrony configuration changes to a *.d config 1. Azure: Azure needs all default pool entries in chrony.conf disabled. This is currently done by commenting out the pool entries in /etc/chrony/chrony.conf. There doesn't seem to be an alternative way to reset the pool set used by chrony through a configuration in *.d directory. 2. Google: GCP images need to set a single server source entry. This is done indirectly through the ntp cloud-init module configuration. The ntp module replaces the default /etc/chrony/chrony.conf with another file that has required server entry and no pool entries. I believe this cannot be done through an override in *.d directory without touching /etc/chrony/chrony.conf. This request perhaps can be extended to ensure that "negating" a configuration in the default /etc/chrony/chrony.conf should be possible through a configuration in /etc/chrony/*.d directory. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2048876 Title: Allow server and pool sources to be overridden through a conf.d or sources.d configuration To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/chrony/+bug/2048876/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
[Bug 2048876] Re: Allow server and pool sources to be overridden through a conf.d or sources.d configuration
Hi Ankush, thanks for reworking the patch, what feedback has Debian provided so far? Can you share the Debian bug or MR where you're discussing this? It's rather late in the noble cycle to be introducing a change of this scope; you're definitely going to need to put in a FFe. Since FF is nearly upon us, I would suggest treating the FFe as SRU (including providing the filled in SRU template and step-by-step testing requirements). -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/2048876 Title: Allow server and pool sources to be overridden through a conf.d or sources.d configuration To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/chrony/+bug/2048876/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
