[PATCH] Variable for vcl_dir in startup scripts
Federico Schwindt
fgsch at lodoss.net
Mon Aug 3 19:16:04 CEST 2015
Hi Gauthier,
We've discussed these on irc today and I've now removed ExecStartPre and
committed your diff for the sysv init script.
Thanks for the diff.
Cheers.
On Mon, Jul 27, 2015 at 9:38 PM, Federico Schwindt <fgsch at lodoss.net> wrote:
> > 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 <
> Gauthier.Delacroix at coreye.fr> 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 <gauthier.delacroix at coreye.fr>
>> 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:fgsch at lodoss.net]
>> Envoyé : mercredi 15 juillet 2015 20:07
>> À : Delacroix, Gauthier <Gauthier.Delacroix at coreye.fr>
>> Cc : varnish-dev at varnish-cache.org; ssm at redpill-linpro.com
>> 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 <
>> Gauthier.Delacroix at coreye.fr> 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:fgsch at lodoss.net]
>> Envoyé : jeudi 25 juin 2015 17:53
>> À : Delacroix, Gauthier
>> Cc : varnish-dev at varnish-cache.org
>> 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 <
>> Gauthier.Delacroix at coreye.fr> 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 <gauthier.delacroix at coreye.fr>
>> 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
>> varnish-dev at varnish-cache.org
>> https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://www.varnish-cache.org/lists/pipermail/varnish-dev/attachments/20150803/5ee99cc7/attachment-0001.html>
More information about the varnish-dev
mailing list