> Seems the prestart is required for reloads. I haven't tested RH but this doesn't seem to be the case in Debian - ExecStartPre is only called before (re)start.
The sysv init script change might be needed though. Setting vmod_dir will also affects the test btw. On Mon, Jul 27, 2015 at 11:04 AM, Delacroix, Gauthier < [email protected]> wrote: > 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
