Hi Jeroen, On Fri, Feb 04, 2011 at 04:16:23PM +0100, [email protected] wrote: > Hi, > > I've updated the resource agent so that it tries to establish an LDAP > connection. Most of the issues where resolved, but I haven't had the time > yet to test with other OSes so I changed /bin/sh to /bin/bash. I'll try to > remove bashisms as soon as possible.
Here's a review of your slapd RA. Took a while, sorry about that. Any news on talking to openldap people? Cheers, Dejan **** > #!/bin/bash > # > # Stand-alone LDAP Daemon (slapd) > # > # Description: Manages Stand-alone LDAP Daemon (slapd) as an OCF resource in > # an high-availability setup. > # > # Author: Jeroen Koekkoek > # License: GNU General Public License (GPL) > # Copyright: (C) 2011 Pagelink B.V. > # > # The OCF code was inspired by the Postfix resource script written by > # Raoul Bhatia <[email protected]>. > # > # The code for managing the slapd instance is based on the the slapd > init > # script found in Debian GNU/Linux 6.0. > # > # OCF parameters: > # OCF_RESKEY_slapd > # OCF_RESKEY_ldapsearch > # OCF_RESKEY_config_file > # OCF_RESKEY_user > # OCF_RESKEY_group > # OCF_RESKEY_services > # OCF_RESKEY_watch_suffix > # OCF_RESKEY_ignore_suffix > # OCF_RESKEY_bind_dn > # OCF_RESKEY_password > # OCF_RESKEY_parameters > # > ################################################################################ > > # Initialization: > > : ${OCF_FUNCTIONS_DIR=${OCF_ROOT}/resource.d/heartbeat} > . ${OCF_FUNCTIONS_DIR}/.ocf-shellfuncs > > : ${OCF_RESKEY_slapd="/usr/sbin/slapd"} > : ${OCF_RESKEY_ldapsearch="/usr/bin/ldapsearch"} > : ${OCF_RESKEY_config_file=""} > : ${OCF_RESKEY_user=""} > : ${OCF_RESKEY_group=""} > : ${OCF_RESKEY_services="ldap:///"} > : ${OCF_RESKEY_watch_suffix=""} > : ${OCF_RESKEY_ignore_suffix=""} > : ${OCF_RESKEY_bind_dn=""} > : ${OCF_RESKEY_password=""} > : ${OCF_RESKEY_parameters=""} > > USAGE="Usage: $0 {start|stop|status|monitor|validate-all|meta-data}" > ORIG_IFS=$IFS > NEWLINE=' > ' > > ################################################################################ > > usage() { > echo $USAGE >&2 > } > > meta_data() > { > cat <<END > <?xml version="1.0"?> > <!DOCTYPE resource-agent SYSTEM "ra-api-1.dtd"> > <resource-agent name="slapd"> > <version>0.1</version> > > <longdesc lang="en"> > Resource script for Stand-alone LDAP Daemon (slapd). It manages a slapd > instance as an OCF resource. > </longdesc> > <shortdesc lang="en">Manages a Stand-alone LDAP Daemon (slapd) > instance</shortdesc> > > <parameters> > > <parameter name="slapd" unique="0" required="0"> > <longdesc lang="en"> > Full path to the slapd binary. > For example, "/usr/sbin/slapd". > </longdesc> > <shortdesc lang="en">Full path to slapd binary</shortdesc> > <content type="string" default="/usr/sbin/slapd" /> > </parameter> > > <parameter name="ldapsearch" unique="0" required="0"> > <longdesc lang="en"> > Full path to the ldapsearch binary. > For example, "/usr/bin/ldapsearch". > </longdesc> > <shortdesc lang="en">Full path to slapd binary</shortdesc> Not slapd, but ldapsearch. > <content type="string" default="/usr/bin/ldapsearch" /> > </parameter> Make this default to just "ldapsearch". It is expected to be in PATH depending on the installation. > <parameter name="config_file" required="0" unique="1"> > <longdesc lang="en"> > Full path to a slapd configuration directory or a slapd configuration file. > For example, "/etc/ldap/slapd.d" or "/etc/ldap/slapd.conf". > </longdesc> > <shortdesc>Full path to configuration directory or file</shortdesc> > <content type="string" default=""/> > </parameter> If this parameter can be a file or a directory, then better change the name. Perhaps just "config". Alternatively, use two parameters (configfile and there's some openldap name I cannot recall now for the cn=config configuration type). Then, exactly one of these parameters must be set. > <parameter name="user" unique="1" required="0"> > <longdesc lang="en"> > User name or id slapd will run with. The group id is also changed to this > user's gid, unless the group parameter is used to override. > </longdesc> > <shortdesc lang="en">User name or id slapd will run with</shortdesc> > <content type="string" default="" /> > </parameter> This is not unique. > <parameter name="group" unique="1" required="0"> > <longdesc lang="en"> > Group name or id slapd will run with. > </longdesc> > <shortdesc lang="en">Group name or id slapd will run with</shortdesc> > <content type="string" default="" /> > </parameter> This is not unique. > <parameter name="services" required="0" unique="1"> > <longdesc lang="en"> > LDAP (and other scheme) URLs slapd will serve. > For example, "ldap://127.0.0.1:389 ldaps:/// ldapi:///" > </longdesc> > <shortdesc>LDAP (and other scheme) URLs to serve</shortdesc> > <content type="string" default="ldap:///"/> > </parameter> Is this unique? Probably. > <parameter name="watch_suffix" required="0" unique="1"> > <longdesc lang="en"> > Suffix (database backend) that will be monitored for availability. Multiple > suffixes can be specified by providing a space seperated list. By providing > one > or more suffixes here, the ignore_suffix parameter is discarded. All suffixes > will be monitored if left blank. > </longdesc> > <shortdesc>Suffix that will be monitored for availability.</shortdesc> > <content type="string" default=""/> > </parameter> Not unique if it can be blank. > <parameter name="ignore_suffix" required="0" unique="1"> > <longdesc lang="en"> > Suffix (database backend) that will not be monitored for availability. > Multiple > suffixes can be specified by providing a space seperated list. No suffix will > be excluded if left blank. > </longdesc> > <shortdesc>Suffix that will not be monitored for availability.</shortdesc> > <content type="string" default=""/> > </parameter> Not unique. > <parameter name="bind_dn" required="0" unique="1"> > <longdesc lang="en"> > Distinguished Name used to bind to the LDAP directory. Leave blank to bind to > the LDAP directory anonymously. > </longdesc> > <shortdesc>Distinguished Name used to bind to the LDAP directory.</shortdesc> > <content type="string" default=""/> > </parameter> > > <parameter name="password" required="0" unique="1"> > <longdesc lang="en"> > Password used to bind to the LDAP directory. > </longdesc> > <shortdesc>Password used to bind to the LDAP directory.</shortdesc> > <content type="string" default=""/> Bind_dn/password for testing? If so, then add that to the description. Also does not have to be unique. > <parameter name="parameters" unique="0" required="0"> > <longdesc lang="en"> > slapd may be called with additional parameters. > Specify any of them here. > </longdesc> > <shortdesc lang="en"></shortdesc> No shortdesc. > <content type="string" default="" /> > </parameter> > > </parameters> > > <actions> > <action name="start" timeout="20s" /> > <action name="stop" timeout="20s" /> > <action name="monitor" depth="0" timeout="20s" interval="60s" /> Did you consider the timeouts or are these copies from another agent? They should be set to the minimum suggested values. > <action name="validate-all" timeout="20s" /> > <action name="meta-data" timeout="5s" /> > </actions> > </resource-agent> > END > } > > terminate() > { > local pid=$1 > local signal=$2 > local recheck=$3 > local result > > if [ -z "$recheck" ]; then > recheck=1 > fi > > kill -$signal $pid >/dev/null 2>&1; result=$? > > if [ $result -eq 0 ] && [ $recheck -gt 0 ]; then > # Grant some time for shutdown and recheck n times. > local i=0 > > while [ $result -eq 0 ] && [ $i -lt $recheck ]; do > kill -0 $pid >/dev/null 2>&1; result=$? > let "i += 1" > > if [ $result -eq 0 ]; then > sleep 1 > fi > done > else > result=1 > fi > > if [ $result -ne 0 ]; then > return 0 > fi > > return 1 > } > > watch_suffix() > { > local result > > if [ -n "$OFC_RESKEY_watch_suffix" ]; then > if echo "'$OCF_RESKEY_watch_suffix'" | grep "'$1'" >/dev/null 2>&1; then > result=0 > else > result=1 > fi > else > if echo "'$OCF_RESKEY_ignore_suffix'" | grep "'$1'" >/dev/null 2>&1; then > result=1 > else > result=0 > fi > fi > > return $result > } > > slapd_pid() > { > local pid > > if [ -f "$pid_file" ]; then > pid=`head -n 1 "$pid_file" 2>/dev/null` > > if [ -n "$pid" ]; then > echo "$pid" > return $OCF_SUCCESS > fi > > ocf_log err "slapd pid file '$pid_file' empty." > return $OCF_ERR_GENERIC > fi > > ocf_log info "slapd pid file '$pid_file' does not exist." > return $OCF_NOT_RUNNING > } > > slapd_status() > { > local pid > local state > > pid=`slapd_pid`; state=$? > > if [ $state -eq $OCF_SUCCESS ]; then > > if ! kill -0 $pid >/dev/null 2>&1; then > return $OCF_NOT_RUNNING > else > echo "$pid" > return $OCF_SUCCESS > fi > fi > > return $state > } This function does unexpected things, i.e. it echos the pid. That's confusing. I suggest to do something like this in callers of slapd_status(): slapd_status `slapd_pid` and to save pid in a local var only in terminate which is actually using it. > slapd_start() > { > local options > local pid > local reason > local result > local state > > pid=`slapd_status`; state=$? > > if [ $state -eq $OCF_SUCCESS ]; then > ocf_log info "slapd already running." > return $state > elif [ $state -eq $OCF_ERR_GENERIC ]; then > ocf_log err "slapd returned error." Is this message necessary? It can only happen if the pid file is empty. > return $state > fi > > options="-u $user -g $group" In the meta-data neither of these two parameters are required. To match this code they should be. > if [ -d "$config_file" ]; then > options="$options -F $config_file" > else > options="$options -f $config_file" > fi > > if [ -n "$parameters" ]; then > options="$options $parameters" > fi > > if [ -n "$services" ]; then > $slapd -h "$services" $options 2>&1; result=$? > else > $slapd $options 2>&1; result=$? > fi > > if [ $result -ne 0 ]; then > ocf_log err "slapd returned error." > > return $OCF_ERR_GENERIC > fi > > ocf_log info "slapd started." > > return $OCF_SUCCESS > } > > slapd_stop() > { > local pid > local result > local state > > pid=`slapd_status`; state=$? > > if [ $state -eq $OCF_NOT_RUNNING ]; then > ocf_log info "slapd already stopped." > return $OCF_SUCCESS > elif [ $state -eq $OCF_ERR_GENERIC ]; then > ocf_log err "slapd returned error." Is this message necessary? This branch can only happen if the pid file is empty. > return $state > fi > > terminate $pid 15 5; result=$? > if [ $result -ne 0 ]; then > ocf_log err "slapd failed to stop. Escalating to KILL." > > terminate $pid 9 5; result=$? > if [ $result -ne 0 ]; then > > ocf_log err "slapd failed to stop." > return $OCF_ERR_GENERIC The best practice is to loop while status returns true and let the higher layer decide when the operation was taking too long. Theoreticaly you can't say how long a process will take to cleanup. > fi > fi > > if [ -f "$pid_file" ]; then > rm -f "$pid_file" >/dev/null 2>&1 > fi > > ocf_log info "slapd stopped." > return $OCF_SUCCESS > } > > slapd_monitor() > { > local options > local pid > local result > local state > local suffix > local suffixes > > pid=`slapd_status`; state=$? > if [ $state -eq $OCF_NOT_RUNNING ]; then > ocf_log debug "slapd is stopped." > return $state > elif [ $state -ne $OCF_SUCCESS ]; then > ocf_log err "slapd returned error." > return $state > fi > > if [ -d "$config_file" ]; then > for suffix in `find "$config_file"/'cn=config' -type f -name olcDatabase* > -exec \ > sed -ne > 's/^[[:space:]]*olcSuffix:[[:space:]]\+\(.\+\)/\1/p' {} \;` > do > suffix=${suffix#\"*} > suffix=${suffix%\"*} > > if watch_suffix $suffix; then > suffixes="$suffixes $suffix" > fi > done > > elif [ -f "$config_file" ]; then > for suffix in `sed -ne 's/^[[:space:]]*suffix[[:space:]]\+\(.\+\)/\1/p' > "$config_file"` > do > suffix=${suffix#\"*} > suffix=${suffix%\"*} > > if watch_suffix $suffix; then > suffixes="$suffixes $suffix" > fi These two loop bodies are the same. > done > fi > > options="-LLL -s base -x" > > if [ -n "$bind_dn" ]; then > options="$options -D '$bind_dn' -w '$password'" > fi > > for suffix in $suffixes; do > ldapsearch -H "$services" -b "$suffix" $options >/dev/null 2>&1; result=$? > > case "$result" in > "0") > ocf_log debug "slapd database with suffix '$suffix' reachable" > ;; > "49") > ocf_log err "slapd database with suffix '$suffic' unreachable. > Invalid credentials." '$suffic' -> '$suffix' > return $OCF_ERR_CONFIGURED > ;; > *) > ocf_log err "slapd database with suffix '$suffix' unreachable." > state=$OCF_ERR_GENERIC > ;; > esac > done > > return $state > } > > slapd_validate_all() > { > if [ ! -x $slapd ]; then > ocf_log err "slapd binary '$slapd' does not exist or cannot be executed." > return $OCF_ERR_INSTALLED > fi > > if [ ! -x $ldapsearch ]; then > ocf_log err "slapd binary '$ldapsearch' does not exist or cannot be > executed." > return $OCF_ERR_INSTALLED > fi check_binary? > if [ ! -d $config_file ] && [ ! -f $config_file ]; then > ocf_log err "slapd configuration file '$config_file' does not exist." > return $OCF_ERR_INSTALLED > fi > > if ! id "$user" >/dev/null 2>&1; then > ocf_log err "slapd user '$user' does not exist $ret" > return $OCF_ERR_INSTALLED > fi > > if ! grep "^$group:" /etc/group >/dev/null 2>&1; then > ocf_log err "slapd group '$group' does not exist" > return $OCF_ERR_INSTALED > fi > > return $OCF_SUCCESS > } > > # > # slapd_validate_dirs: retrieves all slapd database directories from the > # configuration file(s) and verifies existence and > # permissions. > # > slapd_validate_dirs() > { > local dir > local dirs > local groups=`groups $user | sed -ne 's/^[^:]\+: \(.*\)$/\1/p' | sed 's/ > \+/\\\|/' 2>/dev/null` > local perms > local result > local state > > > [ "$user" = "root" ] && return $OCF_SUCCESS > > IFS=$NEWLINE > > if [ -d "$config_file" ]; then > for dir in `find "$config_file"/'cn=config' -type f -name olcDatabase* > -exec \ > sed -ne 's/^olcDbDirectory:[[:space:]]\+\(.\+\)/\1/p' {} \;` > do > dir=${dir#\"*} > dir=${dir%\"*} > dirs="$dirs$NEWLINE$dir" > done > elif [ -f "$config_file" ]; then > for dir in `sed -ne 's/^directory[[:space:]]\+\(.\+\)/\1\n/p' > "$config_file"` > do > dir=${dir#\"*} > dir=${dir%\"*} > dirs="$dirs$NEWLINE$dir" > done > fi > > state=$OCF_SUCCESS > > for dir in $dirs; do > IFS=$ORIG_IFS > > perms=`stat -c "%U:%G:%a" "$dir"`; result=$? > > > if [ $result -eq 0 ]; then > > echo "$perms" | grep "$user:[^:]\+:7.." >/dev/null 2>&1; result=$? > > if [ $result -ne 0 ]; then > > if [ -n "$groups" ]; then > echo "$perms" | grep "[^:]\+:\($group\|$groups\):.7." >/dev/null > 2>&1; result=$? > else > echo "$perms" | grep "[^:]\+:$group:.7." >/dev/null 2>&1; result=$? > fi > > > if [ $result -ne 0 ]; then > > echo "$perms" | grep ":..7" >/dev/null 2>&1; result=$? > > if [ $result -ne 0 ]; then > state=$OCF_ERR_GENERIC > ocf_log err "slapd data directory '$dir' is not writable." > else > ocf_log warn "slapd data directory '$dir' is world writable. Mode > 0700 recommended." > fi > else > ocf_log warn "slapd data directory '$dir' is group writable. Mode > 0700 recommended." > fi > > else > ocf_log warn "slapd data directory '$dir' is accessible by others. > Mode 0700 recommended." > fi > fi > > IFS=$NEWLINE > done > > IFS=$ORIG_IFS > > return $state > } I still find checking directories superfluous. > # > # Main > # > > slapd=$OCF_RESKEY_slapd > ldapsearch=$OCF_RESKEY_ldapsearch > config_file=$OCF_RESKEY_config_file > user=$OCF_RESKEY_user > group=$OCF_RESKEY_group > services=$OCF_RESKEY_services > bind_dn=$OCF_RESKEY_bind_dn > password=$OCF_RESKEY_password > parameters=$OCF_RESKEY_parameters > pid_file='' > > if [ -z "$config_file" ]; then > if [ -e "/etc/ldap/slapd.d" ]; then > config_file="/etc/ldap/slapd.d" > else > config_file="/etc/ldap/slapd.conf" > fi > fi > > if [ -d "$config_file" ]; then > pid_file=`sed -ne \ > 's/^olcPidFile:[[:space:]]\+\(.\+\)[[:space:]]*/\1/p' \ > "$config_file"/'cn=config.ldif' 2>/dev/null` > elif [ -f "$config_file" ]; then > pid_file=`sed -ne \ > 's/^pidfile[[:space:]]\+\(.\+\)/\1/p' \ > "$config_file" 2>/dev/null` > fi It would be good to move parsing of configuration files to separate functions. > if test -z "$user"; then > user=`id -nu 2>/dev/null` > elif ocf_is_decimal "$user"; then > user=`sed -ne "s/^\([^:]\+\):[^:]\+:$user:.*/\1/p" /etc/passwd 2>/dev/null` > fi > > if test -z "$group"; then > group=`id -ng 2>/dev/null` > elif ocf_is_decimal "$group"; then > group=`sed -ne "s/^\([^:]\+\):[^:]\+:$group:.*/\1/p" /etc/group 2>/dev/null` > fi This won't work if the passwd/group databases are for instance in LDAP ;-) Best to keep it simple and just require users to provide names. > if [ $# -ne 1 ]; then > usage > exit $OCF_ERR_ARGS > fi > > case $1 in > meta-data) > meta_data > exit $OCF_SUCCESS > ;; > usage|help) > usage > exit $OCF_SUCCESS > ;; > esac > > slapd_validate_all > > [ $? -eq $OCF_SUCCESS ] || exit $? > > case $1 in > status) > pid=`slapd_status`; state=$? > > if [ $state -eq $OCF_SUCCESS ]; then > ocf_log debug "slapd is running." > elif [ $state -eq $OCF_NOT_RUNNING ]; then > ocf_log debug "slapd is stopped." > fi > > exit $state > ;; > start) > slapd_start > exit $? > ;; > stop) > slapd_stop > exit $? > ;; > monitor) > pid=`slapd_monitor`; state=$? > exit $? > ;; > validate-all) > slapd_validate_dirs > exit $? > ;; > *) > usage > exit $OCF_ERR_UNIMPLEMENTED > ;; > esac _______________________________________________________ Linux-HA-Dev: [email protected] http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev Home Page: http://linux-ha.org/
