MINOR: checks: update dynamic environment variables in external checks

commit 9ede66b0 introduced an environment variable (HAPROXY_SERVER_CURCONN) that
was supposed to be dynamically updated, but it was set only once, during its
initialization.

Most of the code provided in this previous patch has been rewritten in order to
easily update the environment variables without reallocating memory during each
check.

Now, HAPROXY_SERVER_CURCONN will contain the current number of connections on
the server at the time of the check.

diff --git a/include/types/checks.h b/include/types/checks.h
index 2ffbc2927ade423f2bb81782f447912c973f7396..831166e70f263eed68e536610018022f55463150 100644 (file)
--- a/include/types/checks.h
+++ b/include/types/checks.h
@@ -80,6 +80,32 @@ enum {
        HCHK_STATUS_SIZE
 };
 
+/* environment variables memory requirement for different types of data */
+#define EXTCHK_SIZE_EVAL_INIT 0                /* size determined during the init phase,
+                                        * such environment variables are not updatable. */
+#define EXTCHK_SIZE_ULONG     20       /* max string length for an unsigned long value */
+
+/* external checks environment variables */
+enum {
+       EXTCHK_PATH = 0,
+
+       /* Proxy specific environment variables */
+       EXTCHK_HAPROXY_PROXY_NAME,      /* the backend name */
+       EXTCHK_HAPROXY_PROXY_ID,        /* the backend id */
+       EXTCHK_HAPROXY_PROXY_ADDR,      /* the first bind address if available (or empty) */
+       EXTCHK_HAPROXY_PROXY_PORT,      /* the first bind port if available (or empty) */
+
+       /* Server specific environment variables */
+       EXTCHK_HAPROXY_SERVER_NAME,     /* the server name */
+       EXTCHK_HAPROXY_SERVER_ID,       /* the server id */
+       EXTCHK_HAPROXY_SERVER_ADDR,     /* the server address */
+       EXTCHK_HAPROXY_SERVER_PORT,     /* the server port if available (or empty) */
+       EXTCHK_HAPROXY_SERVER_MAXCONN,  /* the server max connections */
+       EXTCHK_HAPROXY_SERVER_CURCONN,  /* the current number of connections on the server */
+
+       EXTCHK_SIZE
+};
+
 
 /* health status for response tracking */
 enum {
@@ -161,6 +187,11 @@ struct check_status {
        char *desc;                     /* long description */
 };
 
+struct extcheck_env {
+       char *name;     /* environment variable name */
+       int vmaxlen;    /* value maximum length, used to determine the required memory allocation */
+};
+
 struct analyze_status {
        char *desc;                             /* description */
        unsigned char lr[HANA_OBS_SIZE];        /* result for l4/l7: 0 = ignore, 1 - error, 2 - OK */

diff --git a/src/checks.c b/src/checks.c
index 794793c51d09ba6849a58b1c8c00f341160fa29d..feca96e2f867c7813d475d23303d1bc54dcd3eb5 100644 (file)
--- a/src/checks.c
+++ b/src/checks.c
@@ -96,6 +96,20 @@ static const struct check_status check_statuses[HCHK_STATUS_SIZE] = {
        [HCHK_STATUS_PROCOK]    = { CHK_RES_PASSED,   "PROCOK",   "External check passed" },
 };
 
+const struct extcheck_env extcheck_envs[EXTCHK_SIZE] = {
+       [EXTCHK_PATH]                   = { "PATH",                   EXTCHK_SIZE_EVAL_INIT },
+       [EXTCHK_HAPROXY_PROXY_NAME]     = { "HAPROXY_PROXY_NAME",     EXTCHK_SIZE_EVAL_INIT },
+       [EXTCHK_HAPROXY_PROXY_ID]       = { "HAPROXY_PROXY_ID",       EXTCHK_SIZE_EVAL_INIT },
+       [EXTCHK_HAPROXY_PROXY_ADDR]     = { "HAPROXY_PROXY_ADDR",     EXTCHK_SIZE_EVAL_INIT },
+       [EXTCHK_HAPROXY_PROXY_PORT]     = { "HAPROXY_PROXY_PORT",     EXTCHK_SIZE_EVAL_INIT },
+       [EXTCHK_HAPROXY_SERVER_NAME]    = { "HAPROXY_SERVER_NAME",    EXTCHK_SIZE_EVAL_INIT },
+       [EXTCHK_HAPROXY_SERVER_ID]      = { "HAPROXY_SERVER_ID",      EXTCHK_SIZE_EVAL_INIT },
+       [EXTCHK_HAPROXY_SERVER_ADDR]    = { "HAPROXY_SERVER_ADDR",    EXTCHK_SIZE_EVAL_INIT },
+       [EXTCHK_HAPROXY_SERVER_PORT]    = { "HAPROXY_SERVER_PORT",    EXTCHK_SIZE_EVAL_INIT },
+       [EXTCHK_HAPROXY_SERVER_MAXCONN] = { "HAPROXY_SERVER_MAXCONN", EXTCHK_SIZE_EVAL_INIT },
+       [EXTCHK_HAPROXY_SERVER_CURCONN] = { "HAPROXY_SERVER_CURCONN", EXTCHK_SIZE_ULONG },
+};
+
 static const struct analyze_status analyze_statuses[HANA_STATUS_SIZE] = {              /* 0: ignore, 1: error, 2: OK */
        [HANA_STATUS_UNKNOWN]           = { "Unknown",                         { 0, 0 }},
 
@@ -1574,50 +1588,60 @@ static int init_pid_list(void) {
        return 0;
 }
 
-#define EXTCHECK_ARG_MAX 5
-#define EXTCHECK_ENV_ROOM 16
-
-#define EXTCHECK_ADD_ENV(check, envname, value, idx) { if (external_check_add_env(check, envname, value, idx)) goto err; }
+/* helper macro to set an environment variable and jump to a specific label on failure. */
+#define EXTCHK_SETENV(check, envidx, value, fail) { if (extchk_setenv(check, envidx, value)) goto fail; }
 
 /*
- * helper function to allocate enough space for new environment variables used
- * by external checks.
+ * helper function to allocate enough memory to store an environment variable.
+ * It will also check that the environment variable is updatable, and silently
+ * fail if not.
  */
-static int external_check_add_env(struct check *check, const char *envname, const char *value, int *idx)
+static int extchk_setenv(struct check *check, int idx, const char *value)
 {
        int len, ret;
+       char *envname;
+       int vmaxlen;
 
-       if (*idx % EXTCHECK_ENV_ROOM == 0) {
-               /* Allocate enough space to store new environment variables */
-               char **tmp = realloc(check->envp, (EXTCHECK_ENV_ROOM * ((*idx / EXTCHECK_ENV_ROOM) + 1) + 1) * sizeof(check->envp));
-               if (tmp == NULL) {
-                       Alert("Failed to allocate memory for new environment variables. Aborting\n");
-                       return 1;
-               } else {
-                       check->envp = tmp;
-               }
+       if (idx < 0 || idx >= EXTCHK_SIZE) {
+               Alert("Illegal environment variable index %d. Aborting.\n", idx);
+               return 1;
        }
+
+       envname = extcheck_envs[idx].name;
+       vmaxlen = extcheck_envs[idx].vmaxlen;
+
+       /* Check if the environment variable is already set, and silently reject
+        * the update if this one is not updatable. */
+       if ((vmaxlen == EXTCHK_SIZE_EVAL_INIT) && (check->envp[idx]))
+               return 0;
+
        /* Instead of sending NOT_USED, sending an empty value is preferable */
        if (strcmp(value, "NOT_USED") == 0) {
                value = "";
        }
-       len = strlen(envname) + strlen(value) + 1;
-       check->envp[*idx] = malloc(len + 1);
-       check->envp[*idx + 1] = NULL;
-       if (!check->envp[*idx]) {
+
+       len = strlen(envname) + 1;
+       if (vmaxlen == EXTCHK_SIZE_EVAL_INIT)
+               len += strlen(value);
+       else
+               len += vmaxlen;
+
+       if (!check->envp[idx])
+               check->envp[idx] = malloc(len + 1);
+
+       if (!check->envp[idx]) {
                Alert("Failed to allocate memory for the environment variable '%s'. Aborting.\n", envname);
                return 1;
        }
-       ret = snprintf(check->envp[*idx], len + 1, "%s=%s", envname, value);
+       ret = snprintf(check->envp[idx], len + 1, "%s=%s", envname, value);
        if (ret < 0) {
                Alert("Failed to store the environment variable '%s'. Reason : %s. Aborting.\n", envname, strerror(errno));
                return 1;
        }
-       else if (ret != len) {
+       else if (ret > len) {
                Alert("Environment variable '%s' was truncated. Aborting.\n", envname);
                return 1;
        }
-       (*idx)++;
        return 0;
 }
 
@@ -1627,7 +1651,6 @@ static int prepare_external_check(struct check *check)
        struct proxy *px = s->proxy;
        struct listener *listener = NULL, *l;
        int i;
-       int envidx = 0;
        const char *path = px->check_path ? px->check_path : DEF_CHECK_PATH;
        char buf[256];
 
@@ -1641,11 +1664,17 @@ static int prepare_external_check(struct check *check)
                }
 
        check->curpid = NULL;
-       check->envp = NULL;
+       check->envp = calloc((EXTCHK_SIZE + 1), sizeof(char *));
+       if (!check->envp) {
+               Alert("Failed to allocate memory for environment variables. Aborting\n");
+               goto err;
+       }
 
-       check->argv = calloc(EXTCHECK_ARG_MAX + 1, sizeof(check->argv));
-       if (!check->argv)
+       check->argv = calloc(6, sizeof(char *));
+       if (!check->argv) {
+               Alert("Starting [%s:%s] check: out of memory.\n", px->id, s->id);
                goto err;
+       }
 
        check->argv[0] = px->check_command;
 
@@ -1668,6 +1697,7 @@ static int prepare_external_check(struct check *check)
                check->argv[2] = strdup("NOT_USED");
        }
        else {
+               Alert("Starting [%s:%s] check: unsupported address family.\n", px->id, s->id);
                goto err;
        }
 
@@ -1676,28 +1706,36 @@ static int prepare_external_check(struct check *check)
        port_to_str(&s->addr, buf, sizeof(buf));
        check->argv[4] = strdup(buf);
 
-       for (i = 0; i < 5; i++)
-               if (!check->argv[i])
+       for (i = 0; i < 5; i++) {
+               if (!check->argv[i]) {
+                       Alert("Starting [%s:%s] check: out of memory.\n", px->id, s->id);
                        goto err;
+               }
+       }
 
-       EXTCHECK_ADD_ENV(check, "PATH", path, &envidx);
+       EXTCHK_SETENV(check, EXTCHK_PATH, path, err);
        /* Add proxy environment variables */
-       EXTCHECK_ADD_ENV(check, "HAPROXY_PROXY_NAME", px->id, &envidx);
-       EXTCHECK_ADD_ENV(check, "HAPROXY_PROXY_ID", ultoa_r(px->uuid, buf, sizeof(buf)), &envidx);
-       EXTCHECK_ADD_ENV(check, "HAPROXY_PROXY_ADDR", check->argv[1], &envidx);
-       EXTCHECK_ADD_ENV(check, "HAPROXY_PROXY_PORT", check->argv[2], &envidx);
+       EXTCHK_SETENV(check, EXTCHK_HAPROXY_PROXY_NAME, px->id, err);
+       EXTCHK_SETENV(check, EXTCHK_HAPROXY_PROXY_ID, ultoa_r(px->uuid, buf, sizeof(buf)), err);
+       EXTCHK_SETENV(check, EXTCHK_HAPROXY_PROXY_ADDR, check->argv[1], err);
+       EXTCHK_SETENV(check, EXTCHK_HAPROXY_PROXY_PORT, check->argv[2], err);
        /* Add server environment variables */
-       EXTCHECK_ADD_ENV(check, "HAPROXY_SERVER_NAME", s->id, &envidx);
-       EXTCHECK_ADD_ENV(check, "HAPROXY_SERVER_ID", ultoa_r(s->puid, buf, sizeof(buf)), &envidx);
-       EXTCHECK_ADD_ENV(check, "HAPROXY_SERVER_ADDR", check->argv[3], &envidx);
-       EXTCHECK_ADD_ENV(check, "HAPROXY_SERVER_PORT", check->argv[4], &envidx);
-       EXTCHECK_ADD_ENV(check, "HAPROXY_SERVER_MAXCONN", ultoa_r(s->maxconn, buf, sizeof(buf)), &envidx);
-       EXTCHECK_ADD_ENV(check, "HAPROXY_SERVER_CURCONN", ultoa_r(s->cur_sess, buf, sizeof(buf)), &envidx);
+       EXTCHK_SETENV(check, EXTCHK_HAPROXY_SERVER_NAME, s->id, err);
+       EXTCHK_SETENV(check, EXTCHK_HAPROXY_SERVER_ID, ultoa_r(s->puid, buf, sizeof(buf)), err);
+       EXTCHK_SETENV(check, EXTCHK_HAPROXY_SERVER_ADDR, check->argv[3], err);
+       EXTCHK_SETENV(check, EXTCHK_HAPROXY_SERVER_PORT, check->argv[4], err);
+       EXTCHK_SETENV(check, EXTCHK_HAPROXY_SERVER_MAXCONN, ultoa_r(s->maxconn, buf, sizeof(buf)), err);
+       EXTCHK_SETENV(check, EXTCHK_HAPROXY_SERVER_CURCONN, ultoa_r(s->cur_sess, buf, sizeof(buf)), err);
+
+       /* Ensure that we don't leave any hole in check->envp */
+       for (i = 0; i < EXTCHK_SIZE; i++)
+               if (!check->envp[i])
+                       EXTCHK_SETENV(check, i, "", err);
 
        return 1;
 err:
        if (check->envp) {
-               for (i = 0; i < envidx; i++)
+               for (i = 0; i < EXTCHK_SIZE; i++)
                        free(check->envp[i]);
                free(check->envp);
                check->envp = NULL;
@@ -1728,6 +1766,7 @@ err:
  */
 static int connect_proc_chk(struct task *t)
 {
+       char buf[256];
        struct check *check = t->context;
        struct server *s = check->server;
        struct proxy *px = s->proxy;
@@ -1749,6 +1788,7 @@ static int connect_proc_chk(struct task *t)
                /* Child */
                extern char **environ;
                environ = check->envp;
+               extchk_setenv(check, EXTCHK_HAPROXY_SERVER_CURCONN, ultoa_r(s->cur_sess, buf, sizeof(buf)));
                execvp(px->check_command, check->argv);
                Alert("Failed to exec process for external health check: %s. Aborting.\n",
                      strerror(errno));