Hi Tuomas,

Tuomas Noraef <tnor...@gmail.com> writes:

> While being in the process of evaluating bcfg2, I came across its
> Debian init scripts, which are IMHO a complete mess. Sorry to come to
> use these terms, but I am really being honest... I hardly believe such
> bugs (really release critical, I guess, as I can't imagine even
> remotely using bcfg2, or, more simply, can't use it, with init scripts
> in this state) have not been reported yet.


I did not write the bcfg2 init scripts, and hadn't really even looked at them
much before this bug report. I would have noticed at least few of these issues
if I had.

Bcfg2 does not have many users on Debian, so noticing bugs may take time. Also
I am not running the squeeze version of the server in production (I am running
that version of the client, and the lenny version of the server, which does
not show most of these issues).

I went through the issues you reported and the script itself, and decided to
rewrite it in a more debian-like way (using start-stop-daemon instead of the
lsb functions, etc) instead of trying to fix the current one. I'm attaching my
version as a patch to this mail, I would appreciate it if you could test it
and make sure that all of the reported problems are fixed by it.

Also it appears that the bugs with server shutdown are currently thought to be
fixed upstream, but the fixes aren't easy to backport to the current stable
version. I'll have a closer look at this, but it may be that we will need to
wait for 1.1 to be released until this is properly fixed and just kill the
daemon from the init script until that happens.

I will also sort out the situation with the bcfg2 client init script, probably
by removing it and noting the removal in NEWS.Debian before uploading a new
version. 

PS. Please use the unified diff format (diff -u) when sending patches, it's
quite a bit more readable.

-- 
Arto Jantunen

--- bcfg2-server.orig	2010-03-30 10:20:45.000000000 +0300
+++ bcfg2-server	2010-08-02 15:45:21.291754838 +0300
@@ -1,112 +1,135 @@
-#!/bin/sh
-#
-# bcfg-server - Bcfg2 configuration daemon
-#
-# chkconfig: 2345 19 81
-# description: bcfg2 server for configuration requests
-#
+#! /bin/sh
 ### BEGIN INIT INFO
 # Provides:          bcfg2-server
-# Required-Start:    $network $remote_fs $named
-# Required-Stop:     $network $remote_fs $named
+# Required-Start:    $network $remote_fs $named $syslog
+# Required-Stop:     $network $remote_fs $named $syslog
 # Default-Start:     2 3 4 5
 # Default-Stop:      0 1 6
 # Short-Description: Configuration management Server
-# Description:       Bcfg2 is a configuration management system that builds
-#                    installs configuration files served by bcfg2-server
+# Description:       The server component of the Bcfg2 configuration management
+#                    system
 ### END INIT INFO
 
-# Include lsb functions
-. /lib/lsb/init-functions
+# Author: Arto Jantunen <vi...@debian.org>
+
+# PATH should only include /usr/* if it runs after the mountnfs.sh script
+PATH=/sbin:/usr/sbin:/bin:/usr/bin
+DESC="Configuration management server"
+NAME=bcfg2-server
+DAEMON=/usr/sbin/$NAME
+PIDFILE=/var/run/$NAME.pid
+DAEMON_ARGS="-D $PIDFILE"
+SCRIPTNAME=/etc/init.d/$NAME
 
-# Commonly used stuff
-DAEMON=/usr/sbin/bcfg2-server
-PIDFILE=/var/run/bcfg2-server.pid
-PARAMS="-D $PIDFILE"
+# Exit if the package is not installed
+[ -x "$DAEMON" ] || exit 0
 
 # Disabled per default
 BCFG2_SERVER_OPTIONS=""
 BCFG2_SERVER_ENABLED=0
 
-# Include default startup configuration if exists
-test -f "/etc/default/bcfg2-server" && . /etc/default/bcfg2-server
+# Read configuration variable file if it is present
+[ -r /etc/default/$NAME ] && . /etc/default/$NAME
+
+# Load the VERBOSE setting and other rcS variables
+. /lib/init/vars.sh
+
+# Define LSB log_* functions.
+# Depend on lsb-base (>= 3.2-14) to ensure that this file is present
+# and status_of_proc is working.
+. /lib/lsb/init-functions
 
 if [ "$BCFG2_SERVER_ENABLED" -eq 0 ] ; then
      log_failure_msg "bcfg2-server is disabled - see /etc/default/bcfg2-server"
      exit 0
 fi
 
-# Exit if $DAEMON doesn't exist and is not executable
-test -x $DAEMON || exit 5
-
-# Internal variables
-BINARY=$(basename $DAEMON)
-
-start () {
-    echo -n "Starting Configuration Management Server: "
-    start_daemon ${DAEMON} ${PARAMS} ${BCFG2_SERVER_OPTIONS}
-    STATUS=$?
-    if [ "$STATUS" = 0 ]
-    then
-        log_success_msg "bcfg2-server"
-        test -d /var/lock/subsys && touch /var/lock/subsys/bcfg2-server
-    else
-        log_failure_msg "bcfg2-server"
-    fi
-    return $STATUS
-}
-
-stop () {
-    echo -n "Stopping Configuration Management Server: "
-    killproc -p $PIDFILE ${BINARY}
-    STATUS=$?
-    if [ "$STATUS" = 0 ]; then
-      log_success_msg "bcfg2-server"
-      test -d /var/lock/subsys && touch /var/lock/subsys/bcfg2-server
-    else
-      log_failure_msg "bcfg2-server"
-    fi
-    return $STATUS
+#
+# Function that starts the daemon/service
+#
+do_start()
+{
+	# Return
+	#   0 if daemon has been started
+	#   1 if daemon was already running
+	#   2 if daemon could not be started
+	start-stop-daemon --start --quiet --pidfile $PIDFILE --name $NAME \
+	    --startas $DAEMON --test > /dev/null || return 1
+	start-stop-daemon --start --quiet --pidfile $PIDFILE --name $NAME \
+	    --startas $DAEMON -- $DAEMON_ARGS || return 2
 }
 
-status () {
-    # Inspired by redhat /etc/init.d/functions status() call
-    PID=$(pidof -x $BINARY)
-    if [ -n "$PID" ]; then
-      echo "$BINARY (pid $PID) is running..."
-      return 0
-    fi
-
-    if [ -f $PIDFILE ]; then
-      if [ -n "$PID" ]; then
-        log_failure_msg "$BINARY dead but pid file exists..."
-        return 1
-      fi
-    fi
-
-    log_failure_msg "$BINARY is not running"
-    return 3
+#
+# Function that stops the daemon/service
+#
+do_stop()
+{
+	# Return
+	#   0 if daemon has been stopped
+	#   1 if daemon was already stopped
+	#   2 if daemon could not be stopped
+	#   other if a failure occurred
+	start-stop-daemon --stop --quiet --retry=TERM/15/KILL/5 --pidfile \
+	    $PIDFILE --name $NAME
+	RETVAL="$?"
+	[ "$RETVAL" = 2 ] && return 2
+	# Many daemons don't delete their pidfiles when they exit.
+	rm -f $PIDFILE
+	return "$RETVAL"
 }
 
 case "$1" in
-    start)
-        start
-    ;;
-    stop)
-        stop
-    ;;
-    status)
-        status
-    ;;
-    restart|reload|force-reload)
-        stop
-        sleep 5
-        start
-    ;;
-    *)
-        log_success_msg "Usage: $0 {start|stop|status|reload|restart|force-reload}"
-        exit 1
-    ;;
+  start)
+	log_daemon_msg "Starting $DESC" "$NAME"
+	do_start
+	case "$?" in
+		0|1) log_end_msg 0 ;;
+		2) log_end_msg 1 ;;
+	esac
+	;;
+  stop)
+	log_daemon_msg "Stopping $DESC" "$NAME"
+	do_stop
+	case "$?" in
+		0|1) log_end_msg 0 ;;
+		2) log_end_msg 1 ;;
+	esac
+	;;
+  status)
+       start-stop-daemon --start --quiet --pidfile $PIDFILE --name $NAME \
+       --startas $DAEMON --test > /dev/null 
+       RETVAL="$?"
+       if [ "$RETVAL" = 0 ]; then
+	   echo "$NAME is not running"
+	   exit 3
+       else
+	   echo "$NAME is running"
+	   exit 0
+       fi
+       ;;
+  restart|force-reload)
+	log_daemon_msg "Restarting $DESC" "$NAME"
+	do_stop
+	case "$?" in
+	  0|1)
+		do_start
+		case "$?" in
+			0) log_end_msg 0 ;;
+			1) log_end_msg 1 ;; # Old process is still running
+			*) log_end_msg 1 ;; # Failed to start
+		esac
+		;;
+	  *)
+	  	# Failed to stop
+		log_end_msg 1
+		;;
+	esac
+	;;
+  *)
+	#echo "Usage: $SCRIPTNAME {start|stop|restart|reload|force-reload}" >&2
+	echo "Usage: $SCRIPTNAME {start|stop|status|restart|force-reload}" >&2
+	exit 3
+	;;
 esac
 
-exit 0
+:

Reply via email to