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 0ed68b8..a7cc5d5 100644
--- a/include/proto/server.h
+++ b/include/proto/server.h
@@ -45,6 +45,7 @@
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 3c75729..4923d86 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -8450,13 +8450,6 @@
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 8198e4e..2b12ba3 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -956,6 +956,9 @@
/* 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 cc1f0d1..f38ee8d 100644
--- a/src/server.c
+++ b/src/server.c
@@ -710,6 +710,40 @@
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 @@
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]);
}