Hi all,

This set of patches brings the ability to change a server's port at run
time and also bring the ability to do it through the CLI.

Please note that the patch 0001 also changes the way HAProxy find out which
port to use for health checking: HAProxy used to do it at startup, when
parsing the configuration file, but now we need to find it when preparing
the health check, at run time.

Baptiste
From 6bb6d7ae5045c4ff76cf9d87ee25e600b52c4e27 Mon Sep 17 00:00:00 2001
From: Baptiste Assmann <bed...@gmail.com>
Date: Mon, 13 Jun 2016 14:15:41 +0200
Subject: [PATCH 2/5] MAJOR: check: find out which port to use for health check
 at run time

HAProxy used to deduce port used for health checks when parsing configuration
at startup time.
Because of this way of working, it makes it complicated to change the port at
run time.

The current patch changes this behavior and makes HAProxy to choose the
port used for health checking when preparing the check task itself.

A new type of error is introduced and reported when no port can be found.

There won't be any impact on performance, since the process to find out the
port value is made of a few 'if' statements.

This patch also introduces a new check state CHK_ST_PORT_MISS: this flag is
used to report an error in the case when HAProxy needs to establish a TCP
connection to a server, to perform a health check but no TCP ports can be
found for it.

And last, it also introduces a new stream termination condition:
SF_ERR_CHK_PORT. Purpose of this flag is to report an error in the event when
HAProxy has to run a health check but no port can be found to perform it.
---
 include/proto/checks.h |  1 +
 include/types/checks.h |  1 +
 include/types/stream.h |  1 +
 src/checks.c           | 71 ++++++++++++++++++++++++++++++++++++++++++++++++--
 src/server.c           | 26 ++----------------
 5 files changed, 74 insertions(+), 26 deletions(-)

diff --git a/include/proto/checks.h b/include/proto/checks.h
index 9ab3e50..ecd4a5c 100644
--- a/include/proto/checks.h
+++ b/include/proto/checks.h
@@ -50,6 +50,7 @@ void free_check(struct check *check);
 
 void send_email_alert(struct server *s, int priority, const char *format, ...)
 	__attribute__ ((format(printf, 3, 4)));
+int srv_check_healthcheck_port(struct check *chk);
 #endif /* _PROTO_CHECKS_H */
 
 /*
diff --git a/include/types/checks.h b/include/types/checks.h
index dd20184..283ff3d 100644
--- a/include/types/checks.h
+++ b/include/types/checks.h
@@ -41,6 +41,7 @@ enum chk_result {
 #define CHK_ST_ENABLED          0x0004  /* this check is currently administratively enabled */
 #define CHK_ST_PAUSED           0x0008  /* checks are paused because of maintenance (health only) */
 #define CHK_ST_AGENT            0x0010  /* check is an agent check (otherwise it's a health check) */
+#define CHK_ST_PORT_MISS        0x0020  /* check can't be send because no port is configured to run it */
 
 /* check status */
 enum {
diff --git a/include/types/stream.h b/include/types/stream.h
index 17e74b8..0d8c500 100644
--- a/include/types/stream.h
+++ b/include/types/stream.h
@@ -73,6 +73,7 @@
 #define SF_ERR_DOWN     0x00009000	/* the proxy killed a stream because the backend became unavailable */
 #define SF_ERR_KILLED   0x0000a000	/* the proxy killed a stream because it was asked to do so */
 #define SF_ERR_UP       0x0000b000	/* the proxy killed a stream because a preferred backend became available */
+#define SF_ERR_CHK_PORT 0x0000c000	/* no port could be found for a health check. TODO: check SF_ERR_SHIFT */
 #define SF_ERR_MASK     0x0000f000	/* mask to get only stream error flags */
 #define SF_ERR_SHIFT    12		/* bit shift */
 
diff --git a/src/checks.c b/src/checks.c
index 5d02642..aef21ed 100644
--- a/src/checks.c
+++ b/src/checks.c
@@ -679,6 +679,12 @@ static void chk_report_conn_err(struct connection *conn, int errno_bck, int expi
 		}
 	}
 
+       if (check->state & CHK_ST_PORT_MISS) {
+		/* NOTE: this is reported after <fall> tries */
+		chunk_printf(chk, "No port available for the TCP connection");
+		set_server_check_status(check, HCHK_STATUS_SOCKERR, err_msg);
+	}
+
 	if ((conn->flags & (CO_FL_CONNECTED|CO_FL_WAIT_L4_CONN)) == CO_FL_WAIT_L4_CONN) {
 		/* L4 not established (yet) */
 		if (conn->flags & CO_FL_ERROR)
@@ -1430,6 +1436,7 @@ static struct task *server_warmup(struct task *t)
  *  - SF_ERR_PRXCOND if the connection has been limited by the proxy (maxconn)
  *  - SF_ERR_RESOURCE if a system resource is lacking (eg: fd limits, ports, ...)
  *  - SF_ERR_INTERNAL for any other purely internal errors
+ *  - SF_ERR_CHK_PORT if no port could be found to run a health check on an AF_INET* socket
  * Additionnally, in the case of SF_ERR_RESOURCE, an emergency log will be emitted.
  * Note that we try to prevent the network stack from sending the ACK during the
  * connect() when a pure TCP check is used (without PROXY protocol).
@@ -1491,10 +1498,19 @@ static int connect_conn_chk(struct task *t)
 		conn->addr.to = s->addr;
 	}
 
-	if (check->port) {
-		set_host_port(&conn->addr.to, check->port);
+       if ((conn->addr.to.ss_family == AF_INET) || (conn->addr.to.ss_family == AF_INET6)) {
+		int i = 0;
+
+		i = srv_check_healthcheck_port(check);
+		if (i == 0) {
+			conn->owner = check;
+			return SF_ERR_CHK_PORT;
+		}
+
+		set_host_port(&conn->addr.to, i);
 	}
 
+
 	proto = protocol_by_family(conn->addr.to.ss_family);
 
 	conn_prepare(conn, proto, check->xprt);
@@ -2072,6 +2088,9 @@ static struct task *process_chk_conn(struct task *t)
 			conn->flags |= CO_FL_ERROR;
 			chk_report_conn_err(conn, errno, 0);
 			break;
+		/* should share same code than cases below */
+		case SF_ERR_CHK_PORT:
+			check->state |= CHK_ST_PORT_MISS;
 		case SF_ERR_PRXCOND:
 		case SF_ERR_RESOURCE:
 		case SF_ERR_INTERNAL:
@@ -3368,6 +3387,54 @@ void send_email_alert(struct server *s, int level, const char *format, ...)
 	enqueue_email_alert(p, buf);
 }
 
+/*
+ * Return value:
+ *   the port to be used for the health check
+ *   0 in case no port could be found for the check
+ */
+int srv_check_healthcheck_port(struct check *chk)
+{
+	int i = 0;
+	struct server *srv = NULL;
+
+	srv = chk->server;
+
+	/* If neither a port nor an addr was specified and no check transport
+	 * layer is forced, then the transport layer used by the checks is the
+	 * same as for the production traffic. Otherwise we use raw_sock by
+	 * default, unless one is specified.
+	 */
+	if (!chk->port && !is_addr(&chk->addr)) {
+#ifdef USE_OPENSSL
+		chk->use_ssl |= (srv->use_ssl || (srv->proxy->options & PR_O_TCPCHK_SSL));
+#endif
+		chk->send_proxy |= (srv->pp_opts);
+	}
+
+	/* by default, we use the health check port ocnfigured */
+	if (chk->port > 0)
+		return chk->port;
+
+	/* try to get the port from check_core.addr if check.port not set */
+	i = get_host_port(&chk->addr);
+	if (i > 0)
+		return i;
+
+	/* try to get the port from server address */
+	/* prevent MAPPORTS from working at this point, since checks could
+	 * not be performed in such case (MAPPORTS impose a relative ports
+	 * based on live traffic)
+	 */
+	if (srv->flags & SRV_F_MAPPORTS)
+		return 0;
+	i = get_host_port(&srv->addr); /* by default */
+	if (i > 0)
+		return i;
+
+	return 0;
+}
+
+
 
 /*
  * Local variables:
diff --git a/src/server.c b/src/server.c
index 62c08b0..0bf9258 100644
--- a/src/server.c
+++ b/src/server.c
@@ -869,7 +869,6 @@ int parse_server(const char *file, int linenum, char **args, struct proxy *curpr
 
 	if (!strcmp(args[0], "server") || !strcmp(args[0], "default-server")) {  /* server address */
 		int cur_arg;
-		short realport = 0;
 		int do_agent = 0, do_check = 0, defsrv = (*args[0] == 'd');
 
 		if (!defsrv && curproxy == defproxy) {
@@ -961,10 +960,6 @@ int parse_server(const char *file, int linenum, char **args, struct proxy *curpr
 				err_code |= ERR_ALERT | ERR_FATAL;
 				goto out;
 			}
-			else {
-				/* used by checks */
-				realport = port1;
-			}
 
 			/* save hostname and create associated name resolution */
 			newsrv->hostname = fqdn;
@@ -1749,34 +1744,17 @@ int parse_server(const char *file, int linenum, char **args, struct proxy *curpr
 				goto out;
 			}
 
-			/* If neither a port nor an addr was specified and no check transport
-			 * layer is forced, then the transport layer used by the checks is the
-			 * same as for the production traffic. Otherwise we use raw_sock by
-			 * default, unless one is specified.
-			 */
-			if (!newsrv->check.port && !is_addr(&newsrv->check.addr)) {
-#ifdef USE_OPENSSL
-				newsrv->check.use_ssl |= (newsrv->use_ssl || (newsrv->proxy->options & PR_O_TCPCHK_SSL));
-#endif
-				newsrv->check.send_proxy |= (newsrv->pp_opts);
-			}
-			/* try to get the port from check_core.addr if check.port not set */
-			if (!newsrv->check.port)
-				newsrv->check.port = get_host_port(&newsrv->check.addr);
-
-			if (!newsrv->check.port)
-				newsrv->check.port = realport; /* by default */
-
 			/*
 			 * We need at least a service port, a check port or the first tcp-check rule must
 			 * be a 'connect' one when checking an IPv4/IPv6 server.
 			 */
-			if (!newsrv->check.port &&
+			if ((srv_check_healthcheck_port(&newsrv->check) == 0) &&
 			    (is_inet_addr(&newsrv->check.addr) ||
 			     (!is_addr(&newsrv->check.addr) && is_inet_addr(&newsrv->addr)))) {
 				struct tcpcheck_rule *r = NULL;
 				struct list *l;
 
+
 				r = (struct tcpcheck_rule *)newsrv->proxy->tcpcheck_rules.n;
 				if (!r) {
 					Alert("parsing [%s:%d] : server %s has neither service port nor check port. Check has been disabled.\n",
-- 
1.9.1

From 5627377042023aa924cc66caa9719fdfce9e2cf2 Mon Sep 17 00:00:00 2001
From: Baptiste Assmann <bed...@gmail.com>
Date: Thu, 11 Aug 2016 23:12:18 +0200
Subject: [PATCH 3/5] MINOR: server: introduction of 3 new server flags

Introduction of 3 new server flags to remember if some parameters were set
during configuration parsing.

* SRV_F_CHECKADDR: this server has a check addr configured
* SRV_F_CHECKPORT: this server has a check port configured
* SRV_F_AGENTADDR: this server has a agent addr configured
---
 include/types/server.h | 3 +++
 src/server.c           | 5 +++++
 2 files changed, 8 insertions(+)

diff --git a/include/types/server.h b/include/types/server.h
index 952671c..ce13820 100644
--- a/include/types/server.h
+++ b/include/types/server.h
@@ -98,6 +98,9 @@ enum srv_admin {
 #define SRV_F_NON_STICK    0x0004        /* never add connections allocated to this server to a stick table */
 #define SRV_F_USE_NS_FROM_PP 0x0008      /* use namespace associated with connection if present */
 #define SRV_F_FORCED_ID    0x0010        /* server's ID was forced in the configuration */
+#define SRV_F_CHECKADDR    0x0020        /* this server has a check addr configured */
+#define SRV_F_CHECKPORT    0x0040        /* this server has a check port configured */
+#define SRV_F_AGENTADDR    0x0080        /* this server has a agent addr configured */
 
 /* configured server options for send-proxy (server->pp_opts) */
 #define SRV_PP_V1          0x0001        /* proxy protocol version 1 */
diff --git a/src/server.c b/src/server.c
index 0bf9258..aff3573 100644
--- a/src/server.c
+++ b/src/server.c
@@ -1002,6 +1002,8 @@ int parse_server(const char *file, int linenum, char **args, struct proxy *curpr
 
 			newsrv->check.use_ssl	= curproxy->defsrv.check.use_ssl;
 			newsrv->check.port	= curproxy->defsrv.check.port;
+			if (newsrv->check.port)
+				newsrv->flags |= SRV_F_CHECKPORT;
 			newsrv->check.inter	= curproxy->defsrv.check.inter;
 			newsrv->check.fastinter	= curproxy->defsrv.check.fastinter;
 			newsrv->check.downinter	= curproxy->defsrv.check.downinter;
@@ -1292,10 +1294,13 @@ int parse_server(const char *file, int linenum, char **args, struct proxy *curpr
 				}
 
 				newsrv->check.addr = newsrv->agent.addr = *sk;
+				newsrv->flags |= SRV_F_CHECKADDR;
+				newsrv->flags |= SRV_F_AGENTADDR;
 				cur_arg += 2;
 			}
 			else if (!strcmp(args[cur_arg], "port")) {
 				newsrv->check.port = atol(args[cur_arg + 1]);
+				newsrv->flags |= SRV_F_CHECKPORT;
 				cur_arg += 2;
 			}
 			else if (!defsrv && !strcmp(args[cur_arg], "backup")) {
-- 
1.9.1

From 5cad49e60387a06f46abcf89a87ecf0f33c70d6a Mon Sep 17 00:00:00 2001
From: Baptiste Assmann <bed...@gmail.com>
Date: Tue, 2 Aug 2016 08:18:55 +0200
Subject: [PATCH 4/5] MINOR: new update_server_addr_port() function to change
 both server's ADDR and service PORT

This function can replace update_server_addr() where the need to change the
server's port as well as the IP address is required.
It performs some validation before performing each type of change.
---
 include/proto/server.h |   1 +
 src/server.c           | 174 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 175 insertions(+)

diff --git a/include/proto/server.h b/include/proto/server.h
index ee45f63..47630fe 100644
--- a/include/proto/server.h
+++ b/include/proto/server.h
@@ -40,6 +40,7 @@ int srv_lastsession(const struct server *s);
 int srv_getinter(const struct check *check);
 int parse_server(const char *file, int linenum, char **args, struct proxy *curproxy, struct proxy *defproxy);
 int update_server_addr(struct server *s, void *ip, int ip_sin_family, const char *updater);
+const char *update_server_addr_port(struct server *s, const char *addr, const char *port, char *updater);
 struct server *server_find_by_id(struct proxy *bk, int id);
 struct server *server_find_by_name(struct proxy *bk, const char *name);
 struct server *server_find_best_match(struct proxy *bk, char *name, int id, int *diff);
diff --git a/src/server.c b/src/server.c
index aff3573..71432a3 100644
--- a/src/server.c
+++ b/src/server.c
@@ -2612,6 +2612,180 @@ int update_server_addr(struct server *s, void *ip, int ip_sin_family, const char
 }
 
 /*
+ * This function update a server's addr and port only for AF_INET and AF_INET6 families.
+ *
+ * Caller can pass its name through <updater> to get it integrated in the response message
+ * returned by the function.
+ *
+ * The function first does the following, in that order:
+ * - validates the new addr and/or port
+ * - checks if an update is required (new IP or port is different than current ones)
+ * - checks the update is allowed:
+ *   - don't switch from/to a family other than AF_INET4 and AF_INET6
+ *   - allow all changes if no CHECKS are configured
+ *   - if CHECK is configured:
+ *     - if switch to port map (SRV_F_MAPPORTS), ensure health check have their own ports
+ * - applies required changes to both ADDR and PORT if both 'required' and 'allowed'
+ *   conditions are met
+ */
+const char *update_server_addr_port(struct server *s, const char *addr, const char *port, char *updater)
+{
+	struct sockaddr_storage sa;
+	int ret, port_change_required;
+	char current_addr[INET6_ADDRSTRLEN];
+	u_int16_t current_port, new_port;
+	struct chunk *msg;
+
+	msg = get_trash_chunk();
+	chunk_reset(msg);
+
+	if (addr) {
+		memset(&sa, 0, sizeof(struct sockaddr_storage));
+		if (str2ip2(addr, &sa, 0) == NULL) {
+			chunk_printf(msg, "Invalid addr '%s'", addr);
+			goto out;
+		}
+
+		/* changes are allowed on AF_INET* families only */
+		if ((sa.ss_family != AF_INET) && (sa.ss_family != AF_INET6)) {
+			chunk_printf(msg, "Update to families other than AF_INET and AF_INET6 supported only through configuration file");
+			goto out;
+		}
+
+		/* collecting data currently setup */
+		memset(current_addr, '\0', sizeof(current_addr));
+		ret = addr_to_str(&s->addr, current_addr, sizeof(current_addr));
+		/* changes are allowed on AF_INET* families only */
+		if ((ret != AF_INET) && (ret != AF_INET6)) {
+			chunk_printf(msg, "Update for the current server address family is only supported through configuration file");
+			goto out;
+		}
+
+		/* applying ADDR changes if required and allowed
+		 * ipcmp returns 0 when both ADDR are the same
+		 */
+		if (ipcmp(&s->addr, &sa) == 0) {
+			chunk_appendf(msg, "no need to change the addr");
+			goto port;
+		}
+		current_port = get_host_port(&s->addr);
+		memset(&s->addr, '\0', sizeof(s->addr));
+		ipcpy(&sa, &s->addr);
+		set_host_port(&s->addr, current_port);
+
+		/* we also need to update check's ADDR only if it uses the server's one */
+		if ((s->check.state & CHK_ST_CONFIGURED) && (s->flags & SRV_F_CHECKADDR)) {
+			current_port = get_host_port(&s->check.addr);
+			memset(&s->check.addr, '\0', sizeof(s->check.addr));
+			ipcpy(&sa, &s->check.addr);
+			set_host_port(&s->check.addr, current_port);
+		}
+
+		/* we also need to update agent ADDR only if it use the server's one */
+		if ((s->agent.state & CHK_ST_CONFIGURED) && (s->flags & SRV_F_AGENTADDR)) {
+			current_port = get_host_port(&s->agent.addr);
+			memset(&s->agent.addr, '\0', sizeof(s->agent.addr));
+			ipcpy(&sa, &s->agent.addr);
+			set_host_port(&s->agent.addr, current_port);
+		}
+
+		/* update report for caller */
+		chunk_printf(msg, "IP changed from '%s' to '%s'", current_addr, addr);
+	}
+
+ port:
+	if (port) {
+		char sign = '\0';
+		char *endptr;
+
+		if (addr)
+			chunk_appendf(msg, ", ");
+
+		/* collecting data currently setup */
+		current_port = get_host_port(&s->addr);
+
+		/* check if PORT change is required */
+		port_change_required = 0;
+
+		sign = *port;
+		new_port = strtol(port, &endptr, 10);
+		if ((errno != 0) || (port == endptr)) {
+			chunk_appendf(msg, "problem converting port '%s' to an int", port);
+			goto out;
+		}
+
+		/* check if caller triggers a port mapped or offset */
+		if (sign == '-' || (sign == '+')) {
+			/* check if server currently uses port map */
+			if (!(s->flags & SRV_F_MAPPORTS)) {
+				/* switch from fixed port to port map mandatorily triggers
+				 * a port change */
+				port_change_required = 1;
+				/* check is configured
+				 * we're switching from a fixed port to a SRV_F_MAPPORTS (mapped) port
+				 * prevent PORT change if check doesn't have it's dedicated port while switching
+				 * to port mapping */
+				if ((s->check.state & CHK_ST_CONFIGURED) && !(s->flags & SRV_F_CHECKPORT)) {
+					chunk_appendf(msg, "can't change <port> to port map because it is incompatible with current health check port configuration (use 'port' statement from the 'server' directive.");
+					goto out;
+				}
+			}
+			/* we're already using port maps */
+			else {
+				port_change_required = current_port != new_port;
+			}
+		}
+		/* fixed port */
+		else {
+			port_change_required = current_port != new_port;
+		}
+
+		/* applying PORT changes if required and update response message */
+		if (port_change_required) {
+			/* apply new port */
+			set_host_port(&s->addr, new_port);
+
+			/* prepare message */
+			chunk_appendf(msg, "port changed from '");
+			if (s->flags & SRV_F_MAPPORTS)
+				chunk_appendf(msg, "+");
+			chunk_appendf(msg, "%d' to '", current_port);
+
+			if (sign == '-') {
+				s->flags |= SRV_F_MAPPORTS;
+				chunk_appendf(msg, "%c", sign);
+				/* just use for result output */
+				new_port = -new_port;
+			}
+			else if (sign == '+') {
+				s->flags |= SRV_F_MAPPORTS;
+				chunk_appendf(msg, "%c", sign);
+			}
+			else {
+				s->flags &= ~SRV_F_MAPPORTS;
+			}
+
+			chunk_appendf(msg, "%d'", new_port);
+
+			/* we also need to update health checks port only if it uses server's realport */
+			if ((s->check.state & CHK_ST_CONFIGURED) && !(s->flags & SRV_F_CHECKPORT)) {
+				s->check.port = new_port;
+			}
+		}
+		else {
+			chunk_appendf(msg, "no need to change the port");
+		}
+	}
+
+out:
+	if (updater)
+		chunk_appendf(msg, " by '%s'", updater);
+	chunk_appendf(msg, "\n");
+	return msg->str;
+}
+
+
+/*
  * update server status based on result of name resolution
  * returns:
  *  0 if server status is updated
-- 
1.9.1

From eb8062ed829490e55fdc16cf811297b2782bf7da Mon Sep 17 00:00:00 2001
From: Baptiste Assmann <bed...@gmail.com>
Date: Wed, 3 Aug 2016 22:34:12 +0200
Subject: [PATCH 5/5] MINOR: cli: ability to change a server's port

Enrichment of the 'set server <b>/<s> addr' cli directive to allow changing
now a server's port.
The new syntax looks like:
  set server <b>/<s> addr [port <port>]
---
 doc/management.txt |  5 ++++-
 src/dumpstats.c    | 15 ++++++++++++++-
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/doc/management.txt b/doc/management.txt
index bde9604..fd53c5c 100644
--- a/doc/management.txt
+++ b/doc/management.txt
@@ -1582,8 +1582,11 @@ set rate-limit ssl-sessions global <value>
   is passed in number of sessions per second sent to the SSL stack. It applies
   before the handshake in order to protect the stack against handshake abuses.
 
-set server <backend>/<server> addr <ip4 or ip6 address>
+set server <backend>/<server> addr <ip4 or ip6 address> [port <port>]
   Replace the current IP address of a server by the one provided.
+  Optionnaly, the port can be changed using the 'port' parameter.
+  Note that changing the port also support switching from/to port mapping
+  (notation with +X or -Y), only if a port is configured for the health check.
 
 set server <backend>/<server> agent [ up | down ]
   Force a server's agent to a new state. This can be useful to immediately
diff --git a/src/dumpstats.c b/src/dumpstats.c
index 8be723f..8705a1d 100644
--- a/src/dumpstats.c
+++ b/src/dumpstats.c
@@ -1800,7 +1800,20 @@ static int stats_sock_parse_request(struct stream_interface *si, char *line)
 				appctx->st0 = STAT_CLI_PRINT;
 			}
 			else if (strcmp(args[3], "addr") == 0) {
-				warning = server_parse_addr_change_request(sv, args[4], "stats command");
+				char *addr = NULL;
+				char *port = NULL;
+				if (strlen(args[4]) == 0) {
+					appctx->ctx.cli.msg = "set server <b>/<s> requires an <addr> .\n";
+					appctx->st0 = STAT_CLI_PRINT;
+					return 1;
+				}
+				else {
+					addr = args[4];
+				}
+				if (strcmp(args[5], "port") == 0) {
+					port = args[6];
+				}
+				warning = update_server_addr_port(sv, addr, port, "stats socket command");
 				if (warning) {
 					appctx->ctx.cli.msg = warning;
 					appctx->st0 = STAT_CLI_PRINT;
-- 
1.9.1

Reply via email to