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.

diff --git a/src/cfgparse.c b/src/cfgparse.c
index 926cd1d66bcee00b54c59758321a0f5a03e93238..d049899b34d9b0226095a602727fc6ccfb27b1b6 100644 (file)
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -3265,9 +3265,6 @@ out_uri_auth_compat:
                                    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;
@@ -3304,9 +3301,7 @@ out_uri_auth_compat:
                                        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 f52647b721721e7737d558b7bcceaf01443d4e9e..b932d25f420c3b3a2592794659b5e86dcab097ec 100644 (file)
--- a/src/checks.c
+++ b/src/checks.c
@@ -3310,6 +3310,10 @@ const char *init_check(struct check *check, int type)
 
 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) {
@@ -3441,7 +3445,6 @@ int init_email_alert(struct mailers *mls, struct proxy *p, char **err)
                struct email_alertq *q     = &queues[i];
                struct check        *check = &q->check;
 
-               task_destroy(check->task);
                free_check(check);
        }
        free(queues);
@@ -3664,17 +3667,6 @@ int srv_check_healthcheck_port(struct check *chk)
 
        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;
@@ -3701,6 +3693,130 @@ int srv_check_healthcheck_port(struct check *chk)
 
 REGISTER_POST_CHECK(start_checks);
 
+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;
+}
+
+static void deinit_srv_check(struct server *srv)
+{
+       if (srv->do_check)
+               free_check(&srv->check);
+}
+
+
+static void deinit_srv_agent_check(struct server *srv)
+{
+       if (srv->do_agent)
+               free_check(&srv->agent);
+       free(srv->agent.send_string);
+}
+
+REGISTER_POST_SERVER_CHECK(init_srv_check);
+REGISTER_POST_SERVER_CHECK(init_srv_agent_check);
+
+REGISTER_SERVER_DEINIT(deinit_srv_check);
+REGISTER_SERVER_DEINIT(deinit_srv_agent_check);
+
 /*
  * Local variables:
  *  c-indent-level: 8

diff --git a/src/haproxy.c b/src/haproxy.c
index a702e7869928743d5321da9f13e576d5eb9f99f6..982f7b94b75869918489f691a599a2bd256fb241 100644 (file)
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2662,23 +2662,11 @@ void deinit(void)
                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);

diff --git a/src/server.c b/src/server.c
index f2f9eb90b57a470874dc331ee6cded5331880b6e..8307ae2ca21b3a7054695049381ed5dd72c042e1 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -1880,154 +1880,6 @@ struct server *new_server(struct proxy *proxy)
        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)
@@ -2059,9 +1911,16 @@ static int server_finalize_init(const char *file, int linenum, char **args, int
 {
        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
@@ -2129,9 +1988,6 @@ static int server_template_init(struct server *srv, struct proxy *px)
        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;
@@ -2148,14 +2004,6 @@ static int server_template_init(struct server *srv, struct proxy *px)
                /* 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 224de28661132516c3bc2df6e9d3592fb4210e98..36f4da3f18b7af6b839afad1fc8f65d94c273c01 100644 (file)
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -5927,8 +5927,6 @@ int ssl_sock_prepare_srv_ctx(struct server *srv)
        }
        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) {