On Tue, Aug 04, 2020 at 07:05:45PM +0200, Miroslav Lichvar wrote:
On Tue, Aug 04, 2020 at 04:32:35PM +0000, Zbigniew Jędrzejewski-Szmek
wrote:
> It's nice to not require any files in /etc (so for example the admin
> can do 'rm -rf /etc/*' to restore vendor defaults). So instead of
> using /etc/chrony.conf to load other files, please consider just
> building this logic directly into the code. There is also no need to
> make those config paths configurable.
That's possible (the paths could be hardcoded in the systemd unit
file), but is it a good idea to force the users to use the new system?
If someone has a modified config, or is using one of the many ansible
roles for configuring NTP for example, why should it stop working? We
don't have to break configuration tools that just generate a config
from scratch.
Backwards compatiblity should be kept obviously. In systemd we look at
{/usr,/usr/local,/etc,/run}/foo.conf and
{/usr,/usr/local,/etc,/run}/foo.conf.d/*.conf, so foo.conf is the main
config file, while the "drop-ins" in /etc/ have higher priority and
server as overrides. This is backwards compatible with having just
/etc/foo.conf.
> > My concern is that it will basically break all existing
tools that
> > need to check and/or modify the configuration (e.g. anaconda). They
> > will need to know the naming of the files which have specific settings
> > in order to override them, or implement a parser duplicating the
> > chronyd logic to figure out which files are loaded from where.
>
> The whole goal of the config-in-dropin-files-logic is that anaconda
> wouldn't parse existing config, it would just write
> /etc/chrony.d/50-anaconda.conf that would override only the parts it
> cares about.
The trouble is that a fragment having a different name cannot disable
servers specified in a different fragment. If anaconda wanted to
override the default servers, it would need to know the name of the
fragment. In some cases users/vendors/products may want to specify
additional servers, sometimes replace them.
This can be handled by having a directive that means "wipe previous
config for this setting". In systemd we handle this by treating an
empty assignment as special:
SystemCallFilter=open
SystemCallFilter=
SystemCallFilter=write
SystemCallFilter=read close
is equivalent to SystemCallFilter=write read close.
> Also, please don't invent a new logic — just follow the same
one that
> systemd does [1]. This has the advantage that if something *needs* to
> look all of config, it can reuse what an existing loader. Also, admins
> don't need to learn a new set of rules. Ideally,
> 'systemd-analyze cat-config chrony.conf' would just work.
I was following the liboverdrop logic, which probably adopted the
systemd convention. For printing the whole configuration there is now
a "chronyd -p" command. The systemd cat-config command seems to expect
the ini-style syntax. That's not going to work here.
Zbyszek