From 2fc4d39577ac8491cd0a07c7666e090a90d39144 Mon Sep 17 00:00:00 2001
From: Amaury Denoyelle <adenoyelle@haproxy.com>
Date: Thu, 22 Jul 2021 16:04:59 +0200
Subject: [PATCH] MEDIUM: server: implement check for dynamic servers

Implement check support for dynamic servers. The "check" keyword is now
enabled for dynamic servers. If used, the server check is initialized
and the check task started in the "add server" CLI handler. The check is
explicitely disabled and must be manually activated via "enable health"
CLI handler.

The dynamic server refcount is incremented if a check is configured. On
"delete server" handler, the check is purged, which decrements the
refcount.
---
 doc/management.txt | 15 +++++++++++++++
 src/check.c        |  2 +-
 src/server.c       | 31 ++++++++++++++++++++++++++++---
 3 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/doc/management.txt b/doc/management.txt
index 4b01ddcb42..eaa6e31cd3 100644
--- a/doc/management.txt
+++ b/doc/management.txt
@@ -1475,20 +1475,33 @@ add server <backend>/<server> [args]*
   another dynamic server. This is to ensure that the tracking chain is kept
   consistent even in the case of dynamic servers deletion.
 
+  Use the "check" keyword to enable health-check support. Note that the
+  health-check is disabled by default and must be enabled independently from
+  the server using the "enable health" command.
+
   Here is the list of the currently supported keywords :
 
   - allow-0rtt
   - alpn
+  - addr
   - backup
   - ca-file
+  - check
+  - check-proto
+  - check-send-proxy
+  - check-via-socks4
   - ciphers
   - ciphersuites
   - crl-file
   - crt
   - disabled
+  - downinter
   - enabled
+  - fall
+  - fastinter
   - force-sslv3/tlsv10/tlsv11/tlsv12/tlsv13
   - id
+  - inter
   - maxconn
   - maxqueue
   - minconn
@@ -1499,8 +1512,10 @@ add server <backend>/<server> [args]*
   - pool-low-conn
   - pool-max-conn
   - pool-purge-delay
+  - port
   - proto
   - proxy-v2-options
+  - rise
   - send-proxy
   - send-proxy-v2
   - send-proxy-v2-ssl
diff --git a/src/check.c b/src/check.c
index 4055c9f27e..637b6c905c 100644
--- a/src/check.c
+++ b/src/check.c
@@ -2386,7 +2386,7 @@ static struct srv_kw_list srv_kws = { "CHK", { }, {
 	{ "agent-inter",         srv_parse_agent_inter,         1,  1,  1 }, /* Set the interval between two agent checks */
 	{ "agent-port",          srv_parse_agent_port,          1,  1,  1 }, /* Set the TCP port used for agent checks. */
 	{ "agent-send",          srv_parse_agent_send,          1,  1,  1 }, /* Set string to send to agent. */
-	{ "check",               srv_parse_check,               0,  1,  0 }, /* Enable health checks */
+	{ "check",               srv_parse_check,               0,  1,  1 }, /* Enable health checks */
 	{ "check-proto",         srv_parse_check_proto,         1,  1,  1 }, /* Set the mux protocol for health checks  */
 	{ "check-send-proxy",    srv_parse_check_send_proxy,    0,  1,  1 }, /* Enable PROXY protocol for health checks */
 	{ "check-via-socks4",    srv_parse_check_via_socks4,    0,  1,  1 }, /* Enable socks4 proxy for health checks */
diff --git a/src/server.c b/src/server.c
index 84b9cd25a8..749d44e5be 100644
--- a/src/server.c
+++ b/src/server.c
@@ -4564,6 +4564,18 @@ static int cli_parse_add_server(char **args, char *payload, struct appctx *appct
 			goto out;
 	}
 
+	/* Init check if configured. The check is manually disabled because a
+	 * dynamic server is started in a disable state. It must be manually
+	 * activated via a "enable health" command.
+	 */
+	if (srv->do_check) {
+		if (init_srv_check(srv))
+			goto out;
+
+		srv->check.state &= ~CHK_ST_ENABLED;
+		srv_use_dynsrv(srv);
+	}
+
 	/* Attach the server to the end of the proxy linked list. Note that this
 	 * operation is not thread-safe so this is executed under thread
 	 * isolation.
@@ -4615,6 +4627,16 @@ static int cli_parse_add_server(char **args, char *payload, struct appctx *appct
 	srv_use_dynsrv(srv);
 	thread_release();
 
+	/* Start the check task. The server must be fully initialized.
+	 *
+	 * <srvpos> and <nbcheck> parameters are set to 1 as there should be no
+	 * need to randomly spread the task interval for dynamic servers.
+	 */
+	if (srv->check.state & CHK_ST_CONFIGURED) {
+		if (!start_check_task(&srv->check, 0, 1, 1))
+			ha_alert("System might be unstable, consider to execute a reload");
+	}
+
 	ha_notice("New server registered.\n");
 	cli_msg(appctx, LOG_INFO, usermsgs_str());
 
@@ -4625,6 +4647,9 @@ static int cli_parse_add_server(char **args, char *payload, struct appctx *appct
 		if (srv->track)
 			release_server_track(srv);
 
+		if (srv->check.state & CHK_ST_CONFIGURED)
+			free_check(&srv->check);
+
 		/* remove the server from the proxy linked list */
 		if (be->srv == srv) {
 			be->srv = srv->next;
@@ -4728,9 +4753,9 @@ static int cli_parse_delete_server(char **args, char *payload, struct appctx *ap
 	if (srv->track)
 		release_server_track(srv);
 
-	/* TODO remove server for check list once 'check' will be implemented for
-	 * dynamic servers
-	 */
+	/* stop the check task if running */
+	if (srv->check.state & CHK_ST_CONFIGURED)
+		check_purge(&srv->check);
 
 	/* detach the server from the proxy linked list
 	 * The proxy servers list is currently not protected by a lock, so this