Debian: /var/run/consul/consul.pid user affinity inconsistent

[weitzj]

The init template for Debian uses start-stop-daemon to create a pidfile owned by the daemon user root. https://github.com/solarkennedy/puppet-consul/blob/master/templates/consul.debian.erb#L57

But:

It passes the -pid-file parameter along to the consul agent. See: https://github.com/solarkennedy/puppet-consul/blob/master/templates/consul.debian.erb#L21

This causes the launch of the consul agent to fail, since it cannot write to /var/run/consul/consul.pid using the user:group => consul:consul

There are 2 solutions:

• Make /var/run/consul/consul.pid writeable by the consul user, which runs consul agent
• Let start-stop-daemon write the pid to /var/run/consul/consul.pid (this means removing the pid-file param from consul agent

[weitzj]

The PR implements the second solution, since NGINX does it this way, too. It also fixes a status check bug.

[EvanKrall]

Couldn't you instead remove --make-pidfile from the start-stop-daemon args? Is it actually preferable to have start-stop-daemon manage the pidfile?

[weitzj]

Yeah, we could do that. I was just inspired by the nginx file on my debian machine, which looks almost the same as the current puppet-consul debian file. https://github.com/solarkennedy/puppet-consul/blob/master/templates/consul.debian.erb#L21

Do you want me to implement the other solution instead?

[EvanKrall]

I don't really have strong opinions either way.

[weitzj]

Nginx script Debine Weezy

e.g.: https://blog.luukhendriks.eu/2013/05/17/nginx-initscript-debian-7.html

#!/bin/sh

### BEGIN INIT INFO
# Provides:          nginx
# Required-Start:    $local_fs $remote_fs $network $syslog
# Required-Stop:     $local_fs $remote_fs $network $syslog
# Default-Start:     2 3 4 5
# Default-Stop:      0 1 6
# Short-Description: starts the nginx web server
# Description:       starts nginx using start-stop-daemon
### END INIT INFO

PATH=/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin
DAEMON=/usr/sbin/nginx
NAME=nginx
DESC=nginx

# Include nginx defaults if available
if [ -f /etc/default/nginx ]; then
    . /etc/default/nginx
fi

test -x $DAEMON || exit 0

set -e

. /lib/lsb/init-functions

test_nginx_config() {
    if $DAEMON -t $DAEMON_OPTS >/dev/null 2>&1; then
        return 0
    else
        $DAEMON -t $DAEMON_OPTS
        return $?
    fi
}

case "$1" in
    start)
        echo -n "Starting $DESC: "
        test_nginx_config
        # Check if the ULIMIT is set in /etc/default/nginx
        if [ -n "$ULIMIT" ]; then
            # Set the ulimits
            ulimit $ULIMIT
        fi
        start-stop-daemon --start --quiet --pidfile /var/run/$NAME.pid \
            --exec $DAEMON -- $DAEMON_OPTS || true
        echo "$NAME."
        ;;

    stop)
        echo -n "Stopping $DESC: "
        start-stop-daemon --stop --quiet --pidfile /var/run/$NAME.pid \
            --exec $DAEMON || true
        echo "$NAME."
        ;;

    restart|force-reload)
        echo -n "Restarting $DESC: "
        start-stop-daemon --stop --quiet --pidfile \
            /var/run/$NAME.pid --exec $DAEMON || true
        sleep 1
        test_nginx_config
        # Check if the ULIMIT is set in /etc/default/nginx
        if [ -n "$ULIMIT" ]; then
            # Set the ulimits
            ulimit $ULIMIT
        fi
        start-stop-daemon --start --quiet --pidfile \
            /var/run/$NAME.pid --exec $DAEMON -- $DAEMON_OPTS || true
        echo "$NAME."
        ;;

    reload)
        echo -n "Reloading $DESC configuration: "
        test_nginx_config
        start-stop-daemon --stop --signal HUP --quiet --pidfile /var/run/$NAME.pid \
            --exec $DAEMON || true
        echo "$NAME."
        ;;

    configtest|testconfig)
        echo -n "Testing $DESC configuration: "
        if test_nginx_config; then
            echo "$NAME."
        else
            exit $?
        fi
        ;;

    status)
        status_of_proc -p /var/run/$NAME.pid "$DAEMON" nginx && exit 0 || exit $?
        ;;
    *)
        echo "Usage: $NAME {start|stop|restart|reload|force-reload|status|configtest}" >&2
        exit 1
        ;;
esac

exit 0

[weitzj]

We could actually implement a real reload command, sending HUP, which is missing in the current solution

[EvanKrall]

Real reload would be nice too, but doesn't necessarily have to happen in the same PR.

[weitzj]

Yes. One should not highjack this on here. :)

[weitzj]

Please review again. Added CHANGELOG

[EvanKrall]

Merged! Thanks for the PR.