From: Aurelien DARRAGON <adarragon@haproxy.com>
Date: Mon, 17 Apr 2023 15:45:58 +0000 (+0200)
Subject: MINOR: server: explicitly commit state change in srv_update_status()
X-Git-Tag: v2.8-dev8~58
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=22151c70bbc8af166ccbe04f50950f80d4a22c7d;p=thirdparty%2Fhaproxy.git

MINOR: server: explicitly commit state change in srv_update_status()

As shown in 8f29829 ("BUG/MEDIUM: checks: a down server going to
maint remains definitely stucked on down state."), state changes
that don't result in explicit lb state change, require us to perform
an explicit server state commit to make sure the next state is
applied before returning from the function.

This is the case for server state changes that don't trigger lb logic
and only perform some logging.

This is quite error prone, we could easily forget a state change
combination that could result in next_state, next_admin or next_eweight
not being applied. (cur_state, cur_admin and cur_eweight would be left
with unexpected values)

To fix this, we explicitly call srv_lb_commit_status() at the end
of srv_update_status() to enforce the new values, even if they were
already applied. (when a state changes requires lb state update
an implicit commit is already performed)

Applying the state change multiple times is safe (since the next value
always points to the current value).

Backport notes:

2.2:

Replace
	struct task *srv_cleanup_toremove_conns(struct task *task, void *context, unsigned int state)

by
	struct task *srv_cleanup_toremove_connections(struct task *task, void *context, unsigned short state)
---

diff --git a/src/server.c b/src/server.c
index b78f8a6888..db0b881be6 100644
--- a/src/server.c
+++ b/src/server.c
@@ -5485,9 +5485,6 @@ static void srv_update_status(struct server *s)
 				free_trash_chunk(tmptrash);
 				tmptrash = NULL;
 			}
-			/* commit new admin status */
-
-			s->cur_admin = s->next_admin;
 		}
 		else {	/* server was still running */
 			check->health = 0; /* failure */
@@ -5702,8 +5699,6 @@ static void srv_update_status(struct server *s)
 			}
 		}
 		/* don't report anything when leaving drain mode and remaining in maintenance */
-
-		s->cur_admin = s->next_admin;
 	}
 
 	if (!(s->next_admin & SRV_ADMF_MAINT)) {
@@ -5821,15 +5816,16 @@ static void srv_update_status(struct server *s)
 				free_trash_chunk(tmptrash);
 				tmptrash = NULL;
 			}
-
-			/* commit new admin status */
-
-			s->cur_admin = s->next_admin;
 		}
 	}
 
 	/* Re-set log strings to empty */
 	*s->adm_st_chg_cause = 0;
+
+	/* explicitly commit state changes (even if it was already applied implicitly
+	 * by some lb state change function), so we don't miss anything
+	 */
+	srv_lb_commit_status(s);
 }
 
 struct task *srv_cleanup_toremove_conns(struct task *task, void *context, unsigned int state)