BUG/MEDIUM: server/checks: Init server check during config validity check
The options and directives related to the configuration of checks in a backend
may be defined after the servers declarations. So, initialization of the check
of each server must not be performed during configuration parsing, because some
info may be missing. Instead, it must be done during the configuration validity
check.
Thus, callback functions are registered to be called for each server after the
config validity check, one for the server check and another one for the server
agent-check. In addition deinit callback functions are also registered to
release these checks.
This patch should be backported as far as 1.7. But per-server post_check
callback functions are only supported since the 2.1. And the initcall mechanism
does not exist before the 1.9. Finally, in 1.7, the code is totally
different. So the backport will be harder on older versions.
(cherry picked from commit 8892e5d30b2c20fce04f2f44202bc1f127076c15)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 698f6c695fe1904b54e978c04f6a3268db9af37f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
diff --git a/include/proto/checks.h b/include/proto/checks.h
index 2b285f3..695fe84 100644
--- a/include/proto/checks.h
+++ b/include/proto/checks.h
@@ -3,7 +3,7 @@
Functions prototypes for the checks.
Copyright (C) 2000-2009 Willy Tarreau - w@1wt.eu
-
+
This library is free software; you can redistribute it and/or
modify it under the terms of the GNU Lesser General Public
License as published by the Free Software Foundation, version 2.1
@@ -50,6 +50,8 @@
const char *init_check(struct check *check, int type);
void free_check(struct check *check);
+void deinit_srv_check(struct server *srv);
+void deinit_srv_agent_check(struct server *srv);
int init_email_alert(struct mailers *mailers, struct proxy *p, char **err);
void send_email_alert(struct server *s, int priority, const char *format, ...)
diff --git a/src/cfgparse.c b/src/cfgparse.c
index cfacb1e..9e8fbab 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -3234,9 +3234,6 @@
proxy_type_str(curproxy), curproxy->id,
newsrv->id);
- /* set the check type on the server */
- newsrv->check.type = curproxy->options2 & PR_O2_CHK_ANY;
-
if (newsrv->trackit) {
struct proxy *px;
struct server *srv, *loop;
@@ -3273,9 +3270,7 @@
goto next_srv;
}
- if (!(srv->check.state & CHK_ST_CONFIGURED) &&
- !(srv->agent.state & CHK_ST_CONFIGURED) &&
- !srv->track && !srv->trackit) {
+ if (!srv->do_check && !srv->do_agent && !srv->track && !srv->trackit) {
ha_alert("config : %s '%s', server '%s': unable to use %s/%s for "
"tracking as it does not have any check nor agent enabled.\n",
proxy_type_str(curproxy), curproxy->id,
diff --git a/src/checks.c b/src/checks.c
index f1cf0e7..2cc6702 100644
--- a/src/checks.c
+++ b/src/checks.c
@@ -3326,6 +3326,10 @@
void free_check(struct check *check)
{
+ task_destroy(check->task);
+ if (check->wait_list.tasklet)
+ tasklet_free(check->wait_list.tasklet);
+
free(check->bi.area);
free(check->bo.area);
if (check->cs) {
@@ -3457,7 +3461,6 @@
struct email_alertq *q = &queues[i];
struct check *check = &q->check;
- task_destroy(check->task);
free_check(check);
}
free(queues);
@@ -3680,17 +3683,6 @@
srv = chk->server;
- /* If neither a port nor an addr was specified and no check transport
- * layer is forced, then the transport layer used by the checks is the
- * same as for the production traffic. Otherwise we use raw_sock by
- * default, unless one is specified.
- */
- if (!chk->port && !is_addr(&chk->addr)) {
- if (!chk->use_ssl)
- chk->use_ssl = srv->use_ssl;
- chk->send_proxy |= (srv->pp_opts);
- }
-
/* by default, we use the health check port ocnfigured */
if (chk->port > 0)
return chk->port;
@@ -3715,6 +3707,146 @@
return 0;
}
+static int init_srv_check(struct server *srv)
+{
+ const char *err;
+ struct tcpcheck_rule *r;
+ int ret = 0;
+
+ if (!srv->do_check)
+ goto out;
+
+
+ /* If neither a port nor an addr was specified and no check transport
+ * layer is forced, then the transport layer used by the checks is the
+ * same as for the production traffic. Otherwise we use raw_sock by
+ * default, unless one is specified.
+ */
+ if (!srv->check.port && !is_addr(&srv->check.addr)) {
+ if (!srv->check.use_ssl && srv->use_ssl != -1) {
+ srv->check.use_ssl = srv->use_ssl;
+ srv->check.xprt = srv->xprt;
+ }
+ else if (srv->check.use_ssl == 1)
+ srv->check.xprt = xprt_get(XPRT_SSL);
+
+ srv->check.send_proxy |= (srv->pp_opts);
+ }
+
+ /* validate <srv> server health-check settings */
+
+ /* We need at least a service port, a check port or the first tcp-check
+ * rule must be a 'connect' one when checking an IPv4/IPv6 server.
+ */
+ if ((srv_check_healthcheck_port(&srv->check) != 0) ||
+ (!is_inet_addr(&srv->check.addr) && (is_addr(&srv->check.addr) || !is_inet_addr(&srv->addr))))
+ goto init;
+
+ if (!LIST_ISEMPTY(&srv->proxy->tcpcheck_rules)) {
+ ha_alert("config: %s '%s': server '%s' has neither service port nor check port.\n",
+ proxy_type_str(srv->proxy), srv->proxy->id, srv->id);
+ ret |= ERR_ALERT | ERR_ABORT;
+ goto out;
+ }
+
+ /* search the first action (connect / send / expect) in the list */
+ r = get_first_tcpcheck_rule(&srv->proxy->tcpcheck_rules);
+ if (!r || (r->action != TCPCHK_ACT_CONNECT) || !r->port) {
+ ha_alert("config: %s '%s': server '%s' has neither service port nor check port "
+ "nor tcp_check rule 'connect' with port information.\n",
+ proxy_type_str(srv->proxy), srv->proxy->id, srv->id);
+ ret |= ERR_ALERT | ERR_ABORT;
+ goto out;
+ }
+
+ /* scan the tcp-check ruleset to ensure a port has been configured */
+ list_for_each_entry(r, &srv->proxy->tcpcheck_rules, list) {
+ if ((r->action == TCPCHK_ACT_CONNECT) && (!r->port)) {
+ ha_alert("config: %s '%s': server '%s' has neither service port nor check port, "
+ "and a tcp_check rule 'connect' with no port information.\n",
+ proxy_type_str(srv->proxy), srv->proxy->id, srv->id);
+ ret |= ERR_ALERT | ERR_ABORT;
+ goto out;
+ }
+ }
+
+ init:
+ err = init_check(&srv->check, srv->proxy->options2 & PR_O2_CHK_ANY);
+ if (err) {
+ ha_alert("config: %s '%s': unable to init check for server '%s' (%s).\n",
+ proxy_type_str(srv->proxy), srv->proxy->id, srv->id, err);
+ ret |= ERR_ALERT | ERR_ABORT;
+ goto out;
+ }
+ srv->check.state |= CHK_ST_CONFIGURED | CHK_ST_ENABLED;
+ global.maxsock++;
+
+ out:
+ return ret;
+}
+
+static int init_srv_agent_check(struct server *srv)
+{
+ const char *err;
+ int ret = 0;
+
+ if (!srv->do_agent)
+ goto out;
+
+ err = init_check(&srv->agent, PR_O2_LB_AGENT_CHK);
+ if (err) {
+ ha_alert("config: %s '%s': unable to init agent-check for server '%s' (%s).\n",
+ proxy_type_str(srv->proxy), srv->proxy->id, srv->id, err);
+ ret |= ERR_ALERT | ERR_ABORT;
+ goto out;
+ }
+
+ if (!srv->agent.inter)
+ srv->agent.inter = srv->check.inter;
+
+ srv->agent.state |= CHK_ST_CONFIGURED | CHK_ST_ENABLED | CHK_ST_AGENT;
+ global.maxsock++;
+
+ out:
+ return ret;
+}
+
+void deinit_srv_check(struct server *srv)
+{
+ if (srv->do_check)
+ free_check(&srv->check);
+}
+
+
+void deinit_srv_agent_check(struct server *srv)
+{
+ if (srv->do_agent)
+ free_check(&srv->agent);
+ free(srv->agent.send_string);
+}
+
+static int init_servers_checks()
+{
+ struct proxy *px;
+ struct server *s;
+ int ret = 0;
+
+ for (px = proxies_list; px; px = px->next) {
+ for (s = px->srv; s; s = s->next) {
+ ret |= init_srv_check(s);
+ if (ret & (ERR_ABORT|ERR_FATAL))
+ goto end;
+ ret |= init_srv_agent_check(s);
+ if (ret & (ERR_ABORT|ERR_FATAL))
+ goto end;
+ }
+ }
+ end:
+ return ret;
+}
+
+/* Must be declared in that order because checks must be initialized first */
+REGISTER_POST_CHECK(init_servers_checks);
REGISTER_POST_CHECK(start_checks);
/*
diff --git a/src/haproxy.c b/src/haproxy.c
index d5ebcdc..8748931 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -107,6 +107,7 @@
#include <proto/auth.h>
#include <proto/backend.h>
#include <proto/channel.h>
+#include <proto/checks.h>
#include <proto/cli.h>
#include <proto/connection.h>
#include <proto/fd.h>
@@ -2476,23 +2477,11 @@
while (s) {
s_next = s->next;
- task_destroy(s->check.task);
- task_destroy(s->agent.task);
-
- if (s->check.wait_list.tasklet)
- tasklet_free(s->check.wait_list.tasklet);
- if (s->agent.wait_list.tasklet)
- tasklet_free(s->agent.wait_list.tasklet);
task_destroy(s->warmup);
free(s->id);
free(s->cookie);
- free(s->check.bi.area);
- free(s->check.bo.area);
- free(s->agent.bi.area);
- free(s->agent.bo.area);
- free(s->agent.send_string);
free(s->hostname_dn);
free((char*)s->conf.file);
free(s->idle_conns);
@@ -2500,6 +2489,8 @@
free(s->safe_conns);
free(s->idle_orphan_conns);
free(s->curr_idle_thr);
+ deinit_srv_check(s);
+ deinit_srv_agent_check(s);
if (s->use_ssl == 1 || s->check.use_ssl == 1 || (s->proxy->options & PR_O_TCPCHK_SSL)) {
if (xprt_get(XPRT_SSL) && xprt_get(XPRT_SSL)->destroy_srv)
diff --git a/src/server.c b/src/server.c
index 12d17ce..4541bdf 100644
--- a/src/server.c
+++ b/src/server.c
@@ -1858,154 +1858,6 @@
return srv;
}
-/*
- * Validate <srv> server health-check settings.
- * Returns 0 if everything is OK, -1 if not.
- */
-static int server_healthcheck_validate(const char *file, int linenum, struct server *srv)
-{
- struct tcpcheck_rule *r = NULL;
- struct list *l;
-
- /*
- * We need at least a service port, a check port or the first tcp-check rule must
- * be a 'connect' one when checking an IPv4/IPv6 server.
- */
- if ((srv_check_healthcheck_port(&srv->check) != 0) ||
- (!is_inet_addr(&srv->check.addr) && (is_addr(&srv->check.addr) || !is_inet_addr(&srv->addr))))
- return 0;
-
- r = (struct tcpcheck_rule *)srv->proxy->tcpcheck_rules.n;
- if (!r) {
- ha_alert("parsing [%s:%d] : server %s has neither service port nor check port. "
- "Check has been disabled.\n",
- file, linenum, srv->id);
- return -1;
- }
-
- /* search the first action (connect / send / expect) in the list */
- l = &srv->proxy->tcpcheck_rules;
- list_for_each_entry(r, l, list) {
- if (r->action != TCPCHK_ACT_COMMENT)
- break;
- }
-
- if ((r->action != TCPCHK_ACT_CONNECT) || !r->port) {
- ha_alert("parsing [%s:%d] : server %s has neither service port nor check port "
- "nor tcp_check rule 'connect' with port information. Check has been disabled.\n",
- file, linenum, srv->id);
- return -1;
- }
-
- /* scan the tcp-check ruleset to ensure a port has been configured */
- l = &srv->proxy->tcpcheck_rules;
- list_for_each_entry(r, l, list) {
- if ((r->action == TCPCHK_ACT_CONNECT) && (!r->port)) {
- ha_alert("parsing [%s:%d] : server %s has neither service port nor check port, "
- "and a tcp_check rule 'connect' with no port information. Check has been disabled.\n",
- file, linenum, srv->id);
- return -1;
- }
- }
-
- return 0;
-}
-
-/*
- * Initialize <srv> health-check structure.
- * Returns the error string in case of memory allocation failure, NULL if not.
- */
-static const char *do_health_check_init(struct server *srv, int check_type, int state)
-{
- const char *ret;
-
- if (!srv->do_check)
- return NULL;
-
- ret = init_check(&srv->check, check_type);
- if (ret)
- return ret;
-
- srv->check.state |= state;
- global.maxsock++;
-
- return NULL;
-}
-
-static int server_health_check_init(const char *file, int linenum,
- struct server *srv, struct proxy *curproxy)
-{
- const char *ret;
-
- if (!srv->do_check)
- return 0;
-
- if (srv->trackit) {
- ha_alert("parsing [%s:%d]: unable to enable checks and tracking at the same time!\n",
- file, linenum);
- return ERR_ALERT | ERR_FATAL;
- }
-
- if (server_healthcheck_validate(file, linenum, srv) < 0)
- return ERR_ALERT | ERR_ABORT;
-
- /* note: check type will be set during the config review phase */
- ret = do_health_check_init(srv, 0, CHK_ST_CONFIGURED | CHK_ST_ENABLED);
- if (ret) {
- ha_alert("parsing [%s:%d] : %s.\n", file, linenum, ret);
- return ERR_ALERT | ERR_ABORT;
- }
-
- return 0;
-}
-
-/*
- * Initialize <srv> agent check structure.
- * Returns the error string in case of memory allocation failure, NULL if not.
- */
-static const char *do_server_agent_check_init(struct server *srv, int state)
-{
- const char *ret;
-
- if (!srv->do_agent)
- return NULL;
-
- ret = init_check(&srv->agent, PR_O2_LB_AGENT_CHK);
- if (ret)
- return ret;
-
- if (!srv->agent.inter)
- srv->agent.inter = srv->check.inter;
-
- srv->agent.state |= state;
- global.maxsock++;
-
- return NULL;
-}
-
-static int server_agent_check_init(const char *file, int linenum,
- struct server *srv, struct proxy *curproxy)
-{
- const char *ret;
-
- if (!srv->do_agent)
- return 0;
-
- if (!srv->agent.port) {
- ha_alert("parsing [%s:%d] : server %s does not have agent port. Agent check has been disabled.\n",
- file, linenum, srv->id);
- return ERR_ALERT | ERR_FATAL;
- }
-
- ret = do_server_agent_check_init(srv, CHK_ST_CONFIGURED | CHK_ST_ENABLED | CHK_ST_AGENT);
- if (ret) {
- ha_alert("parsing [%s:%d] : %s.\n", file, linenum, ret);
- return ERR_ALERT | ERR_ABORT;
- }
-
- return 0;
-}
-
#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
static int server_sni_expr_init(const char *file, int linenum, char **args, int cur_arg,
struct server *srv, struct proxy *proxy)
@@ -2037,9 +1889,16 @@
{
int ret;
- if ((ret = server_health_check_init(file, linenum, srv, px)) != 0 ||
- (ret = server_agent_check_init(file, linenum, srv, px)) != 0) {
- return ret;
+ if (srv->do_check && srv->trackit) {
+ ha_alert("parsing [%s:%d]: unable to enable checks and tracking at the same time!\n",
+ file, linenum);
+ return ERR_ALERT | ERR_FATAL;
+ }
+
+ if (srv->do_agent && !srv->agent.port) {
+ ha_alert("parsing [%s:%d] : server %s does not have agent port. Agent check has been disabled.\n",
+ file, linenum, srv->id);
+ return ERR_ALERT | ERR_FATAL;
}
#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
@@ -2109,9 +1968,6 @@
struct server *newsrv;
for (i = srv->tmpl_info.nb_low + 1; i <= srv->tmpl_info.nb_high; i++) {
- int check_init_state;
- int agent_init_state;
-
newsrv = new_server(px);
if (!newsrv)
goto err;
@@ -2128,14 +1984,6 @@
/* Set this new server ID. */
srv_set_id_from_prefix(newsrv, srv->tmpl_info.prefix, i);
- /* Initial checks states. */
- check_init_state = CHK_ST_CONFIGURED | CHK_ST_ENABLED;
- agent_init_state = CHK_ST_CONFIGURED | CHK_ST_ENABLED | CHK_ST_AGENT;
-
- if (do_health_check_init(newsrv, px->options2 & PR_O2_CHK_ANY, check_init_state) ||
- do_server_agent_check_init(newsrv, agent_init_state))
- goto err;
-
/* Linked backwards first. This will be restablished after parsing. */
newsrv->next = px->srv;
px->srv = newsrv;
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index e679142..868808c 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -4737,8 +4737,6 @@
}
if (srv->use_ssl == 1)
srv->xprt = &ssl_sock;
- if (srv->check.use_ssl == 1)
- srv->check.xprt = &ssl_sock;
ctx = SSL_CTX_new(SSLv23_client_method());
if (!ctx) {