From e3f35843dc33fa6a3fb3071c5c89ecde9db1ce5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tibor=20Dudl=C3=A1k?= Date: Thu, 2 May 2019 10:57:11 +0200 Subject: [PATCH] Moving prompt for NTP options to install_check In a interactive installation of freeipa server a promt asks for NTP related options after install_check has been called. As it may cause confusion to users moving to install_check methods where the prompt for other options is being done. Refactored sync_time() method to use passed parameters ntp_servers and ntp_pool. Resolves: https://pagure.io/freeipa/issue/7930 Reviewed-By: Rob Crittenden Reviewed-By: Oleg Kozlov --- ipaclient/install/client.py | 51 ++++++++++++++++++++--------- ipaclient/install/timeconf.py | 2 +- ipaserver/install/server/install.py | 47 +++++++++++++++++--------- 3 files changed, 67 insertions(+), 33 deletions(-) diff --git a/ipaclient/install/client.py b/ipaclient/install/client.py index be00771ba..4f361bef1 100644 --- a/ipaclient/install/client.py +++ b/ipaclient/install/client.py @@ -2085,10 +2085,13 @@ def install_check(options): try: timeconf.check_timedate_services() except timeconf.NTPConflictingService as e: - print("WARNING: conflicting time&date synchronization service '{}'" - " will be disabled".format(e.conflicting_service)) - print("in favor of chronyd") - print("") + print( + "WARNING: conflicting time&date synchronization service " + "'{}' will be disabled in favor of chronyd\n".format( + e.conflicting_service + ) + ) + except timeconf.NTPConfigurationError: pass @@ -2374,6 +2377,11 @@ def install_check(options): "Proceed with fixed values and no DNS discovery?", False): raise ScriptError(rval=CLIENT_INSTALL_ERROR) + if options.conf_ntp: + if not options.on_master and not options.unattended and not ( + options.ntp_servers or options.ntp_pool): + options.ntp_servers, options.ntp_pool = timeconf.get_time_source() + cli_realm = ds.realm cli_realm_source = ds.realm_source logger.debug("will use discovered realm: %s", cli_realm) @@ -2401,6 +2409,14 @@ def install_check(options): logger.info("BaseDN: %s", cli_basedn) logger.debug("BaseDN source: %s", cli_basedn_source) + if not options.on_master: + if options.ntp_servers: + for server in options.ntp_servers: + logger.info("NTP server: %s", server) + + if options.ntp_pool: + logger.info("NTP pool: %s", options.ntp_pool) + # ipa-join would fail with IP address instead of a FQDN for srv in cli_server: try: @@ -2466,7 +2482,7 @@ def update_ipa_nssdb(): (nickname, sys_db.secdir, e)) -def sync_time(options, fstore, statestore): +def sync_time(ntp_servers, ntp_pool, fstore, statestore): """ Will disable any other time synchronization service and configure chrony with given ntp(chrony) server and/or pool using Augeas. @@ -2478,21 +2494,24 @@ def sync_time(options, fstore, statestore): # disable other time&date services first timeconf.force_chrony(statestore) - if not options.ntp_servers and not options.ntp_pool: + if not ntp_servers and not ntp_pool: + # autodiscovery happens in case that NTP configuration isn't explicitly + # disabled and user did not provide any NTP server addresses or + # NTP pool address to the installer interactively or as an cli argument ds = discovery.IPADiscovery() - ntp_servers = ds.ipadns_search_srv(cli_domain, '_ntp._udp', - None, break_on_first=False) - if not ntp_servers and not options.unattended: - options.ntp_servers, options.ntp_pool = timeconf.get_time_source() - else: - options.ntp_servers = ntp_servers + ntp_servers = ds.ipadns_search_srv( + cli_domain, '_ntp._udp', None, break_on_first=False + ) + if ntp_servers: + for server in ntp_servers: + # when autodiscovery found server records + logger.debug("Found DNS record for NTP server: \t%s", server) logger.info('Synchronizing time') configured = False - if options.ntp_servers or options.ntp_pool: - configured = timeconf.configure_chrony(options.ntp_servers, - options.ntp_pool, + if ntp_servers or ntp_pool: + configured = timeconf.configure_chrony(ntp_servers, ntp_pool, fstore, statestore) else: logger.warning("No SRV records of NTP servers found and no NTP server " @@ -2577,7 +2596,7 @@ def _install(options): if options.conf_ntp: # Attempt to configure and sync time with NTP server (chrony). - sync_time(options, fstore, statestore) + sync_time(options.ntp_servers, options.ntp_pool, fstore, statestore) elif options.on_master: # If we're on master skipping the time sync here because it was done # in ipa-server-install diff --git a/ipaclient/install/timeconf.py b/ipaclient/install/timeconf.py index 4afe5b32c..cb4b78e11 100644 --- a/ipaclient/install/timeconf.py +++ b/ipaclient/install/timeconf.py @@ -65,7 +65,7 @@ def get_time_source(): ntp_pool = user_input("Enter a NTP source pool address, " "or press Enter to skip", allow_empty=True) if ntp_pool: # if user input is not '' (empty) - logger.debug("User provided NTP pool:\n\t%s", ntp_pool) + logger.debug("User provided NTP pool:\t%s", ntp_pool) return ntp_servers, ntp_pool diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py index 78180b4f5..36ef2ddff 100644 --- a/ipaserver/install/server/install.py +++ b/ipaserver/install/server/install.py @@ -16,7 +16,9 @@ import textwrap import six -from ipaclient.install.client import check_ldap_conf +from ipaclient.install import timeconf +from ipaclient.install.client import ( + check_ldap_conf, sync_time, restore_time_sync) from ipaclient.install.ipachangeconf import IPAChangeConf from ipalib.install import certmonger, sysrestore from ipapython import ipautil, version @@ -33,7 +35,6 @@ from ipalib.util import ( validate_domain_name, no_matching_interface_for_ip_address_warning, ) -import ipaclient.install.timeconf from ipaserver.install import ( adtrust, bindinstance, ca, dns, dsinstance, httpinstance, installutils, kra, krbinstance, @@ -427,13 +428,15 @@ def install_check(installer): if not options.no_ntp: try: - ipaclient.install.timeconf.check_timedate_services() - except ipaclient.install.timeconf.NTPConflictingService as e: - print("WARNING: conflicting time&date synchronization service '{}'" - " will be disabled".format(e.conflicting_service)) - print("in favor of chronyd") - print("") - except ipaclient.install.timeconf.NTPConfigurationError: + timeconf.check_timedate_services() + except timeconf.NTPConflictingService as e: + print( + "WARNING: conflicting time&date synchronization service " + "'{}' will be disabled in favor of chronyd\n".format( + e.conflicting_service + ) + ) + except timeconf.NTPConfigurationError: pass if not options.setup_dns and installer.interactive: @@ -672,6 +675,10 @@ def install_check(installer): if options.ip_addresses or options.setup_dns: installer._update_hosts_file = True + if not options.no_ntp and not options.unattended and not ( + options.ntp_servers or options.ntp_pool): + options.ntp_servers, options.ntp_pool = timeconf.get_time_source() + print() print("The IPA Master Server will be configured with:") print("Hostname: %s" % host_name) @@ -709,6 +716,14 @@ def install_check(installer): "Directory unless\nthe realm name of the IPA server matches " "its domain name.\n\n") + if options.ntp_servers or options.ntp_pool: + if options.ntp_servers: + for server in options.ntp_servers: + print("NTP server:\t{}".format(server)) + + if options.ntp_pool: + print("NTP pool:\t{}".format(options.ntp_pool)) + if installer.interactive and not user_input( "Continue to configure the system with these values?", False): raise ScriptError("Installation aborted") @@ -781,11 +796,11 @@ def install(installer): # As chrony configuration is moved from client here, unconfiguration of # chrony will be handled here in uninstall() method as well by invoking # the ipa-server-install --uninstall - if not options.no_ntp: - if not ipaclient.install.client.sync_time(options, fstore, sstore): - print("Warning: IPA was unable to sync time with chrony!") - print(" Time synchronization is required for IPA " - "to work correctly") + if not options.no_ntp and not sync_time( + options.ntp_servers, options.ntp_pool, fstore, sstore): + print("Warning: IPA was unable to sync time with chrony!") + print(" Time synchronization is required for IPA " + "to work correctly") if options.dirsrv_cert_files: ds = dsinstance.DsInstance(fstore=fstore, @@ -1112,7 +1127,7 @@ def uninstall(installer): except Exception: pass - ipaclient.install.client.restore_time_sync(sstore, fstore) + restore_time_sync(sstore, fstore) kra.uninstall() @@ -1144,7 +1159,7 @@ def uninstall(installer): sstore._load() - ipaclient.install.timeconf.restore_forced_timeservices(sstore) + timeconf.restore_forced_timeservices(sstore) # Clean up group_exists (unused since IPA 2.2, not being set since 4.1) sstore.restore_state("install", "group_exists")