Mailing List Archive

[master] 49d31c8d7 Limit automatic active VCL selection to startup VCLs
commit 49d31c8d72cc4c23ecd67594912eba03392c1362
Author: Martin Blix Grydeland <martin@varnish-software.com>
Date: Thu Nov 11 14:33:23 2021 +0100

Limit automatic active VCL selection to startup VCLs

Limit the selection of the active VCL from MGT's view point to be the last
startup VCL given. That would be the VCL from the very last -f argument
given to varnishd, or the autogenerated VCL from the -b argument (-f and
-b are mutually exclusive). Because all startup VCLs are always set to
state AUTO, and AUTO VCLs are made warm when the child is not
running (which it is not at the time the startup VCLs are compiled), this
ensures that it is a warm VCL that is selected as the active VCL.

With this patch, VCLs loaded through the use of an initial CLI command
script (-I option) will not cause a VCL to automatically be selected as
the active VCL. Rather, it is expected that the initial CLI command script
should have an explicit 'vcl.use' statement to select the active VCL. When
an active VCL is not set, attempts to start the child will fail, which
again will fail the varnishd daemon startup (unless -d is given) with an
error code.

The behaviour prior to this patch when using -I, -f '' (empty field), -F
or -d was not well defined. The first VCL loaded (either by -I startup CLI
script or a CLI 'vcl.load' command) would become the active VCL, even if
that VCL is loaded cold. That is an illegal state and would lead to
asserts. It is also not very useful functionality, given the typical use
case for -I is to set up VCL labels. Those require the tips of the VCL
tree dependency graph to be loaded first, and then the VCLs that selects
the label. This means that the active VCL will typically be the last VCL
loaded, which then will require the use of a 'vcl.use' statement in the -I
script anyways. This makes it an acceptable change of default behaviour
that should not affect users.

diff --git a/bin/varnishd/mgt/mgt_vcl.c b/bin/varnishd/mgt/mgt_vcl.c
index a45645e5d..69b35371e 100644
--- a/bin/varnishd/mgt/mgt_vcl.c
+++ b/bin/varnishd/mgt/mgt_vcl.c
@@ -463,9 +463,6 @@ mgt_new_vcl(struct cli *cli, const char *vclname, const char *vclsrc,
AZ(C_flag);
vp->fname = lib;

- if (mgt_vcl_active == NULL)
- mgt_vcl_active = vp;
-
if ((cli->result == CLIS_OK || cli->result == CLIS_TRUNCATED) &&
vcl_count > mgt_param.max_vcl &&
mgt_param.max_vcl_handling == 1) {
@@ -499,6 +496,9 @@ mgt_vcl_startup(struct cli *cli, const char *vclsrc, const char *vclname,
{
char buf[20];
static int n = 0;
+ struct vclprog *vp;
+
+ AZ(MCH_Running());

AN(vclsrc);
AN(origin);
@@ -506,8 +506,13 @@ mgt_vcl_startup(struct cli *cli, const char *vclsrc, const char *vclname,
bprintf(buf, "boot%d", n++);
vclname = buf;
}
- mgt_vcl_active = NULL;
- (void)mgt_new_vcl(cli, vclname, vclsrc, origin, NULL, C_flag);
+ vp = mgt_new_vcl(cli, vclname, vclsrc, origin, NULL, C_flag);
+ if (vp != NULL) {
+ /* Last startup VCL becomes the automatically selected
+ * active VCL. */
+ AN(vp->warm);
+ mgt_vcl_active = vp;
+ }
}

/*--------------------------------------------------------------------*/
diff --git a/bin/varnishtest/tests/u00000.vtc b/bin/varnishtest/tests/u00000.vtc
index dbed3ce58..5103f85b3 100644
--- a/bin/varnishtest/tests/u00000.vtc
+++ b/bin/varnishtest/tests/u00000.vtc
@@ -105,6 +105,10 @@ shell -expect {VCL compiled.} {
varnishadm -n ${tmpdir}/v1 vcl.load vcl1 ${tmpdir}/vcl
}

+shell -expect {VCL 'vcl1' now active} {
+ varnishadm -n ${tmpdir}/v1 vcl.use vcl1
+}
+
shell -expect {active auto warm - vcl1} {
varnishadm -n ${tmpdir}/v1 vcl.list
}
_______________________________________________
varnish-commit mailing list
varnish-commit@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-commit