MAJOR: init: start all listeners via protocols and not via proxies anymore

Ever since the protocols were added in 1.3.13, listeners used to be
started twice:
  - once by start_proxies(), which iteratees over all proxies then all
    listeners ;
  - once by protocol_bind_all() which iterates over all protocols then
    all listeners ;

It's a real mess because error reporting is not even consistent, and
more importantly now that some protocols do not appear in regular
proxies (peers, logs), there is no way to retry their binding should
it fail on the last step.

What this patch does is to make sure that listeners are exclusively
started by protocols. The failure to start a listener now causes the
emission of an error indicating the proxy's name (as it used to be
the case per proxy), and retryable failures are silently ignored
during all but last attempts.

The start_proxies() function was kept solely for setting the proxy's
state to READY and emitting the "Proxy started" message and log that
some have likely got used to seeking in their logs.

include/haproxy/protocol.h

@@ -38,9 +38,9 @@ void protocol_register(struct protocol *proto);
 void protocol_unregister(struct protocol *proto);
 
 /* binds all listeners of all registered protocols. Returns a composition
- * of ERR_NONE, ERR_RETRYABLE, ERR_FATAL.
+ * of ERR_NONE, ERR_RETRYABLE, ERR_FATAL, ERR_ABORT.
  */
-int protocol_bind_all(char *errmsg, int errlen);
+int protocol_bind_all(int verbose);
 
 /* unbinds all listeners of all registered protocols. They are also closed.
  * This must be performed before calling exit() in order to get a chance to

include/haproxy/proxy.h

@@ -39,7 +39,7 @@ extern struct eb_root proxy_by_name;    /* tree of proxies sorted by name */
 extern const struct cfg_opt cfg_opts[];
 extern const struct cfg_opt cfg_opts2[];
 
-int start_proxies(int verbose);
+void start_proxies(void);
 struct task *manage_proxy(struct task *t, void *context, unsigned short state);
 void soft_stop(void);
 int pause_proxy(struct proxy *p);

src/haproxy.c

@@ -2941,7 +2941,6 @@ int main(int argc, char **argv)
 {
 	int err, retry;
 	struct rlimit limit;
-	char errmsg[100];
 	int pidfd = -1;
 
 	setvbuf(stdout, NULL, _IONBF, 0);
@@ -3059,7 +3058,7 @@ int main(int argc, char **argv)
 	err = ERR_NONE;
 	while (retry >= 0) {
 		struct timeval w;
-		err = start_proxies(retry == 0 || nb_oldpids == 0);
+		err = protocol_bind_all(retry == 0 || nb_oldpids == 0);
 		/* exit the loop on no error or fatal error */
 		if ((err & (ERR_RETRYABLE|ERR_FATAL)) != ERR_RETRYABLE)
 			break;
@@ -3082,8 +3081,9 @@ int main(int argc, char **argv)
 		retry--;
 	}
 
-	/* Note: start_proxies() sends an alert when it fails. */
+	/* Note: protocol_bind_all() sends an alert when it fails. */
 	if ((err & ~ERR_WARN) != ERR_NONE) {
+		ha_alert("[%s.main()] Some protocols failed to start their listeners! Exiting.\n", argv[0]);
 		if (retry != MAX_START_RETRIES && nb_oldpids) {
 			protocol_unbind_all(); /* cleanup everything we can */
 			tell_old_pids(SIGTTIN);
@@ -3091,6 +3091,8 @@ int main(int argc, char **argv)
 		exit(1);
 	}
 
+	start_proxies();
+
 	if (!(global.mode & MODE_MWORKER_WAIT) && listeners == 0) {
 		ha_alert("[%s.main()] No enabled listener found (check for 'bind' directives) ! Exiting.\n", argv[0]);
 		/* Note: we don't have to send anything to the old pids because we
@@ -3098,20 +3100,7 @@ int main(int argc, char **argv)
 		exit(1);
 	}
 
-	err = protocol_bind_all(errmsg, sizeof(errmsg));
+	/* Ok, all listeners should now be bound, close any leftover sockets
-	if ((err & ~ERR_WARN) != ERR_NONE) {
-		if ((err & ERR_ALERT) || (err & ERR_WARN))
-			ha_alert("[%s.main()] %s.\n", argv[0], errmsg);
-
-		ha_alert("[%s.main()] Some protocols failed to start their listeners! Exiting.\n", argv[0]);
-		protocol_unbind_all(); /* cleanup everything we can */
-		if (nb_oldpids)
-			tell_old_pids(SIGTTIN);
-		exit(1);
-	} else if (err & ERR_WARN) {
-		ha_alert("[%s.main()] %s.\n", argv[0], errmsg);
-	}
-	/* Ok, all listener should now be bound, close any leftover sockets
 	 * the previous process gave us, we don't need them anymore
 	 */
 	while (xfer_sock_list != NULL) {

src/protocol.c

@@ -18,6 +18,7 @@
 #include <haproxy/list.h>
 #include <haproxy/listener.h>
 #include <haproxy/protocol.h>
+#include <haproxy/proxy.h>
 #include <haproxy/tools.h>
 
 
@@ -55,20 +56,37 @@ void protocol_unregister(struct protocol *proto)
 /* binds all listeners of all registered protocols. Returns a composition
  * of ERR_NONE, ERR_RETRYABLE, ERR_FATAL.
  */
-int protocol_bind_all(char *errmsg, int errlen)
+int protocol_bind_all(int verbose)
 {
 	struct protocol *proto;
 	struct listener *listener;
-	int err;
+	char msg[100];
+	int err, lerr;
 
 	err = 0;
 	HA_SPIN_LOCK(PROTO_LOCK, &proto_lock);
 	list_for_each_entry(proto, &protocols, list) {
 		list_for_each_entry(listener, &proto->listeners, proto_list) {
-			err |= proto->bind(listener, errmsg, errlen);
+			lerr = proto->bind(listener, msg, sizeof(msg));
-			if (err & ERR_ABORT)
+
+			/* errors are reported if <verbose> is set or if they are fatal */
+			if (verbose || (lerr & (ERR_FATAL | ERR_ABORT))) {
+				struct proxy *px = listener->bind_conf->frontend;
+
+				if (lerr & ERR_ALERT)
+					ha_alert("Starting %s %s: %s\n",
+						 proxy_type_str(px), px->id, msg);
+				else if (lerr & ERR_WARN)
+					ha_warning("Starting %s %s: %s\n",
+						   proxy_type_str(px), px->id, msg);
+			}
+
+			err |= lerr;
+			if (lerr & ERR_ABORT)
 				break;
 		}
+		if (err & ERR_ABORT)
+			break;
 	}
 	HA_SPIN_UNLOCK(PROTO_LOCK, &proto_lock);
 	return err;

src/proxy.c

@@ -1046,65 +1046,18 @@ void init_new_proxy(struct proxy *p)
 }
 
 /*
- * This function creates all proxy sockets. It should be done very early,
+ * This function finishes the startup of proxies by marking them ready. */
- * typically before privileges are dropped. The sockets will be registered
+void start_proxies(void)
- * but not added to any fd_set, in order not to loose them across the fork().
- * The proxies also start in READY state because they all have their listeners
- * bound.
- *
- * Its return value is composed from ERR_NONE, ERR_RETRYABLE and ERR_FATAL.
- * Retryable errors will only be printed if <verbose> is not zero.
- */
-int start_proxies(int verbose)
 {
 	struct proxy *curproxy;
-	struct listener *listener;
-	int lerr, err = ERR_NONE;
-	int pxerr;
-	char msg[100];
 
 	for (curproxy = proxies_list; curproxy != NULL; curproxy = curproxy->next) {
 		if (curproxy->state != PR_STNEW)
 			continue; /* already initialized */
 
-		pxerr = 0;
+		curproxy->state = PR_STREADY;
-		list_for_each_entry(listener, &curproxy->conf.listeners, by_fe) {
+		send_log(curproxy, LOG_NOTICE, "Proxy %s started.\n", curproxy->id);
-			if (listener->state != LI_ASSIGNED)
-				continue; /* already started */
-
-			lerr = listener->proto->bind(listener, msg, sizeof(msg));
-
-			/* errors are reported if <verbose> is set or if they are fatal */
-			if (verbose || (lerr & (ERR_FATAL | ERR_ABORT))) {
-				if (lerr & ERR_ALERT)
-					ha_alert("Starting %s %s: %s\n",
-						 proxy_type_str(curproxy), curproxy->id, msg);
-				else if (lerr & ERR_WARN)
-					ha_warning("Starting %s %s: %s\n",
-						   proxy_type_str(curproxy), curproxy->id, msg);
-			}
-
-			err |= lerr;
-			if (lerr & (ERR_ABORT | ERR_FATAL)) {
-				pxerr |= 1;
-				break;
-			}
-			else if (lerr & ERR_CODE) {
-				pxerr |= 1;
-				continue;
-			}
-		}
-
-		if (!pxerr) {
-			curproxy->state = PR_STREADY;
-			send_log(curproxy, LOG_NOTICE, "Proxy %s started.\n", curproxy->id);
-		}
-
-		if (err & ERR_ABORT)
-			break;
 	}
-
-	return err;
 }