BUG/MEDIUM: server: server stuck in maintenance after FQDN change

Pierre Bonnat reported that SRV-based server-template recently stopped
to work properly.

After reviewing the changes, it was found that the regression was caused
by a4d04c6 ("BUG/MINOR: server: make sure the HMAINT state is part of MAINT")

Indeed, HMAINT is not a regular maintenance flag. It was implemented in
b418c122 a4d04c6 ("BUG/MINOR: server: make sure the HMAINT state is part
of MAINT"). This flag is only set (and never removed) when the server FQDN
is changed from its initial config-time value. This can happen with "set
server fqdn" command as well as SRV records updates from the DNS. This
flag should ideally belong to server flags.. but it was stored under
srv_admin enum because cur_admin is properly exported/imported via server
state-file while regular server's flags are not.

Due to a4d04c6, when a server FQDN changes, the server is considered in
maintenance, and since the HMAINT flag is never removed, the server is
stuck in maintenance.

To fix the issue, we partially revert a4d04c6. But this latter commit is
right on one point: HMAINT flag was way too confusing and mixed-up between
regular MAINT flags, thus there's nothing to blame about a4d04c6 as it was
error-prone anyway.. To prevent such kind of bugs from happening again,
let's rename HMAINT to something more explicit (SRV_ADMF_FQDN_CHANGED) and
make it stand out under srv_admin enum so we're not tempted to mix it with
regular maintenance flags anymore.

Since a4d04c6 was set to be backported in all versions, this patch must
be backported there as well.

diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
index b29106d4c02f94f9476874b895d19e8aec6c8c4e..84b3b87c0c845b70b114959caf446df3793e91ad 100644 (file)
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -82,9 +82,13 @@ enum srv_admin {
        SRV_ADMF_IDRAIN    = 0x10,        /* the server has inherited the drain status from a tracked server */
        SRV_ADMF_DRAIN     = 0x18,        /* mask to check if any drain flag is present */
        SRV_ADMF_RMAINT    = 0x20,        /* the server is down because of an IP address resolution failure */
-       SRV_ADMF_HMAINT    = 0x40,        /* the server FQDN has been set from socket stats */
 
-       SRV_ADMF_MAINT     = 0x63,        /* mask to check if any maintenance flag except CMAINT is present */
+       SRV_ADMF_MAINT     = 0x23,        /* mask to check if any maintenance flag except CMAINT is present */
+
+       SRV_ADMF_FQDN_CHANGED = 0x40,     /* Special value: set (and never removed) if the server fqdn has
+                                          * changed (from cli or resolvers) since its initial value from
+                                          * config. This flag is exported and restored through state-file
+                                          */
 } __attribute__((packed));
 
 /* options for servers' "init-addr" parameter

diff --git a/src/server.c b/src/server.c
index 58862c5a17b5f8ee3d0e731d05a1e8d7a93ab9a4..b3900933d58520097aeb3431ac0d38f0bdb26d44 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -5115,8 +5115,8 @@ const char *srv_update_fqdn(struct server *server, const char *fqdn, const char
                goto out;
        }
 
-       /* Flag as FQDN set from stats socket. */
-       server->next_admin |= SRV_ADMF_HMAINT;
+       /* Flag as FQDN changed (e.g.: set from stats socket or resolvers) */
+       server->next_admin |= SRV_ADMF_FQDN_CHANGED;
 
  out:
        if (updater)

diff --git a/src/server_state.c b/src/server_state.c
index 38cf383f44a32afc634f61cd161a18e248ccc513..131f339ea2487c0586a3634f9a11db7f39bfc8b3 100644 (file)
--- a/src/server_state.c
+++ b/src/server_state.c
@@ -53,7 +53,7 @@ static void srv_state_srv_update(struct server *srv, int version, char **params)
        int srv_check_state, srv_agent_state;
        int bk_f_forced_id;
        int srv_f_forced_id;
-       int fqdn_set_by_cli;
+       int fqdn_changed;
        const char *fqdn;
        const char *port_st;
        unsigned int port_svc;
@@ -112,12 +112,12 @@ static void srv_state_srv_update(struct server *srv, int version, char **params)
        p = NULL;
        errno = 0;
        srv_admin_state = strtol(params[2], &p, 10);
-       fqdn_set_by_cli = !!(srv_admin_state & SRV_ADMF_HMAINT);
+       fqdn_changed = !!(srv_admin_state & SRV_ADMF_FQDN_CHANGED);
 
        /* inherited statuses will be recomputed later.
-        * Also disable SRV_ADMF_HMAINT flag (set from stats socket fqdn).
+        * Also disable SRV_ADMF_FQDN_CHANGED flag (set from stats socket fqdn).
         */
-       srv_admin_state &= ~SRV_ADMF_IDRAIN & ~SRV_ADMF_IMAINT & ~SRV_ADMF_HMAINT & ~SRV_ADMF_RMAINT;
+       srv_admin_state &= ~SRV_ADMF_IDRAIN & ~SRV_ADMF_IMAINT & ~SRV_ADMF_RMAINT & ~SRV_ADMF_FQDN_CHANGED;
 
        if ((p == params[2]) || errno == EINVAL || errno == ERANGE ||
            (srv_admin_state != 0 &&
@@ -372,7 +372,7 @@ static void srv_state_srv_update(struct server *srv, int version, char **params)
                         * So we must reset the 'set from stats socket FQDN' flag to be consistent with
                         * any further FQDN modification.
                         */
-                       srv->next_admin &= ~SRV_ADMF_HMAINT;
+                       srv->next_admin &= ~SRV_ADMF_FQDN_CHANGED;
                }
                else {
                        /* If the FDQN has been changed from stats socket,
@@ -380,10 +380,10 @@ static void srv_state_srv_update(struct server *srv, int version, char **params)
                         * from stats socket).
                         * Also ensure the runtime resolver will process this resolution.
                         */
-                       if (fqdn_set_by_cli) {
+                       if (fqdn_changed) {
                                srv_set_fqdn(srv, fqdn, 0);
                                srv->flags &= ~SRV_F_NO_RESOLUTION;
-                               srv->next_admin |= SRV_ADMF_HMAINT;
+                               srv->next_admin |= SRV_ADMF_FQDN_CHANGED;
                        }
                }
        }