Subject: [alpine-devel] varnish conf.d / init.d breakage

From: Timo Teras <[email-masked]>
Date: Wed, 30 Oct 2013 10:45:02 +0200

Hi,

I just noticed that varnish broke after upgrade on 2.6-stable.

Seems to be due to this commit:

> commit 7fb51995c49d292b40a4cf2429cafec98ec72f55
> Author: V.Krishn <[email-masked]>
> Date:   Wed May 8 13:21:21 2013 +0000
> 
>     Improved confd and add checkpath, build fixes

While it fixes improves many things (Thank you for that!) it also
breaks certain things. And could be improved further.

> diff --git a/main/varnish/APKBUILD b/main/varnish/APKBUILD
> index 6fafe20..adb1162 100644
> --- a/main/varnish/APKBUILD
> +++ b/main/varnish/APKBUILD
> @@ -50,7 +53,10 @@ package() {
>  		|| return 1
>  	install -Dm644 "$srcdir"/varnishd.logrotate \
>  		"$pkgdir"/etc/logrotate.d/varnishd || return 1
> -	mkdir -p "$pkgdir"/var/log/varnish
> +        install -d -o varnish -g varnish \
> +                "$pkgdir"/var/cache/varnish \
> +                "$pkgdir"/var/log/varnish \
> +                || return 1

I think this should include /var/lib/varnish. Check also white space.

>  	find "$pkgdir" -name *.la -print | xargs rm
>  }
>  
> diff --git a/main/varnish/varnish.pre-install
> b/main/varnish/varnish.pre-install new file mode 100644
> index 0000000..0ce9831
> --- /dev/null
> +++ b/main/varnish/varnish.pre-install
> @@ -0,0 +1,5 @@
> +#!/bin/sh
> +
> +addgroup varnish 2>/dev/null

Should have -S flag for addgroup too.

> +adduser -S -H -h /var/lib/varnish -s /sbin/nologin -D -G varnish
> varnish 2>/dev/null +exit 0
> diff --git a/main/varnish/varnishd.confd b/main/varnish/varnishd.confd
> index 1b46706..1471f82 100644
> --- a/main/varnish/varnishd.confd
> +++ b/main/varnish/varnishd.confd
> @@ -1,12 +1,36 @@
>  # /etc/conf.d/varnishd
>  
> +
> +# Varnish configuration file
> +VARNISH_CONF="/etc/varnish/default.vcl"
> +

Should be default in init.d. Also this should default to default.vcl
for regular init.d. Or if running /etc/init.d/varnish.instanceid, the
vcl should be instanceid.vcl. See e.g. openvpn init.d script on how to
use ${SVCNAME} to accompilish this.

> +VARNISH_SECRET_FILE="/etc/varnish/secret"
> +
> +# Varnish address
> +VARNISH_LISTEN_ADDRESS=
> +VARNISH_PORT=8080
> +
> +# Varnish work files
> +VARNISH_WORKING_DIR="/var/cache/varnish"

This is not needed. See below comments on VARNISHD_OPTS.

> +VARNISH_STORAGE_FILE="/var/cache/varnish/varnish_storage.bin"
> +VARNISH_STORAGE_SIZE=50M
> +VARNISH_STORAGE="file,${VARNISH_STORAGE_FILE},${VARNISH_STORAGE_SIZE}"

Perhaps add commented line with:
VARNISH_STORAGE="malloc,${VARNISH_STORAGE_SIZE}"

> +
> +# PID files
> +VARNISHD_PID_FILE=/var/run/varnish/varnishd.pid
> +VARNISHNCSA_PID_FILE=/var/run/varnish/varnishncsa.pid

I think these should be defaults in init.d script, and not needing
explicit mentioning in conf.d except if overridden. This actually broke
my install: *_PID_FILE was not set in my conf.d and init.d did not
handle that. Please move these to be defaults in init.d. The pid files
should also have ${SVCNAME} in them for non-default instance.

> +# Varnish admin
>  ADMINHOSTPORT="127.0.0.1:65080"
> -CFG_FILE="/etc/varnish/default.vcl"
>  
>  # options passed to varnish on startup
>  # please see the varnishd man page for more options
> -VARNISHD_OPTS="-a 127.0.0.1:8080 -T $ADMINHOSTPORT -f $CFG_FILE"
> +VARNISHD_OPTS="-a ${VARNISH_LISTEN_ADDRESS}:${VARNISH_PORT} -f
> ${VARNISH_CONF} -s ${VARNISH_STORAGE} -n $VARNISH_WORKING_DIR -T
> $ADMINHOSTPORT" + +# add user

Adding "-n $VARNISH_WORKING_DIR" properly relocates varnishd, but it
breaks varnishncsa logging. I think it would be better to not set -n as
the other tools do not support it nicely. IMHO, /var/cache makes sense
for cache file - but for the shm file we really should use /var/lib. So
using -n with /var/cache/... would be technically incorrect too.

Would be nice to add "-i ${SVCNAME}" though.

> +VARNISHD_OPTS="-u varnish $VARNISHD_OPTS"
>  
>  # arguments passed to varnishncsa
>  # please see the varnishncsa man page for more options
>  VARNISHNCSA_ARGS="-c -a -w /var/log/varnish/access.log"

Awkwardly, varnishncsa would need "-n ${SVCNAME}" to match with
varnishd's -i flag.

> diff --git a/main/varnish/varnishd.initd b/main/varnish/varnishd.initd
> index b8c5653..54fb4a0 100755
> --- a/main/varnish/varnishd.initd
> +++ b/main/varnish/varnishd.initd
> @@ -3,6 +3,7 @@
>  # Distributed under the terms of the GNU General Public License v2
>  #
> $Header: /var/cvsroot/gentoo-x86/www-servers/varnish/files/varnishd.initd,v
> 1.7 2009/08/30 06:28:07 hollow Exp $
> +VARNISH_CONF=${VARNISH_CONF:-${CFG_FILE}} extra_commands="reload
> flush" 
>  depend() {
> @@ -11,26 +12,34 @@ depend() {
>  
>  start() {
>  	ebegin "Starting varnish"
> +        checkpath --directory --owner varnish:varnish \
> +	          --mode 755 ${VARNISHD_PID_FILE%/*}
>  	#allow varnishd to lock logfile to memory
>  	ulimit -l 82000
> -	start-stop-daemon --quiet --start
> --pidfile /var/run/varnishd.pid --exec /usr/sbin/varnishd --
> -P /var/run/varnishd.pid ${VARNISHD_OPTS} &> /dev/null
> +	start-stop-daemon --quiet --start \
> +	                  --pidfile ${VARNISHD_PID_FILE} \
> +			  --exec /usr/sbin/varnishd \
> +			  -- -P ${VARNISHD_PID_FILE}
> ${VARNISHD_OPTS} &> /dev/null eend $?
>  
>  	if [ "${VARNISHNCSA_ARGS}" != "" ]; then
>  		ebegin "Starting varnish logging"
> -		start-stop-daemon --quiet --start
> --pidfile /var/run/varnishncsa.pid --exec /usr/bin/varnishncsa -- -D
> -P /var/run/varnishncsa.pid ${VARNISHNCSA_ARGS}
> +		start-stop-daemon --quiet --start \
> +				--pidfile ${VARNISHNCSA_PID_FILE} \
> +				--exec /usr/bin/varnishncsa \
> +				-- -D -P ${VARNISHNCSA_PID_FILE}
> ${VARNISHNCSA_ARGS} eend $?
>  	fi
>  }
>  
>  stop() {
>  	ebegin "Stopping varnish"
> -	start-stop-daemon --quiet --stop
> --pidfile /var/run/varnishd.pid
> +	start-stop-daemon --quiet --stop --pidfile
> ${VARNISHD_PID_FILE} eend $?
>  
> -	if [ -e /var/run/varnishncsa.pid ]; then
> +	if [ -e ${VARNISHNCSA_PID_FILE} ]; then
>  		ebegin "Stopping varnish logging"
> -		start-stop-daemon --quiet --stop
> --pidfile /var/run/varnishncsa.pid
> +		start-stop-daemon --quiet --stop --pidfile
> ${VARNISHNCSA_PID_FILE} eend $?
>  	fi
>  }
> @@ -47,7 +56,7 @@ reload() {
>  
>  	# reload new one
>  	NOW=$(date +%Y%m%d-%H%M%S-%s)
> -	/usr/bin/varnishadm -T $ADMINHOSTPORT vcl.load reload-$NOW
> $CFG_FILE > /dev/null
> +	/usr/bin/varnishadm -T $ADMINHOSTPORT vcl.load reload-$NOW
> ${VARNISH_CONF} > /dev/null /usr/bin/varnishadm -T $ADMINHOSTPORT
> vcl.use  reload-$NOW > /dev/null 
>  	eend $?

If ${SVCNAME} support is added; varnishadm would need -n ${SVCNAME}.

Thanks,
 Timo

---
Unsubscribe:  [email-masked]
Help:         [email-masked]
---

---

From: Natanael Copa <[email-masked]>
Date: Wed, 30 Oct 2013 15:19:38 +0100

On Wed, 30 Oct 2013 10:45:02 +0200

> breaks certain things. And could be improved further.

I have fixes the things you mentioned. Please test.

Thanks!

-nc

---