Hi,

The two attached patches address some issues that affect
ipa-client-install when syncing time from the NTP server.  Now that we
use ntpd to perform the time sync, the client install can end up hanging
forever when the server is not reachable (firewall issues, etc.).  These
patches address the issues in two different ways:

1 - Don't attempt to sync time when --no-ntp is specified.

2 - Implement a timeout capability that is used when we run ntpd to
perform the time sync to prevent indefinite hanging.

The one potentially contentious issue is that this introduces a new
dependency on python-subprocess32 to allow us to have timeout support
when using Python 2.x.  This is packaged for Fedora, but I don't see it
on RHEL or CentOS currently.  It would need to be packaged there.

https://fedorahosted.org/freeipa/ticket/4842

Thanks,
-NGK
From 80514f225f628f7c7993b85e562a851e7ee40644 Mon Sep 17 00:00:00 2001
From: Nathan Kinder <nkin...@redhat.com>
Date: Wed, 25 Feb 2015 14:22:02 -0800
Subject: [PATCH 1/2] Skip time sync during client install when using --no-ntp

When --no-ntp is specified during ipa-client-install, we still
attempt to perform a time sync before obtaining a TGT from the
KDC.  We should not be attempting to sync time with the KDC if
we are explicitly told to not configure ntp.

Ticket: https://fedorahosted.org/freeipa/ticket/4842
---
 ipa-client/ipa-install/ipa-client-install | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index ccaab55..a625fbd 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -2324,8 +2324,9 @@ def install(options, env, fstore, statestore):
         # hostname if different from system hostname
         tasks.backup_and_replace_hostname(fstore, statestore, options.hostname)
 
-    if not options.on_master:
+    if not options.on_master and options.conf_ntp:
         # Attempt to sync time with IPA server.
+        # If we're skipping NTP configuration, we also skip the time sync here.
         # We assume that NTP servers are discoverable through SRV records in the DNS
         # If that fails, we try to sync directly with IPA server, assuming it runs NTP
         root_logger.info('Synchronizing time with KDC...')
-- 
1.9.3

From 7ba0745b10145f516536a2b1b711203b5bb8cb09 Mon Sep 17 00:00:00 2001
From: Nathan Kinder <nkin...@redhat.com>
Date: Wed, 25 Feb 2015 15:19:47 -0800
Subject: [PATCH 2/2] Timeout when performing time sync during client install

We use ntpd now to sync time before fetching a TGT during client
install.  Unfortuantely, ntpd will hang forever if it is unable to
reach the NTP server.

This patch adds the ability for commands run via ipautil.run() to
have an optional timeout.  This capability is used by the NTP sync
code that is run during ipa-client-install.

This new timeout capability requires a new dependency on the
subprocess32 module, which is a backport of the Python 3.x subprocess
module for Python 2.x.

Ticket: https://fedorahosted.org/freeipa/ticket/4842
---
 freeipa.spec.in                 |  1 +
 ipa-client/ipaclient/ntpconf.py |  4 +++-
 ipapython/ipautil.py            | 21 +++++++++++++++------
 3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index b186d9f..8379b31 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -285,6 +285,7 @@ Requires: libipa_hbac-python
 Requires: python-qrcode-core >= 5.0.0
 Requires: python-pyasn1
 Requires: python-dateutil
+Requires: python-subprocess32
 Requires: python-yubico
 Requires: wget
 Requires: dbus-python
diff --git a/ipa-client/ipaclient/ntpconf.py b/ipa-client/ipaclient/ntpconf.py
index e1ac55a..2cc3332 100644
--- a/ipa-client/ipaclient/ntpconf.py
+++ b/ipa-client/ipaclient/ntpconf.py
@@ -149,7 +149,9 @@ def synconce_ntp(server_fqdn):
 
     tmp_ntp_conf = ipautil.write_tmp_file('server %s' % server_fqdn)
     try:
-        ipautil.run([ntpd, '-qgc', tmp_ntp_conf.name])
+        # The ntpd command will never exit if it is unable to reach the
+        # server, so timeout after 15 seconds.
+        ipautil.run([ntpd, '-qgc', tmp_ntp_conf.name], timeout=15)
         return True
     except ipautil.CalledProcessError:
         return False
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 4116d97..41046fc 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -20,6 +20,7 @@
 import string
 import tempfile
 import subprocess
+import subprocess32
 import random
 import os, sys, traceback
 import copy
@@ -38,6 +39,7 @@ import krbV
 import pwd
 from dns import resolver, rdatatype
 from dns.exception import DNSException
+from subprocess32 import TimeoutExpired
 
 from ipapython.ipa_log_manager import *
 from ipapython import ipavalidate
@@ -249,7 +251,7 @@ def shell_quote(string):
 
 def run(args, stdin=None, raiseonerr=True,
         nolog=(), env=None, capture_output=True, skip_output=False, cwd=None,
-        runas=None):
+        runas=None, timeout=None):
     """
     Execute a command and return stdin, stdout and the process return code.
 
@@ -277,6 +279,8 @@ def run(args, stdin=None, raiseonerr=True,
     :param cwd: Current working directory
     :param runas: Name of a user that the command shold be run as. The spawned
         process will have both real and effective UID and GID set.
+    :param timeout: Timeout if the command hasn't returned within the specified
+        number of seconds.  A TimeoutExpired exception will be raised.
     """
     p_in = None
     p_out = None
@@ -295,16 +299,17 @@ def run(args, stdin=None, raiseonerr=True,
         env = copy.deepcopy(os.environ)
         env["PATH"] = "/bin:/sbin:/usr/kerberos/bin:/usr/kerberos/sbin:/usr/bin:/usr/sbin"
     if stdin:
-        p_in = subprocess.PIPE
+        p_in = subprocess32.PIPE
     if skip_output:
         p_out = p_err = open(paths.DEV_NULL, 'w')
     elif capture_output:
-        p_out = subprocess.PIPE
-        p_err = subprocess.PIPE
+        p_out = subprocess32.PIPE
+        p_err = subprocess32.PIPE
 
     arg_string = nolog_replace(' '.join(shell_quote(a) for a in args), nolog)
     root_logger.debug('Starting external process')
     root_logger.debug('args=%s' % arg_string)
+    root_logger.debug('timeout=%s' % timeout)
 
     preexec_fn = None
     if runas is not None:
@@ -316,15 +321,19 @@ def run(args, stdin=None, raiseonerr=True,
                               os.setreuid(pent.pw_uid, pent.pw_uid))
 
     try:
-        p = subprocess.Popen(args, stdin=p_in, stdout=p_out, stderr=p_err,
+        p = subprocess32.Popen(args, stdin=p_in, stdout=p_out, stderr=p_err,
                              close_fds=True, env=env, cwd=cwd,
                              preexec_fn=preexec_fn)
-        stdout,stderr = p.communicate(stdin)
+        stdout,stderr = p.communicate(stdin, timeout)
         stdout,stderr = str(stdout), str(stderr)    # Make pylint happy
     except KeyboardInterrupt:
         root_logger.debug('Process interrupted')
         p.wait()
         raise
+    except TimeoutExpired:
+        root_logger.debug('Process execution timed out')
+        p.kill()
+        stdout,stderr = p.communicate(stdin)
     except:
         root_logger.debug('Process execution failed')
         raise
-- 
1.9.3

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to