From: Willy Tarreau Date: Thu, 3 Nov 2016 18:22:19 +0000 (+0100) Subject: BUG/MEDIUM: servers: properly propagate the maintenance states during startup X-Git-Tag: v1.7-dev6~42 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=757478e900ebc1e67c1fe060b9b561d276cab3e1;p=thirdparty%2Fhaproxy.git 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 0ed68b8c1a..a7cc5d5691 100644 --- 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 3c757296cd..4923d86094 100644 --- 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 8198e4efcb..2b12ba3bf9 100644 --- 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 cc1f0d10ab..f38ee8d0c2 100644 --- 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 . + * 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 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]); }