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.

diff --git a/include/haproxy/protocol.h b/include/haproxy/protocol.h
index 44d7b0334a1220ffa937e1eacee41c2dea06ac96..3a3da90b538db1d404d1b7cfc26b27548c2847a7 100644 (file)
--- a/include/haproxy/protocol.h
+++ b/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

diff --git a/include/haproxy/proxy.h b/include/haproxy/proxy.h
index f6d8944a6950c98525bfb460683ebc71aa192761..bf94c2279677b47dc869d661d2c202954bc50df1 100644 (file)
--- a/include/haproxy/proxy.h
+++ b/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);

diff --git a/src/haproxy.c b/src/haproxy.c
index 8f41cf57967160af79fe15ed1f3dfd48f8d18dd6..409aef79cf25e271deaa60d98d14b0c922310512 100644 (file)
--- a/src/haproxy.c
+++ b/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));
-       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
+       /* Ok, all listeners should now be bound, close any leftover sockets
         * the previous process gave us, we don't need them anymore
         */
        while (xfer_sock_list != NULL) {

diff --git a/src/protocol.c b/src/protocol.c
index 84c85d2dd8dc56ac466c619ae20ccf5782933d86..a942e44930c65df970a48c85285374ce099c9572 100644 (file)
--- a/src/protocol.c
+++ b/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);
-                       if (err & ERR_ABORT)
+                       lerr = 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))) {
+                               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;

diff --git a/src/proxy.c b/src/proxy.c
index 3e42370d808d5acebae88484f4b30a6c4df99c5f..8fbb8171dfef5a2d6f4c9f07e9235a228d2249ab 100644 (file)
--- a/src/proxy.c
+++ b/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,
- * typically before privileges are dropped. The sockets will be registered
- * 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)
+ * This function finishes the startup of proxies by marking them ready. */
+void start_proxies(void)
 {
        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;
-               list_for_each_entry(listener, &curproxy->conf.listeners, by_fe) {
-                       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;
+               curproxy->state = PR_STREADY;
+               send_log(curproxy, LOG_NOTICE, "Proxy %s started.\n", curproxy->id);
        }
-
-       return err;
 }