Hi,

Actually, the problem was deeper than my first thought.
In its current state, statefile and SRV records are simply not compatible.
I had to add a new field in the state file format to add support to this.

Could you please confirm the patch attached fixes your issues?

Baptiste



On Mon, Jun 25, 2018 at 11:48 AM, Baptiste <bed...@gmail.com> wrote:

> Hi,
>
> Forget the backend id, it's the wrong answer to that problem.
> I was investigating an other potential issue, but this does not fix the
> original problem reported here.
>
> Here is the answer I delivered today on discourse, where other people have
> also reported the same issue:
>
>    Just to let you know that I think I found the cause of the issue but I
> don’t have a fix yet.
>    I’ll come back to you this week with more info and hopefully a fix.
>    The issue seem to be in srv_init_addr(), because srv->hostname is not
> set (null).
>
> Baptiste
>
>
>
From 6899b19b9686b6dadc65b89adfb32c8792004663 Mon Sep 17 00:00:00 2001
From: Baptiste Assmann <bed...@gmail.com>
Date: Mon, 2 Jul 2018 17:00:54 +0200
Subject: [PATCH 1/2] BUG/MEDIUM: dns: fix incomatibility between SRV
 resolution and server state file

Server state file has no indication that a server is currently managed
by a DNS SRV resolution.
And thus, both feature (DNS SRV resolution and server state), when used
together, does not provide the expected behavior: a smooth experience...

This patch introduce the "SRV record name" in the server state file and
loads and applies it if found and wherever required.
---
 include/types/server.h |  7 ++++---
 src/proxy.c            | 10 ++++++++--
 src/server.c           | 45 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 57 insertions(+), 5 deletions(-)

diff --git a/include/types/server.h b/include/types/server.h
index 0cd20c0..29b88f8 100644
--- a/include/types/server.h
+++ b/include/types/server.h
@@ -126,10 +126,11 @@ enum srv_initaddr {
     "bk_f_forced_id "             \
     "srv_f_forced_id "            \
     "srv_fqdn "                   \
-    "srv_port"
+    "srv_port"                    \
+    "srvrecord"
 
-#define SRV_STATE_FILE_MAX_FIELDS 19
-#define SRV_STATE_FILE_NB_FIELDS_VERSION_1 18
+#define SRV_STATE_FILE_MAX_FIELDS 20
+#define SRV_STATE_FILE_NB_FIELDS_VERSION_1 19
 #define SRV_STATE_LINE_MAXLEN 512
 
 /* server flags -- 32 bits */
diff --git a/src/proxy.c b/src/proxy.c
index c262966..c1c41ba 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -1429,6 +1429,7 @@ static int dump_servers_state(struct stream_interface *si, struct chunk *buf)
 	char srv_addr[INET6_ADDRSTRLEN + 1];
 	time_t srv_time_since_last_change;
 	int bk_f_forced_id, srv_f_forced_id;
+	char *srvrecord;
 
 	/* we don't want to report any state if the backend is not enabled on this process */
 	if (px->bind_proc && !(px->bind_proc & pid_bit))
@@ -1458,18 +1459,23 @@ static int dump_servers_state(struct stream_interface *si, struct chunk *buf)
 		bk_f_forced_id = px->options & PR_O_FORCED_ID ? 1 : 0;
 		srv_f_forced_id = srv->flags & SRV_F_FORCED_ID ? 1 : 0;
 
+		srvrecord = NULL;
+		if (srv->srvrq && srv->srvrq->name)
+			srvrecord = srv->srvrq->name;
+
 		chunk_appendf(buf,
 				"%d %s "
 				"%d %s %s "
 				"%d %d %d %d %ld "
 				"%d %d %d %d %d "
-				"%d %d %s %u"
+				"%d %d %s %u %s"
 				"\n",
 				px->uuid, px->id,
 				srv->puid, srv->id, srv_addr,
 				srv->cur_state, srv->cur_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, srv->hostname ? srv->hostname : "-", srv->svc_port);
+				bk_f_forced_id, srv_f_forced_id, srv->hostname ? srv->hostname : "-", srv->svc_port,
+				srvrecord ? srvrecord : "-");
 		if (ci_putchk(si_ic(si), &trash) == -1) {
 			si_applet_cant_put(si);
 			return 0;
diff --git a/src/server.c b/src/server.c
index 277d140..cb13793 100644
--- a/src/server.c
+++ b/src/server.c
@@ -2678,6 +2678,7 @@ static void srv_update_state(struct server *srv, int version, char **params)
 	const char *fqdn;
 	const char *port_str;
 	unsigned int port;
+	char *srvrecord;
 
 	fqdn = NULL;
 	port = 0;
@@ -2701,6 +2702,7 @@ static void srv_update_state(struct server *srv, int version, char **params)
 			 * srv_f_forced_id:      params[12]
 			 * srv_fqdn:             params[13]
 			 * srv_port:             params[14]
+			 * srvrecord:            params[15]
 			 */
 
 			/* validating srv_op_state */
@@ -2833,6 +2835,13 @@ static void srv_update_state(struct server *srv, int version, char **params)
 				}
 			}
 
+			/* SRV record
+			 * NOTE: in HAProxy, SRV records must start with an underscore '_'
+			 */
+			srvrecord = params[15];
+			if (srvrecord && *srvrecord != '_')
+				srvrecord = NULL;
+
 			/* don't apply anything if one error has been detected */
 			if (msg->len)
 				goto out;
@@ -2965,6 +2974,41 @@ static void srv_update_state(struct server *srv, int version, char **params)
 					}
 				}
 			}
+			/* If all the conditions below are validated, this means
+			 * we're evaluating a server managed by SRV resolution
+			 */
+			else if (fqdn && !srv->hostname && srvrecord) {
+				int res;
+
+				/* we can't apply previous state if SRV record has changed */
+				if (srv->srvrq && strcmp(srv->srvrq->name, srvrecord) != 0) {
+					chunk_appendf(msg, ", SRV record mismatch between configuration ('%s') and state file ('%s) for server '%s'. Previous state not applied", srv->srvrq->name, srvrecord, srv->id);
+					HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
+					goto out;
+				}
+
+				/* create or find a SRV resolution for this srv record */
+				if (srv->srvrq == NULL && (srv->srvrq = find_srvrq_by_name(srvrecord, srv->proxy)) == NULL)
+					srv->srvrq = new_dns_srvrq(srv, srvrecord);
+				if (srv->srvrq == NULL) {
+					chunk_appendf(msg, ", can't create or find SRV resolution '%s' for server '%s'", srvrecord, srv->id);
+					HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
+					goto out;
+				}
+
+				/* prepare DNS resolution for this server */
+				res = srv_prepare_for_resolution(srv, fqdn);
+				if (res == -1) {
+					chunk_appendf(msg, ", can't allocate memory for DNS resolution for server '%s'", srv->id);
+					HA_SPIN_UNLOCK(SERVER_LOCK, &srv->lock);
+					goto out;
+				}
+
+				/* configure check.port accordingly */
+				if ((srv->check.state & CHK_ST_CONFIGURED) &&
+				    !(srv->flags & SRV_F_CHECKPORT))
+					srv->check.port = port;
+			}
 
 			if (port_str)
 				srv->svc_port = port;
@@ -3219,6 +3263,7 @@ void apply_server_state(void)
 							 * srv_f_forced_id:      params[16] => srv_params[12]
 							 * srv_fqdn:             params[17] => srv_params[13]
 							 * srv_port:             params[18] => srv_params[14]
+							 * srvrecord:            params[19] => srv_params[15]
 							 */
 							if (arg >= 4) {
 								srv_params[srv_arg] = cur;
-- 
2.7.4

Reply via email to