www/varnish4: Fix race condition on start

varnishlog and varnishncsa had a race condition with varnishd on
startup as they will notice varnishd's leftover _.vsm file and try to
connect to varnishd but occasionally fail due to varnishd not being
fully operational yet. By adding "-t off" flag as default to varnishncsa
and varnishlog they will wait indefinitely for varnishd to start up
instead of just exiting.

Other bugs fixed:

* varnishncsa_logformat in rc.conf mostly works

Unfortunately due to shell expansion we still cannot pass
quotes (") so if you need those in your log format you will have to
manage varnishncsa startup yourself. I apologize, but there's not much
we can do at the moment. I'm working with upstream to allow loading
configuration from a file. Note that Debian has complained too...

* Default permissions for log and pidfiles have been corrected

They are now correctly set to 664 for pidfiles and 644 for log files

* Description of default flags for varnishncsa and varnishlog have been
adjusted to reflect reality

New features:

* We now support "checkconfig" and "reload" options like on RedHat
distributions

You can do "service varnishd checkconfig" to validate your vcl is
functional. Doing a "service varnishd reload" also works without
interrupting service and creates a new vcl reflecting the date+timestamp.
Varnishd now does a checkconfig before starting and will also prevent
you from breaking a running instance by doing a "restart" with an
invalid vcl.

Thanks to many for the reports and testing.

Differential Revision:	https://reviews.freebsd.org/D4016

4 files changed, 76 insertions, 57 deletions

diff --git a/www/varnish4/Makefile b/www/varnish4/Makefile
index 4d06d5b0e414..cbd491e07a1b 100644
--- a/www/varnish4/Makefile
+++ b/www/varnish4/Makefile
@@ -2,7 +2,7 @@
 
 PORTNAME=	varnish
 PORTVERSION=	4.1.0
-PORTREVISION=	1
+PORTREVISION=	2
 CATEGORIES=	www
 MASTER_SITES=	http://repo.varnish-cache.org/source/
 PKGNAMESUFFIX=	4
diff --git a/www/varnish4/files/varnishd.in b/www/varnish4/files/varnishd.in
index 270204cd8f8b..6e75a96a700b 100644
--- a/www/varnish4/files/varnishd.in
+++ b/www/varnish4/files/varnishd.in
@@ -65,6 +65,11 @@ load_rc_config ${name}
 
 command="%%PREFIX%%/sbin/${name}"
 pidfile="${varnishd_pidfile}"
+configtest_cmd="varnishd_checkconfig"
+reload_cmd="varnishd_reload"
+restart_precmd="varnishd_checkconfig"
+start_precmd="varnishd_precmd"
+extra_commands="status reload configtest"
 
 if [ -n "${varnishd_config}" ] ; then
 	: ${varnishd_flags:="-j unix,user=${varnishd_jailuser} -P ${varnishd_pidfile} -a ${varnishd_listen} -T ${varnishd_admin} -f ${varnishd_config} -s ${varnishd_storage} -h ${varnishd_hash} ${varnishd_extra_flags}"}
@@ -72,4 +77,52 @@ else
 	: ${varnishd_flags:="-j unix,user=${varnishd_jailuser} -P ${varnishd_pidfile} -a ${varnishd_listen} -T ${varnishd_admin} -b ${varnishd_backend} -s ${varnishd_storage} -h ${varnishd_hash} ${varnishd_extra_flags}"}
 fi
 
+varnishd_checkconfig()
+{
+	if [ -z "${varnishd_config}" ]; then
+		echo "${name}: nothing to check, no configuration file defined, builtin VCL used"
+	else
+		echo "Performing sanity check on ${name} configuration:"
+		if eval ${command} -C -f "${varnishd_config}" 2> /dev/null ; then
+			echo "${name}: the configuration file ${varnishd_config} syntax is ok"
+		else
+			err 1 "${name}: the configuration file ${varnishd_config} syntax is NOT ok"
+		fi
+	fi
+}
+
+# Adapted from work done by Ingvar Hagelund <ingvar@redpill-linpro.com> (see redhat/varnish_reload_vcl)
+varnishd_reload()
+{
+	local _current_config_name _new_config_name _varnishadm_cmd
+
+	if [ -z "${varnishd_config}" ]; then
+		echo "${name}: nothing to reload, no configuration file defined, builtin VCL used"
+	else
+		_new_config_name="rc.d_reloaded@$(date +%Y%m%d%H%M%S)"
+		_varnishadm_cmd="%%PREFIX%%/bin/varnishadm ${varnish_cli_flags}"
+		if ! eval ${_varnishadm_cmd} vcl.list > /dev/null; then
+			err 1 "${name}: can't connect to varnishadm"
+		fi
+		_current_config_name=$(${_varnishadm_cmd} vcl.list | awk ' /^active/ { print $3 } ')
+		if ! eval ${_varnishadm_cmd} vcl.load ${_new_config_name} ${varnishd_config} > /dev/null; then
+			err 1 "${name}: vcl.load failed, you're still using previous rules (${_current_config_name})"
+		fi
+		if eval ${_varnishadm_cmd} vcl.use ${_new_config_name} > /dev/null; then
+			echo "VCL file \"${varnishd_config}\" has been successfully loaded as \"${_new_config_name}\""
+			echo "To remove previous loaded configurations, you should run \"${_varnishadm_cmd} vcl.discard <configname>\" by yourself"
+		else
+			err 1 "${name}: vcl.use failed, you're still using previous rules (${_current_config_name})"
+		fi
+	fi
+
+	return 0
+}
+
+varnishd_precmd()
+{
+	# Check config before starting
+	varnishd_checkconfig
+}
+
 run_rc_command "$1"
diff --git a/www/varnish4/files/varnishlog.in b/www/varnish4/files/varnishlog.in
index dc29a0d6c056..5da94cca8216 100644
--- a/www/varnish4/files/varnishlog.in
+++ b/www/varnish4/files/varnishlog.in
@@ -21,12 +21,12 @@
 #	default: "/var/log/varnish.log"
 #
 # varnishlog_flags - command line arguments.
-#	default: "-D -P ${varnishlog_pidfile} -a -w ${varnishlog_file}"
+#	default: "-t off -P ${varnishlog_pidfile} -D -a -A -w ${varnishlog_file}"
 #
 # Add the following line to /etc/newsyslog.conf to rotate the log file
 # once a day:
 #
-# /var/log/varnish.log 640 7 * @T00 JB /var/run/varnishlog.pid
+# /var/log/varnish.log varnishlog:varnish 640 7 * @T00 JB /var/run/varnishlog.pid
 #
 # See varnishlog(1) for a detailed overview of command-line options.
 #
@@ -40,43 +40,26 @@ load_rc_config ${name}
 : ${varnishlog_enable:=NO}
 : ${varnishlog_pidfile=/var/run/${name}.pid}
 : ${varnishlog_file=/var/log/varnish.log}
-: ${varnishlog_flags="-P ${varnishlog_pidfile} -D -a -A -w ${varnishlog_file}"}
-: ${varnishlog_user=varnishlog}
+: ${varnishlog_flags="-t off -P ${varnishlog_pidfile} -D -a -A -w ${varnishlog_file}"}
 
-command="%%PREFIX%%/bin/${name}"
+procname="%%PREFIX%%/bin/${name}"
+command="/usr/sbin/daemon"
+command_args="-f -u varnishlog ${procname} ${varnishlog_flags}"
 pidfile=${varnishlog_pidfile}
 start_precmd=precmd
 
 precmd()
 {
+        # varnishlog_flags gets applied too early if we don't do this.
+        rc_flags=""
+
 	if [ ! -e ${pidfile} ]; then
-		install -o ${varnishlog_user} -g varnish /dev/null ${pidfile};
+		install -o varnishlog -g varnish -m 644 /dev/null ${pidfile};
 	fi
 
 	if [ ! -e ${varnishlog_file} ]; then
-		install -o ${varnishlog_user} -g varnish /dev/null ${varnishlog_file};
+		install -o varnishlog -g varnish -m 640 /dev/null ${varnishlog_file};
 	fi
-
-	waitcount=0
-	vsm=%%PREFIX%%/varnish/$(hostname)/_.vsm
-	
-        # wait no longer than 5s for  _.vsm file to arrive from varnishd
-	while :
-	do
-		if [ -f ${vsm} ] ; then
-			break
-		else
-			case "${waitcount}" in
-				5)	echo "${name}: Failed to start. Missing ${vsm} ?"
-					break
-					;;
-				*)	echo "${name}: waiting for ${vsm}"
-					waitcount=$((waitcount+1))
-					sleep 1
-					;;
-			esac
-		fi
-	done
 }
 
 run_rc_command "$1"
diff --git a/www/varnish4/files/varnishncsa.in b/www/varnish4/files/varnishncsa.in
index c03ad60ba7d6..74cfb6c03b44 100644
--- a/www/varnish4/files/varnishncsa.in
+++ b/www/varnish4/files/varnishncsa.in
@@ -21,7 +21,7 @@
 #	default: "/var/log/varnishncsa.log"
 #
 # varnishncsa_flags - command line arguments.
-#	default: "-D -P ${varnishncsa_pidfile} -a -w ${varnishncsa_file}${varnishncsa_logformat:+ -P \"$varnishncsa_logformat\""
+#	default: "-t off -P ${varnishncsa_pidfile} -D -a -w ${varnishncsa_file}${varnishncsa_logformat:+ -F \"$varnishncsa_logformat\"}"
 #
 # varnishncsa_logformat - log file format.
 #       default: "" (uses varnishncsa's default format)
@@ -30,7 +30,7 @@
 # Add the following line to /etc/newsyslog.conf to rotate the log file
 # once a day:
 #
-# /var/log/varnishncsa.log 640 7 * @T00 JB /var/run/varnishncsa.pid
+# /var/log/varnishncsa.log varnishlog:varnish 640 7 * @T00 JB /var/run/varnishncsa.pid
 #
 # See varnishncsa(1) for a detailed overview of command-line options.
 #
@@ -45,43 +45,26 @@ load_rc_config ${name}
 : ${varnishncsa_enable:=NO}
 : ${varnishncsa_pidfile=/var/run/${name}.pid}
 : ${varnishncsa_file=/var/log/${name}.log}
-: ${varnishncsa_flags="-P ${varnishncsa_pidfile} -D -a -w ${varnishncsa_file}${varnishncsa_logformat:+ -F \"$varnishncsa_logformat\"}"}
-: ${varnishncsa_user=varnishlog}
+: ${varnishncsa_flags="-t off -P ${varnishncsa_pidfile} -D -a -w ${varnishncsa_file} ${varnishncsa_logformat:+-F \"$varnishncsa_logformat\"}"}
 
-command="%%PREFIX%%/bin/${name}"
+procname="%%PREFIX%%/bin/${name}"
+command="/usr/sbin/daemon"
+command_args="-f -u varnishlog ${procname} ${varnishncsa_flags}"
 pidfile=${varnishncsa_pidfile}
 start_precmd=precmd
 
 precmd()
 {
+        # $varnishncsa_flags gets applied too early if we don't do this.
+        rc_flags=""
+
 	if [ ! -e ${pidfile} ]; then
-		install -o ${varnishncsa_user} -g varnish /dev/null ${pidfile};
+		install -o varnishlog -g varnish -m 644 /dev/null ${pidfile};
 	fi
 
 	if [ ! -e ${varnishncsa_file} ]; then
-		install -o ${varnishncsa_user} -g varnish /dev/null ${varnishncsa_file};
+		install -o varnishlog -g varnish -m 640 /dev/null ${varnishncsa_file};
 	fi
-
-	waitcount=0
-	vsm=%%PREFIX%%/varnish/$(hostname)/_.vsm
-	
-        # wait no longer than 5s for  _.vsm file to arrive from varnishd
-	while :
-	do
-		if [ -f ${vsm} ] ; then
-			break
-		else
-			case "${waitcount}" in
-				5)	echo "${name}: Failed to start. Missing ${vsm} ?"
-					break
-					;;
-				*)	echo "${name}: waiting for ${vsm}"
-					waitcount=$((waitcount+1))
-					sleep 1
-					;;
-			esac
-		fi
-	done
 }
 
 run_rc_command "$1"