Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

overlay: Add 21dhcp-chrony to set default DHCP NTP settings #412

Merged
merged 3 commits into from
Aug 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,21 @@ case "${platform}" in
*) exit 0 ;;
esac

# If not set already (by host customization or this script), set
# PEERNTP=no so that DHCP-provided NTP servers are not added to chrony.
# By doing this we assume the better NTP server choice is the
# platform-provided link-local NTP server rather than others from DHCP.
# TODO: once https://bugzilla.redhat.com/show_bug.cgi?id=1828434 is
# resolved, this won't be required.
if [ ! -e /etc/sysconfig/network ] || ! grep -q "PEERNTP" /etc/sysconfig/network; then
cat <<EOF >> /etc/sysconfig/network
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait but...is this file actually read on FCOS today? That's part of the "initscripts networking" bits...which NetworkManager might read...let me see.
It does look like it reads /etc/sysconfig/network, but rg -i peerntp has zero hits in NM git.

Ahh but I see, we're sourcing that ourselves in the other script. OK.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes - as far as I could tell, only the script added here, and the dhclient handler shipped in chrony, https://src.fedoraproject.org/rpms/chrony/blob/cafc2b7a759c623057666ae578e847c78b6e7811/f/chrony.dhclient#_7, will read that environment variable. If something else is installed and happens to read and parse PEERNTP=no and not propagate the time sources (e.g. dhclient or ntpd), it should be expected behavior that the cloud time source is used by default when on the cloud platform and DHCP sources are not propagated.

# PEERNTP=no is automatically added by default when a platform-provided time
# source is available, but this behavior may be overridden through an Ignition
# config specifying PEERNTP=yes. See https://github.com/coreos/fedora-coreos-config/pull/412.
PEERNTP=no
EOF
fi

if ! cmp {/usr,}/etc/chrony.conf >/dev/null; then
echo "$self: /etc/chrony.conf is modified; not changing the default"
exit 0
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
#!/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.
#
# Carried in Fedora CoreOS and RHEL CoreOS temporarily.
# See: https://bugzilla.redhat.com/show_bug.cgi?id=1800901

export LC_ALL=C

# Exit if support for dhcp4-change landed in the chrony package.
# See upstream thread: https://listengine.tuxfamily.org/chrony.tuxfamily.org/chrony-dev/2020/05/msg00022.html
[ -f /usr/lib/NetworkManager/dispatcher.d/20-chrony-dhcp ] && \
grep -q "dhcp4-change" /usr/lib/NetworkManager/dispatcher.d/20-chrony-dhcp && \
exit 0
[ -f /etc/NetworkManager/dispatcher.d/20-chrony-dhcp ] && \
grep -q "dhcp4-change" /etc/NetworkManager/dispatcher.d/20-chrony-dhcp && \
exit 0

# If a dhclient installation is present, avoid a redundant operation
# with dhclient which handles NTP server config through its own
# NetworkManager dispatcher script 11-dhclient.
[ -e /usr/sbin/dhclient ] && exit 0

interface=$1
action=$2

chrony_helper=/usr/libexec/coreos-chrony-helper
default_server_options=iburst
server_dir=/var/run/coreos-chrony-dhcp

dhcp_server_file=$server_dir/chrony.servers.$interface
# DHCP4_NTP_SERVERS is passed from DHCP options by NetworkManager.
nm_dhcp_servers=$DHCP4_NTP_SERVERS

[ -f /etc/sysconfig/network ] && . /etc/sysconfig/network
[ -f /etc/sysconfig/network-scripts/ifcfg-"${interface}" ] && \
. /etc/sysconfig/network-scripts/ifcfg-"${interface}"

add_servers_from_dhcp() {
rm -f "$dhcp_server_file"

# Don't add NTP servers if PEERNTP=no specified; return early.
[ "$PEERNTP" = "no" ] && return

for server in $nm_dhcp_servers; do
echo "$server ${NTPSERVERARGS:-$default_server_options}" >> "$dhcp_server_file"
done
$chrony_helper update-daemon || :
}

clear_servers_from_dhcp() {
if [ -f "$dhcp_server_file" ]; then
rm -f "$dhcp_server_file"
$chrony_helper update-daemon || :
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
102 changes: 102 additions & 0 deletions overlay.d/21dhcp-chrony/usr/libexec/coreos-chrony-helper
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
#!/bin/bash
#
# This is a copy of /usr/libexec/chrony-helper carried in FCOS/RHCOS
# temporarily while upstream changes and the package update to
# chrony-helper percolate (see
# https://src.fedoraproject.org/rpms/chrony/pull-request/3). It should
# not by used by any script other than the 20-coreos-chrony-dhcp NM
# dispatcher script.
#
# This script configures running chronyd to use NTP servers obtained from
# DHCP and _ntp._udp DNS SRV records. Files with servers from DHCP are managed
# externally (e.g. by a dhclient script). Files with servers from DNS SRV
# records are updated here using the dig utility. The script can also list
# and set static sources in the chronyd configuration file.

chronyc=/usr/bin/chronyc
helper_dir=/var/run/coreos-chrony-helper
server_dir=/var/run/coreos-chrony-dhcp

network_sysconfig_file=/etc/sysconfig/network
dhcp_servers_files="${server_dir}/chrony.servers.*"
added_servers_file=$helper_dir/added_servers

. $network_sysconfig_file &> /dev/null

chrony_command() {
$chronyc -a -n -m "$1"
}

is_running() {
chrony_command "tracking" &> /dev/null
}

get_servers_files() {
[ "$PEERNTP" != "no" ] && echo "$dhcp_servers_files"
}

is_update_needed() {
for file in $(get_servers_files) $added_servers_file; do
[ -e "$file" ] && return 0
done
return 1
}

update_daemon() {
local all_servers_with_args all_servers added_servers

if ! is_running; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, won't this potentially race if NM and chrony start up at the same time?

rm -f $added_servers_file
return 0
fi

all_servers_with_args=$(cat $(get_servers_files) 2> /dev/null)

all_servers=$(
echo "$all_servers_with_args" |
while read -r server serverargs; do
echo "$server"
done | sort -u)
added_servers=$( (
cat $added_servers_file 2> /dev/null
echo "$all_servers_with_args" |
while read -r server serverargs; do
[ -z "$server" ] && continue
chrony_command "add server $server $serverargs" &> /dev/null &&
echo "$server"
done) | sort -u)

comm -23 <(echo -n "$added_servers") <(echo -n "$all_servers") |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could use a comment here, I have never seen comm used before.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was a first for me too :)

while read -r server; do
chrony_command "delete $server" &> /dev/null
done

added_servers=$(comm -12 <(echo -n "$added_servers") <(echo -n "$all_servers"))

if [ -n "$added_servers" ]; then
echo "$added_servers" > $added_servers_file
else
rm -f $added_servers_file
fi
}

prepare_helper_dir() {
mkdir -p $helper_dir
exec 100> $helper_dir/lock
if ! flock -w 20 100; then
echo "Failed to lock $helper_dir" >&2
return 1
fi
}

case "$1" in
update-daemon)
is_update_needed || exit 0
prepare_helper_dir && update_daemon
;;
*)
echo "unhandled command in coreos-chrony-helper"
exit 2
esac

exit $?
19 changes: 19 additions & 0 deletions overlay.d/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,25 @@ This part is separate from 05core to aid RHCOS, which still uses Ignition spec 2

Disables the Red Hat Linux legacy `ifcfg` format.

20platform-chrony
-----------------

Add static chrony configuration for NTP servers provided on platforms
such as `azure`, `aws`, `gcp`. The chrony config for these NTP servers
should override other chrony configuration (e.g. DHCP-provided)
configuration.

21dhcp-chrony
-------------

Handle DHCP-provided NTP servers and configure chrony to use them,
without overwriting platform-specific configuration. Can be removed
once changes in upstream chrony with support for per-platform
defaults (https://bugzilla.redhat.com/show_bug.cgi?id=1828434),
and handling in 20-chrony and chrony-helper using the defaults
lands in downstream packages. See upstream thread:
https://listengine.tuxfamily.org/chrony.tuxfamily.org/chrony-dev/2020/05/msg00022.html

15fcos
------

Expand Down