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

[Rob Crittenden]

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