[PATCH] Variable for vcl_dir in startup scripts
Federico Schwindt
fgsch at lodoss.net
Mon Jul 27 22:38:02 CEST 2015
> 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/20150727/bc935c87/attachment.html>
More information about the varnish-dev
mailing list