BUG/MEDIUM: servers: properly propagate the maintenance states during startup

Right now there is an issue with the way the maintenance flags are
propagated upon startup. They are not propagate, just copied from the
tracked server. This implies that depending on the server's order, some
tracking servers may not be marked down. For example this configuration
does not work as expected :

        server s1 1.1.1.1:8000 track s2
        server s2 1.1.1.1:8000 track s3
        server s3 1.1.1.1:8000 track s4
        server s4 wtap:8000 check inter 1s disabled

It results in s1/s2 being up, and s3/s4 being down, while all of them
should be down.

The only clean way to process this is to run through all "root" servers
(those not tracking any other server), and to propagate their state down
to all their trackers. This is the same algorithm used to propagate the
state changes. It has to be done both to compute the IDRAIN flag and the
IMAINT flag. However, doing so requires that tracking servers are not
marked as inherited maintenance anymore while parsing the configuration
(and given that it is wrong, better drop it).

This fix also addresses another side effect of the bug above which is
that the IDRAIN/IMAINT flags are stored in the state files, and if
restored while the tracked server doesn't have the equivalent flag,
the servers may end up in a situation where it's impossible to remove
these flags. For example in the configuration above, after removing
"disabled" on server s4, the other servers would have remained down,
and not anymore with this fix. Similarly, the combination of IMAINT
or IDRAIN with their respective forced modes was not accepted on
reload, which is wrong as well.

This bug has been present at least since 1.5, maybe even 1.4 (it came
with tracking support). The fix needs to be backported there, though
the srv-state parts are irrelevant.

This commit relies on previous patch to silence warnings on startup.

diff --git a/include/proto/server.h b/include/proto/server.h
index 0ed68b8c1a0651dc3ec0f73e5c6a13a96952a50b..a7cc5d56919f71908c60a4f33a65cfc3287ce519 100644 (file)
--- a/include/proto/server.h
+++ b/include/proto/server.h
@@ -45,6 +45,7 @@ struct server *server_find_by_id(struct proxy *bk, int id);
 struct server *server_find_by_name(struct proxy *bk, const char *name);
 struct server *server_find_best_match(struct proxy *bk, char *name, int id, int *diff);
 void apply_server_state(void);
+void srv_compute_all_admin_states(struct proxy *px);
 
 /* functions related to server name resolution */
 int snr_update_srv_status(struct server *s);

diff --git a/src/cfgparse.c b/src/cfgparse.c
index 3c757296cd96be7a391fd673c6c91b508348fde5..4923d8609478d1cc23339df43381602f67264304 100644 (file)
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -8450,13 +8450,6 @@ out_uri_auth_compat:
                                        goto next_srv;
                                }
 
-                               /* if the other server is forced disabled, we have to do the same here */
-                               if (srv->admin & SRV_ADMF_MAINT) {
-                                       newsrv->admin |= SRV_ADMF_IMAINT;
-                                       newsrv->state = SRV_ST_STOPPED;
-                                       newsrv->check.health = 0;
-                               }
-
                                newsrv->track = srv;
                                newsrv->tracknext = srv->trackers;
                                srv->trackers = newsrv;

diff --git a/src/haproxy.c b/src/haproxy.c
index 8198e4efcbafa1ef2696f52616632ae92992df0d..2b12ba3bf97c958940f2b3addbb803c2ba5c0e50 100644 (file)
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -956,6 +956,9 @@ void init(int argc, char **argv)
        /* Apply server states */
        apply_server_state();
 
+       for (px = proxy; px; px = px->next)
+               srv_compute_all_admin_states(px);
+
        global_listener_queue_task = task_new();
        if (!global_listener_queue_task) {
                Alert("Out of memory when initializing global task\n");

diff --git a/src/server.c b/src/server.c
index cc1f0d10ab0bd127b0bdcd59a112497cd5c47184..f38ee8d0c280d188c490ff7f03bce67f43aaa9ff 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -710,6 +710,40 @@ void srv_clr_admin_flag(struct server *s, enum srv_admin mode)
                srv_clr_admin_flag(srv, mode);
 }
 
+/* principle: propagate maint and drain to tracking servers. This is useful
+ * upon startup so that inherited states are correct.
+ */
+static void srv_propagate_admin_state(struct server *srv)
+{
+       struct server *srv2;
+
+       if (!srv->trackers)
+               return;
+
+       for (srv2 = srv->trackers; srv2; srv2 = srv2->tracknext) {
+               if (srv->admin & (SRV_ADMF_MAINT | SRV_ADMF_CMAINT))
+                       srv_set_admin_flag(srv2, SRV_ADMF_IMAINT);
+
+               if (srv->admin & SRV_ADMF_DRAIN)
+                       srv_set_admin_flag(srv2, SRV_ADMF_IDRAIN);
+       }
+}
+
+/* Compute and propagate the admin states for all servers in proxy <px>.
+ * Only servers *not* tracking another one are considered, because other
+ * ones will be handled when the server they track is visited.
+ */
+void srv_compute_all_admin_states(struct proxy *px)
+{
+       struct server *srv;
+
+       for (srv = px->srv; srv; srv = srv->next) {
+               if (srv->track)
+                       continue;
+               srv_propagate_admin_state(srv);
+       }
+}
+
 /* Note: must not be declared <const> as its list will be overwritten.
  * Please take care of keeping this list alphabetically sorted, doing so helps
  * all code contributors.
@@ -2028,15 +2062,17 @@ static void srv_update_state(struct server *srv, int version, char **params)
                        p = NULL;
                        errno = 0;
                        srv_admin_state = strtol(params[2], &p, 10);
+
+                       /* inherited statuses will be recomputed later */
+                       srv_admin_state &= ~SRV_ADMF_IDRAIN & ~SRV_ADMF_IMAINT;
+
                        if ((p == params[2]) || errno == EINVAL || errno == ERANGE ||
                            (srv_admin_state != 0 &&
                             srv_admin_state != SRV_ADMF_FMAINT &&
-                            srv_admin_state != SRV_ADMF_IMAINT &&
                             srv_admin_state != SRV_ADMF_CMAINT &&
                             srv_admin_state != (SRV_ADMF_CMAINT | SRV_ADMF_FMAINT) &&
                             srv_admin_state != (SRV_ADMF_CMAINT | SRV_ADMF_FDRAIN) &&
-                            srv_admin_state != SRV_ADMF_FDRAIN &&
-                            srv_admin_state != SRV_ADMF_IDRAIN)) {
+                            srv_admin_state != SRV_ADMF_FDRAIN)) {
                                chunk_appendf(msg, ", invalid srv_admin_state value '%s'", params[2]);
                        }