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/

Reply via email to