Merge pull request #5275 from garlick/issue#5273

broker: avoid spurious overlay peer warning

Author: mergify[bot]

src/broker/boot_config.c

@@ -13,6 +13,9 @@
 #if HAVE_CONFIG_H
 #include "config.h"
 #endif
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <netdb.h>
 #include <unistd.h>
 #include <stdbool.h>
 #include <assert.h>
@@ -22,6 +25,7 @@
 #include <flux/hostlist.h>
 #include <flux/taskmap.h>
 
+#include "src/common/libyuarel/yuarel.h"
 #include "src/common/libutil/log.h"
 #include "src/common/libutil/errno_safe.h"
 #include "ccan/str/str.h"
@@ -495,6 +499,38 @@ static int set_broker_boot_method_attr (attr_t *attrs, const char *value)
     return 0;
 }
 
+/* Zeromq treats failed hostname resolution as a transient error, and silently
+ * retries in the background, which can make config problems hard to diagnose.
+ * Parse the URI in advance, and if the host portion is invalid, log it.
+ * Ref: flux-framework/flux-core#5009
+ */
+static void warn_of_invalid_host (flux_t *h, const char *uri)
+{
+    char *cpy;
+    struct yuarel u = { 0 };
+    struct addrinfo *result;
+    int e;
+
+    if (!(cpy = strdup (uri))
+        || yuarel_parse (&u, cpy) < 0
+        || !u.scheme
+        || !u.host
+        || !streq (u.scheme, "tcp"))
+        goto done;
+    /* N.B. this URI will be used for zmq_connect(), therefore it must
+     * be a valid peer address, not an interface name or wildcard.
+     */
+    if ((e = getaddrinfo (u.host, NULL, NULL, &result)) == 0) {
+        freeaddrinfo (result);
+        goto done;
+    }
+    log_msg ("Warning: unable to resolve upstream peer %s: %s",
+             u.host,
+             gai_strerror (e));
+done:
+    free (cpy);
+}
+
 int boot_config (flux_t *h, struct overlay *overlay, attr_t *attrs)
 {
     struct boot_conf conf;
@@ -623,6 +659,7 @@ int boot_config (flux_t *h, struct overlay *overlay, attr_t *attrs)
                                       parent_uri,
                                       sizeof (parent_uri)) < 0)
             goto error;
+        warn_of_invalid_host (h, parent_uri);
         if (overlay_set_parent_uri (overlay, parent_uri) < 0) {
             log_err ("overlay_set_parent_uri %s", parent_uri);
             goto error;

src/broker/overlay.c

@@ -32,7 +32,6 @@
 #include "src/common/libutil/errprintf.h"
 #include "src/common/librouter/rpc_track.h"
 #include "src/common/libccan/ccan/ptrint/ptrint.h"
-#include "src/common/libyuarel/yuarel.h"
 
 #include "overlay.h"
 #include "attr.h"
@@ -1264,48 +1263,13 @@ static void parent_monitor_cb (struct zmqutil_monitor *mon, void *arg)
     }
 }
 
-/* Zeromq treats failed hostname resolution as a transient error, and silently
- * retries in the background, which can make config problems hard to diagnose.
- * Parse the URI in advance, and if the host portion is invalid, log it.
- * Ref: flux-framework/flux-core#5009
- */
-static void warn_of_invalid_host (flux_t *h, const char *uri)
-{
-    char *cpy;
-    struct yuarel u = { 0 };
-    struct addrinfo *result;
-    int e;
-
-    if (!(cpy = strdup (uri))
-        || yuarel_parse (&u, cpy) < 0
-        || !u.scheme
-        || !u.host
-        || !streq (u.scheme, "tcp"))
-        goto done;
-    /* N.B. this URI will be used for zmq_connect(), therefore it must
-     * be a valid peer address, not an interface name or wildcard.
-     */
-    if ((e = getaddrinfo (u.host, NULL, NULL, &result)) == 0) {
-        freeaddrinfo (result);
-        goto done;
-    }
-    flux_log (h,
-              LOG_ERR,
-              "Warning: unable to resolve upstream peer %s: %s",
-              u.host,
-              gai_strerror (e));
-done:
-    free (cpy);
-}
-
 int overlay_connect (struct overlay *ov)
 {
     if (ov->rank > 0) {
         if (!ov->h || ov->rank == FLUX_NODEID_ANY || !ov->parent.uri) {
             errno = EINVAL;
             return -1;
         }
-        warn_of_invalid_host (ov->h, ov->parent.uri);
         if (!(ov->parent.zsock = zsock_new_dealer (NULL)))
             goto nomem;
         /* The socket monitor is only used for logging.

t/t0013-config-file.t

@@ -1,6 +1,9 @@
 #!/bin/sh
 #
 
+# Append --logfile option if FLUX_TESTS_LOGFILE is set in environment:
+test -n "$FLUX_TESTS_LOGFILE" && set -- "$@" --logfile
+
 test_description='Test config file overlay bootstrap'
 
 . `dirname $0`/sharness.sh
@@ -260,6 +263,43 @@ test_expect_success 'start size=3 instance with ipc:// and custom topology' '
 	test_cmp conf8a.exp conf8a.out
 '
 
+waitgrep() {
+	local pattern=$1
+	local file=$2
+	local iter=$3
+	while test $iter -gt 0; do
+		grep "$pattern" $file 2>/dev/null && return 0
+		sleep 0.3
+		iter=$(($iter-1))
+	done
+	return 1
+}
+
+# RFC 2606 reserves the .invalid domain for testing
+test_expect_success NO_CHAIN_LINT 'a warning is printed when upstream URI has unknown host' '
+	mkdir conf8b &&
+	cat <<-EOT >conf8b/bootstrap.toml &&
+	[bootstrap]
+	curve_cert = "testcert"
+	[[bootstrap.hosts]]
+	host = "fake0"
+	connect = "tcp://foo.invalid:1234"
+	[[bootstrap.hosts]]
+	host = "fake1"
+	EOT
+	FLUX_FAKE_HOSTNAME=fake1 \
+		flux broker -vv -Sbroker.rc1_path=,-Sbroker.rc3_path= \
+		--config-path=conf8b 2>warn.err &
+	echo $! >warn.pid &&
+	waitgrep "unable to resolve upstream peer" warn.err 30
+'
+# In case warn.pid actually refers to a libtool wrapper, try pkill(1) -P
+# first to kill its children, then kill(1).  See flux-framework/flux-core#5275.
+test_expect_success NO_CHAIN_LINT 'clean up broker from previous test' '
+	warnpid=$(cat warn.pid) &&
+	pkill -15 -P $warnpid || kill -15 $warnpid
+'
+
 getport() {
 	flux python -c \
 	'from socket import socket; s=socket(); s.bind(("", 0)); print(s.getsockname()[1])'