MINOR: server: srv_append_status refacto
srv_append_status() has become a swiss-knife function over time.
It is used from server code and also from checks code, with various
inputs and distincts code paths, making it very hard to guess the
actual behavior of the function (resulting string output).
To simplify the logic behind it, we're dividing it in multiple contextual
functions that take simple inputs and do explicit things, making them
more predictable and easier to maintain.
diff --git a/include/haproxy/check.h b/include/haproxy/check.h
index 485c494..c90d3e7 100644
--- a/include/haproxy/check.h
+++ b/include/haproxy/check.h
@@ -67,6 +67,7 @@
const char *get_check_status_info(short check_status);
int httpchk_build_status_header(struct server *s, struct buffer *buf);
void __health_adjust(struct server *s, short status);
+void check_append_info(struct buffer *msg, struct check *check);
void set_server_check_status(struct check *check, short status, const char *desc);
void chk_report_conn_err(struct check *check, int errno_bck, int expired);
void check_notify_failure(struct check *check);
diff --git a/src/check.c b/src/check.c
index f39f88d..2ff3bdb 100644
--- a/src/check.c
+++ b/src/check.c
@@ -428,6 +428,31 @@
return analyze_statuses[HANA_STATUS_UNKNOWN].desc;
}
+/* append check info to buffer msg */
+void check_append_info(struct buffer *msg, struct check *check)
+{
+ if (!check)
+ return;
+ chunk_appendf(msg, ", reason: %s", get_check_status_description(check->status));
+
+ if (check->status >= HCHK_STATUS_L57DATA)
+ chunk_appendf(msg, ", code: %d", check->code);
+
+ if (check->desc[0]) {
+ struct buffer src;
+
+ chunk_appendf(msg, ", info: \"");
+
+ chunk_initlen(&src, check->desc, 0, strlen(check->desc));
+ chunk_asciiencode(msg, &src, '"');
+
+ chunk_appendf(msg, "\"");
+ }
+
+ if (check->duration >= 0)
+ chunk_appendf(msg, ", check duration: %ldms", check->duration);
+}
+
/* Sets check->status, update check->duration and fill check->result with an
* adequate CHK_RES_* value. The new check->health is computed based on the
* result.
@@ -528,7 +553,7 @@
(check->result == CHK_RES_CONDPASS) ? "conditionally ":"",
(check->result >= CHK_RES_PASSED) ? "succeeded" : "failed");
- srv_append_status(&trash, s, check, -1, 0);
+ check_append_info(&trash, check);
chunk_appendf(&trash, ", status: %d/%d %s",
(check->health >= check->rise) ? check->health - check->rise + 1 : check->health,
diff --git a/src/server.c b/src/server.c
index 9483e89..501657b 100644
--- a/src/server.c
+++ b/src/server.c
@@ -1511,57 +1511,33 @@
srv_shutdown_streams(srv, why);
}
-/* Appends some information to a message string related to a server going UP or
- * DOWN. If both <forced> and <reason> are null and the server tracks another
- * one, a "via" information will be provided to know where the status came from.
- * If <check> is non-null, an entire string describing the check result will be
- * appended after a comma and a space (eg: to report some information from the
- * check that changed the state). In the other case, the string will be built
- * using the check results stored into the struct server if present.
+static void srv_append_op_chg_cause(struct buffer *msg, struct server *s, struct check *check)
+{
+ if (check)
+ check_append_info(msg, check);
+ else if (s->op_st_chg.reason && *s->op_st_chg.reason)
+ chunk_appendf(msg, ", %s", s->op_st_chg.reason);
+}
+
+static void srv_append_adm_chg_cause(struct buffer *msg, struct server *s)
+{
+ if (s->adm_st_chg_cause)
+ chunk_appendf(msg, " (%s)", srv_adm_st_chg_cause(s->adm_st_chg_cause));
+}
+
+/* Appends some information to a message string related to a server tracking
+ * or requeued connections info.
+ *
+ * If <forced> is null and the server tracks another one, a "via"
* If <xferred> is non-negative, some information about requeued sessions are
* provided.
*
* Must be called with the server lock held.
*/
-void srv_append_status(struct buffer *msg, struct server *s,
- struct check *check, int xferred, int forced)
+static void srv_append_more(struct buffer *msg, struct server *s,
+ int xferred, int forced)
{
- short status = s->op_st_chg.status;
- short code = s->op_st_chg.code;
- long duration = s->op_st_chg.duration;
- char *desc = s->op_st_chg.reason;
-
- if (check) {
- status = check->status;
- code = check->code;
- duration = check->duration;
- desc = check->desc;
- }
-
- if (status != -1) {
- chunk_appendf(msg, ", reason: %s", get_check_status_description(status));
-
- if (status >= HCHK_STATUS_L57DATA)
- chunk_appendf(msg, ", code: %d", code);
-
- if (desc && *desc) {
- struct buffer src;
-
- chunk_appendf(msg, ", info: \"");
-
- chunk_initlen(&src, desc, 0, strlen(desc));
- chunk_asciiencode(msg, &src, '"');
-
- chunk_appendf(msg, "\"");
- }
-
- if (duration >= 0)
- chunk_appendf(msg, ", check duration: %ldms", duration);
- }
- else if (desc && *desc) {
- chunk_appendf(msg, ", %s", desc);
- }
- else if (!forced && s->track) {
+ if (!forced && s->track) {
chunk_appendf(msg, " via %s/%s", s->track->proxy->id, s->track->id);
}
@@ -5354,7 +5330,9 @@
"%sServer %s/%s is DOWN", s->flags & SRV_F_BACKUP ? "Backup " : "",
s->proxy->id, s->id);
- srv_append_status(tmptrash, s, NULL, xferred, 0);
+ srv_append_op_chg_cause(tmptrash, s, NULL);
+ srv_append_more(tmptrash, s, xferred, 0);
+
ha_warning("%s.\n", tmptrash->area);
/* we don't send an alert if the server was previously paused */
@@ -5382,7 +5360,8 @@
"%sServer %s/%s is stopping", s->flags & SRV_F_BACKUP ? "Backup " : "",
s->proxy->id, s->id);
- srv_append_status(tmptrash, s, NULL, xferred, 0);
+ srv_append_op_chg_cause(tmptrash, s, NULL);
+ srv_append_more(tmptrash, s, xferred, 0);
ha_warning("%s.\n", tmptrash->area);
send_log(s->proxy, LOG_NOTICE, "%s.\n",
@@ -5424,7 +5403,9 @@
"%sServer %s/%s is UP", s->flags & SRV_F_BACKUP ? "Backup " : "",
s->proxy->id, s->id);
- srv_append_status(tmptrash, s, NULL, xferred, 0);
+ srv_append_op_chg_cause(tmptrash, s, NULL);
+ srv_append_more(tmptrash, s, xferred, 0);
+
ha_warning("%s.\n", tmptrash->area);
send_log(s->proxy, LOG_NOTICE, "%s.\n",
tmptrash->area);
@@ -5460,13 +5441,10 @@
tmptrash = alloc_trash_chunk();
if (tmptrash) {
chunk_printf(tmptrash,
- "%sServer %s/%s was DOWN and now enters maintenance%s%s%s",
- s->flags & SRV_F_BACKUP ? "Backup " : "", s->proxy->id, s->id,
- (s->adm_st_chg_cause) ? " (" : "",
- srv_adm_st_chg_cause(s->adm_st_chg_cause),
- (s->adm_st_chg_cause) ? ")" : "");
-
- srv_append_status(tmptrash, s, NULL, -1, (s->next_admin & SRV_ADMF_FMAINT));
+ "%sServer %s/%s was DOWN and now enters maintenance",
+ s->flags & SRV_F_BACKUP ? "Backup " : "", s->proxy->id, s->id);
+ srv_append_adm_chg_cause(tmptrash, s);
+ srv_append_more(tmptrash, s, -1, (s->next_admin & SRV_ADMF_FMAINT));
if (!(global.mode & MODE_STARTING)) {
ha_warning("%s.\n", tmptrash->area);
@@ -5500,14 +5478,11 @@
tmptrash = alloc_trash_chunk();
if (tmptrash) {
chunk_printf(tmptrash,
- "%sServer %s/%s is going DOWN for maintenance%s%s%s",
+ "%sServer %s/%s is going DOWN for maintenance",
s->flags & SRV_F_BACKUP ? "Backup " : "",
- s->proxy->id, s->id,
- (s->adm_st_chg_cause) ? " (" : "",
- srv_adm_st_chg_cause(s->adm_st_chg_cause),
- (s->adm_st_chg_cause) ? ")" : "");
-
- srv_append_status(tmptrash, s, NULL, xferred, (s->next_admin & SRV_ADMF_FMAINT));
+ s->proxy->id, s->id);
+ srv_append_adm_chg_cause(tmptrash, s);
+ srv_append_more(tmptrash, s, xferred, (s->next_admin & SRV_ADMF_FMAINT));
if (!(global.mode & MODE_STARTING)) {
ha_warning("%s.\n", tmptrash->area);
@@ -5686,13 +5661,10 @@
tmptrash = alloc_trash_chunk();
if (tmptrash) {
- chunk_printf(tmptrash, "%sServer %s/%s enters drain state%s%s%s",
- s->flags & SRV_F_BACKUP ? "Backup " : "", s->proxy->id, s->id,
- (s->adm_st_chg_cause) ? " (" : "",
- srv_adm_st_chg_cause(s->adm_st_chg_cause),
- (s->adm_st_chg_cause) ? ")" : "");
-
- srv_append_status(tmptrash, s, NULL, xferred, (s->next_admin & SRV_ADMF_FDRAIN));
+ chunk_printf(tmptrash, "%sServer %s/%s enters drain state",
+ s->flags & SRV_F_BACKUP ? "Backup " : "", s->proxy->id, s->id);
+ srv_append_adm_chg_cause(tmptrash, s);
+ srv_append_more(tmptrash, s, xferred, (s->next_admin & SRV_ADMF_FDRAIN));
if (!(global.mode & MODE_STARTING)) {
ha_warning("%s.\n", tmptrash->area);