[Freeipa-devel] [PATCH] 1014 configurable service timeout

[Rob Crittenden]

Martin Kosek wrote:
> On 05/29/2012 04:31 PM, Rob Crittenden wrote:
>> Martin Kosek wrote:
>>> On Thu, 2012-05-24 at 11:38 -0400, Rob Crittenden wrote:
>>>> Petr Viktorin wrote:
>>>>> On 05/18/2012 10:03 PM, Rob Crittenden wrote:
>>>>>> Rob Crittenden wrote:
>>>>>>> A hardcoded timeout was used in ipactl for service restarts, set rather
>>>>>>> low. A separate timeout was hardcoded into the installer.
>>>>>>>
>>>>>>> I centralized them into a single timeout, configurable in the standard
>>>>>>> way in /etc/ipa/*.conf.
>>>>>>>
>>>>>>> On install it will always default to 120 seconds and remain there unless
>>>>>>> changed in default.conf (not replicated either).
>>>>>>>
>>>>>>> I tested this on systemd systems and sysV systems and it works ok for
>>>>>>> me. You'll also want to double-check that this works when other 389-ds
>>>>>>> instances are installed.
>>>>>>>
>>>>>>> Getting the naming of instances right was a bit tricky.
>>>>>>
>>>>>> Noticed a problem on upgrades and fixed that. Updated patch attached.
>>>>>>
>>>>>> rob
>>>>>>
>>>>>>
>>>>>
>>>>> Please rebase the patch onto current master.
>>>>>
>>>>>
>>>>
>>>> Done
>>>
>>> This is a good start. I just found few places where I found that the
>>> remaining wait function calls are redundant:
>>>
>>> 1) install/tools/ipactl:
>>>
>>>           if lurl.urlscheme == 'ldapi':
>>> -            wait_for_open_socket(lurl.hostport, timeout=6)
>>> +            wait_for_open_socket(lurl.hostport,
>>> timeout=api.env.startup_timeout)
>>>            else:
>>>                (host,port) = lurl.hostport.split(':')
>>> -            wait_for_open_ports(host, [int(port)], timeout=6)
>>> +            wait_for_open_ports(host, [int(port)],
>>> timeout=api.env.startup_timeout)
>>>
>>> Aren't these calls redundant? We already wait for ports when dirsrv is
>>> started (dirsrv.start()) or restarted (dirsrv.restart()).
>>
>> It is redundant in some cases but there are some calls we make where this is
>> used to determine the availability of the service. This call is needed.
>>
>>> 2) ipaserver/install/replication.py:
>>> -        installutils.wait_for_open_ports('localhost', [389, 636], 300)
>>> +        ipautil.wait_for_open_ports('localhost', [389, 636], 300)
>>>
>>> Isn't this now redundant? Port check should be done in service restart.
>>
>> Yes, looks like this call can go.
>>
>>> 3) ipaserver/install/plugins/updateclient.py:
>>>
>>> -            installutils.wait_for_open_socket(socket_name)
>>> +            wait_for_open_socket(socket_name)
>>>
>>> Also seems redundant, dirsrv should be already up as it was restarted
>>> via our Service framework. Though we only check for ports in the Service
>>> framework, I wonder if this is enough and we can be sure that when ports
>>> are up, the LDAPI socket is also up.
>>
>> No, sockets and ports are separate, particularly when updating. In fact, we
>> disable the ports so a wait_for_port() will always fail which is why I added
>> the wait flag. This may be a case I missed with upgrades. Let me test upgrades
>> again...
>>
>> rob
>
> I think we want to either send a revised patch to this ticket to get it to Beta
> 1 or to defer it to some future version...
>
> Martin
>

Here is a rebased patch.

rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-rcrit-1014-4-timeout.patch
Type: text/x-diff
Size: 72619 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120629/9571611b/attachment.bin>