Hello all,

With the patch in the attachment, you can add a "port" field to a probe
definition, so that a backend can respond to health probes from a
different port.

By default, probes work as before (the port is the same as in the
backend definition if you don't have the port field), so existing
configs are unchanged.

This is a Varnish 4 re-implementation of a solution we've had working in
production under Varnish 3 for about a year and a half. Our use case is
a third-party app as the backend, for which the vendor supplies a health
check under a different port, and our developers can't easily change
that. (The app is SD LFredhopper.)

The patch adds some vtc tests that are not in order in the numbering
(starting at c00100.vtc and v00100.vtc), so as not to overlap with
anyone else's tests. But of course they can be easily renumbered.


Best,
Geoff
-- 
** * * UPLEX - Nils Goroll Systemoptimierung

Scheffelstraße 32
22301 Hamburg

Tel +49 40 2880 5731
Mob +49 176 636 90917
Fax +49 40 42949753

http://uplex.de
From dc609db7f64e6067be6958aabf32af40c898eaf0 Mon Sep 17 00:00:00 2001
From: Geoff Simmons <[email protected]>
Date: Fri, 14 Jun 2013 09:06:21 +0200
Subject: [PATCH 1/1] add a "port" field to probe definitions, to allow
 backends to respond to health probes from a different
 port

---
 bin/varnishd/cache/cache_backend_poll.c |   55 +++++++++++++++++++-----
 bin/varnishtest/tests/c00100.vtc        |   47 ++++++++++++++++++++
 bin/varnishtest/tests/c00101.vtc        |   47 ++++++++++++++++++++
 bin/varnishtest/tests/c00102.vtc        |   71 +++++++++++++++++++++++++++++++
 bin/varnishtest/tests/v00038.vtc        |   18 ++++++++
 bin/varnishtest/tests/v00100.vtc        |   60 ++++++++++++++++++++++++++
 doc/sphinx/reference/vcl.rst            |    3 ++
 include/vrt.h                           |    1 +
 include/vsa.h                           |    3 ++
 lib/libvarnish/vsa.c                    |   43 +++++++++++++++++++
 lib/libvcc/vcc_backend.c                |   15 ++++++-
 11 files changed, 352 insertions(+), 11 deletions(-)
 create mode 100644 bin/varnishtest/tests/c00100.vtc
 create mode 100644 bin/varnishtest/tests/c00101.vtc
 create mode 100644 bin/varnishtest/tests/c00102.vtc
 create mode 100644 bin/varnishtest/tests/v00100.vtc

diff --git a/bin/varnishd/cache/cache_backend_poll.c b/bin/varnishd/cache/cache_backend_poll.c
index 4773785..a41adb1 100644
--- a/bin/varnishd/cache/cache_backend_poll.c
+++ b/bin/varnishd/cache/cache_backend_poll.c
@@ -49,6 +49,7 @@
 #include "vrt.h"
 #include "vtcp.h"
 #include "vtim.h"
+#include "vsa.h"
 
 /* Default averaging rate, we want something pretty responsive */
 #define AVG_RATE			4
@@ -76,6 +77,9 @@ struct vbp_target {
 	char				*req;
 	int				req_len;
 
+	struct suckaddr			*ipv4;
+	struct suckaddr			*ipv6;
+
 	char				resp_buf[128];
 	unsigned			good;
 
@@ -97,6 +101,34 @@ static VTAILQ_HEAD(, vbp_target)	vbp_list =
 
 static struct lock			vbp_mtx;
 
+static void
+copy_suckaddrs(struct vbp_target *vt, const struct backend *b)
+{
+        CHECK_OBJ_NOTNULL(vt, VBP_TARGET_MAGIC);
+        CHECK_OBJ_NOTNULL(b, BACKEND_MAGIC);
+
+        if (b->ipv4 != NULL) {
+                vt->ipv4 = VSA_Copy(b->ipv4);
+                AN(VSA_Sane(vt->ipv4));
+        }
+
+        if (b->ipv6 != NULL) {
+                vt->ipv6 = VSA_Copy(b->ipv6);
+                AN(VSA_Sane(vt->ipv6));
+        }
+}
+
+static void
+reset_port(struct vbp_target *vt, const struct vrt_backend_probe *p)
+{
+        if (p->port != 0) {
+                if (vt->ipv4 != NULL)
+                        VSA_SetPort(vt->ipv4, p->port);
+                if (vt->ipv6 != NULL)
+                        VSA_SetPort(vt->ipv6, p->port);
+        }
+}
+
 /*--------------------------------------------------------------------
  * Poke one backend, once, but possibly at both IPv4 and IPv6 addresses.
  *
@@ -126,34 +158,30 @@ vbp_poke(struct vbp_target *vt)
 	int s, tmo, i;
 	double t_start, t_now, t_end;
 	unsigned rlen, resp;
-	struct backend *bp;
 	char buf[8192], *p;
 	struct pollfd pfda[1], *pfd = pfda;
 
-	bp = vt->backend;
-	CHECK_OBJ_NOTNULL(bp, BACKEND_MAGIC);
-
 	t_start = t_now = VTIM_real();
 	t_end = t_start + vt->probe.timeout;
 	tmo = (int)round((t_end - t_now) * 1e3);
 
 	s = -1;
-	if (cache_param->prefer_ipv6 && bp->ipv6 != NULL) {
-		s = vbp_connect(PF_INET6, bp->ipv6, tmo);
+	if (cache_param->prefer_ipv6 && vt->ipv6 != NULL) {
+		s = vbp_connect(PF_INET6, vt->ipv6, tmo);
 		t_now = VTIM_real();
 		tmo = (int)round((t_end - t_now) * 1e3);
 		if (s >= 0)
 			vt->good_ipv6 |= 1;
 	}
-	if (tmo > 0 && s < 0 && bp->ipv4 != NULL) {
-		s = vbp_connect(PF_INET, bp->ipv4, tmo);
+	if (tmo > 0 && s < 0 && vt->ipv4 != NULL) {
+		s = vbp_connect(PF_INET, vt->ipv4, tmo);
 		t_now = VTIM_real();
 		tmo = (int)round((t_end - t_now) * 1e3);
 		if (s >= 0)
 			vt->good_ipv4 |= 1;
 	}
-	if (tmo > 0 && s < 0 && bp->ipv6 != NULL) {
-		s = vbp_connect(PF_INET6, bp->ipv6, tmo);
+	if (tmo > 0 && s < 0 && vt->ipv6 != NULL) {
+		s = vbp_connect(PF_INET6, vt->ipv6, tmo);
 		t_now = VTIM_real();
 		tmo = (int)round((t_end - t_now) * 1e3);
 		if (s >= 0)
@@ -499,6 +527,7 @@ VBP_Insert(struct backend *b, const struct vrt_backend_probe *p,
 		b->probe = vt;
 		startthread = 1;
 		VTAILQ_INSERT_TAIL(&vbp_list, vt, list);
+		copy_suckaddrs(vt, b);
 	} else {
 		vt = b->probe;
 	}
@@ -511,6 +540,8 @@ VBP_Insert(struct backend *b, const struct vrt_backend_probe *p,
 	VTAILQ_INSERT_TAIL(&vt->vcls, vcl, list);
 	Lck_Unlock(&vbp_mtx);
 
+	reset_port(vt, p);
+
 	if (startthread) {
 		for (u = 0; u < vcl->probe.initial; u++) {
 			vbp_start_poke(vt);
@@ -586,6 +617,10 @@ VBP_Remove(struct backend *b, struct vrt_backend_probe const *p)
 	VTAILQ_REMOVE(&vbp_list, vt, list);
 	b->probe = NULL;
 	VSB_delete(vt->vsb);
+	if (vt->ipv4 != NULL)
+		VSA_Fini(vt->ipv4);
+	if (vt->ipv6 != NULL)
+		VSA_Fini(vt->ipv6);
 	FREE_OBJ(vt);
 }
 
diff --git a/bin/varnishtest/tests/c00100.vtc b/bin/varnishtest/tests/c00100.vtc
new file mode 100644
index 0000000..5a07b1e
--- /dev/null
+++ b/bin/varnishtest/tests/c00100.vtc
@@ -0,0 +1,47 @@
+varnishtest "Test Backend Polling with a separate port (cf. c00017.vtc)"
+
+server s1 {
+} -start
+
+server s2 {
+	# Probes 
+	loop 8 {
+		rxreq
+		expect req.url == "/"
+		txresp -hdr "Bar: foo" -body "foobar" 
+		accept
+	}
+
+	loop 3 {
+		rxreq
+		expect req.url == "/"
+		txresp -status 404 -hdr "Bar: foo" -body "foobar" 
+		accept
+	}
+	loop 2 {
+		rxreq
+		expect req.url == "/"
+		txresp -proto "FROBOZ" -status 200 -hdr "Bar: foo" -body "foobar" 
+		accept
+	}
+
+	sema r1 sync 2
+} -start
+
+varnish v1 -vcl { 
+
+	backend foo {
+		.host = "${s1_addr}";
+		.port = "${s1_port}";
+		.probe = {
+			.timeout = 1 s;
+			.interval = 0.1 s;
+			.port = ${s2_port};
+		}
+	}
+	
+} -start
+
+sema r1 sync 2
+
+varnish v1 -cli "debug.health"
diff --git a/bin/varnishtest/tests/c00101.vtc b/bin/varnishtest/tests/c00101.vtc
new file mode 100644
index 0000000..dfc38f6
--- /dev/null
+++ b/bin/varnishtest/tests/c00101.vtc
@@ -0,0 +1,47 @@
+varnishtest "Dropping polling of a backend with a separate port (cf. c00035.vtc)"
+
+server s1 {
+	rxreq
+	expect req.url == /foo
+	txresp -bodylen 4
+} -start
+
+server s2 -repeat 1 {
+	rxreq
+	txresp
+} -start
+
+varnish v1 -vcl {
+	backend s1 {
+		.host = "${s1_addr}";
+		.port = "${s1_port}";
+		.probe = {
+			.window = 8;
+			.initial = 7;
+			.threshold = 8;
+			.interval = 0.1s;
+			.port = ${s2_port};
+		}
+	}
+} -start
+
+delay 1
+
+varnish v1 -vcl {
+	backend s1 {
+		.host = "${s1_addr}";
+		.port = "${s1_port}";
+	}
+} -cliok "vcl.use vcl2" -cliok "vcl.discard vcl1"
+
+delay 1
+
+varnish v1 -cliok "vcl.list"
+varnish v1 -cliok "debug.health"
+
+client c1 {
+	txreq -url /foo
+	rxresp
+	expect resp.status == 200
+	expect resp.bodylen == 4
+} -run
diff --git a/bin/varnishtest/tests/c00102.vtc b/bin/varnishtest/tests/c00102.vtc
new file mode 100644
index 0000000..9492317
--- /dev/null
+++ b/bin/varnishtest/tests/c00102.vtc
@@ -0,0 +1,71 @@
+varnishtest "Forcing health of backends with separate probe port (cf. c00048.vtc)"
+
+server s1 -repeat 3 {
+	rxreq
+	txresp
+} -start
+
+server s2 -repeat 3 {
+	rxreq
+	txresp
+} -start
+
+varnish v1 -vcl {
+	backend s1 {
+		.host = "${s1_addr}";
+		.port = "${s1_port}";
+		.probe = {
+			.window = 8;
+			.initial = 7;
+			.threshold = 8;
+			.interval = 10s;
+			.port = ${s2_port};
+		}
+	}
+
+	sub vcl_recv {
+		return(pass);
+	}
+
+} -start
+
+delay 1
+
+varnish v1 -cliok "backend.list"
+varnish v1 -cliok "backend.set_health s1 auto"
+varnish v1 -cliok "backend.list s"
+
+client c1 {
+	txreq
+	rxresp
+	expect resp.status == 200
+} -run
+
+varnish v1 -cliok "backend.list (:)"
+varnish v1 -cliok "backend.set_health s1 sick"
+varnish v1 -cliok "backend.list (1:)"
+
+client c1 {
+	txreq
+	rxresp
+	expect resp.status == 503
+} -run
+
+varnish v1 -cliok "backend.list"
+varnish v1 -cliok "backend.set_health s1 healthy"
+varnish v1 -cliok "backend.list"
+
+client c1 {
+	txreq
+	rxresp
+	expect resp.status == 200
+} -run
+
+varnish v1 -clierr 106 "backend.set_health s1 foo"
+varnish v1 -clierr 106 "backend.set_health s2 foo"
+varnish v1 -clierr 106 "backend.set_health s2 auto"
+varnish v1 -cliok "backend.list (foo)"
+varnish v1 -clierr 300 "backend.list ("
+varnish v1 -clierr 300 {backend.list " ( : ) -"}
+varnish v1 -cliok {backend.list "a ( b : c )"}
+
diff --git a/bin/varnishtest/tests/v00038.vtc b/bin/varnishtest/tests/v00038.vtc
index 7de4517..33c3347 100644
--- a/bin/varnishtest/tests/v00038.vtc
+++ b/bin/varnishtest/tests/v00038.vtc
@@ -65,3 +65,21 @@ varnish v1 -errvcl "Expected '{' or name of probe, got" {
 		.probe = "NONE";
 	}
 }
+
+varnish v1 -errvcl ".port specification out of range" {
+	backend b1 {
+		.host = "127.0.0.1";
+		.probe = {
+			.port = 65536;
+		}
+	}
+}
+
+varnish v1 -errvcl ".port specification out of range" {
+	backend b1 {
+		.host = "127.0.0.1";
+		.probe = {
+			.port = 0;
+		}
+	}
+}
diff --git a/bin/varnishtest/tests/v00100.vtc b/bin/varnishtest/tests/v00100.vtc
new file mode 100644
index 0000000..c7d4922
--- /dev/null
+++ b/bin/varnishtest/tests/v00100.vtc
@@ -0,0 +1,60 @@
+varnishtest "Check std.healthy() with separate probe port (cf. v00014.vtc)"
+
+server s1 {
+	rxreq
+	expect req.url == "/"
+	txresp -body "slash"
+	rxreq
+	expect req.url == "/foo"
+	txresp -body "foobar"
+} -start
+
+server s2 -repeat 3 {
+	rxreq
+	expect req.url == "/"
+	txresp -body "slash"
+} -start
+
+varnish v1 -vcl {
+
+	import ${vmod_std};
+
+	probe foo {
+		.url = "/";
+		.timeout = 1s;
+		.interval = 1s;
+		.window = 3;
+		.threshold = 2;
+		.initial = 0;
+		.port = ${s2_port};
+	}
+
+	backend default {
+		.host = "${s1_addr}";
+		.port = "${s1_port}";
+		.max_connections = 1;
+		.probe = foo;
+	}
+
+	sub vcl_recv {
+		if (std.healthy(default)) {
+			return(synth(200,"Backend healthy"));
+		} else {
+			return(synth(500,"Backend sick"));
+		}
+	}
+} -start
+
+client c1 {
+	txreq
+	rxresp
+	expect resp.status == 500
+} -run
+
+delay 2
+
+client c1 {
+	txreq -url "/foo"
+	rxresp
+	expect resp.status == 200
+} -run
diff --git a/doc/sphinx/reference/vcl.rst b/doc/sphinx/reference/vcl.rst
index 90ac019..0c9a6ca 100644
--- a/doc/sphinx/reference/vcl.rst
+++ b/doc/sphinx/reference/vcl.rst
@@ -258,6 +258,9 @@ There are no mandatory options. These are the options you can set:
     How many of the polls in .window must have succeeded for us to
     consider the backend healthy. Defaults to 3.
 
+  port
+    The port to which Varnish should connect for health probes. Default
+    is the same port used for the backend.
 
 Access Control List (ACL)
 -------------------------
diff --git a/include/vrt.h b/include/vrt.h
index 1c5eea2..66f0092 100644
--- a/include/vrt.h
+++ b/include/vrt.h
@@ -146,6 +146,7 @@ struct vrt_backend_probe {
 	unsigned	window;
 	unsigned	threshold;
 	unsigned	initial;
+	unsigned short	port;
 };
 
 /*
diff --git a/include/vsa.h b/include/vsa.h
index 26f6419..c7285a3 100644
--- a/include/vsa.h
+++ b/include/vsa.h
@@ -36,7 +36,10 @@ extern const int vsa_suckaddr_len;
 int VSA_Sane(const struct suckaddr *);
 socklen_t VSA_Len(const struct suckaddr *);
 unsigned VSA_Port(const struct suckaddr *);
+void VSA_SetPort(struct suckaddr *, unsigned short port);
 int VSA_Compare(const struct suckaddr *, const struct suckaddr *);
+struct suckaddr *VSA_Copy(const struct suckaddr *);
+void VSA_Fini(struct suckaddr *);
 
 const struct sockaddr *VSA_Get_Sockaddr(const struct suckaddr *, socklen_t *sl);
 
diff --git a/lib/libvarnish/vsa.c b/lib/libvarnish/vsa.c
index c2838d6..5c95f60 100644
--- a/lib/libvarnish/vsa.c
+++ b/lib/libvarnish/vsa.c
@@ -337,3 +337,46 @@ VSA_Port(const struct suckaddr *sua)
 			return (0);
 	}
 }
+
+struct suckaddr *
+VSA_Copy(const struct suckaddr *sua)
+{
+	socklen_t len;
+	const void *sa;
+        
+	CHECK_OBJ_NOTNULL(sua, SUCKADDR_MAGIC);
+	switch(sua->sa.sa_family) {
+		case PF_INET:
+                        sa = (const void *) &sua->sa4;
+                        len = sizeof(sua->sa4);
+                        break;
+		case PF_INET6:
+                        sa = (const void *) &sua->sa6;
+                        len = sizeof(sua->sa6);
+                        break;
+		default:
+			return (NULL);
+	}
+	return VSA_Malloc(sa, len);
+}
+
+void
+VSA_SetPort(struct suckaddr *sua, unsigned short port)
+{
+	CHECK_OBJ_NOTNULL(sua, SUCKADDR_MAGIC);
+	switch(sua->sa.sa_family) {
+		case PF_INET:
+			sua->sa4.sin_port = htons(port);
+		case PF_INET6:
+			sua->sa6.sin6_port = htons(port);
+		default:
+			break;
+	}
+}
+
+void
+VSA_Fini(struct suckaddr *sua)
+{
+	CHECK_OBJ_NOTNULL(sua, SUCKADDR_MAGIC);
+	FREE_OBJ(sua);
+}
diff --git a/lib/libvcc/vcc_backend.c b/lib/libvcc/vcc_backend.c
index 7c079a9..8427c09 100644
--- a/lib/libvcc/vcc_backend.c
+++ b/lib/libvcc/vcc_backend.c
@@ -104,7 +104,7 @@ vcc_ParseProbeSpec(struct vcc *tl)
 	struct token *t_field;
 	struct token *t_did = NULL, *t_window = NULL, *t_threshold = NULL;
 	struct token *t_initial = NULL;
-	unsigned window, threshold, initial, status;
+	unsigned window, threshold, initial, status, port;
 	double t;
 
 	fs = vcc_FldSpec(tl,
@@ -116,6 +116,7 @@ vcc_ParseProbeSpec(struct vcc *tl)
 	    "?window",
 	    "?threshold",
 	    "?initial",
+	    "?port",
 	    NULL);
 
 	SkipToken(tl, '{');
@@ -124,6 +125,7 @@ vcc_ParseProbeSpec(struct vcc *tl)
 	threshold = 0;
 	initial = 0;
 	status = 0;
+	port = 0;
 	Fh(tl, 0, "static const struct vrt_backend_probe vgc_probe__%d = {\n",
 	    tl->nprobe++);
 	while (tl->t->tok != '}') {
@@ -183,6 +185,16 @@ vcc_ParseProbeSpec(struct vcc *tl)
 			t_threshold = tl->t;
 			threshold = vcc_UintVal(tl);
 			ERRCHK(tl);
+		} else if (vcc_IdIs(t_field, "port")) {
+			port = vcc_UintVal(tl);
+			/* Assuming in_port_t == uint16_t */
+			if (port == 0 || port > UINT16_MAX) {
+				VSB_printf(tl->sb,
+					   ".port specification out of range");
+				vcc_ErrWhere(tl, tl->t);
+				return;
+			}
+			ERRCHK(tl);
 		} else {
 			vcc_ErrToken(tl, t_field);
 			vcc_ErrWhere(tl, t_field);
@@ -230,6 +242,7 @@ vcc_ParseProbeSpec(struct vcc *tl)
 		Fh(tl, 0, "\t.initial = ~0U,\n");
 	if (status > 0)
 		Fh(tl, 0, "\t.exp_status = %u,\n", status);
+	Fh(tl, 0, "\t.port = %u,\n", port);
 	Fh(tl, 0, "};\n");
 	SkipToken(tl, '}');
 }
-- 
1.7.10.4

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
varnish-dev mailing list
[email protected]
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Reply via email to