On 2006-07-28T01:57:29, Rajat Upadhyaya <[EMAIL PROTECTED]> wrote:
Thanks for this posting!
Some general feedback:
- Please, send the files as patches vs the Linux HA source tree; in your
case, this mean to supply a patch providing the PureFTPd.in file,
making the changes to the Makefiles to build this et cetera. Then we
can be sure we're using the same substitutions as you want us to.
- Please, send them as attachments, not in-line. That makes it easier to
handle, and while prevent GroupWise from wrapping the lines (which it
apparently did here, too).
> This is the OCF RA for Pure-FTPd.
The OCF one is all what is needed, as we have a wrapper to make OCF
scripts available as heartbeat ones too.
Some comments on the script itself:
> #!/bin/sh
> #
> # License: GNU General Public License (GPL)
> #
> # This script is an OCF resource script for using Pure-FTPd in
> # an Active-Passive setup.
> #
> # usage: $0 {start|stop|status|monitor|validate-all|meta-data}
> #
> # The "start" arg starts Pure-FTPd.
> #
> # Surprisingly, the "stop" arg stops it.
> #
> # OCF parameters are as below
> # OCF_RESKEY_script
> # OCF_RESKEY_conffile
> #
> #######################################################################
> # Initialization:
>
> . /usr/lib/heartbeat/ocf-shellfuncs
>
> HA_VARRUNDIR=/var/run/heartbeat
> IFCONFIG=/sbin/ifconfig
> IFCONFIG_A_OPT=
BTW, these would be very likely candidates for substitution -
@halibdir@ and so on. The ifconfig variables are probably superfluous?
> USAGE="usage: $0 {start|stop|status|monitor|validate-all|meta-data}";
>
> #######################################################################
> LC_ALL=C
> export LC_ALL
> HA_D=/etc/ha.d
> . ${HA_D}/shellfuncs
You shouldn't have to source shellfuncs if you're loading the OCF
shellfuncs already.
> SYSTYPE="`uname -s`"
> case "$SYSTYPE" in
> SunOS)
> # `uname -r` = 5.9 -> SYSVERSION = 9
> SYSVERSION="`uname -r | cut -d. -f 2`"
> ;;
> Darwin)
> # Treat Darwin the same as the other BSD variants (matched as
> *BSD)
> SYSTYPE="${SYSTYPE}BSD"
> ;;
> *)
> ;;
> esac
This is also dead code - nothing in your script requires the systype to
be known.
> meta_data() {
> cat <<END
> <?xml version="1.0"?>
> <!DOCTYPE resource-agent SYSTEM "ra-api-1.dtd">
> <resource-agent name="Pure-FTPd">
> <version>1.0</version>
> <longdesc lang="en">
> This script manages Pure-FTPd in an Active-Passive setup
> </longdesc>
> <shortdesc lang="en">OCF Resource Agent compliant FTP
> script.</shortdesc>
>
> <parameters>
> <parameter name="script" unique="1" required="1">
> <longdesc lang="en">
> The full path to the Pure-FTPd startup script.
> For example, "/sbin/pure-config.pl"
> </longdesc>
> <shortdesc lang="en">Script name with full path</shortdesc>
> <content type="string" default="/sbin/pure-config.pl" />
> </parameter>
> <parameter name="conffile" unique="1" required="1">
> <longdesc lang="en">
> The Pure-FTPd configuration file name with full path.
> For example, "/etc/pure-ftpd/pure-ftpd.conf"
> </longdesc>
> <shortdesc lang="en">Configuration file name with full
> path</shortdesc>
> <content type="string" default="/etc/pure-ftpd/pure-ftpd.conf" />
> </parameter>
> </parameters>
>
> <actions>
> <action name="start" timeout="90" />
> <action name="stop" timeout="100" />
> <action name="monitor" depth="10" timeout="20s" interval="5s"
> start-delay="1s" />
Do you really want the monitor to be run every 5s by default? Of course,
that's your call...
> <action name="validate-all" timeout="30s" />
> <action name="meta-data" timeout="5s" />
> </actions>
> </resource-agent>
> END
> exit $OCF_SUCCESS
> }
>
> PureFTPd_status()
> {
> pgrep pure-ftpd >> /dev/null
> return $?
> }
Sorry, but this is hardly a good check. There can be several pureftpd
instances running, can there not? This one will return success whenever
any single one is running.
> PureFTPd_start() {
> #
> # make a few checks and start pure-ftpd
> #
> if ocf_is_root ; then : ; else
> ocf_log err "You must be root."
> exit $OCF_ERR_PERM
> fi
Always? I don't think this is a good check. The user could be running
pureftpd on a non-privileged port. (It doesn't matter, because all RAs
_do_ get run as root, I'm just pointing it out for completeness.)
> #if pure-ftpd is running return success
>
> PureFTPd_status
> if [ $? -eq 0 ]; then
> exit $OCF_SUCCESS
> fi
Again, this will result in the script pretending the RA was started
whenever _any_ pureftpd instance is running on the system.
> if [ -n $OCF_RESKEY_script -a -n $OCF_RESKEY_conffile ]; then
> $OCF_RESKEY_script $OCF_RESKEY_conffile
> else
> ocf_log err "One or more empty arguments"
> exit $OCF_ERR_GENERIC
> fi
If you want to test for empty arguments, you need to put "" around them
in the if statement.
> if [ $? -ne 0 ]; then
> ocf_log info "Pure-FTPd returned error" $?
> exit $OCF_ERR_GENERIC
> fi
>
> exit $OCF_SUCCESS
> }
>
>
> PureFTPd_stop()
> {
> PureFTPd_status
> if [ $? -eq 0 ] ; then
> pkill pure-ftpd
> exit $OCF_SUCCESS
> fi
Same here, just worse. This will kill ALL pureftpd instances on the
system, not just the one specified.
> exit $OCF_SUCCESS
> }
>
> PureFTPd_monitor() {
>
> PureFTPd_status
> if [ $? = "" ]; then
> return $OCF_NOT_RUNNING
> fi
>
> return $OCF_SUCCESS
> }
And of course, this never will detect a hung ftp server, which might be
something to add.
Thanks,
Lars
--
High Availability & Clustering
SUSE Labs, Research and Development
SUSE LINUX Products GmbH - A Novell Business -- Charles Darwin
"Ignorance more frequently begets confidence than does knowledge"
_______________________________________________________
Linux-HA-Dev: [email protected]
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/
