BUG/MINOR: server: Properly handle init-state value during haproxy startup

Unlike stated in the configuration manual, the server 'init-state' parameter
was not evaluated during haproxy startup/reload. After a review, it appeared
there were also issues if combined with the 'track' parameter. In addtition,
this parameter was only evaluated when health-checks were enabled for the
server, leading to unexpected behavior if the serve settings are dynamically
changed via the CLI.

To fix those issues, behavior of the 'init-state' parameter was slightly
adapted. It is always evaluated, even when there is no running health-checks
for the server. An error is reported if the 'track' parameter is also
defined. Both cannot work together.

In addition, the "none" state was introduced to be able to restore the
default behavior. It will be especially useful when the parameter is
inherited from a 'default-server' directive.

This patch should fix the issue #3298. It must be backported as far as 3.2.

diff --git a/doc/configuration.txt b/doc/configuration.txt
index a51701fcf8e863638cc74ff515df0e126074f45a..0d6ba70d1b39cdc9451271334b989fbab8f323fb 100644 (file)
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -18972,7 +18972,7 @@ downinter <delay>
   "inter" setting will have a very limited effect as it will not be able to
   reduce the time spent in the queue.
 
-init-state { fully-up | up | down | fully-down }
+init-state { fully-up | up | down | fully-down | none }
   May be used in the following contexts: tcp, http
 
   May be used in sections :   defaults | frontend | listen | backend
@@ -18980,20 +18980,25 @@ init-state { fully-up | up | down | fully-down }
 
   The "init-state" option sets the initial state of the server:
     - when set to 'fully-up', the server is considered immediately available
-      and can turn to the DOWN state when ALL health checks fail.
-    - when set to 'up' (the default), the server is considered immediately
-      available and will initiate a health check that can turn it to the DOWN
-      state immediately if it fails.
-    - when set to 'down', the server initially is considered unavailable and
-      will initiate a health check that can turn it to the UP state immediately
-      if it succeeds.
+      and, if health checks are enabled for this server, it will be turned to
+      the DOWN state when ALL health checks fail.
+    - when set to 'up', the server is considered immediately available and, if
+      health checks are enabled for this server, it will be turned to the DOWN
+      state immediately if the next health check fails.
+    - when set to 'down', the server initially is considered unavailable and,
+      if health checks are enabled for this server, it can be turned to the UP
+      state if the next health check succeeds.
     - when set to 'fully-down', the server is initially considered unavailable
-      and can turn to the UP state when ALL health checks succeed.
+      and, if health checks are enabled for this server, it will turned to the
+      UP state when ALL health checks succeed.
+    - when set to 'none' (the default value), init-state management is
+      disabled. It can be used to restore the default behavior when this
+      parameter was inherited from a 'default-server' directive.
 
   The server's init-state is considered when the HAProxy instance is
   (re)started, a new server is detected (for example via service discovery /
   DNS resolution), a dynamic server is inlived, a server exits maintenance,
-  etc.
+  etc. This directive cannot be used when the server is tracking another one.
 
   Examples:
       # pass client traffic ONLY to Redis "master" node

diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
index c8f318c5ce541def2c3414c91890989c8687b29e..69265cb068eee7c09c130698117422575a0c934f 100644 (file)
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -111,7 +111,8 @@ enum srv_initaddr {
  * at start up time.
  */
 enum srv_init_state {
-       SRV_INIT_STATE_FULLY_DOWN = 0,     /* the server should initially be considered DOWN until it passes all health checks. Please keep set to zero. */
+       SRV_INIT_STATE_NONE = 0,
+       SRV_INIT_STATE_FULLY_DOWN,         /* the server should initially be considered DOWN until it passes all health checks. Please keep set to zero. */
        SRV_INIT_STATE_DOWN,               /* the server should initially be considered DOWN until it passes one health check. */
        SRV_INIT_STATE_UP,                 /* the server should initially be considered UP, but will go DOWN if it fails one health check. */
        SRV_INIT_STATE_FULLY_UP,           /* the server should initially be considered UP, but will go DOWN if it fails all health checks. */

diff --git a/include/haproxy/server.h b/include/haproxy/server.h
index 5ffc06880f1eda342a4069db03b0501bcafeeed6..0d58929683077864173bd3c8c734b64be412a4d2 100644 (file)
--- a/include/haproxy/server.h
+++ b/include/haproxy/server.h
@@ -278,6 +278,35 @@ static inline void srv_adm_set_ready(struct server *s)
        srv_clr_admin_flag(s, SRV_ADMF_FMAINT);
 }
 
+static inline void srv_set_init_state(struct server *srv)
+{
+       /* no init-state configured or the server is already disabled: don't eval init-state */
+       if (srv->init_state == SRV_INIT_STATE_NONE ||
+           srv->next_admin & (SRV_ADMF_CMAINT | SRV_ADMF_FMAINT))
+               return;
+
+       if (srv->init_state == SRV_INIT_STATE_FULLY_UP) {
+               /* initially UP, when all checks fail to bring server DOWN */
+               srv->next_state = SRV_ST_RUNNING;
+               srv->check.health = srv->check.rise + srv->check.fall - 1;
+       }
+       else if (srv->init_state == SRV_INIT_STATE_UP) {
+               /* initially UP, when one check fails check brings server DOWN */
+               srv->next_state = SRV_ST_RUNNING;
+               srv->check.health = srv->check.rise;
+       }
+       else if (srv->init_state == SRV_INIT_STATE_DOWN) {
+              /* initially DOWN, when one check is successful bring server UP */
+               srv->next_state = SRV_ST_STOPPED;
+               srv->check.health = srv->check.rise - 1;
+       }
+       else if (srv->init_state == SRV_INIT_STATE_FULLY_DOWN) {
+              /* initially DOWN, when all checks are successful bring server UP */
+               srv->next_state = SRV_ST_STOPPED;
+               srv->check.health = 0;
+       }
+}
+
 /* appends an initaddr method to the existing list. Returns 0 on failure. */
 static inline int srv_append_initaddr(unsigned int *list, enum srv_initaddr addr)
 {

diff --git a/src/server.c b/src/server.c
index fce4ab71e235ab733bf805baef901dc57b348949..014b5e9a35e74f1a04b0d74cc7712b54273da8f5 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -935,6 +935,8 @@ static int srv_parse_enabled(char **args, int *cur_arg,
        newsrv->next_state = SRV_ST_RUNNING;
        newsrv->check.state &= ~CHK_ST_PAUSED;
        newsrv->check.health = newsrv->check.rise;
+
+       srv_set_init_state(newsrv);
        return 0;
 }
 
@@ -1124,22 +1126,25 @@ static int srv_parse_init_addr(char **args, int *cur_arg,
 
 /* Parse the "init-state" server keyword */
 static int srv_parse_init_state(char **args, int *cur_arg,
-                                                          struct proxy *curproxy, struct server *newsrv, char **err)
+                               struct proxy *curproxy, struct server *newsrv, char **err)
 {
-       if (strcmp(args[*cur_arg + 1], "fully-up") == 0)
-               newsrv->init_state= SRV_INIT_STATE_FULLY_UP;
+       if (strcmp(args[*cur_arg + 1], "none") == 0)
+               newsrv->init_state = SRV_INIT_STATE_NONE;
+       else if (strcmp(args[*cur_arg + 1], "fully-up") == 0)
+               newsrv->init_state = SRV_INIT_STATE_FULLY_UP;
        else if (strcmp(args[*cur_arg + 1], "up") == 0)
                newsrv->init_state = SRV_INIT_STATE_UP;
        else if (strcmp(args[*cur_arg + 1], "down") == 0)
-               newsrv->init_state= SRV_INIT_STATE_DOWN;
+               newsrv->init_state = SRV_INIT_STATE_DOWN;
        else if (strcmp(args[*cur_arg + 1], "fully-down") == 0)
-               newsrv->init_state= SRV_INIT_STATE_FULLY_DOWN;
+               newsrv->init_state = SRV_INIT_STATE_FULLY_DOWN;
        else {
-               memprintf(err, "'%s' expects one of 'fully-up', 'up', 'down', or 'fully-down' but got '%s'",
+               memprintf(err, "'%s' expects one of 'none', 'fully-up', 'up', 'down', or 'fully-down' but got '%s'",
                                  args[*cur_arg], args[*cur_arg + 1]);
                return ERR_ALERT | ERR_FATAL;
        }
 
+       srv_set_init_state(newsrv);
        return 0;
 }
 
@@ -2880,7 +2885,7 @@ void srv_settings_init(struct server *srv)
        srv->agent.fall = DEF_AGENT_FALLTIME;
        srv->agent.port = 0;
 
-       srv->init_state = SRV_INIT_STATE_UP;
+       srv->init_state = SRV_INIT_STATE_NONE;
 
        srv->maxqueue = 0;
        srv->minconn = 0;
@@ -3014,7 +3019,9 @@ void srv_settings_cpy(struct server *srv, const struct server *src, int srv_tmpl
        srv->check.rise = srv->check.health = src->check.rise;
        srv->check.fall               = src->check.fall;
 
-       /* Here we check if 'disabled' is the default server state */
+       /* Here we check if 'disabled' is the default server state. Otherwise,
+        * we check 'init-state' parameter
+        */
        if (src->next_admin & (SRV_ADMF_CMAINT | SRV_ADMF_FMAINT)) {
                srv->next_admin |= SRV_ADMF_CMAINT | SRV_ADMF_FMAINT;
                srv->next_state        = SRV_ST_STOPPED;
@@ -3042,6 +3049,7 @@ void srv_settings_cpy(struct server *srv, const struct server *src, int srv_tmpl
        srv->init_addr                = src->init_addr;
 
        srv->init_state               = src->init_state;
+       srv_set_init_state(srv);
 #if defined(USE_OPENSSL)
        srv_ssl_settings_cpy(srv, src);
 #endif
@@ -3958,6 +3966,10 @@ static int _srv_parse_finalize(char **args, int cur_arg,
                ha_alert("unable to enable checks and tracking at the same time!\n");
                return ERR_ALERT | ERR_FATAL;
        }
+       if (srv->init_state != SRV_INIT_STATE_NONE && srv->trackit) {
+               ha_alert("unable to set init-state and tracking at the same time!\n");
+               return ERR_ALERT | ERR_FATAL;
+       }
 
        if (srv->do_agent && !srv->agent.port) {
                ha_alert("server %s does not have agent port. Agent check has been disabled.\n",
@@ -5927,6 +5939,7 @@ static int cli_parse_enable_health(char **args, char *payload, struct appctx *ap
 
        HA_SPIN_LOCK(SERVER_LOCK, &sv->lock);
        sv->check.state |= CHK_ST_ENABLED;
+       srv_set_init_state(sv);
        HA_SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
        return 1;
 }
@@ -6971,20 +6984,12 @@ static int _srv_update_status_adm(struct server *s, enum srv_adm_st_chg_cause ca
                 */
                if (s->check.state & CHK_ST_ENABLED) {
                        s->check.state &= ~CHK_ST_PAUSED;
-                       if(s->init_state == SRV_INIT_STATE_FULLY_UP) {
-                               s->check.health = s->check.rise + s->check.fall - 1; /* initially UP, when all checks fail to bring server DOWN */
-                       }
-                       else if(s->init_state == SRV_INIT_STATE_DOWN) {
-                               s->check.health = s->check.rise - 1; /* initially DOWN, when one check is successful bring server UP */
-                       }
-                       else if(s->init_state == SRV_INIT_STATE_FULLY_DOWN) {
-                               s->check.health = 0; /* initially DOWN, when all checks are successful bring server UP */
-                       } else {
-                               s->check.health = s->check.rise; /* initially UP, when one check fails check brings server DOWN */
-                       }
+                       s->check.health = s->check.rise; /* start OK but check immediately */
                }
+               srv_set_init_state(s);
 
-               if ((!s->track || s->track->next_state != SRV_ST_STOPPED) &&
+               if (s->init_state == SRV_INIT_STATE_NONE &&
+                   (!s->track || s->track->next_state != SRV_ST_STOPPED) &&
                    (!(s->agent.state & CHK_ST_ENABLED) || (s->agent.health >= s->agent.rise)) &&
                    (!(s->check.state & CHK_ST_ENABLED) || (s->check.health >= s->check.rise))) {
                        if (s->track && s->track->next_state == SRV_ST_STOPPING) {