From 2545d7fd438338263d0cc52eb8e7433fb61afee7 Mon Sep 17 00:00:00 2001 From: Takuya ASADA Date: Thu, 8 Apr 2021 21:40:19 +0900 Subject: [PATCH 1/2] scylla_util.py: return bool value on systemd_unit.is_active() Currently, 'if unit.is_active():' is always True since is_active() returns result in string (active, inactive, unknown). To avoid such scripting bug, change return value in bool. --- dist/common/scripts/scylla_util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dist/common/scripts/scylla_util.py b/dist/common/scripts/scylla_util.py index da4b752ea4..05be6bf905 100644 --- a/dist/common/scripts/scylla_util.py +++ b/dist/common/scripts/scylla_util.py @@ -802,7 +802,7 @@ class systemd_unit: return run('systemctl {} disable {}'.format(self.ctlparam, self._unit), shell=True, check=True) def is_active(self): - return run('systemctl {} is-active {}'.format(self.ctlparam, self._unit), shell=True, capture_output=True, encoding='utf-8').stdout.strip() + return True if run('systemctl {} is-active {}'.format(self.ctlparam, self._unit), shell=True, capture_output=True, encoding='utf-8').stdout.strip() == 'active' else False def mask(self): return run('systemctl {} mask {}'.format(self.ctlparam, self._unit), shell=True, check=True) From 735c83b27fcc613a31b7c7011bd02e86b71e737a Mon Sep 17 00:00:00 2001 From: Takuya ASADA Date: Wed, 7 Apr 2021 09:35:40 +0900 Subject: [PATCH 2/2] scylla_ntp_setup: detect already installed ntp client On current implementation, we may re-run ntp configuration even it already configured. Also, the system may configured with non-default ntp client, we just ignoring that and configure with default ntp client. This patch minimize unnecessary re-configuration of ntp client. It run in following order: 1. Check NTP client is already running. If it running, skip setup 2. Check NTP client is alrady installed. If it installed, use it 3. If there is non of NTP client package installed, - if it's CentOS, install chrony - if it's on other distributions, install systemd-timesyncd Related with #8344, #8339 --- dist/common/scripts/scylla_ntp_setup | 94 +++++++++++++++++++++++----- 1 file changed, 79 insertions(+), 15 deletions(-) diff --git a/dist/common/scripts/scylla_ntp_setup b/dist/common/scripts/scylla_ntp_setup index 155c5d96e3..62cc746103 100755 --- a/dist/common/scripts/scylla_ntp_setup +++ b/dist/common/scripts/scylla_ntp_setup @@ -29,6 +29,30 @@ import shutil from scylla_util import * from subprocess import run +from pathlib import Path + +def get_chrony_unit(): + try: + unit = systemd_unit('chronyd') + except: + unit = systemd_unit('chrony') + return unit + +def get_chrony_conf(): + p = Path('/etc/chrony/chrony.conf') + if p.exists(): + return p + p = Path('/etc/chrony.conf') + if p.exists(): + return p + raise FileNotFoundError('chrony.conf not found') + +def get_ntp_unit(): + try: + unit = systemd_unit('ntpd') + except: + unit = systemd_unit('ntp') + return unit if __name__ == '__main__': if os.getuid() > 0: @@ -40,29 +64,69 @@ if __name__ == '__main__': help='specify subdomain of pool.ntp.org (ex: centos, fedora or amazon)') args = parser.parse_args() - # use systemd-timesyncd - if systemd_unit.available('systemd-timesyncd'): + target = None + if os.path.exists('/lib/systemd/systemd-timesyncd'): + if systemd_unit('systemd-timesyncd').is_active(): + print('ntp is already configured, skip setup') + sys.exit(0) + target = 'systemd-timesyncd' + if shutil.which('chronyd'): + if get_chrony_unit().is_active(): + print('ntp is already configured, skip setup') + sys.exit(0) + if not target: + target = 'chrony' + if shutil.which('ntpd'): + if get_ntp_unit().is_active(): + print('ntp is already configured, skip setup') + sys.exit(0) + if not target: + target = 'ntp' + if not target: + if is_redhat_variant(): + target = 'chrony' + else: + target = 'systemd-timesyncd' + + print(f'Use {target} to configure ntp') + if target == 'systemd-timesyncd': + if not os.path.exists('/lib/systemd/systemd-timesyncd'): + # On some distribution, systemd-timesyncd is part of systemd package, + # but we can ignore it since systemd must be installed by default. + pkg_install('systemd-timesyncd') run('timedatectl set-ntp true', shell=True, check=True) - # use chrony - else: - if shutil.which('ntpd'): - pkg_uninstall('ntp') + elif target == 'chrony': if not shutil.which('chronyd'): pkg_install('chrony') - if is_debian_variant(): - confpath = '/etc/chrony/chrony.conf' - chronyd = 'chrony.service' - else: - confpath = '/etc/chrony.conf' - chronyd = 'chronyd.service' - with open(confpath) as f: + confp = get_chrony_conf() + with confp.open() as f: conf = f.read() if args.subdomain: conf2 = re.sub(r'pool\s+([0-9]+)\.(\S+)\.pool\.ntp\.org', 'pool \\1.{}.pool.ntp.org'.format(args.subdomain), conf, flags=re.MULTILINE) - with open(confpath, 'w') as f: + with confp.open(confpath, 'w') as f: f.write(conf2) conf = conf2 - chronyd = systemd_unit(chronyd) + chronyd = get_chrony_unit() chronyd.enable() chronyd.restart() run('chronyc -a makestep', shell=True, check=True) + elif target == 'ntp': + if not shutil.which('ntpd'): + pkg_install('ntp') + if not shutil.which('ntpdate'): + pkg_install('ntpdate') + confp = Path('/etc/ntp.conf') + with confp.open() as f: + conf = f.read() + if args.subdomain: + conf2 = re.sub(r'(server|pool)\s+([0-9]+)\.(\S+)\.pool\.ntp\.org', '\\1 \\2.{}.pool.ntp.org'.format(args.subdomain), conf, flags=re.MULTILINE) + with confp.open(confpath, 'w') as f: + f.write(conf2) + conf = conf2 + match = re.search(r'^(server|pool)\s+(\S*)(\s+\S+)?', conf, flags=re.MULTILINE) + server = match.group(2) + ntpd = get_ntpd_unit() + ntpd.stop() + run(f'ntpdate {server}', shell=True, check=True) + ntpd.enable() + ntpd.start()