Hi Achim,
On Thu, Nov 20, 2008 at 01:32:06PM +0100, Achim Stumpf wrote:
> Hi,
>
> I am still waiting for further advice to finish the commit
> process to your repository. ;o)
I'd have a few suggestions as well:
1. You could drop both connect_timeout and max_time parameters.
It is advisable to have the upper layer decide when an operation
should timeout (by setting the operation's "timeout" attribute).
2. kill -0 "$1"
returns success with zombie processes too.
3. Poftpd: a typo in a message.
4. In proftpd_start()
# check that the proftpd binary exists
if [ ! -x "$OCF_RESKEY_binary" ]; then
ocf_log err "Proftpd binary $OCF_RESKEY_binary does not exist"
exit $OCF_ERR_CONFIGURED
This belongs to validate. Should also be OCF_ERR_INSTALLED.
5. In proftpd_stop()
...
TRIES=0
while isRunning "$PID" && [ $TRIES -lt 5 ]
do
sleep 1
kill $PID > /dev/null 2>&1
ocf_log info "Killing Proftpd PID $PID"
TRIES=`expr $TRIES + 1`
done
Move the kill command in front of the loop: it's enough to send a
signal once. The same for the latter 'kill -9'. You can also try
other signals, such as HUP (see, perhaps, the proftpd docs).
Also, could it be that 5 seconds is a bit too short to let the
ftpd exit?
As in 1. above, it's better to leave the timeout handling to
lrmd. You can just do an infinite look here:
while isRunning "$PID"; do
sleep 1
...
done
6. In proftpd_monitor()
ocf_log err "Proftpd monitor on PID $PID failed"
It may or may not be an error if proftpd is not running. Better
to keep silent.
ocf_log err "$OCF_RESKEY_curl_binary does not exist"
return $OCF_ERR_CONFIGURED
This belongs to the validate action. The error should be
OCF_ERR_INSTALLED.
See the validate action in other OCF agents. Note that a failure
to validate the environment and parameters shouldn't influence
the meta-data operation.
And... very sorry for taking this long.
Cheers,
Dejan
>
>
> Achim
>
>
> Achim Stumpf wrote:
>> Hi,
>>
>> What's about the proftpd script? Maybe we could finish that one... ;o)
>>
>>
>> Thanks,
>>
>> Achim
>>
>>
>>
>> Achim Stumpf wrote:
>>> Hi,
>>>
>>> Lars Marowsky-Bree wrote:
>>>>
>>>> "start_delay" should not be necessary, unless your start operation is
>>>> broken.
>>>>
>>>
>>> I have removed the start_delay.
>>>
>>>>
>>>> You also need to do a check_binary on the proftpd binary somewhere prior
>>>> to calling this command.
>>>>
>>>
>>> All config files and binaries get checked now.
>>>
>>>>> proftpd_stop()
>>>>> {
>>>>> if proftpd_status ; then
>>>>> PID=`head -n 1 $OCF_RESKEY_pidfile`
>>>>> if [ ! -z $PID ] ; then
>>>>> kill $PID
>>>>> if [ $? -ne 0 ]; then
>>>>> ocf_log err "Proftpd couldn't be stopped"
>>>>> return $OCF_ERR_GENERIC
>>>>> fi
>>>>> fi
>>>>> fi
>>>>
>>>> You need to check afterwards that the process is really gone. kill is
>>>> not sufficient; it might send the signal, but if the process is stuck on
>>>> IO, it might not stop. I'd also recommend kill -9.
>>>>
>>>
>>> I have adapted some code of the apache ocf script:
>>>
>>> proftpd_stop()
>>> {
>>> if proftpd_status ; then
>>> PID=`head -n 1 $OCF_RESKEY_pidfile`
>>> if [ ! -z "$PID" ]; then
>>> kill $PID > /dev/null 2>&1 if [ $? -eq 0 ]; then
>>> TRIES=0
>>> while isRunning "$PID" && [ $TRIES -lt 5 ]
>>> do
>>> sleep 1
>>> kill $PID > /dev/null 2>&1
>>> ocf_log info "Killing Proftpd PID $PID"
>>> TRIES=`expr $TRIES + 1`
>>> done
>>> TRIES=0
>>> while isRunning "$PID" && [ $TRIES -lt 3 ]
>>> do
>>> sleep 1
>>> kill -9 $PID > /dev/null 2>&1
>>> ocf_log info "Killing Proftpd PID $PID with SIGKILL"
>>> TRIES=`expr $TRIES + 1`
>>> done
>>> if isRunning "$PID" ; then
>>> ocf_log err "Killing Proftpd PID $PID FAILED"
>>> exit $OCF_ERR_GENERIC
>>> fi
>>> else
>>> ocf_log err "Killing Proftpd PID $PID FAILED"
>>> exit $OCF_ERR_GENERIC
>>> fi
>>> fi
>>> fi exit $OCF_SUCCESS
>>> }
>>>
>>> 1. kill -15 2. if it is still running try max. 5 times to kill again
>>> with kill -15
>>> 3. after that if it is still running try 3 times with kill -9
>>>
>>> Hope that it meets your needs ;o)
>>>
>>> And actually I got the idea to send in the while loop with kill -9 a kill
>>> -9 signal to all the child processes as returned by:
>>>
>>> ps --no-headers -o pid --ppid $PID
>>>
>>> If you like the above code and the idea with kill -9 to the child
>>> processes, I will implement that too.
>>>
>>>>> proftpd_monitor()
>>>>> {
>>>>> if [ $OCF_CHECK_LEVEL -eq 0 ]; then if proftpd_status ;
>>>>> then
>>>>> ocf_log debug "Proftpd monitor succeded"
>>>>> return $OCF_SUCCESS
>>>>> fi
>>>>> else
>>>>> ${OCF_RESKEY_curl_binary} -sS --connect-timeout
>>>>> ${OCF_RESKEY_connect_timeout} --max-time ${OCF_RESKEY_max_time} -u
>>>>> "${OCF_RESKEY_test_user}:${OCF_RESKEY_test_pass}" ftp://localhost/ >
>>>>> /dev/null 2>&1
>>>>
>>>> The curl_binary also needs to be "check_binary"ed somewhere.
>>>>
>>>
>>> done
>>>
>>>> And you might be managing several proftpd instances, none of which might
>>>> bind to "localhost"; I think the URL needs to be configurable.
>>>
>>> done
>>>
>>>
>>> Thanks for your advice. Let's see what is wrong now... ;o)
>>>
>>>
>>> Achim
>>>
>>>
>>>
> _______________________________________________________
> Linux-HA-Dev: [email protected]
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/
_______________________________________________________
Linux-HA-Dev: [email protected]
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/