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

[Robert Fairley <rfairley@xxxxxxxxxx>]

On Thu, Jun 18, 2020 at 6:31 AM Miroslav Lichvar <mlichvar@xxxxxxxxxx> wrote:
>
> I think it could be a fragment now, and probably everything else
> except the default servers. I'll see if I can add an example of
> a fragmented configuration, which downstreams could use.
>

That'd help a lot! Would then rebase
https://src.fedoraproject.org/rpms/chrony/pull-request/3
on that.

> The patches look good, except the commit message still mentions the
> helper. Can you please update that?
>

Sorry I had missed that, had not checked the commit messages again
after removing the helper commit. Fixed.

> > +++ b/examples/chrony.nm-dispatcher.dhcp.in
> > @@ -0,0 +1,44 @@
> > +#!/bin/sh
> > +# This is a NetworkManager dispatcher script for chronyd to update
> > +# its NTP sources passed from DHCP options. Note that this script is
> > +# specific to NetworkManager-dispatcher due to use of the
> > +# DHCP4_NTP_SERVERS environment variable. For networkd-dispatcher,
> > +# an alternative approach is external means such as a dhclient hook.
>
> Maybe don't mention networkd-dispatcher here at all? If someone adds
> support to this script, or adds a new script, it will be mentioned
> there.
>

Makes sense, updated!

Thanks,

Robert

From bec10018a7d4468fbfc531438ae312e12debbf23 Mon Sep 17 00:00:00 2001
From: Robert Fairley <rfairley@xxxxxxxxxx>
Date: Thu, 4 Jun 2020 14:48:35 -0400
Subject: [PATCH 1/2] examples: add dispatcher for NTP servers from DHCP

Add new NM dispatcher script for NTP servers given by DHCP through
NetworkManager in a similar way to how distributions have done in
11-dhclient, e.g. [1]. New NTP servers are written as entries to a
file per-interface in CHRONY_SERVER_DIR, which is re-read by
chronyd upon executing `chronyc reload sources`.

This provides a way for NTP server configuration to be carried over
from NetworkManager DHCP events to chrony, for DHCP clients other
than dhclient. Part of fixing integration where the NetworkManager
internal client is used, e.g [2].

Paths and options are made configurable through macros that can be
substituted at install time with distribution-specific values.

[1] https://src.fedoraproject.org/rpms/dhcp/blob/master/f/11-dhclient
[2] https://bugzilla.redhat.com/show_bug.cgi?id=1800901
---
 examples/chrony.nm-dispatcher.dhcp.in | 43 +++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 examples/chrony.nm-dispatcher.dhcp.in

diff --git a/examples/chrony.nm-dispatcher.dhcp.in b/examples/chrony.nm-dispatcher.dhcp.in
new file mode 100644
index 0000000..bbd9430
--- /dev/null
+++ b/examples/chrony.nm-dispatcher.dhcp.in
@@ -0,0 +1,43 @@
+#!/bin/sh
+# This is a NetworkManager dispatcher script for chronyd to update
+# its NTP sources passed from DHCP options. Note that this script is
+# specific to NetworkManager-dispatcher due to use of the
+# DHCP4_NTP_SERVERS environment variable.
+
+export LC_ALL=C
+
+interface=$1
+action=$2
+
+chronyc=@CHRONYC@
+default_server_options=@CHRONY_DEFAULT_SERVER_OPTIONS@
+server_dir=@CHRONY_SERVER_DIR@
+
+dhcp_server_file=$server_dir/$interface.sources
+# DHCP4_NTP_SERVERS is passed from DHCP options by NetworkManager.
+nm_dhcp_servers=$DHCP4_NTP_SERVERS
+
+add_servers_from_dhcp() {
+    rm -f "$dhcp_server_file"
+    for server in $nm_dhcp_servers; do
+        echo "server $server $default_server_options" >> "$dhcp_server_file"
+    done
+    $chronyc reload sources > /dev/null 2>&1 || :
+}
+
+clear_servers_from_dhcp() {
+    if [ -f "$dhcp_server_file" ]; then
+        rm -f "$dhcp_server_file"
+        $chronyc reload sources > /dev/null 2>&1 || :
+    fi
+}
+
+mkdir -p $server_dir
+
+if [ "$action" = "up" ] || [ "$action" = "dhcp4-change" ]; then
+    add_servers_from_dhcp
+elif [ "$action" = "down" ]; then
+    clear_servers_from_dhcp
+fi
+
+exit 0
-- 
2.26.2

From 1d1abaa07c96249fd15a3a2676d362acbbf7916b Mon Sep 17 00:00:00 2001
From: Robert Fairley <rfairley@xxxxxxxxxx>
Date: Thu, 4 Jun 2020 14:48:40 -0400
Subject: [PATCH 2/2] examples/nm-dispatcher: make chronyc configurable

Similar to the DHCP dispatcher, add the @CHRONYC@ macro to make the
chronyc command path configurable at install time.

Add the suffix `.in` to the filename to indicate this must be
substituted.

Also give an `.onoffline` suffix to more clearly differentiate
this script from `chrony.nm-dispatcher.dhcp.in`.
---
 ...chrony.nm-dispatcher => chrony.nm-dispatcher.onoffline.in} | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
 rename examples/{chrony.nm-dispatcher => chrony.nm-dispatcher.onoffline.in} (87%)

diff --git a/examples/chrony.nm-dispatcher b/examples/chrony.nm-dispatcher.onoffline.in
similarity index 87%
rename from examples/chrony.nm-dispatcher
rename to examples/chrony.nm-dispatcher.onoffline.in
index 0b0c3e7..7c0e2dd 100644
--- a/examples/chrony.nm-dispatcher
+++ b/examples/chrony.nm-dispatcher.onoffline.in
@@ -5,11 +5,13 @@
 
 export LC_ALL=C
 
+chronyc=@CHRONYC@
+
 # For NetworkManager consider only up/down events
 [ $# -ge 2 ] && [ "$2" != "up" ] && [ "$2" != "down" ] && exit 0
 
 # Note: for networkd-dispatcher routable.d ~= on and off.d ~= off
 
-chronyc onoffline > /dev/null 2>&1
+$chronyc onoffline > /dev/null 2>&1
 
 exit 0
-- 
2.26.2

diff --git a/examples/chrony.nm-dispatcher.dhcp.in b/examples/chrony.nm-dispatcher.dhcp.in
index d1f34eb..bbd9430 100644
--- a/examples/chrony.nm-dispatcher.dhcp.in
+++ b/examples/chrony.nm-dispatcher.dhcp.in
@@ -2,8 +2,7 @@
 # This is a NetworkManager dispatcher script for chronyd to update
 # its NTP sources passed from DHCP options. Note that this script is
 # specific to NetworkManager-dispatcher due to use of the
-# DHCP4_NTP_SERVERS environment variable. For networkd-dispatcher,
-# an alternative approach is external means such as a dhclient hook.
+# DHCP4_NTP_SERVERS environment variable.
 
 export LC_ALL=C