From 1e06e57d7ae02cf48c9f045c4a53edf031d2bbf1 Mon Sep 17 00:00:00 2001 From: Patrik Lundin Date: Fri, 18 Oct 2024 14:07:35 +0200 Subject: [PATCH 1/3] Replace ntp with chrony on our servers This affects Ubuntu 23.04 and up as well as Debian 12 and up. It is unfortunate that we need to modify sunet::ntp as well, but the problem is that ops-repos sometimes pull in sunet::ntp by hand (sometimes to set specific configuration in cosmos-rules etc) so in those cases it is hard to do a clean cutover between them. For that reason sunet::ntp now does nothing on hosts matching the same criteria as the ones where sunet::server applies sunet::chrony. This way both can be applied to a host at the same time. --- manifests/chrony.pp | 22 ++++++++++ manifests/ntp.pp | 101 +++++++++++++++++++++++--------------------- manifests/server.pp | 11 ++++- 3 files changed, 86 insertions(+), 48 deletions(-) diff --git a/manifests/chrony.pp b/manifests/chrony.pp index cd203737d..5f30be543 100644 --- a/manifests/chrony.pp +++ b/manifests/chrony.pp @@ -22,6 +22,28 @@ Array[String] $ntsservercerts = [], Array[String] $ntsserverkeys = [], ) { + + ### BEGIN sunet::ntp cleanup + # Cleanup potential remains from previous usage of sunet::ntp + package { 'ntp': ensure => 'purged' } + package { 'ntpsec': ensure => 'purged' } + + # Some files that are left behind even after purge because they are created + # manually by sunet::ntp on Ubuntu. + if $facts['os']['distro']['id'] == 'Ubuntu' { + include sunet::systemd_reload + + file { '/etc/systemd/system/multi-user.target.wants/ntp.service': + ensure => 'absent', + notify => [Class['sunet::systemd_reload']] + } + file { '/etc/systemd/system/ntp.service': + ensure => 'absent', + notify => [Class['sunet::systemd_reload']] + } + } + ### END sunet::ntp cleanup + package { 'chrony': ensure => 'installed' } service { 'chrony': diff --git a/manifests/ntp.pp b/manifests/ntp.pp index 509ab4370..3be7ae823 100644 --- a/manifests/ntp.pp +++ b/manifests/ntp.pp @@ -9,62 +9,69 @@ $distro = $facts['os']['distro']['id'] $release = $facts['os']['distro']['release']['full'] - # Help Puppet understand to use systemd for Ubuntu 16.04 hosts - if $distro == 'Ubuntu' and versioncmp($release, '15.04') >= 0 { - Service { - provider => 'systemd', + # Only do anything on older Ubuntu and Debian. For later versions + # sunet::server will use sunet::chrony instead. This if-statement is needed + # because not every ops-repo uses sunet::server to select what ntp class to + # use. If changing this if-statement you might need to update it in + # sunet::server as well. + if ($distro == 'Ubuntu' and versioncmp($release, '22.04') <= 0) or ($distro == 'Debian' and versioncmp($release, '11') <= 0) { + # Help Puppet understand to use systemd for Ubuntu 16.04 hosts + if $distro == 'Ubuntu' and versioncmp($release, '15.04') >= 0 { + Service { + provider => 'systemd', + } } - } - package { 'ntp': ensure => 'installed' } - service { 'ntp': - ensure => running, - name => 'ntp', - enable => true, - hasrestart => true, - require => Package['ntp'], - } + package { 'ntp': ensure => 'installed' } + service { 'ntp': + ensure => running, + name => 'ntp', + enable => true, + hasrestart => true, + require => Package['ntp'], + } - # Don't use pool.ntp.org servers, but rather DHCP provided NTP servers - $_disable_pool = $disable_pool_ntp_org ? { - true => ['rm pool[.]'], - false => [], - } + # Don't use pool.ntp.org servers, but rather DHCP provided NTP servers + $_disable_pool = $disable_pool_ntp_org ? { + true => ['rm pool[.]'], + false => [], + } - # in cases where DHCP does not provide servers, or the machinery doesn't - # work well (Ubuntu 16.04, looking at you), add some servers manually - $_set_servers = map(flatten([$set_servers, $add_servers])) |$index, $server| { - sprintf('set server[%s] %s', $index + 1, $server) - } - $changes = flatten([$_disable_pool, - $_set_servers ? { - [] => [], - default => ['rm server[.]', - $_set_servers], - },]) + # in cases where DHCP does not provide servers, or the machinery doesn't + # work well (Ubuntu 16.04, looking at you), add some servers manually + $_set_servers = map(flatten([$set_servers, $add_servers])) |$index, $server| { + sprintf('set server[%s] %s', $index + 1, $server) + } + $changes = flatten([$_disable_pool, + $_set_servers ? { + [] => [], + default => ['rm server[.]', + $_set_servers], + },]) - if $changes != [] { - include augeas + if $changes != [] { + include augeas - augeas { 'ntp.conf': - context => '/files/etc/ntp.conf', - changes => $changes, - require => Package['ntp'], - notify => Service['ntp'], + augeas { 'ntp.conf': + context => '/files/etc/ntp.conf', + changes => $changes, + require => Package['ntp'], + notify => Service['ntp'], + } } - } - if $distro == 'Ubuntu' and versioncmp($release, '15.04') >= 0 { - include sunet::systemd_reload + if $distro == 'Ubuntu' and versioncmp($release, '15.04') >= 0 { + include sunet::systemd_reload - # replace init.d script with systemd service file to get Restart=always - file { - '/etc/systemd/system/ntp.service': - content => template('sunet/ntp/ntp.service.erb'), - notify => [Class['sunet::systemd_reload'], - Service['ntp'], - ], - ; + # replace init.d script with systemd service file to get Restart=always + file { + '/etc/systemd/system/ntp.service': + content => template('sunet/ntp/ntp.service.erb'), + notify => [Class['sunet::systemd_reload'], + Service['ntp'], + ], + ; + } } } } diff --git a/manifests/server.pp b/manifests/server.pp index 7f575dacd..94df90803 100644 --- a/manifests/server.pp +++ b/manifests/server.pp @@ -44,8 +44,17 @@ } } + $distro = $facts['os']['distro']['id'] + $release = $facts['os']['distro']['release']['full'] + if $ntpd_config { - include sunet::ntp + # If changing this if-statement you might need to update it in sunet::ntp + # as well + if ($distro == 'Ubuntu' and versioncmp($release, '22.04') <= 0) or ($distro == 'Debian' and versioncmp($release, '11') <= 0) { + include sunet::ntp + } else { + include sunet::chrony + } } if $scriptherder { From 8274809fad933716e8d72454f1fba9a611b5cb7f Mon Sep 17 00:00:00 2001 From: Patrik Lundin Date: Mon, 21 Oct 2024 09:04:43 +0200 Subject: [PATCH 2/3] Make version comparisions compatible with Debian While 'full' on ubuntu is never more specific than e.g. '24.04', on debian it also includes a minor component, e.g. '12.7'. The 'major' version is identical to 'full' on ubuntu while it is e.g. '12' on debian which in general is what we are looking for in comparisions. --- manifests/ntp.pp | 2 +- manifests/server.pp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/manifests/ntp.pp b/manifests/ntp.pp index 3be7ae823..cc4774642 100644 --- a/manifests/ntp.pp +++ b/manifests/ntp.pp @@ -7,7 +7,7 @@ # Get facts for distro/release $distro = $facts['os']['distro']['id'] - $release = $facts['os']['distro']['release']['full'] + $release = $facts['os']['distro']['release']['major'] # Only do anything on older Ubuntu and Debian. For later versions # sunet::server will use sunet::chrony instead. This if-statement is needed diff --git a/manifests/server.pp b/manifests/server.pp index 94df90803..7de8da189 100644 --- a/manifests/server.pp +++ b/manifests/server.pp @@ -45,7 +45,7 @@ } $distro = $facts['os']['distro']['id'] - $release = $facts['os']['distro']['release']['full'] + $release = $facts['os']['distro']['release']['major'] if $ntpd_config { # If changing this if-statement you might need to update it in sunet::ntp From ba3ea5467fd70347399b9d2d32f39e4a107a7e42 Mon Sep 17 00:00:00 2001 From: Patrik Lundin Date: Tue, 22 Oct 2024 09:46:46 +0200 Subject: [PATCH 3/3] Start using fact for enabling chrony No need to keep if-statements in sync across manifests this way. Script basically a copy of our existing nftables fact. Idea from @mickenordin --- facts.d/chrony_enabled.sh | 14 ++++++++++++++ manifests/ntp.pp | 5 ++--- manifests/server.pp | 11 +++-------- 3 files changed, 19 insertions(+), 11 deletions(-) create mode 100755 facts.d/chrony_enabled.sh diff --git a/facts.d/chrony_enabled.sh b/facts.d/chrony_enabled.sh new file mode 100755 index 000000000..a2f7b3ca8 --- /dev/null +++ b/facts.d/chrony_enabled.sh @@ -0,0 +1,14 @@ +#!/bin/sh + +enabled="no" + +if [ -f /etc/sunet-chrony-opt-in ]; then + enabled="yes" +fi + +vendor=$(lsb_release -is) +version=$(lsb_release -rs) +test "$vendor" = "Debian" && dpkg --compare-versions "${version}" "ge" "12" && enabled="yes" +test "$vendor" = "Ubuntu" && dpkg --compare-versions "${version}" "ge" "23.04" && enabled="yes" + +echo "sunet_chrony_enabled=${enabled}" diff --git a/manifests/ntp.pp b/manifests/ntp.pp index cc4774642..dff17d3a1 100644 --- a/manifests/ntp.pp +++ b/manifests/ntp.pp @@ -12,9 +12,8 @@ # Only do anything on older Ubuntu and Debian. For later versions # sunet::server will use sunet::chrony instead. This if-statement is needed # because not every ops-repo uses sunet::server to select what ntp class to - # use. If changing this if-statement you might need to update it in - # sunet::server as well. - if ($distro == 'Ubuntu' and versioncmp($release, '22.04') <= 0) or ($distro == 'Debian' and versioncmp($release, '11') <= 0) { + # use. + if $::facts['sunet_chrony_enabled'] != 'yes' { # Help Puppet understand to use systemd for Ubuntu 16.04 hosts if $distro == 'Ubuntu' and versioncmp($release, '15.04') >= 0 { Service { diff --git a/manifests/server.pp b/manifests/server.pp index 7de8da189..122d2412a 100644 --- a/manifests/server.pp +++ b/manifests/server.pp @@ -44,16 +44,11 @@ } } - $distro = $facts['os']['distro']['id'] - $release = $facts['os']['distro']['release']['major'] - if $ntpd_config { - # If changing this if-statement you might need to update it in sunet::ntp - # as well - if ($distro == 'Ubuntu' and versioncmp($release, '22.04') <= 0) or ($distro == 'Debian' and versioncmp($release, '11') <= 0) { - include sunet::ntp - } else { + if $::facts['sunet_chrony_enabled'] == 'yes' { include sunet::chrony + } else { + include sunet::ntp } }