[Freeipa-devel] [PATCHES 0001-0002] ipa-client-install NTP fixes
Nathan Kinder
nkinder at redhat.com
Fri Feb 27 20:09:27 UTC 2015
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
>
More information about the Freeipa-devel
mailing list