[master] dda20238b Limit automatic active VCL selection to startup VCLs

[Nils Goroll]

commit dda20238b9dc27f4c907e50fe6e207501683fa9a
Author: Martin Blix Grydeland <martin at 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
 }