Re: [chrony-dev] [PATCH] nm-dispatcher: handle NTP servers from DHCP

[Robert Fairley <rfairley@xxxxxxxxxx>]

On Mon, May 25, 2020 at 7:27 AM Miroslav Lichvar <mlichvar@xxxxxxxxxx> wrote:
> I'd rather avoid having any distribution-specific code in the
> examples. If there was a downstream change in one of the directories
> for instance, we would need to support different versions of the
> distribution.
>
> If you would like to submit the chrony-helper script, please remove
> the sysconfig/PEERNTP stuff. It can be put back in a downstream patch.
> The various locations in the script can be set to strings like
> @CHRONYC@, @CHRONYCONF@, @DHCP_SERVERS_FILES@ to indicate they need to
> be edited before use.
>
> Also, please rename the dhclient_servers_files variable to
> dhcp_servers_file and remove the add/remove-dhclient-servers commands.
>
> The NM dispatcher script should follow the same idea, e.g. the check
> of /sbin/dhclient should be in a downstream patch.

Thank you for the feedback and suggestions on the approach, makes sense
to keep the files as general examples without the distribution-specific
handling. I have updated the patch to include chrony.helper.in, following
these points. One part of the above description which I deviated from
while making the edits is the following:

- Use @DHCP_SERVERS_DIR@ instead of @DHCP_SERVERS_FILES@. In the
helper, dhcp_servers_files is a glob expression, but
dhcp_server_file in the NM dispatcher is variable based on the
${interface} argument. To avoid two different macros for these two
variables, and avoid passing in a variable reference "${interface}" as
part of a config string, I made only the directory configurable.
That way, both the helper and NM dispatcher can use one macro
@DHCP_SERVERS_DIR@.

I have updated https://src.fedoraproject.org/rpms/chrony/pull-request/3
with the downstream changes for adding back sysconfig, etc., which is
the `examples-use-sysconfig-detect-dhclient.patch` file. The other two
`.patch` files in that PR add in the upstream patches proposed in this
email.

So far, I have tested these changes by manually applying the files,
with the downstream patch for Fedora, in Fedora CoreOS (the F32-based
"next" stream) with and without dhclient present on the system, with
a DHCP server on the same network using the `ntp-servers` option. In
both cases NTP server config files are written to `/var/lib/dhclient`
or `/var/lib/chrony/servers`, and the NTP servers from the DHCP server
are listed with `chronyc sources`.