Hi Federico, Seems the prestart is required for reloads.
Here is a new patch, just including DAEMON_OPTS in both tests. Gauthier From 4c11e58d6270591f93be58551496412e3118aeda Mon Sep 17 00:00:00 2001 From: Gauthier Delacroix <[email protected]> Date: Mon, 27 Jul 2015 11:55:13 +0200 Subject: [PATCH] Include DAEMON_OPTS in service prestart tests --- redhat/varnish.initrc | 2 +- redhat/varnish.service | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/redhat/varnish.initrc b/redhat/varnish.initrc index 117e334..03f47fb 100755 --- a/redhat/varnish.initrc +++ b/redhat/varnish.initrc @@ -126,7 +126,7 @@ rh_status_q() { configtest() { if [ -f "$VARNISH_VCL_CONF" ]; then - $exec -f "$VARNISH_VCL_CONF" -C -n /tmp > /dev/null && echo "Syntax ok" + $exec -f "$VARNISH_VCL_CONF" -C -n /tmp $DAEMON_OPTS > /dev/null && echo "Syntax ok" else echo "VARNISH_VCL_CONF is unset or does not point to a file" fi diff --git a/redhat/varnish.service b/redhat/varnish.service index a4f3355..ca93b3d 100644 --- a/redhat/varnish.service +++ b/redhat/varnish.service @@ -27,7 +27,7 @@ EnvironmentFile=/etc/varnish/varnish.params Type=forking PIDFile=/var/run/varnish.pid PrivateTmp=true -ExecStartPre=/usr/sbin/varnishd -C -f $VARNISH_VCL_CONF +ExecStartPre=/usr/sbin/varnishd -C -f $VARNISH_VCL_CONF $DAEMON_OPTS ExecStart=/usr/sbin/varnishd \ -P /var/run/varnish.pid \ -f $VARNISH_VCL_CONF \ -- 1.8.3.msysgit.0 De : Federico Schwindt [mailto:[email protected]] Envoyé : mercredi 15 juillet 2015 20:07 À : Delacroix, Gauthier <[email protected]> Cc : [email protected]; [email protected] Objet : Re: [PATCH] Variable for vcl_dir in startup scripts Hi Gauthier, Apologies for the delay. In all fairness I copied this from the Debian systemd files [1] in an attempt to unify the service files. I don't see any particular reason for having this, specially if it's the source of issues. Stig, is there any reason for the ExecStartPre? The commit message says "debian/varnish.service: Test configuration before starting, add reload". I don't see any difference in the output for an invalid config with ExecStartPre when I run either `systemctl start' or `systemctl status'. Thanks. 1. http://anonscm.debian.org/cgit/pkg-varnish/pkg-varnish.git/tree/debian On Thu, Jun 25, 2015 at 5:12 PM, Delacroix, Gauthier <[email protected]> wrote: Federico, I've initially set vcl_dir in DAEMON_OPTS in varnish.params with 4.0.2, but it's not included in the syntax check added in 4.0.3, making my config unable to start without changing varnish.service. I'm not aware of the reason behind this syntax check (won't a syntax error make startup fail anyway ?) but I assumed you had a good reason not to include DAEMON_OPTS. As it's better to change varnish.params rather than varnish.service, my proposal was to add a way to include parameters needed to make the config work in the syntax check, without including the full DAEMON_OPTS. If you finally think DAEMON_OPTS can be included in the syntax check, then I can send a really smaller patch. Gauthier De : Federico Schwindt [mailto:[email protected]] Envoyé : jeudi 25 juin 2015 17:53 À : Delacroix, Gauthier Cc : [email protected] Objet : Re: [PATCH] Variable for vcl_dir in startup scripts Hi, My first inclination is to ensure that the ExecStartPre line uses the same parameters as ExecStart. So my questions is how do you set vcl_dir? Do you edit varnish.service or varnish.params? Wouldn't adding DAEMON_OPTS to ExecStartPre (and configtest) do it? On Thu, Jun 25, 2015 at 3:26 PM, Delacroix, Gauthier <[email protected]> wrote: Here is another patch proposal to make syntax check handle parameters required to compile the VCL (vcl_dir, etc.) without creating a startup variable for each parameter. It just adds a COMPILE_OPTS that is merged in DAEMON_OPTS to start Varnish but is used alone in the syntax check. Gauthier From d1567a956d53a489aa4ace66ce0b1c1ef745570b Mon Sep 17 00:00:00 2001 From: Gauthier Delacroix <[email protected]> Date: Thu, 25 Jun 2015 15:06:08 +0200 Subject: [PATCH] Add COMPILE_OPTS in startup scripts to make syntax check check handle compilation parameters --- redhat/varnish.initrc | 6 ++++-- redhat/varnish.params | 8 ++++++++ redhat/varnish.service | 3 ++- redhat/varnish.sysconfig | 11 ++++++++++- 4 files changed, 24 insertions(+), 4 deletions(-) diff --git a/redhat/varnish.initrc b/redhat/varnish.initrc index 117e334..0bde074 100755 --- a/redhat/varnish.initrc +++ b/redhat/varnish.initrc @@ -126,9 +126,11 @@ rh_status_q() { configtest() { if [ -f "$VARNISH_VCL_CONF" ]; then - $exec -f "$VARNISH_VCL_CONF" -C -n /tmp > /dev/null && echo "Syntax ok" + $exec -f "$VARNISH_VCL_CONF" -C -n /tmp $COMPILE_OPTS > /dev/null \ + && echo "Syntax ok" else - echo "VARNISH_VCL_CONF is unset or does not point to a file" + echo "VARNISH_VCL_CONF is unset or does not point to a file" + echo "Also check that COMPILE_OPTS is set depending on the VCL config" fi } diff --git a/redhat/varnish.params b/redhat/varnish.params index 27a14dd..970d088 100644 --- a/redhat/varnish.params +++ b/redhat/varnish.params @@ -31,5 +31,13 @@ VARNISH_TTL=120 VARNISH_USER=varnish VARNISH_GROUP=varnish +# Startup options required to compile the configuration. +# The following run-time parameters must be defined here, if needed: +# cc_command, group_cc, vcc_allow_inline_c, vcc_err_unref, vcc_unsafe_path, +# vcl_dir, vmod_dir +# Defining them in DAEMON_OPTS may result in a syntax check failure. +# See the man page varnishd(1). +#COMPILE_OPTS="-p vcl_dir=/etc/varnish -p vcc_err_unref=on" + # Other options, see the man page varnishd(1) #DAEMON_OPTS="-p thread_pool_min=5 -p thread_pool_max=500 -p thread_pool_timeout=300" diff --git a/redhat/varnish.service b/redhat/varnish.service index a4f3355..a08db58 100644 --- a/redhat/varnish.service +++ b/redhat/varnish.service @@ -27,7 +27,7 @@ EnvironmentFile=/etc/varnish/varnish.params Type=forking PIDFile=/var/run/varnish.pid PrivateTmp=true -ExecStartPre=/usr/sbin/varnishd -C -f $VARNISH_VCL_CONF +ExecStartPre=/usr/sbin/varnishd -C -f $VARNISH_VCL_CONF $COMPILE_OPTS ExecStart=/usr/sbin/varnishd \ -P /var/run/varnish.pid \ -f $VARNISH_VCL_CONF \ @@ -38,6 +38,7 @@ ExecStart=/usr/sbin/varnishd \ -g $VARNISH_GROUP \ -S $VARNISH_SECRET_FILE \ -s $VARNISH_STORAGE \ + $COMPILE_OPTS \ $DAEMON_OPTS ExecReload=/usr/sbin/varnish_reload_vcl diff --git a/redhat/varnish.sysconfig b/redhat/varnish.sysconfig index 6aa2354..0e376ff 100644 --- a/redhat/varnish.sysconfig +++ b/redhat/varnish.sysconfig @@ -91,6 +91,14 @@ VARNISH_STORAGE="malloc,${VARNISH_STORAGE_SIZE}" # # Default TTL used when the backend does not specify one VARNISH_TTL=120 # +# Startup options required to compile the configuration. +# The following run-time parameters must be defined here, if needed: +# cc_command, group_cc, vcc_allow_inline_c, vcc_err_unref, vcc_unsafe_path, +# vcl_dir, vmod_dir +# Defining them in DAEMON_OPTS may result in a syntax check failure. +# See the man page varnishd(1). +#COMPILE_OPTS="-p vcl_dir=/etc/varnish -p vcc_err_unref=on" +# # # DAEMON_OPTS is used by the init script. If you add or remove options, make # # sure you update this section, too. DAEMON_OPTS="-a ${VARNISH_LISTEN_ADDRESS}:${VARNISH_LISTEN_PORT} \ @@ -102,7 +110,8 @@ DAEMON_OPTS="-a ${VARNISH_LISTEN_ADDRESS}:${VARNISH_LISTEN_PORT} \ -p thread_pool_timeout=${VARNISH_THREAD_TIMEOUT} \ -u varnish -g varnish \ -S ${VARNISH_SECRET_FILE} \ - -s ${VARNISH_STORAGE}" + -s ${VARNISH_STORAGE}" \ + ${COMPILE_OPTS} # -- 1.8.3.msysgit.0 _______________________________________________ varnish-dev mailing list [email protected] https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev _______________________________________________ varnish-dev mailing list [email protected] https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev
