On 04/27/2017 03:20 PM, Frederic Lecaille wrote:
On 04/27/2017 02:56 PM, Baptiste wrote:
On Thu, Apr 27, 2017 at 2:44 PM, Frederic Lecaille
<flecai...@haproxy.com <mailto:flecai...@haproxy.com>> wrote:
On 04/27/2017 12:43 PM, Baptiste wrote:
On Thu, Apr 27, 2017 at 11:22 AM, Frederic Lecaille
<flecai...@haproxy.com <mailto:flecai...@haproxy.com>
<mailto:flecai...@haproxy.com <mailto:flecai...@haproxy.com>>>
wrote:
On 04/27/2017 10:47 AM, Frederic Lecaille wrote:
Hello HAProxy ML,
Please find attached to this mail a patch proposal which
allows
server FQDNs changes from stats socket.
These FQDNs are also added to server-state file.
Regards,
Fred.
A new version of this patch which fixes a memleak (server
hostname
was strdup() both in srv_set_fqdn() and
srv_alloc_dns_resolution()).
Hi Fred,
Hi Baptiste,
I did a quick read of the patch and I noticed the following
points:
- in update_server_fqdn(), I don't think it's relevant to
perform the
name resolution validation (the call to str2ip2)
First, str2ip2 uses libc resolver, which may be different
from the
runtime resolver of HAProxy and second, it's up to the
admin/devops/script which performs this change to ensure he is
not
messing up...
Ok but will the new FQDN be resolved in any case after any update
from stats socket to match with the server IP address or is this
something the operator must also take care of (provide new FQDNs
which may resolve to the current IP address???)?
From my point of view, if no runtime resolution is configured, then
changing the fqdn through the CLI should have no impact (since this is
HAProxy's behavior since the beginning). Unless I misunderstood the
purpose of this feature...
By the way, I think that the call to getaddrinfo() in str2ip2 is
blocking... Hence, if I'm not wrong about getaddrinfo, it's a definitive
no-go to call str2ip2 at runtime.
I do not know all the internals of haproxy but I agree that if str2ip2()
may make any haproxy internal processing block I have definitively to
remove this call to str2ip2(). ;)
And modify two lines which launch another call 2 str2ip2() (see the
comment: /* Reset lastaddr value. etc. */).
Thank you Baptiste.
Here is a new patch version which takes into an account Baptiste remarks.
Thank you again Baptiste.
>From 54ce2b25e74c4b95a86d3da87b5bea261a7183c5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20L=C3=A9caille?= <flecai...@haproxy.com>
Date: Wed, 26 Apr 2017 11:24:02 +0200
Subject: [PATCH] MINOR: server: cli: Add server FQDNs to server-state file and
stats socket.
This patch adds a new stats socket command to modify server
FQDNs at run time.
Its syntax:
set server <backend>/<server> fqdn <FQDN>
This patch also adds FQDNs to server state file at the end
of each line for backward compatibility ("-" if not present).
---
doc/management.txt | 7 +++
include/types/server.h | 22 +++++++-
src/proxy.c | 4 +-
src/server.c | 148 +++++++++++++++++++++++++++++++++++++++++++++----
4 files changed, 167 insertions(+), 14 deletions(-)
diff --git a/doc/management.txt b/doc/management.txt
index 3368277..d1ea1f6 100644
--- a/doc/management.txt
+++ b/doc/management.txt
@@ -1650,6 +1650,9 @@ set server <backend>/<server> weight <weight>[%]
Change a server's weight to the value passed in argument. This is the exact
equivalent of the "set weight" command below.
+set server <backend>/<server> fqdn <FQDN>
+ Change a server's FQDN to the value passed in argument.
+
set ssl ocsp-response <response>
This command is used to update an OCSP Response for a certificate (see "crt"
on "bind" lines). Same controls are performed as during the initial loading of
@@ -1962,6 +1965,9 @@ show servers state [<backend>]
0x20 = SRV_ADMF_RMAINT
The server is in maintenance because of an
IP address resolution failure.
+ 0x40 = SRV_ADMF_HMAINT
+ The server FQDN was set from stats socket.
+
srv_uweight: User visible server's weight.
srv_iweight: Server's initial weight.
srv_time_since_last_change: Time since last operational change.
@@ -2003,6 +2009,7 @@ show servers state [<backend>]
configuration.
srv_f_forced_id: Flag to know if the server's ID is forced by
configuration.
+ srv_fqdn: Server FQDN.
show sess
Dump all known sessions. Avoid doing this on slow connections as this can
diff --git a/include/types/server.h b/include/types/server.h
index 8d68dcb..83b1e80 100644
--- a/include/types/server.h
+++ b/include/types/server.h
@@ -82,6 +82,7 @@ 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 */
};
/* options for servers' "init-addr" parameter
@@ -103,7 +104,26 @@ enum srv_initaddr {
#define SRV_STATE_FILE_VERSION 1
#define SRV_STATE_FILE_VERSION_MIN 1
#define SRV_STATE_FILE_VERSION_MAX 1
-#define SRV_STATE_FILE_FIELD_NAMES "be_id be_name srv_id srv_name srv_addr srv_op_state srv_admin_state srv_uweight srv_iweight srv_time_since_last_change srv_check_status srv_check_result srv_check_health srv_check_state srv_agent_state bk_f_forced_id srv_f_forced_id"
+#define SRV_STATE_FILE_FIELD_NAMES \
+ "be_id " \
+ "be_name " \
+ "srv_id " \
+ "srv_name " \
+ "srv_addr " \
+ "srv_op_state " \
+ "srv_admin_state " \
+ "srv_uweight " \
+ "srv_iweight " \
+ "srv_time_since_last_change " \
+ "srv_check_status " \
+ "srv_check_result " \
+ "srv_check_health " \
+ "srv_check_state " \
+ "srv_agent_state " \
+ "bk_f_forced_id " \
+ "srv_f_forced_id " \
+ "srv_fqdn"
+
#define SRV_STATE_FILE_MAX_FIELDS 18
#define SRV_STATE_FILE_NB_FIELDS_VERSION_1 18
#define SRV_STATE_LINE_MAXLEN 512
diff --git a/src/proxy.c b/src/proxy.c
index dc70213..f6a3214 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -1458,13 +1458,13 @@ static int dump_servers_state(struct stream_interface *si, struct chunk *buf)
"%d %s %s "
"%d %d %d %d %ld "
"%d %d %d %d %d "
- "%d %d"
+ "%d %d %s"
"\n",
px->uuid, px->id,
srv->puid, srv->id, srv_addr,
srv->state, srv->admin, srv->uweight, srv->iweight, (long int)srv_time_since_last_change,
srv->check.status, srv->check.result, srv->check.health, srv->check.state, srv->agent.state,
- bk_f_forced_id, srv_f_forced_id);
+ bk_f_forced_id, srv_f_forced_id, srv->hostname ? srv->hostname : "-");
if (bi_putchk(si_ic(si), &trash) == -1) {
si_applet_cant_put(si);
return 0;
diff --git a/src/server.c b/src/server.c
index 69c1ec3..536ab6b 100644
--- a/src/server.c
+++ b/src/server.c
@@ -46,6 +46,7 @@
static void srv_update_state(struct server *srv, int version, char **params);
static int srv_apply_lastaddr(struct server *srv, int *err_code);
+static int srv_set_fqdn(struct server *srv, const char *fqdn);
/* List head of all known server keywords */
static struct srv_kw_list srv_keywords = {
@@ -1652,16 +1653,18 @@ static void srv_ssl_settings_cpy(struct server *srv, struct server *src)
/*
* Allocate <srv> server dns resolution.
* May be safely called with a default server as <src> argument (without hostname).
+ * Returns -1 in case of any allocation failure, 0 if not.
*/
-static void srv_alloc_dns_resolution(struct server *srv, const char *hostname)
+static int srv_alloc_dns_resolution(struct server *srv, const char *hostname)
{
char *hostname_dn;
int hostname_dn_len;
struct dns_resolution *dst_dns_rslt;
if (!hostname)
- return;
+ return 0;
+ free(srv->hostname);
srv->hostname = strdup(hostname);
dst_dns_rslt = calloc(1, sizeof *dst_dns_rslt);
hostname_dn_len = dns_str_to_dn_label_len(hostname);
@@ -1687,13 +1690,24 @@ static void srv_alloc_dns_resolution(struct server *srv, const char *hostname)
/* a first resolution has been done by the configuration parser */
srv->resolution->last_resolution = 0;
- return;
+ return 0;
err:
free(srv->hostname);
srv->hostname = NULL;
free(hostname_dn);
free(dst_dns_rslt);
+ return -1;
+}
+
+static void srv_free_dns_resolution(struct server *srv)
+{
+ if (!srv->resolution)
+ return;
+
+ free(srv->resolution->hostname_dn);
+ free(srv->resolution);
+ srv->resolution = NULL;
}
/*
@@ -2888,7 +2902,10 @@ static void srv_update_state(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;
+ const char *fqdn;
+ fqdn = NULL;
msg = get_trash_chunk();
switch (version) {
case 1:
@@ -2907,6 +2924,7 @@ static void srv_update_state(struct server *srv, int version, char **params)
* srv_agent_state: params[10]
* bk_f_forced_id: params[11]
* srv_f_forced_id: params[12]
+ * srv_fqdn: params[13]
*/
/* validating srv_op_state */
@@ -2925,9 +2943,12 @@ static void srv_update_state(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);
- /* inherited statuses will be recomputed later */
- srv_admin_state &= ~SRV_ADMF_IDRAIN & ~SRV_ADMF_IMAINT;
+ /* inherited statuses will be recomputed later.
+ * Also disable SRV_ADMF_HMAINT flag (set from stats socket fqdn).
+ */
+ srv_admin_state &= ~SRV_ADMF_IDRAIN & ~SRV_ADMF_IMAINT & ~SRV_ADMF_HMAINT;
if ((p == params[2]) || errno == EINVAL || errno == ERANGE ||
(srv_admin_state != 0 &&
@@ -3018,6 +3039,14 @@ static void srv_update_state(struct server *srv, int version, char **params)
if (p == params[12] || errno == EINVAL || errno == ERANGE || !((srv_f_forced_id == 0) || (srv_f_forced_id == 1)))
chunk_appendf(msg, ", invalid srv_f_forced_id value '%s'", params[12]);
+ /* validating srv_fqdn */
+ fqdn = params[13];
+ if (fqdn && *fqdn == '-')
+ fqdn = NULL;
+ if (fqdn && (strlen(fqdn) > DNS_MAX_NAME_SIZE || invalid_domainchar(fqdn))) {
+ chunk_appendf(msg, ", invalid srv_fqdn value '%s'", params[13]);
+ fqdn = NULL;
+ }
/* don't apply anything if one error has been detected */
if (msg->len)
@@ -3122,6 +3151,34 @@ static void srv_update_state(struct server *srv, int version, char **params)
/* load server IP address */
srv->lastaddr = strdup(params[0]);
+
+ if (fqdn && srv->hostname) {
+ if (!strcmp(srv->hostname, fqdn)) {
+ /* Here we reset the 'set from stats socket FQDN' flag
+ * to support such transitions:
+ * Let's say initial FQDN value is foo1 (in configuration file).
+ * - FQDN changed from stats socket, from foo1 to foo2 value,
+ * - FQDN changed again from file configuration (with the same previous value
+ set from stats socket, from foo1 to foo2 value),
+ * - reload for any other reason than a FQDN modification,
+ * the configuration file FQDN matches the fqdn server state file value.
+ * So we must reset the 'set from stats socket FQDN' flag to be consistent with
+ * any futher FQDN modification.
+ */
+ srv->admin &= ~SRV_ADMF_HMAINT;
+ }
+ else {
+ /* If the FDQN has been changed from stats socket,
+ * apply fqdn state file value (which is the value set
+ * from stats socket).
+ */
+ if (fqdn_set_by_cli) {
+ srv_set_fqdn(srv, fqdn);
+ srv->admin |= SRV_ADMF_HMAINT;
+ }
+ }
+ }
+
break;
default:
chunk_appendf(msg, ", version '%d' not supported", version);
@@ -3152,8 +3209,8 @@ void apply_server_state(void)
char *cur, *end;
char mybuf[SRV_STATE_LINE_MAXLEN];
int mybuflen;
- char *params[SRV_STATE_FILE_MAX_FIELDS];
- char *srv_params[SRV_STATE_FILE_MAX_FIELDS];
+ char *params[SRV_STATE_FILE_MAX_FIELDS] = {0};
+ char *srv_params[SRV_STATE_FILE_MAX_FIELDS] = {0};
int arg, srv_arg, version, diff;
FILE *f;
char *filepath;
@@ -3331,12 +3388,18 @@ void apply_server_state(void)
while (isspace(*cur))
++cur;
- if (cur == end)
+ /* Ignore empty or commented lines */
+ if (cur == end || *cur == '#')
continue;
- /* ignore comment line */
- if (*cur == '#')
+ /* truncated lines */
+ if (mybuf[mybuflen - 1] != '\n') {
+ Warning("server-state file '%s': truncated line\n", filepath);
continue;
+ }
+
+ /* Removes trailing '\n' */
+ mybuf[mybuflen - 1] = '\0';
/* we're now ready to move the line into *srv_params[] */
params[0] = cur;
@@ -3364,6 +3427,7 @@ void apply_server_state(void)
* srv_agent_state: params[14] => srv_params[10]
* bk_f_forced_id: params[15] => srv_params[11]
* srv_f_forced_id: params[16] => srv_params[12]
+ * srv_fqdn: params[17] => srv_params[13]
*/
if (arg >= 4) {
srv_params[srv_arg] = cur;
@@ -4016,6 +4080,16 @@ int srv_set_addr_via_libc(struct server *srv, int *err_code)
return 0;
}
+/* Set the server's FDQN (->hostname) from <hostname>.
+ * Returns -1 if failed, 0 if not.
+ */
+int srv_set_fqdn(struct server *srv, const char *hostname)
+{
+ srv_free_dns_resolution(srv);
+
+ return srv_alloc_dns_resolution(srv, hostname);
+}
+
/* Sets the server's address (srv->addr) from srv->lastaddr which was filled
* from the state file. This is suited for initial address configuration.
* Returns 0 on success otherwise a non-zero error code. In case of error,
@@ -4139,6 +4213,46 @@ int srv_init_addr(void)
return return_code;
}
+const char *update_server_fqdn(struct server *server, const char *fqdn, const char *updater)
+{
+
+ struct chunk *msg;
+
+ msg = get_trash_chunk();
+ chunk_reset(msg);
+
+ if (!strcmp(fqdn, server->hostname)) {
+ chunk_appendf(msg, "no need to change the FDQN");
+ goto out;
+ }
+
+ if (strlen(fqdn) > DNS_MAX_NAME_SIZE || invalid_domainchar(fqdn)) {
+ chunk_appendf(msg, "invalid fqdn '%s'", fqdn);
+ goto out;
+ }
+
+ chunk_appendf(msg, "%s/%s changed its FQDN from %s to %s",
+ server->proxy->id, server->id, server->hostname, fqdn);
+
+ if (srv_set_fqdn(server, fqdn) < 0) {
+ chunk_reset(msg);
+ chunk_appendf(msg, "could not update %s/%s FQDN",
+ server->proxy->id, server->id);
+ goto out;
+ }
+
+ /* Flag as FQDN set from stats socket. */
+ server->admin |= SRV_ADMF_HMAINT;
+
+ out:
+ if (updater)
+ chunk_appendf(msg, " by '%s'", updater);
+ chunk_appendf(msg, "\n");
+
+ return msg->str;
+}
+
+
/* Expects to find a backend and a server in <arg> under the form <backend>/<server>,
* and returns the pointer to the server. Otherwise, display adequate error messages
* on the CLI, sets the CLI's state to CLI_ST_PRINT and returns NULL. This is only
@@ -4319,8 +4433,20 @@ static int cli_parse_set_server(char **args, struct appctx *appctx, void *privat
}
srv_clr_admin_flag(sv, SRV_ADMF_RMAINT);
}
+ else if (strcmp(args[3], "fqdn") == 0) {
+ if (!*args[4]) {
+ appctx->ctx.cli.msg = "set server <b>/<s> fqdn requires a FQDN.\n";
+ appctx->st0 = CLI_ST_PRINT;
+ return 1;
+ }
+ warning = update_server_fqdn(sv, args[4], "stats socket command");
+ if (warning) {
+ appctx->ctx.cli.msg = warning;
+ appctx->st0 = CLI_ST_PRINT;
+ }
+ }
else {
- appctx->ctx.cli.msg = "'set server <srv>' only supports 'agent', 'health', 'state', 'weight', 'addr' and 'check-port'.\n";
+ appctx->ctx.cli.msg = "'set server <srv>' only supports 'agent', 'health', 'state', 'weight', 'addr', 'fqdn' and 'check-port'.\n";
appctx->st0 = CLI_ST_PRINT;
}
return 1;
--
2.1.4
