Re: [Freeipa-devel] [PATCH] 0039 Try continue ipa-client-automount even if nsslapd-minssf 0.

2015-02-27 Thread Martin Basti

On 27/02/15 14:21, Martin Basti wrote:

On 26/02/15 15:54, David Kupka wrote:

On 02/26/2015 02:55 PM, Rob Crittenden wrote:

Martin Basti wrote:

On 26/02/15 10:57, David Kupka wrote:

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


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

Works for me, ACK.


NACK.

If you simply pass in /etc/ipa/ca.crt as the cacert path then it will
use TLS.

rob

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



Thanks for the catch Rob. Updated patch attached.


Hello, I tested it again, just nitpick:

1)
Can you also update the commit message?

Never mind, I accidentally read old commit message. sorry.


And question:
I found, if you erase /etc/ipa/ca.crt from client and use --server 
option pointing to different IPA server (LDAP repectively) out of 
realm, ipa-client-atomount  returns success. Is this behavior good? 
This happens without this patch as well.


Martin^2




--
Martin Basti

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


Re: [Freeipa-devel] [PATCH] 0039 Try continue ipa-client-automount even if nsslapd-minssf 0.

2015-02-27 Thread Martin Basti

On 26/02/15 15:54, David Kupka wrote:

On 02/26/2015 02:55 PM, Rob Crittenden wrote:

Martin Basti wrote:

On 26/02/15 10:57, David Kupka wrote:

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


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

Works for me, ACK.


NACK.

If you simply pass in /etc/ipa/ca.crt as the cacert path then it will
use TLS.

rob

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



Thanks for the catch Rob. Updated patch attached.


Hello, I tested it again, just nitpick:

1)
Can you also update the commit message?

And question:
I found, if you erase /etc/ipa/ca.crt from client and use --server 
option pointing to different IPA server (LDAP repectively) out of realm, 
ipa-client-atomount  returns success. Is this behavior good? This 
happens without this patch as well.


Martin^2

--
Martin Basti

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


Re: [Freeipa-devel] [PATCH] 0039 Try continue ipa-client-automount even if nsslapd-minssf 0.

2015-02-27 Thread David Kupka

On 02/27/2015 02:26 PM, Martin Basti wrote:

On 27/02/15 14:21, Martin Basti wrote:

On 26/02/15 15:54, David Kupka wrote:

On 02/26/2015 02:55 PM, Rob Crittenden wrote:

Martin Basti wrote:

On 26/02/15 10:57, David Kupka wrote:

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


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

Works for me, ACK.


NACK.

If you simply pass in /etc/ipa/ca.crt as the cacert path then it will
use TLS.

rob

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



Thanks for the catch Rob. Updated patch attached.


Hello, I tested it again, just nitpick:

1)
Can you also update the commit message?

Never mind, I accidentally read old commit message. sorry.


And question:
I found, if you erase /etc/ipa/ca.crt from client and use --server
option pointing to different IPA server (LDAP repectively) out of
realm, ipa-client-atomount  returns success. Is this behavior good?
This happens without this patch as well.


First of all this never happens if you rely on DNS discovery so most
user will never encounter this behavior,

BUT it would be nice to add a check and warn the user that he is doing 
something unwise and will probably regret :-)

Could you please file a ticket?



Martin^2






--
David Kupka

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


Re: [Freeipa-devel] [PATCH] 0039 Try continue ipa-client-automount even if nsslapd-minssf 0.

2015-02-27 Thread Rob Crittenden
David Kupka wrote:
 On 02/27/2015 02:26 PM, Martin Basti wrote:
 On 27/02/15 14:21, Martin Basti wrote:
 On 26/02/15 15:54, David Kupka wrote:
 On 02/26/2015 02:55 PM, Rob Crittenden wrote:
 Martin Basti wrote:
 On 26/02/15 10:57, David Kupka wrote:
 https://fedorahosted.org/freeipa/ticket/4902


 ___
 Freeipa-devel mailing list
 Freeipa-devel@redhat.com
 https://www.redhat.com/mailman/listinfo/freeipa-devel
 Works for me, ACK.

 NACK.

 If you simply pass in /etc/ipa/ca.crt as the cacert path then it will
 use TLS.

 rob

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


 Thanks for the catch Rob. Updated patch attached.

 Hello, I tested it again, just nitpick:

 1)
 Can you also update the commit message?
 Never mind, I accidentally read old commit message. sorry.

 And question:
 I found, if you erase /etc/ipa/ca.crt from client and use --server
 option pointing to different IPA server (LDAP repectively) out of
 realm, ipa-client-atomount  returns success. Is this behavior good?
 This happens without this patch as well.
 
 First of all this never happens if you rely on DNS discovery so most
 user will never encounter this behavior,
 
 BUT it would be nice to add a check and warn the user that he is doing
 something unwise and will probably regret :-)
 Could you please file a ticket?

Hmm, interesting. Yeah, I suppose trying to get a host ticket would be
good defensive programming.

ACK on the new patch from me too.

rob

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


Re: [Freeipa-devel] [PATCHES 0001-0002] ipa-client-install NTP fixes

2015-02-27 Thread Rob Crittenden
Nathan Kinder wrote:
 
 
 On 02/26/2015 12:55 AM, Martin Kosek wrote:
 On 02/26/2015 03:28 AM, Nathan Kinder wrote:
 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

 Thanks for Patches. For the second patch, I would really prefer to avoid new
 dependency, especially if it's not packaged in RHEL/CentOS. Maybe we could 
 use
 some workaround instead, as in:

 http://stackoverflow.com/questions/3733270/python-subprocess-timeout
 
 I don't like having to add an additional dependency either, but the
 alternative seems more risky.  Utilizing the subprocess32 module (which
 is really just a backport of the normal subprocess module from Python
 3.x) is not invasive for our code in ipautil.run().  Adding some sort of
 a thread that has to kill the spawned subprocess seems more risky (see
 the discussion about a race condition in the stackoverflow thread
 above).  That said, I'm sure the thread/poll method can be made to work
 if you and others feel strongly that this is a better approach than
 adding a new dependency.

Why not use /usr/bin/timeout from coreutils?

rob

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


Re: [Freeipa-devel] IPA Server upgrade 4.2 design

2015-02-27 Thread Rob Crittenden
Martin Basti wrote:
 On 26/02/15 10:45, Petr Spacek wrote:
 On 25.2.2015 17:49, Martin Basti wrote:
 On 25/02/15 17:15, Petr Spacek wrote:
 On 24.2.2015 19:10, Martin Basti wrote:
 Hello all,

 please read the design page, any objections/suggestions appreciated
 http://www.freeipa.org/page/V4/Server_Upgrade_Refactoring
 Thank you for the design, I have only few nitpicks.

 Increase update files numbers range
 Update files number will be extended into 4 digits values.
 IMHO the dependency on particular number format should be removed
 altogether.
 It should be perfectly enough to say that updates are executed in ASCII
 lexicographic order and be done with it.
 4.1.3-2  4.1.3-12 in lexicographic order, this will not fit.
 Well, sure, but it allows you to use
 00-a
 01-b

 and renumber it to

 001-a
 002-b

 at will without changes to code. (Lexicographic order is what 'ls'
 uses by
 default so you can see the real ordering at any time very easily.)

 Also, as you pointed out, it allows you to do things like
 12.345-a
 12.666-bbb
 without changing code, again :-)
 Oh stupid me, I read it wrong, I replied with IPA version compare.
 
 sounds good to me, any objections anyone?

This makes sense as long as we don't abuse it. The numbers are there to
apply some amount of order but flexibility is good, and will avoid the
problem of having humongous update files.

I'm fine with allowing DM given that it allows running as non-root
(pretty much the only condition that ldapi wouldn't work), but I think a
full upgrade will fail w/o root given that you are combining the two
commands.

On ipactl, would it be overkill if there is a tty to prompt the user to
upgrade? In a non-container world it might be surprising to have an
upgrade happen esp since upgrades take a while.

With --skip-version-check what sorts of problems can we foresee? I
assume a big warning will be added to at least the man page, if not the cli?

Where does platform come from? I'm wondering how Debian will handle this.

Looks really good.

rob

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


Re: [Freeipa-devel] [PATCHES 0001-0002] ipa-client-install NTP fixes

2015-02-27 Thread Nathan Kinder


On 02/26/2015 12:55 AM, Martin Kosek wrote:
 On 02/26/2015 03:28 AM, Nathan Kinder wrote:
 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
 
 Thanks for Patches. For the second patch, I would really prefer to avoid new
 dependency, especially if it's not packaged in RHEL/CentOS. Maybe we could use
 some workaround instead, as in:
 
 http://stackoverflow.com/questions/3733270/python-subprocess-timeout

I don't like having to add an additional dependency either, but the
alternative seems more risky.  Utilizing the subprocess32 module (which
is really just a backport of the normal subprocess module from Python
3.x) is not invasive for our code in ipautil.run().  Adding some sort of
a thread that has to kill the spawned subprocess seems more risky (see
the discussion about a race condition in the stackoverflow thread
above).  That said, I'm sure the thread/poll method can be made to work
if you and others feel strongly that this is a better approach than
adding a new dependency.

-NGK

 
 ?
 
 Martin
 

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


Re: [Freeipa-devel] [PATCHES 0001-0002] ipa-client-install NTP fixes

2015-02-27 Thread Nathan Kinder


On 02/27/2015 12:20 PM, Rob Crittenden wrote:
 Nathan Kinder wrote:


 On 02/26/2015 12:55 AM, Martin Kosek wrote:
 On 02/26/2015 03:28 AM, Nathan Kinder wrote:
 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

 Thanks for Patches. For the second patch, I would really prefer to avoid new
 dependency, especially if it's not packaged in RHEL/CentOS. Maybe we could 
 use
 some workaround instead, as in:

 http://stackoverflow.com/questions/3733270/python-subprocess-timeout

 I don't like having to add an additional dependency either, but the
 alternative seems more risky.  Utilizing the subprocess32 module (which
 is really just a backport of the normal subprocess module from Python
 3.x) is not invasive for our code in ipautil.run().  Adding some sort of
 a thread that has to kill the spawned subprocess seems more risky (see
 the discussion about a race condition in the stackoverflow thread
 above).  That said, I'm sure the thread/poll method can be made to work
 if you and others feel strongly that this is a better approach than
 adding a new dependency.
 
 Why not use /usr/bin/timeout from coreutils?

That sounds like a perfectly good idea.  I wasn't aware of it's
existence (or it's possible that I forgot about it).  Thanks for the
suggestion!  I'll test out a reworked version of the patch.

Do you think that there is value in leaving the timeout capability
centrally in ipautil.run()?  We only need it for the call to 'ntpd'
right now, but there might be a need for using a timeout for other
commands in the future.  The alternative is to just modify
synconce_ntp() to use /usr/bin/timeout and leave ipautil.run() alone.

-NGK

 
 rob
 

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


[Freeipa-devel] [PATCHES] SPEC: Require python2 version of sssd bindings

2015-02-27 Thread Lukas Slebodnik
ehlo,

Please review attached patches and fix freeipa in fedora 22 ASAP.

I think the most critical is 1st patch

sh$ git grep SSSDConfig  | grep import
install/tools/ipa-upgradeconfig:import SSSDConfig
ipa-client/ipa-install/ipa-client-automount:import SSSDConfig
ipa-client/ipa-install/ipa-client-install:import SSSDConfig

BTW package python-sssdconfig is provides since sssd-1.10.0alpha1 (2013-04-02)
but it was not explicitely required.

The latest python3 changes in sssd (fedora 22) is just a result of negligent
packaging of freeipa.

LS
From 09bfbd420ab83f8cb571f9dc04a5cd9c7f15d604 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik lsleb...@redhat.com
Date: Fri, 27 Feb 2015 20:40:06 +0100
Subject: [PATCH 1/3] SPEC: Explicitly requires python-sssdconfig

Resolves:
https://fedorahosted.org/freeipa/ticket/4929
---
 freeipa.spec.in | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 
b186d9fdff31118ea4d929f024f4dc16a75b1d0b..9513f45c6c933a1109390393cb90d68e8c697dc7
 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -122,6 +122,7 @@ Requires: mod_auth_kerb = 5.4-16
 Requires: mod_nss = 1.0.8-26
 Requires: python-ldap = 2.4.15
 Requires: python-krbV
+Requires: python-sssdconfig
 Requires: acl
 Requires: python-pyasn1
 Requires: memcached
@@ -228,6 +229,7 @@ Requires: wget
 Requires: libcurl = 7.21.7-2
 Requires: xmlrpc-c = 1.27.4
 Requires: sssd = 1.12.3
+Requires: python-sssdconfig
 Requires: certmonger = 0.76.8
 Requires: nss-tools
 Requires: bind-utils
-- 
2.1.0

From 3efd525d72d20734d926c4e804a7080ea01cb580 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik lsleb...@redhat.com
Date: Fri, 27 Feb 2015 20:43:38 +0100
Subject: [PATCH 2/3] SPEC: Require python2 version of sssd bindings

Python modules pysss and pysss_murmur was part of package sssd-common.
Fedora 22 tries to get rid of python2 and therefore these modules were
extracted from package sssd-common to separate packages python-sss and
python-sss-murmur and python3 version of packages python3-sss
python3-sss-murmur

git grep pysss  | grep import
ipalib/plugins/trust.py:import pysss_murmur #pylint: disable=F0401
ipaserver/dcerpc.py:import pysss

ipaserver/dcerpc.py is pacakged in freeipa-server-trust-ad
palib/plugins/trust.py is packaged in freeipa-python

Resolves:
https://fedorahosted.org/freeipa/ticket/4929
---
 freeipa.spec.in | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 
9513f45c6c933a1109390393cb90d68e8c697dc7..7a1ff8b50ef1b462ad14fb2328149c3c2ed2fb38
 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -195,6 +195,9 @@ Requires: samba = %{samba_version}
 Requires: samba-winbind
 Requires: libsss_idmap
 Requires: libsss_nss_idmap-python
+%if (0%{?fedora} = 22)
+Requires: python-sss
+%endif
 # We use alternatives to divert winbind_krb5_locator.so plugin to libkrb5
 # on the installes where server-trust-ad subpackage is installed because
 # IPA AD trusts cannot be used at the same time with the locator plugin
@@ -288,6 +291,9 @@ Requires: python-qrcode-core = 5.0.0
 Requires: python-pyasn1
 Requires: python-dateutil
 Requires: python-yubico
+%if (0%{?fedora} = 22)
+Requires: python-sss-murmur
+%endif
 Requires: wget
 Requires: dbus-python
 
-- 
2.1.0

From 5d963dda2d6423007cea803940be5d34fdcbc377 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik lsleb...@redhat.com
Date: Fri, 27 Feb 2015 21:02:51 +0100
Subject: [PATCH 3/3] SPEC: Add missing requires for python-libsss_nss_idmap

git grep pysss_nss_idmap  | grep import
ipalib/plugins/trust.py:import pysss_nss_idmap #pylint: disable=F0401
ipaserver/dcerpc.py:import pysss_nss_idmap

ipaserver/dcerpc.py is packaged in freeipa-server-trust-ad
palib/plugins/trust.py is packaged in freeipa-python

Resolves:
https://fedorahosted.org/freeipa/ticket/4929
---
 freeipa.spec.in | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 
7a1ff8b50ef1b462ad14fb2328149c3c2ed2fb38..fafec414849735854ee97752f20e941f01dc92ce
 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -198,6 +198,7 @@ Requires: libsss_nss_idmap-python
 %if (0%{?fedora} = 22)
 Requires: python-sss
 %endif
+Requires: python-libsss_nss_idmap
 # We use alternatives to divert winbind_krb5_locator.so plugin to libkrb5
 # on the installes where server-trust-ad subpackage is installed because
 # IPA AD trusts cannot be used at the same time with the locator plugin
@@ -294,6 +295,7 @@ Requires: python-yubico
 %if (0%{?fedora} = 22)
 Requires: python-sss-murmur
 %endif
+Requires: python-libsss_nss_idmap
 Requires: wget
 Requires: dbus-python
 
-- 
2.1.0

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

Re: [Freeipa-devel] [PATCHES 0001-0002] ipa-client-install NTP fixes

2015-02-27 Thread Nathan Kinder


On 02/27/2015 01:08 PM, Rob Crittenden wrote:
 Nathan Kinder wrote:


 On 02/27/2015 12:20 PM, Rob Crittenden wrote:
 Nathan Kinder wrote:


 On 02/26/2015 12:55 AM, Martin Kosek wrote:
 On 02/26/2015 03:28 AM, Nathan Kinder wrote:
 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

 Thanks for Patches. For the second patch, I would really prefer to avoid 
 new
 dependency, especially if it's not packaged in RHEL/CentOS. Maybe we 
 could use
 some workaround instead, as in:

 http://stackoverflow.com/questions/3733270/python-subprocess-timeout

 I don't like having to add an additional dependency either, but the
 alternative seems more risky.  Utilizing the subprocess32 module (which
 is really just a backport of the normal subprocess module from Python
 3.x) is not invasive for our code in ipautil.run().  Adding some sort of
 a thread that has to kill the spawned subprocess seems more risky (see
 the discussion about a race condition in the stackoverflow thread
 above).  That said, I'm sure the thread/poll method can be made to work
 if you and others feel strongly that this is a better approach than
 adding a new dependency.

 Why not use /usr/bin/timeout from coreutils?

 That sounds like a perfectly good idea.  I wasn't aware of it's
 existence (or it's possible that I forgot about it).  Thanks for the
 suggestion!  I'll test out a reworked version of the patch.

 Do you think that there is value in leaving the timeout capability
 centrally in ipautil.run()?  We only need it for the call to 'ntpd'
 right now, but there might be a need for using a timeout for other
 commands in the future.  The alternative is to just modify
 synconce_ntp() to use /usr/bin/timeout and leave ipautil.run() alone.
 
 I think it would require a lot of research. One of the programs spawned
 by this is pkicreate which could take quite some time, and spawning a
 clone in particular.
 
 It is definitely an interesting idea but I think it is safest for now to
 limit it to just NTP for now.

What I meant was that we would have an optional keyword timeout
parameter to ipautil.run() that defaults to None, just like my
subprocess32 approach.  If a timeout is not passed in, we would use
subprocess.Popen() to run the specified command just like we do today.
We would only actually pass the timeout parameter to ipautil.run() in
synconce_ntp() for now, so no other commands would have a timeout in
effect.  The capability would be available for other commands this way
though.

Let me propose a patch with this implementation, and if you don't like
it, we can leave ipautil.run() alone and restrict the changes to
synconce_ntp().

 
 rob
 

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


Re: [Freeipa-devel] [PATCHES 0001-0002] ipa-client-install NTP fixes

2015-02-27 Thread Rob Crittenden
Nathan Kinder wrote:
 
 
 On 02/27/2015 12:20 PM, Rob Crittenden wrote:
 Nathan Kinder wrote:


 On 02/26/2015 12:55 AM, Martin Kosek wrote:
 On 02/26/2015 03:28 AM, Nathan Kinder wrote:
 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

 Thanks for Patches. For the second patch, I would really prefer to avoid 
 new
 dependency, especially if it's not packaged in RHEL/CentOS. Maybe we could 
 use
 some workaround instead, as in:

 http://stackoverflow.com/questions/3733270/python-subprocess-timeout

 I don't like having to add an additional dependency either, but the
 alternative seems more risky.  Utilizing the subprocess32 module (which
 is really just a backport of the normal subprocess module from Python
 3.x) is not invasive for our code in ipautil.run().  Adding some sort of
 a thread that has to kill the spawned subprocess seems more risky (see
 the discussion about a race condition in the stackoverflow thread
 above).  That said, I'm sure the thread/poll method can be made to work
 if you and others feel strongly that this is a better approach than
 adding a new dependency.

 Why not use /usr/bin/timeout from coreutils?
 
 That sounds like a perfectly good idea.  I wasn't aware of it's
 existence (or it's possible that I forgot about it).  Thanks for the
 suggestion!  I'll test out a reworked version of the patch.
 
 Do you think that there is value in leaving the timeout capability
 centrally in ipautil.run()?  We only need it for the call to 'ntpd'
 right now, but there might be a need for using a timeout for other
 commands in the future.  The alternative is to just modify
 synconce_ntp() to use /usr/bin/timeout and leave ipautil.run() alone.

I think it would require a lot of research. One of the programs spawned
by this is pkicreate which could take quite some time, and spawning a
clone in particular.

It is definitely an interesting idea but I think it is safest for now to
limit it to just NTP for now.

rob

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