From 602aa4f7604cf4d4cb673cdb1f5a8e995b45d481 Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Wed, 11 Dec 2019 06:39:19 -0800
Subject: [PATCH 01/16] cmd/flux-start: drop annoying warning

Problem: flux start --size=N where N>1 warns unhelpfully:
"warning: setting --bootstrap=selfpmi due to --size option".

Drop the warning message.
---
 src/cmd/flux-start.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/cmd/flux-start.c b/src/cmd/flux-start.c
index e496df3ab325..01d0532ace55 100644
--- a/src/cmd/flux-start.c
+++ b/src/cmd/flux-start.c
@@ -194,7 +194,6 @@ int main (int argc, char *argv[])
         if (bootstrap != BOOTSTRAP_SELFPMI) {
             if (!optparse_hasopt (ctx.opts, "bootstrap")) {
                 bootstrap = BOOTSTRAP_SELFPMI;
-                log_msg("warning: setting --bootstrap=selfpmi due to --size option");
             } else {
                 log_errn_exit(EINVAL, "--size can only be used with --bootstrap=selfpmi");
             }

From f0e398db29ce19fd40d07c5f939679e1f8906237 Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Wed, 18 Dec 2019 16:24:56 -0800
Subject: [PATCH 02/16] broker/log: open file only on rank 0

Problem: when the broker is started with the
'log-filename' attribute set on the command line,
the log file is opened from all broker ranks.
This does not match the documentation or the code.

Move initialization of logbuf->rank in logbuf_initialize()
to before logbuf_register_attrs() is called.  Now the test
for rank 0 is only true on rank 0.

Move the logbuf->h initialization too for consistency.

Fixes #2581
---
 src/broker/log.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/broker/log.c b/src/broker/log.c
index 4982cb45ec4d..bc920ba8b84c 100644
--- a/src/broker/log.c
+++ b/src/broker/log.c
@@ -680,6 +680,8 @@ int logbuf_initialize (flux_t *h, uint32_t rank, attr_t *attrs)
     logbuf_t *logbuf = logbuf_create ();
     if (!logbuf)
         goto error;
+    logbuf->h = h;
+    logbuf->rank = rank;
     if (logbuf_register_attrs (logbuf, attrs) < 0)
         goto error;
     if (fake_rank (h, rank) < 0)
@@ -688,8 +690,6 @@ int logbuf_initialize (flux_t *h, uint32_t rank, attr_t *attrs)
         goto error;
     flux_log_set_appname (h, "broker");
     flux_log_set_redirect (h, logbuf_append_redirect, logbuf);
-    logbuf->h = h;
-    logbuf->rank = rank;
     flux_aux_set (h, "flux::logbuf", logbuf, logbuf_finalize);
     return 0;
 error:

From d51f3b5107b73507d6b9ae7e139be81c61d8380e Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Wed, 18 Dec 2019 16:28:58 -0800
Subject: [PATCH 03/16] broker/log: drop redundant rank setting

Problem: logbuf_initialize() calls flux_attr_set_cacheonly()
on the "rank" attribute.  The broker call to create_dummyattrs()
right after the call to logbuf_initialize() handles this,
and there is no requirement to set it before that.

Drop this call.
---
 src/broker/log.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/src/broker/log.c b/src/broker/log.c
index bc920ba8b84c..d0a1093256ba 100644
--- a/src/broker/log.c
+++ b/src/broker/log.c
@@ -668,13 +668,6 @@ static void logbuf_finalize (void *arg)
     /* FIXME: need logbuf_unregister_attrs() */
 }
 
-static int fake_rank (flux_t *h, uint32_t rank)
-{
-    char buf[16];
-    snprintf (buf, sizeof (buf), "%u", rank);
-    return flux_attr_set_cacheonly (h, "rank", buf);
-}
-
 int logbuf_initialize (flux_t *h, uint32_t rank, attr_t *attrs)
 {
     logbuf_t *logbuf = logbuf_create ();
@@ -684,8 +677,6 @@ int logbuf_initialize (flux_t *h, uint32_t rank, attr_t *attrs)
     logbuf->rank = rank;
     if (logbuf_register_attrs (logbuf, attrs) < 0)
         goto error;
-    if (fake_rank (h, rank) < 0)
-        goto error;
     if (flux_msg_handler_addvec (h, htab, logbuf, &logbuf->handlers) < 0)
         goto error;
     flux_log_set_appname (h, "broker");

From 6b890efb9a4f8d3d7b312d39f119c9de52afa8de Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Sat, 14 Dec 2019 13:09:57 -0800
Subject: [PATCH 04/16] libidset: add idset_ndecode()

Problem: there is no convenient way to decode an idset
from a bounded substring.

Add idset_ndecode() which accepts a length.
---
 src/common/libidset/idset.h        | 5 +++++
 src/common/libidset/idset_decode.c | 9 +++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/common/libidset/idset.h b/src/common/libidset/idset.h
index b0ac43ce38b2..83993f02836b 100644
--- a/src/common/libidset/idset.h
+++ b/src/common/libidset/idset.h
@@ -52,6 +52,11 @@ char *idset_encode (const struct idset *idset, int flags);
  */
 struct idset *idset_decode (const char *s);
 
+/* Decode 'len' chars of string 's' to an idset.
+ * Returns idset on success, or NULL on failure with errno set.
+ */
+struct idset *idset_ndecode (const char *s, size_t len);
+
 /* Add id (or range [lo-hi]) to idset.
  * Return 0 on success, -1 on failure with errno set.
  */
diff --git a/src/common/libidset/idset_decode.c b/src/common/libidset/idset_decode.c
index 39c1f549aa1d..bc6412ebc7d1 100644
--- a/src/common/libidset/idset_decode.c
+++ b/src/common/libidset/idset_decode.c
@@ -61,7 +61,7 @@ static char *trim_brackets (char *s)
     return p;
 }
 
-struct idset *idset_decode (const char *str)
+struct idset *idset_ndecode (const char *str, size_t size)
 {
     struct idset *idset;
     char *cpy = NULL;
@@ -74,7 +74,7 @@ struct idset *idset_decode (const char *str)
     }
     if (!(idset = idset_create (0, IDSET_FLAG_AUTOGROW)))
         return NULL;
-    if (!(cpy = strdup (str)))
+    if (!(cpy = strndup (str, size)))
         goto error;
     a1 = trim_brackets (cpy);
     saveptr = NULL;
@@ -104,6 +104,11 @@ struct idset *idset_decode (const char *str)
     return NULL;
 }
 
+struct idset *idset_decode (const char *str)
+{
+    return idset_ndecode (str, str ? strlen (str) : 0);
+}
+
 /*
  * vi:tabstop=4 shiftwidth=4 expandtab
  */

From 3131f31f32ce4de6dccf6fc4c6f5f5a705d2ca3e Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Sat, 14 Dec 2019 16:39:13 -0800
Subject: [PATCH 05/16] libidset: add idset_format_map()

Add idset_format_map() to simplify expansion of strings that
have embedded idsets.

Given a string with zero or more embedded bracketed idsets,
expands idsets, calling a map function for each formatted string.
For example, an input string of "r[0-1]n[0-1]" would result in
map function calls for "r0n0", "r0n1", "r1n0", "r1n1".
Returns number of iterations, or -1 on failure with errno set.

Iteration can be halted early from the map function (no error).
The map function can also return -1 with errno set to abort
iteration with an error.
---
 src/common/libidset/Makefile.am     |   3 +-
 src/common/libidset/idset.h         |  19 +++++
 src/common/libidset/idset_format.c  | 117 ++++++++++++++++++++++++++++
 src/common/libidset/idset_private.h |   5 ++
 4 files changed, 143 insertions(+), 1 deletion(-)
 create mode 100644 src/common/libidset/idset_format.c

diff --git a/src/common/libidset/Makefile.am b/src/common/libidset/Makefile.am
index 5825b8e14a35..984fbce398f5 100644
--- a/src/common/libidset/Makefile.am
+++ b/src/common/libidset/Makefile.am
@@ -16,7 +16,8 @@ fluxinclude_HEADERS = idset.h
 libidset_la_SOURCES = idset.c \
 		      idset_private.h \
 		      idset_decode.c \
-		      idset_encode.c
+		      idset_encode.c \
+		      idset_format.c
 
 libidset_la_CPPFLAGS = \
 	$(AM_CPPFLAGS)
diff --git a/src/common/libidset/idset.h b/src/common/libidset/idset.h
index 83993f02836b..ffb9a67878ad 100644
--- a/src/common/libidset/idset.h
+++ b/src/common/libidset/idset.h
@@ -100,6 +100,25 @@ size_t idset_count (const struct idset *idset);
  */
 bool idset_equal (const struct idset *set1, const struct idset *set2);
 
+/* Expand bracketed idset string(s) in 's', calling 'fun()' for each
+ * expanded string.  'fun()' should return 0 on success, or -1 on failure
+ * with errno set.  A fun() failure causes idset_format_map () to immediately
+ * return -1.  'fun()' may may halt iteration without triggering an error
+ * by setting *stop = true.
+ *
+ * idset_format_map () returns the number of times the map function was called
+ * (including the stopping one, if any), or -1 on failure with errno set.
+ *
+ * This function recursively expands multiple bracketed idset strings from
+ * left to right, so for example, "r[0-1]n[0-1]" expands to "r0n0", "r0n1",
+ * "r1n0", "r1n1".
+ *
+ */
+typedef int (*idset_format_map_f)(const char *s, bool *stop, void *arg);
+
+int idset_format_map (const char *s, idset_format_map_f fun, void *arg);
+
+
 #endif /* !FLUX_IDSET_H */
 
 /*
diff --git a/src/common/libidset/idset_format.c b/src/common/libidset/idset_format.c
new file mode 100644
index 000000000000..30e6bea90d4d
--- /dev/null
+++ b/src/common/libidset/idset_format.c
@@ -0,0 +1,117 @@
+/************************************************************\
+ * Copyright 2014 Lawrence Livermore National Security, LLC
+ * (c.f. AUTHORS, NOTICE.LLNS, COPYING)
+ *
+ * This file is part of the Flux resource manager framework.
+ * For details, see https://github.com/flux-framework.
+ *
+ * SPDX-License-Identifier: LGPL-3.0
+\************************************************************/
+
+#if HAVE_CONFIG_H
+#include "config.h"
+#endif
+#include <sys/param.h>
+#include <errno.h>
+#include <string.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <stdarg.h>
+#include <stdbool.h>
+
+#include "src/common/libutil/errno_safe.h"
+
+#include "idset.h"
+#include "idset_private.h"
+
+static int find_brackets (const char *s, const char **start, const char **end)
+{
+    if (!(*start = strchr (s, '[')))
+        return -1;
+    if (!(*end = strchr (*start + 1, ']')))
+        return -1;
+    return 0;
+}
+
+int format_first (char *buf,
+                  size_t bufsz,
+                  const char *fmt,
+                  unsigned int id)
+{
+    const char *start, *end;
+
+    if (!buf || !fmt || find_brackets (fmt, &start, &end) < 0) {
+        errno = EINVAL;
+        return -1;
+    }
+    if (snprintf (buf,
+                  bufsz,
+                  "%.*s%u%s",
+                  (int)(start - fmt),
+                  fmt,
+                  id,
+                  end + 1) >= bufsz) {
+        errno = EOVERFLOW;
+        return -1;
+    }
+    return 0;
+}
+
+static int idset_format_map_ex (const char *s,
+                                size_t maxsize,
+                                idset_format_map_f fun,
+                                void *arg,
+                                bool *stop)
+
+{
+    const char *start, *end;
+    struct idset *idset = NULL;
+    char *buf = NULL;
+    int count = 0;
+    unsigned int id;
+    int n;
+
+    if (find_brackets (s, &start, &end) == 0) {
+        if (!(idset = idset_ndecode (start, end - start + 1)))
+            goto error;
+        if (!(buf = malloc (maxsize)))
+            goto error;
+        id = idset_first (idset);
+        while (id != IDSET_INVALID_ID) {
+            if (format_first (buf, maxsize, s, id) < 0)
+                goto error;
+            if ((n = idset_format_map_ex (buf, maxsize, fun, arg, stop)) < 0)
+                goto error;
+            count += n;
+            if (*stop)
+                break;
+            id = idset_next (idset, id);
+        }
+        free (buf);
+        idset_destroy (idset);
+    }
+    else {
+        if (fun && fun (s, stop, arg) < 0)
+            goto error;
+        count = 1;
+    }
+    return count;
+error:
+    ERRNO_SAFE_WRAP (free, buf);
+    idset_destroy (idset);
+    return -1;
+}
+
+int idset_format_map (const char *s, idset_format_map_f fun, void *arg)
+{
+    bool stop = false;
+    if (!s) {
+        errno = EINVAL;
+        return -1;
+    }
+    return idset_format_map_ex (s, 4096, fun, arg, &stop);
+}
+
+/*
+ * vi:tabstop=4 shiftwidth=4 expandtab
+ */
diff --git a/src/common/libidset/idset_private.h b/src/common/libidset/idset_private.h
index 492d565c11ba..a0e0a6b08a80 100644
--- a/src/common/libidset/idset_private.h
+++ b/src/common/libidset/idset_private.h
@@ -27,6 +27,11 @@ struct idset {
 
 int validate_idset_flags (int flags, int allowed);
 
+int format_first (char *buf,
+                  size_t bufsz,
+                  const char *fmt,
+                  unsigned int id);
+
 /*
  * vi:tabstop=4 shiftwidth=4 expandtab
  */

From 2981f80c8898366add83efd62912cc3d4c5b3a1b Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Sat, 14 Dec 2019 17:00:06 -0800
Subject: [PATCH 06/16] libidset: add unit tests for idset_format_map()

---
 src/common/libidset/Makefile.am  |   3 +-
 src/common/libidset/test/idset.c | 183 +++++++++++++++++++++++++++++++
 2 files changed, 185 insertions(+), 1 deletion(-)

diff --git a/src/common/libidset/Makefile.am b/src/common/libidset/Makefile.am
index 984fbce398f5..111e9e2b04bf 100644
--- a/src/common/libidset/Makefile.am
+++ b/src/common/libidset/Makefile.am
@@ -39,4 +39,5 @@ test_idset_t_CPPFLAGS = $(AM_CPPFLAGS)
 test_idset_t_LDADD = \
 	$(top_builddir)/src/common/libtap/libtap.la \
 	$(top_builddir)/src/common/libidset/libidset.la \
-	$(top_builddir)/src/common/libutil/libutil.la
+	$(top_builddir)/src/common/libutil/libutil.la \
+	$(ZMQ_LIBS)
diff --git a/src/common/libidset/test/idset.c b/src/common/libidset/test/idset.c
index b90c063ad503..96b80f85ed59 100644
--- a/src/common/libidset/test/idset.c
+++ b/src/common/libidset/test/idset.c
@@ -8,9 +8,13 @@
  * SPDX-License-Identifier: LGPL-3.0
 \************************************************************/
 
+#if HAVE_CONFIG_H
+#include "config.h"
+#endif
 #include <errno.h>
 #include <string.h>
 #include <stdbool.h>
+#include <czmq.h>
 
 #include "src/common/libtap/tap.h"
 #include "src/common/libidset/idset.h"
@@ -523,6 +527,183 @@ void test_autogrow (void)
     idset_destroy (idset);
 }
 
+/* N.B. internal function */
+void test_format_first (void)
+{
+    char buf[64];
+
+    ok (format_first (buf, sizeof (buf), "[]xyz", 42) == 0
+        && !strcmp (buf, "42xyz"),
+        "format_first works with leading idset");
+    ok (format_first (buf, sizeof (buf), "abc[]xyz", 42) == 0
+        && !strcmp (buf, "abc42xyz"),
+        "format_first works with mid idset");
+    ok (format_first (buf, sizeof (buf), "abc[]", 42) == 0
+        && !strcmp (buf, "abc42"),
+        "format_first works with end idset");
+
+    errno = 0;
+    ok (format_first (buf, sizeof (buf), "abc", 42) < 0
+        && errno == EINVAL,
+        "format_first fails with EINVAL no brackets");
+
+    errno = 0;
+    ok (format_first (buf, sizeof (buf), "abc[", 42) < 0
+        && errno == EINVAL,
+        "format_first fails with EINVAL with no close bracket");
+
+    errno = 0;
+    ok (format_first (buf, sizeof (buf), "abc]", 42) < 0
+        && errno == EINVAL,
+        "format_first fails with EINVAL with no open bracket");
+
+    errno = 0;
+    ok (format_first (buf, sizeof (buf), "abc][", 42) < 0
+        && errno == EINVAL,
+        "format_first fails with EINVAL with backwards brackets");
+
+    errno = 0;
+    ok (format_first (buf, 4, "abc[]", 1) < 0
+        && errno == EOVERFLOW,
+        "format_first fails with EOVERFLOW when buffer exhausted");
+}
+
+bool verify_map (zlist_t *list, const char **expected, int count)
+{
+    char *s;
+    int i;
+
+    s = zlist_first (list);
+    for (i = 0; i < count; i++) {
+        if (strcmp (s, expected[i]) != 0) {
+            diag ("map called with %s, expected %s", s, expected[i]);
+            return false;
+        }
+        s = zlist_next (list);
+    }
+    return true;
+}
+
+void empty_list (zlist_t *list)
+{
+    char *s;
+    while ((s = zlist_pop (list)))
+        free (s);
+}
+
+int mapfun (const char *s, bool *stop, void *arg)
+{
+    zlist_t *list = arg;
+    char *cpy;
+
+    if (!(cpy = strdup (s)))
+        BAIL_OUT ("strdup failed");
+    if (zlist_append (list, cpy) < 0)
+        BAIL_OUT ("zlist_append failed");
+
+    return 0;
+}
+
+int mapfun_err3 (const char *s, bool *stop, void *arg)
+{
+    zlist_t *list = arg;
+    char *cpy;
+
+    if (zlist_size (list) == 3) {
+        errno = EPERM; // arbitrary
+        return -1;
+    }
+    if (!(cpy = strdup (s)))
+        BAIL_OUT ("strdup failed");
+    if (zlist_append (list, cpy) < 0)
+        BAIL_OUT ("zlist_append failed");
+    return 0;
+}
+
+int mapfun_stop3 (const char *s, bool *stop, void *arg)
+{
+    zlist_t *list = arg;
+    char *cpy;
+
+    if (!(cpy = strdup (s)))
+        BAIL_OUT ("strdup failed");
+    if (zlist_append (list, cpy) < 0)
+        BAIL_OUT ("zlist_append failed");
+    if (zlist_size (list) == 3)
+        *stop = true;
+    return 0;
+}
+struct maptest {
+    const char *input;
+    const char *expected[16];
+    int count;
+};
+
+struct maptest maptests[] = {
+    { "n[0-3]", { "n0", "n1", "n2", "n3" }, 4 },
+    { "r[0-1]n[0-1]", { "r0n0", "r0n1", "r1n0", "r1n1" }, 4 },
+    { "[0-1][0-1][0-2]", { "000", "001", "002", "010", "011", "012",
+                           "100", "101", "102", "110", "111", "112"}, 12 },
+    { "n[0,99-100]x", { "n0x", "n99x", "n100x" }, 3 },
+    { "foo", { "foo" }, 1 },
+    { "foo[", { "foo[" }, 1 },
+    { "foo]", { "foo]" }, 1 },
+    { "foo][", { "foo][" }, 1 },
+    { "foo[]", { }, 0 },
+    { "", { "" }, 1 },
+    { NULL, {}, 0 },
+};
+
+void test_format_map (void)
+{
+    zlist_t *list;
+    int i;
+    int rc;
+
+    if (!(list = zlist_new ()))
+        BAIL_OUT ("zlist_new failed");
+
+    /* bad params */
+    errno = 0;
+    ok (idset_format_map (NULL, mapfun, NULL) < 0
+        && errno == EINVAL,
+        "idset_format_map input=NULL fails with EINVAL");
+
+    /* bad idset, but correctly embedded */
+    errno = 0;
+    ok (idset_format_map ("[foo]", mapfun, NULL) < 0
+        && errno == EINVAL,
+        "idset_format_map input=[foo] fails with EINVAL");
+
+    /* check for expected expansion */
+    for (i = 0; maptests[i].input != NULL; i++) {
+        rc = idset_format_map (maptests[i].input, mapfun, list);
+        ok (rc == maptests[i].count
+            && verify_map (list, maptests[i].expected, maptests[i].count),
+            "idset_format_map input='%s' works",
+            maptests[i].input);
+
+        empty_list (list);
+    }
+
+    /* map() returns -1 with errno == EPERM on 4th call */
+    errno = 0;
+    ok (idset_format_map ("h[0-15]", mapfun_err3, list) < 0
+        && errno == EPERM
+        && zlist_size (list) == 3,
+        "idset_format_map input handles map() failure OK");
+    empty_list (list);
+
+    /* map() pokes *stop on 4th call */
+    errno = 0;
+    ok (idset_format_map ("h[0-15]", mapfun_stop3, list) == 3
+        && zlist_size (list) == 3,
+        "idset_format_map input handles *stop = true OK");
+    empty_list (list);
+
+    zlist_destroy (&list);
+}
+
 void issue_1974(void)
 {
     struct idset *idset;
@@ -579,6 +760,8 @@ int main (int argc, char *argv[])
     test_equal ();
     test_copy ();
     test_autogrow ();
+    test_format_first ();
+    test_format_map ();
     issue_1974 ();
     issue_2336 ();
 

From 4fbd36bf365f2aa2d4167e19cbb002e5fce7c952 Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Fri, 13 Dec 2019 06:43:51 -0800
Subject: [PATCH 07/16] broker/boot_config: convert to new config format

Change the [bootstrap] configuration to allow bind URI,
and connect URI to be expressed independently, and to
determine the rank by matching the hostname, not the
local interface addresses.

Rename the rank-ordered array from 'tbon-endpoints' to
'hosts' for clarity.  A hosts entry is a table containing
the keys 'host' (required), 'bind' (optional), and 'connect'
(optional).

The keys 'default_port', 'default_bind', and 'default_connect'
may be defined.  The default bind and connect values are
used when those keys are not explicitly set in the hosts entry.
The 'default_port' value is substituted for the token "%p" if
it occurs in bind or connect strings.  The 'host' value in a
hosts entry entry is substituted for the token "%h" if it
occurs in bind or connect strings.

The 'host' key may included embedded bracketed idsets.

Example 1024 node cluster config:

[bootstrap]

default_port = 8050
default_bind = "tcp://en0:%p"
default_connect = "tcp://e%h:%p"

hosts = [
    {
        host="fluke0",
        bind="tcp://en4:9001",
        connect="tcp://fluke-mgmt:9001"
    },
    { host = "fluke[1-1023]" },
]

In the example, the bind address is "tcp:/en4:9001" on rank 0
and "tcp://en0:8050" on ranks 1-1023.  The connect address for
rank 0 is "tcp://fluke-mgmt:9001"; "tcp://efluke1:8050" for rank 1;
"tcp://efluke2:8050" for rank 2, etc..

Update existing sharness test.

Update installed config file.

Fixes #2572
---
 etc/Makefile.am                               |   3 +-
 etc/conf.d/boot.toml                          |   7 -
 etc/conf.d/bootstrap.toml                     |   9 +
 src/broker/boot_config.c                      | 536 +++++++++++++-----
 src/broker/boot_config.h                      |  59 +-
 t/Makefile.am                                 |  20 +-
 .../{bad-intendpoints => bad-hosts}/boot.toml |   2 +-
 t/conf.d/bad-hosts2/boot.toml                 |   3 +
 t/conf.d/bad-intendpoints2/boot.toml          |   3 -
 t/conf.d/bad-nobootstrap/boot.toml            |   4 +-
 t/conf.d/bad-noendpoints/boot.toml            |   3 -
 t/conf.d/bad-nomatch/boot.toml                |   8 +
 t/conf.d/bad-rank/boot.toml                   |  10 -
 t/conf.d/bad-rank2/boot.toml                  |  10 -
 t/conf.d/good-emptyhosts/boot.toml            |   4 +
 t/conf.d/good-ipc2/boot.toml                  |   9 +
 t/conf.d/good-nohosts/bad-nohosts/boot.toml   |   3 +
 t/conf.d/good-nohosts/boot.toml               |   2 +
 t/conf.d/good-tcp4/boot.toml                  |  13 +
 t/conf.d/priv2-0/boot.toml                    |  11 -
 t/conf.d/priv2-1/boot.toml                    |  11 -
 t/conf.d/private/boot.toml                    |   8 -
 t/conf.d/shared/boot.toml                     |   5 -
 t/conf.d/shared_ipc/boot.toml                 |   5 -
 t/conf.d/shared_none/boot.toml                |   5 -
 t/t0013-config-file.t                         |  90 ++-
 26 files changed, 523 insertions(+), 320 deletions(-)
 delete mode 100644 etc/conf.d/boot.toml
 create mode 100644 etc/conf.d/bootstrap.toml
 rename t/conf.d/{bad-intendpoints => bad-hosts}/boot.toml (53%)
 create mode 100644 t/conf.d/bad-hosts2/boot.toml
 delete mode 100644 t/conf.d/bad-intendpoints2/boot.toml
 delete mode 100644 t/conf.d/bad-noendpoints/boot.toml
 create mode 100644 t/conf.d/bad-nomatch/boot.toml
 delete mode 100644 t/conf.d/bad-rank/boot.toml
 delete mode 100644 t/conf.d/bad-rank2/boot.toml
 create mode 100644 t/conf.d/good-emptyhosts/boot.toml
 create mode 100644 t/conf.d/good-ipc2/boot.toml
 create mode 100644 t/conf.d/good-nohosts/bad-nohosts/boot.toml
 create mode 100644 t/conf.d/good-nohosts/boot.toml
 create mode 100644 t/conf.d/good-tcp4/boot.toml
 delete mode 100644 t/conf.d/priv2-0/boot.toml
 delete mode 100644 t/conf.d/priv2-1/boot.toml
 delete mode 100644 t/conf.d/private/boot.toml
 delete mode 100644 t/conf.d/shared/boot.toml
 delete mode 100644 t/conf.d/shared_ipc/boot.toml
 delete mode 100644 t/conf.d/shared_none/boot.toml

diff --git a/etc/Makefile.am b/etc/Makefile.am
index 110c3125d07e..41e58780e82a 100644
--- a/etc/Makefile.am
+++ b/etc/Makefile.am
@@ -2,7 +2,8 @@
 systemdsystemunit_DATA = flux.service
 #endif
 
-dist_fluxcf_DATA = conf.d/boot.toml
+dist_fluxcf_DATA = \
+	conf.d/bootstrap.toml
 
 noinst_DATA = \
 	flux/curve
diff --git a/etc/conf.d/boot.toml b/etc/conf.d/boot.toml
deleted file mode 100644
index 07aa24a2c321..000000000000
--- a/etc/conf.d/boot.toml
+++ /dev/null
@@ -1,7 +0,0 @@
-[bootstrap]
-
-#mcast-endpoint = "epgm://eth0;229.0.0.1:8050"
-
-tbon-endpoints = [
-    "tcp://127.0.0.1:8020",	# rank 0
-]
diff --git a/etc/conf.d/bootstrap.toml b/etc/conf.d/bootstrap.toml
new file mode 100644
index 000000000000..03391c2d9561
--- /dev/null
+++ b/etc/conf.d/bootstrap.toml
@@ -0,0 +1,9 @@
+#
+# This bootstrap configuration is for a single-node
+# (i.e. workstation) Flux system configuration.
+#
+# See flux-config-bootstrap(5) for more information on this
+# configuration section.
+#
+
+[bootstrap]
diff --git a/src/broker/boot_config.c b/src/broker/boot_config.c
index 6a31a7363341..985739caabfe 100644
--- a/src/broker/boot_config.c
+++ b/src/broker/boot_config.c
@@ -14,226 +14,470 @@
 #include "config.h"
 #endif
 #include <unistd.h>
-#include <limits.h>
 #include <stdbool.h>
-#include <argz.h>
-#include <fnmatch.h>
+#include <assert.h>
 #include <jansson.h>
 #include <flux/core.h>
 
 #include "src/common/libutil/log.h"
-#include "src/common/libutil/ipaddr.h"
 #include "src/common/libutil/kary.h"
+#include "src/common/libutil/errno_safe.h"
+#include "src/common/libidset/idset.h"
 
 #include "attr.h"
 #include "overlay.h"
 #include "boot_config.h"
 
-/* Attributes that may not be set for this boot method.
+
+/* Copy 'fmt' into 'buf', substuting the following tokens:
+ * - %h  host
+ * - %p  port
+ * Returns 0 on success, or -1 on overflow.
  */
-static const char *broker_attr_blacklist[] = {
-    "tbon.endpoint",
-    NULL,
+int boot_config_format_uri (char *buf,
+                            int bufsz,
+                            const char *fmt,
+                            const char *host,
+                            int port)
+{
+    const char *cp;
+    int len = 0;
+    bool tok_flag = false;
+    int n;
+
+    cp = fmt;
+    while (*cp) {
+        if (tok_flag) {
+            switch (*cp) {
+            case 'h': /* %h - host */
+                if (host) {
+                    n = snprintf (&buf[len], bufsz - len, "%s", host);
+                    if (n >= bufsz - len)
+                        goto overflow;
+                    len += n;
+                }
+                else {
+                    buf[len++] = '%';
+                    buf[len++] = 'h';
+                }
+                break;
+            case 'p': /* %p - port */
+                n = snprintf (&buf[len], bufsz - len, "%d", port);
+                if (n >= bufsz - len)
+                    goto overflow;
+                len += n;
+                break;
+            case '%': /* %% - literal % */
+                buf[len++] = '%';
+                break;
+            default:
+                buf[len++] = '%';
+                buf[len++] = *cp;
+                break;
+            }
+            tok_flag = false;
+        }
+        else {
+            if (*cp == '%')
+                tok_flag = true;
+            else
+                buf[len++] = *cp;
+        }
+        if (len >= bufsz)
+            goto overflow;
+        cp++;
+    }
+    buf[len] = '\0';
+    return 0;
+overflow:
+    return -1;
+}
+
+struct map_arg {
+    json_t *hosts;
+    json_t *entry;
 };
 
-static int parse_endpoint_by_rank (const json_t *endpoints,
-                                   int rank,
-                                   const char **entry)
+/* Add a host entry to maparg->hosts for the formatted hostname in 's',
+ * cloned from the original host entry object in maparg->entry.
+ * N.B. idset_map_f footprint
+ */
+static int boot_config_map_hosts (const char *s, bool *stop, void *arg)
 {
-    const json_t *value;
-    const char *s;
+    struct map_arg *maparg = arg;
+    json_t *nentry;
+    json_t *o;
 
-    if (!(value = json_array_get (endpoints, rank))) {
-        log_msg ("Config file error [bootstrap]: rank out of range");
-        return -1;
-    }
-    if (!(s = json_string_value (value))) {
-        log_msg ("Config file error [bootstrap]: malformed tbon-endpoints");
-        log_msg ("Hint: all array entries must be strings");
-        return -1;
+    if (!(nentry = json_deep_copy (maparg->entry)))
+        goto nomem;
+    if (!(o = json_string (s)))
+        goto nomem;
+    if (json_object_set_new (nentry, "host", o) < 0) {
+        json_decref (o);
+        goto nomem;
     }
-    *entry = s;
+    if (json_array_append_new (maparg->hosts, nentry) < 0)
+        goto nomem;
     return 0;
+nomem:
+    json_decref (nentry);
+    errno = ENOMEM;
+    return -1;
 }
 
-/* Search array of configured endpoints, ordered by rank, for one that
- * matches a local address.  For testing support, greedily match any ipc://
- * or tcp://127.0.0.0 address.  On success, array index (rank) is returned.
- * On failure, -1 is returned with diagnostics on stderr.
+/* Build a new hosts array, expanding any entries "compressed" with
+ * embedded idsets, so that json_array_size() is the number of hosts,
+ * and json_array_get() can be used to fetch an entry by rank.
+ * Caller must free.
  */
-static int find_local_endpoint (const json_t *endpoints)
+static json_t *boot_config_expand_hosts (json_t *hosts)
 {
-    char *addrs = NULL;
-    size_t addrs_len = 0;
-    char error[200];
-    int rank;
-    const json_t *value;
-
-    if (ipaddr_getall (&addrs, &addrs_len, error, sizeof (error)) < 0) {
-        log_msg ("%s", error);
-        return -1;
+    json_t *nhosts;
+    size_t index;
+    json_t *value;
+
+    if (!(nhosts = json_array ())) {
+        log_msg ("Config file error [bootstrap]: out of memory");
+        return NULL;
     }
-    json_array_foreach (endpoints, rank, value) {
-        const char *s = json_string_value (value);
-        const char *entry = NULL;
-
-        if (!s) // array element is not a string
-            break;
-        if (fnmatch ("tcp://127.0.0.*:*", s, 0) == 0)
-            break;
-        if (fnmatch("ipc://*", s, 0) == 0)
-            break;
-        while ((entry = argz_next (addrs, addrs_len, entry))) {
-            char pat[128];
-            snprintf (pat, sizeof (pat), "tcp://%s:*", entry);
-            if (fnmatch (pat, s, 0) == 0)
-                break;
+    json_array_foreach (hosts, index, value) {
+        struct map_arg maparg = { .hosts = nhosts, .entry = value};
+        const char *host;
+
+        if (json_unpack (value, "{s:s}", "host", &host) < 0) {
+            log_msg ("Config file error [bootstrap]: missing host field");
+            log_msg ("Hint: hosts entries must be table containing a host key");
+            goto error;
+        }
+        if (idset_format_map (host, boot_config_map_hosts, &maparg) < 0) {
+            log_err ("Config file error [bootstrap]");
+            log_msg ("Hint: text enclosed by square brackets must be an idset");
+            goto error;
         }
-        if (entry != NULL) // found a match in 'addrs'
-            break;
     }
-    free (addrs);
-    if (rank == json_array_size (endpoints))
-        return -1;
-
-    return rank;
+    return nhosts;
+error:
+    json_decref (nhosts);
+    return NULL;
 }
 
-static int parse_config_file (flux_t *h,
-                              int *rankp,
-                              int *sizep,
-                              const json_t **endpointsp)
+/* Parse the [bootstrap] configuration.
+ * Capture the portion that is owned by the flux_t handle in 'conf'.
+ * Post-process conf->hosts to expand any embedded idsets and return
+ * a new hosts JSON array on success.  On failure, log a human readable
+ * message and return NULL.
+ */
+int boot_config_parse (flux_t *h, struct boot_conf *conf, json_t **hostsp)
 {
-    const flux_conf_t *conf;
-    int size = INT_MAX; // optional in config file
-    int rank = INT_MAX; // optional in config file
-    const json_t *endpoints;
     flux_conf_error_t error;
+    const char *default_bind = NULL;
+    const char *default_connect = NULL;
+    json_t *hosts = NULL;
 
-    /* N.B. the broker parses config early, and treats missing/malformed
-     * TOML as a fatal error.  The checks that we make here are specific
-     * to the [bootstrap] section.
-     */
-    conf = flux_get_conf (h, NULL);
-
-    if (flux_conf_unpack (conf,
+    memset (conf, 0, sizeof (*conf));
+    if (flux_conf_unpack (flux_get_conf (h, NULL),
                           &error,
-                          "{s:{s?:i s?:i s:o}}",
+                          "{s:{s?:i s?:s s?:s s?:o}}",
                           "bootstrap",
-                            "size", &size,
-                            "rank", &rank,
-                            "tbon-endpoints", &endpoints) < 0) {
+                            "default_port", &conf->default_port,
+                            "default_bind", &default_bind,
+                            "default_connect", &default_connect,
+                            "hosts", &conf->hosts) < 0) {
         log_msg ("Config file error [bootstrap]: %s", error.errbuf);
         return -1;
     }
-    if (json_array_size (endpoints) == 0) { // returns 0 if non-array
-        log_msg ("Config file error [bootstrap]: malformed tbon-endpoints");
-        log_msg ("Hint: must be an array with at least one element.");
-        return -1;
+
+    /* Take care of %p substitution in the default bind/connect URI's
+     * If %h occurs in these values, it is preserved (since host=NULL here).
+     */
+    if (default_bind) {
+        if (boot_config_format_uri (conf->default_bind,
+                                    sizeof (conf->default_bind),
+                                    default_bind,
+                                    NULL,
+                                    conf->default_port) < 0) {
+            log_msg ("Config file error [bootstrap] %s",
+                     "buffer overflow building default bind URI");
+            return -1;
+        }
     }
-    /* If size and/or rank were unspecified, infer them by looking at the
-     * size of the tbon-endpoints array, and/or by scanning the array for
-     * an address that matches a local interface.
+    if (default_connect) {
+        if (boot_config_format_uri (conf->default_connect,
+                                    sizeof (conf->default_connect),
+                                    default_connect,
+                                    NULL,
+                                    conf->default_port) < 0) {
+            log_msg ("Config file error [bootstrap] %s",
+                     "buffer overflow building default connect URI");
+            return -1;
+        }
+    }
+
+    /* Expand any embedded idsets in hosts entries.
      */
-    if (size == INT_MAX)
-        size = json_array_size (endpoints);
-    if (rank == INT_MAX) {
-        if ((rank = find_local_endpoint (endpoints)) < 0) {
-            log_msg ("Config file error [bootstrap]: could not determine rank");
-            log_msg ("Hint: set rank in config file or ensure tbon-endpoints");
-            log_msg ("  contains a local address");
+    if (conf->hosts) {
+        if (!json_is_array (conf->hosts)) {
+            log_msg ("Config file error [bootstrap] hosts must be array type");
             return -1;
         }
+        if (json_array_size (conf->hosts) > 0) {
+            if (!(hosts = boot_config_expand_hosts (conf->hosts)))
+                return -1;
+        }
+    }
+
+    *hostsp = hosts;
+    return 0;
+}
+
+/* Find host 'name' in hosts array, and set 'rank' to its array index.
+ * Return 0 on success, -1 on failure.
+ */
+int boot_config_getrankbyname (json_t *hosts, const char *name, uint32_t *rank)
+{
+    size_t index;
+    json_t *entry;
+
+    json_array_foreach (hosts, index, entry) {
+        const char *host;
+
+        /* N.B. missing host key already detected by boot_config_parse().
+         */
+        if (json_unpack (entry, "{s:s}", "host", &host) == 0
+                    && !strcmp (name, host)) {
+            *rank = index;
+            return 0;
+        }
+    }
+    log_msg ("Config file error [bootstrap]: %s not found in hosts", name);
+    return -1;
+}
+
+static int gethostentry (json_t *hosts,
+                         struct boot_conf *conf,
+                         uint32_t rank,
+                         const char **host,
+                         const char **bind,
+                         const char **uri)
+{
+    json_t *entry;
+
+    if (!(entry = json_array_get (hosts, rank))) {
+        log_msg ("Config file error [bootstrap] rank %u not found in hosts",
+                 (unsigned int)rank);
+        return -1;
     }
-    if (rank < 0 || rank > size - 1) {
-        log_msg ("Config file error [bootstrap]: invalid rank %d for size %d",
-                 rank,
-                 size);
+    /* N.B. missing host key already detected by boot_config_parse().
+     */
+    if (json_unpack (entry,
+                     "{s:s s?:s s?:s}",
+                     "host",
+                     host,
+                     "bind",
+                     bind,
+                     "connect",
+                     uri) < 0) {
+        log_msg ("Config file error [bootstrap]: rank %u bad hosts entry",
+                 (unsigned int)rank);
+        log_msg ("Hint: bind and connect keys, if present, are type string");
         return -1;
     }
-    *rankp = rank;
-    *sizep = size;
-    *endpointsp = endpoints;
     return 0;
 }
 
-/* Find attributes that, if set, should cause a fatal error.  Unlike PMI boot,
- * this method has no mechanism to share info with other ranks before
- * bootstrap/wireup completes.  Therefore, all bootstrap info has to come
- * from the shared config file.  Return 0 on success (no bad attrs),
- * or -1 on failure (one or more), with diagnostics on stderr.
+/* Look up the host entry for 'rank', then copy that entry's bind address
+ * into 'buf'.  If the entry doesn't provide an explicit bind address,
+ * use the default.  Perform any host or port substitutions while copying.
+ * Return 0 on success, -1 on failure.
  */
-static int find_blacklist_attrs (attr_t *attrs)
+int boot_config_getbindbyrank (json_t *hosts,
+                               struct boot_conf *conf,
+                               uint32_t rank,
+                               char *buf,
+                               int bufsz)
 {
-    int errors = 0;
-    const char **cp;
-
-    for (cp = &broker_attr_blacklist[0]; *cp != NULL; cp++) {
-        if (attr_get (attrs, *cp, NULL, NULL) == 0) {
-            log_msg ("attribute %s may not be set with boot_method=config",
-                     *cp);
-            errors++;
-        }
+    const char *uri = NULL;
+    const char *bind = NULL;
+    const char *host = NULL;
+
+    if (gethostentry (hosts, conf, rank, &host, &bind, &uri) < 0)
+        return -1;
+    if (!bind)
+        bind = conf->default_bind;
+    if (strlen (bind) == 0) {
+        log_msg ("Config file error [bootstrap]: rank %u missing bind URI",
+                 (unsigned int)rank);
+        return -1;
     }
-    if (errors > 0)
+    if (boot_config_format_uri (buf,
+                                bufsz,
+                                bind,
+                                host,
+                                conf->default_port) < 0) {
+        log_msg ("Config file error [bootstrap]: buffer overflow");
         return -1;
+    }
     return 0;
 }
 
-int boot_config (flux_t *h, overlay_t *overlay, attr_t *attrs, int tbon_k)
+/* Look up the host entry for 'rank', then copy that entry's connect address
+ * into 'buf'.  If the entry doesn't provide an explicit connect address,
+ * use the default.  Perform any host or port substitutions while copying.
+ * Return 0 on success, -1 on failure.
+ */
+int boot_config_geturibyrank (json_t *hosts,
+                              struct boot_conf *conf,
+                              uint32_t rank,
+                              char *buf,
+                              int bufsz)
 {
-    int size;
-    int rank;
-    const json_t *endpoints;
-    const char *this_endpoint;
+    const char *uri = NULL;
+    const char *bind = NULL;
+    const char *host = NULL;
 
-    if (find_blacklist_attrs (attrs) < 0)
+    if (gethostentry (hosts, conf, rank, &host, &bind, &uri) < 0)
+        return -1;
+    if (!uri)
+        uri = conf->default_connect;
+    if (strlen (uri) == 0) {
+        log_msg ("Config file error [bootstrap]: rank %u missing connect URI",
+                 (unsigned int)rank);
         return -1;
-    if (parse_config_file (h, &rank, &size, &endpoints) < 0)
+    }
+    if (boot_config_format_uri (buf,
+                                bufsz,
+                                uri,
+                                host,
+                                conf->default_port) < 0) {
+        log_msg ("Config file error [bootstrap]: buffer overflow");
         return -1;
+    }
+    return 0;
+}
+
+int boot_config (flux_t *h, overlay_t *overlay, attr_t *attrs, int tbon_k)
+{
+    struct boot_conf conf;
+    uint32_t rank;
+    uint32_t size;
+    json_t *hosts = NULL;
 
-    /* Initialize overlay network parameters.
+    /* Throw an error if 'tbon.endpoint' attribute is already set.
+     * flux-start sets this, and it's not compatible with the
+     * config boot method as it would be overwritten below.
      */
-    if (overlay_init (overlay, size, rank, tbon_k) < 0)
-        return -1;
-    if (parse_endpoint_by_rank (endpoints, rank, &this_endpoint) < 0)
+    if (attr_get (attrs, "tbon.endpoint", NULL, NULL) == 0) {
+        log_msg ("attr tbon.endpoint may not be set with boot_method=config");
         return -1;
-    if (overlay_set_child (overlay, this_endpoint) < 0) {
-        log_err ("overlay_set_child %s", this_endpoint);
+    }
+
+    /* Ingest the [bootstrap] stanza.
+     */
+    if (boot_config_parse (h, &conf, &hosts) < 0)
         return -1;
+
+    /* If hosts array was specified, match hostname to determine rank,
+     * and size is the length of the hosts array.  O/w rank=0, size=1.
+     */
+    if (hosts != NULL) {
+        const char *fakehost = getenv ("FLUX_FAKE_HOSTNAME"); // for testing;
+        char hostname[MAXHOSTNAMELEN + 1];
+
+        if (gethostname (hostname, sizeof (hostname)) < 0) {
+            log_err ("gethostbyname");
+            goto error;
+        }
+        if (boot_config_getrankbyname (hosts,
+                                      fakehost ? fakehost : hostname,
+                                      &rank) < 0)
+            goto error;
+        size = json_array_size (hosts);
+    }
+    else {
+        size = 1;
+        rank = 0;
     }
-    if (rank > 0) {
-        int parent_rank = kary_parentof (tbon_k, rank);
-        const char *parent_endpoint;
 
-        if (parse_endpoint_by_rank (endpoints,
-                                    parent_rank,
-                                    &parent_endpoint) < 0)
-            return -1;
-        if (overlay_set_parent (overlay, parent_endpoint) < 0) {
-            log_err ("overlay_set_parent %s", parent_endpoint);
-            return -1;
+    /* Tell overlay network this broker's rank, size, and branching factor.
+     */
+    if (overlay_init (overlay, size, rank, tbon_k) < 0)
+        goto error;
+
+    /* If broker has "downstream" peers, determine the URI to bind to
+     * from the config and tell overlay.  Also, set the tbon.endpoint
+     * attribute to the URI peers will connect to.  If broker has no
+     * downstream peers, set tbon.endpoint to NULL.
+     */
+    if (kary_childof (tbon_k, size, rank, 0) != KARY_NONE) {
+        char bind_uri[MAX_URI + 1];
+        char my_uri[MAX_URI + 1];
+
+        assert (hosts != NULL);
+
+        if (boot_config_getbindbyrank (hosts,
+                                       &conf,
+                                       rank,
+                                       bind_uri,
+                                       sizeof (bind_uri)) < 0)
+            goto error;
+        if (overlay_set_child (overlay, bind_uri) < 0) {
+            log_err ("overlay_set_child %s", bind_uri);
+            goto error;
+        }
+        if (boot_config_geturibyrank (hosts,
+                                      &conf,
+                                      rank,
+                                      my_uri,
+                                      sizeof (my_uri)) < 0)
+            goto error;
+        if (attr_add (attrs,
+                      "tbon.endpoint",
+                      my_uri,
+                      FLUX_ATTRFLAG_IMMUTABLE) < 0) {
+            log_err ("setattr tbon.endpoint %s", my_uri);
+            goto error;
+        }
+    }
+    else {
+        if (attr_add (attrs,
+                      "tbon.endpoint",
+                      NULL,
+                      FLUX_ATTRFLAG_IMMUTABLE) < 0) {
+            log_err ("setattr tbon.endpoint NULL");
+            goto error;
         }
     }
 
-    /* Update attributes.
+    /* If broker has an "upstream" peer, determine its URI and tell overlay.
      */
-    if (attr_add (attrs,
-                  "tbon.endpoint",
-                  this_endpoint,
-                  FLUX_ATTRFLAG_IMMUTABLE) < 0) {
-        log_err ("setattr tbon.endpoint %s", this_endpoint);
-        return -1;
+    if (rank > 0) {
+        char parent_uri[MAX_URI + 1];
+        if (boot_config_geturibyrank (hosts,
+                                      &conf,
+                                      kary_parentof (tbon_k, rank),
+                                      parent_uri,
+                                      sizeof (parent_uri)) < 0)
+            goto error;
+        if (overlay_set_parent (overlay, parent_uri) < 0) {
+            log_err ("overlay_set_parent %s", parent_uri);
+            goto error;
+        }
     }
+
+    /* instance-level (position in instance hierarchy) is always zero here.
+     */
     if (attr_add (attrs,
                   "instance-level",
                   "0",
                   FLUX_ATTRFLAG_IMMUTABLE) < 0) {
         log_err ("setattr instance-level 0");
-        return -1;
+        goto error;
     }
-
+    json_decref (hosts);
     return 0;
+error:
+    ERRNO_SAFE_WRAP (json_decref, hosts);
+    return -1;
 }
 
 /*
diff --git a/src/broker/boot_config.h b/src/broker/boot_config.h
index c3825f9629d1..9cfe45e33621 100644
--- a/src/broker/boot_config.h
+++ b/src/broker/boot_config.h
@@ -13,33 +13,6 @@
 
 /* boot_config - bootstrap broker/overlay from config file */
 
-/* Usage: flux broker -Sboot.method=config
- *
- * Example config file (TOML):
- *
- *   [bootstrap]
-
- *   # tbon-endpoints array is ordered by rank (zeromq URI format)
- *   tbon-endpoints = [
- *       "tcp://192.168.1.100:8020",  # rank 0
- *       "tcp://192.168.1.101:8020",  # rank 1
- *       "tcp://192.168.1.102:8020",  # rank 2
- *   ]
- *
- *   # if commented out, rank is determined by scanning tbon-endpoints
- *   # for a local address
- *   rank = 2
- *
- *   # if commented out, instance size is the size of tbon-endpoints.
- *   size = 3
- *
- * Caveat: the tbon-endpoints array entries specify a ZMQ endpoint used
- * in both "bind" and "connect" API calls.  ZMQ "bind" endpoints accept
- * an IP address or an interface name.  ZMQ "connect" endpoints accept
- * an IP address or hostname.  Therefore only IPs -- not hostnames, and not
- * interface names -- may be used here.
- */
-
 #include "attr.h"
 #include "overlay.h"
 
@@ -47,9 +20,39 @@
  *   tbon.endpoint (w)
  *   instance-level (w)
  */
-
 int boot_config (flux_t *h, overlay_t *overlay, attr_t *attrs, int tbon_k);
 
+/* The following is exported for unit testing.
+ */
+#define MAX_URI 2048
+
+struct boot_conf {
+    int default_port;
+    char default_bind[MAX_URI + 1];
+    char default_connect[MAX_URI + 1];
+    json_t *hosts;
+};
+
+int boot_config_geturibyrank (json_t *hosts,
+                              struct boot_conf *conf,
+                              uint32_t rank,
+                              char *buf,
+                              int bufsz);
+int boot_config_getbindbyrank (json_t *hosts,
+                               struct boot_conf *conf,
+                               uint32_t rank,
+                               char *buf,
+                               int bufsz);
+int boot_config_getrankbyname (json_t *hosts,
+                               const char *name,
+                               uint32_t *rank);
+int boot_config_parse (flux_t *h, struct boot_conf *conf, json_t **hosts);
+int boot_config_format_uri (char *buf,
+                            int bufsz,
+                            const char *fmt,
+                            const char *host,
+                            int port);
+
 #endif /* BROKER_BOOT_CONFIG_H */
 
 /*
diff --git a/t/Makefile.am b/t/Makefile.am
index 90a1af35a4ad..38130e31f7c6 100644
--- a/t/Makefile.am
+++ b/t/Makefile.am
@@ -262,19 +262,15 @@ dist_check_DATA = \
 	hwloc-data/1N/nonoverlapping/02-brokers/0.xml \
 	hwloc-data/1N/nonoverlapping/02-brokers/1.xml \
 	valgrind/valgrind.supp \
-	conf.d/private/boot.toml \
-	conf.d/shared/boot.toml \
-	conf.d/shared_ipc/boot.toml \
-	conf.d/shared_none/boot.toml \
-	conf.d/bad-toml/boot.toml \
-	conf.d/bad-noendpoints/boot.toml \
-	conf.d/bad-intendpoints/boot.toml \
-	conf.d/bad-intendpoints2/boot.toml \
+	conf.d/bad-hosts/boot.toml \
+	conf.d/bad-hosts2/boot.toml \
 	conf.d/bad-nobootstrap/boot.toml \
-	conf.d/bad-rank/boot.toml \
-	conf.d/bad-rank2/boot.toml \
-	conf.d/priv2-0/boot.toml \
-	conf.d/priv2-1/boot.toml
+	conf.d/bad-nomatch/boot.toml \
+	conf.d/bad-toml/boot.toml \
+	conf.d/good-nohosts/boot.toml \
+	conf.d/good-emptyhosts/boot.toml \
+	conf.d/good-ipc2/boot.toml \
+	conf.d/good-tcp4/boot.toml
 
 test_ldadd = \
         $(top_builddir)/src/common/libflux-internal.la \
diff --git a/t/conf.d/bad-intendpoints/boot.toml b/t/conf.d/bad-hosts/boot.toml
similarity index 53%
rename from t/conf.d/bad-intendpoints/boot.toml
rename to t/conf.d/bad-hosts/boot.toml
index 4e7c89f3ee09..c5ca12ff5d6d 100644
--- a/t/conf.d/bad-intendpoints/boot.toml
+++ b/t/conf.d/bad-hosts/boot.toml
@@ -1,5 +1,5 @@
 [bootstrap]
 
-tbon-endpoints = [
+hosts = [
     42
 ]
diff --git a/t/conf.d/bad-hosts2/boot.toml b/t/conf.d/bad-hosts2/boot.toml
new file mode 100644
index 000000000000..fcb381ff495a
--- /dev/null
+++ b/t/conf.d/bad-hosts2/boot.toml
@@ -0,0 +1,3 @@
+[bootstrap]
+
+hosts = 42
diff --git a/t/conf.d/bad-intendpoints2/boot.toml b/t/conf.d/bad-intendpoints2/boot.toml
deleted file mode 100644
index 66a52a8a408d..000000000000
--- a/t/conf.d/bad-intendpoints2/boot.toml
+++ /dev/null
@@ -1,3 +0,0 @@
-[bootstrap]
-
-tbon-endpoints = 42
diff --git a/t/conf.d/bad-nobootstrap/boot.toml b/t/conf.d/bad-nobootstrap/boot.toml
index 4191de54b206..37fab957c7b2 100644
--- a/t/conf.d/bad-nobootstrap/boot.toml
+++ b/t/conf.d/bad-nobootstrap/boot.toml
@@ -1,4 +1,4 @@
 
-tbon-endpoints = [
-    "tcp://127.0.0.1:8500",
+hosts = [
+    { host = "foo" },
 ]
diff --git a/t/conf.d/bad-noendpoints/boot.toml b/t/conf.d/bad-noendpoints/boot.toml
deleted file mode 100644
index 8cbdf1b4ac84..000000000000
--- a/t/conf.d/bad-noendpoints/boot.toml
+++ /dev/null
@@ -1,3 +0,0 @@
-[bootstrap]
-
-# missing tbon-endpoints array
diff --git a/t/conf.d/bad-nomatch/boot.toml b/t/conf.d/bad-nomatch/boot.toml
new file mode 100644
index 000000000000..0c6cf61f3ced
--- /dev/null
+++ b/t/conf.d/bad-nomatch/boot.toml
@@ -0,0 +1,8 @@
+[bootstrap]
+
+default_bind = "ipc://@flux-testipc-1-0"
+default_connect = "ipc://@flux-testipc-1-0"
+
+hosts = [
+    { host = "matchnobody" },
+]
diff --git a/t/conf.d/bad-rank/boot.toml b/t/conf.d/bad-rank/boot.toml
deleted file mode 100644
index 93ba170b59c6..000000000000
--- a/t/conf.d/bad-rank/boot.toml
+++ /dev/null
@@ -1,10 +0,0 @@
-[bootstrap]
-
-size = 1
-
-# rank is out of range
-rank = -1
-
-tbon-endpoints = [
-    "ipc://@test",
-]
diff --git a/t/conf.d/bad-rank2/boot.toml b/t/conf.d/bad-rank2/boot.toml
deleted file mode 100644
index f06982574f11..000000000000
--- a/t/conf.d/bad-rank2/boot.toml
+++ /dev/null
@@ -1,10 +0,0 @@
-[bootstrap]
-
-size = 1
-
-# rank is out of range
-rank = 42
-
-tbon-endpoints = [
-    "ipc://@test",
-]
diff --git a/t/conf.d/good-emptyhosts/boot.toml b/t/conf.d/good-emptyhosts/boot.toml
new file mode 100644
index 000000000000..ae6907bd2c25
--- /dev/null
+++ b/t/conf.d/good-emptyhosts/boot.toml
@@ -0,0 +1,4 @@
+[bootstrap]
+
+hosts = [
+]
diff --git a/t/conf.d/good-ipc2/boot.toml b/t/conf.d/good-ipc2/boot.toml
new file mode 100644
index 000000000000..57d824cc6082
--- /dev/null
+++ b/t/conf.d/good-ipc2/boot.toml
@@ -0,0 +1,9 @@
+[bootstrap]
+
+hosts = [
+    { host="fake0",
+      bind="ipc:///tmp/test-ipc2-0",
+      connect="ipc:///tmp/test-ipc2-0"
+    },
+    { host="fake1" }
+]
diff --git a/t/conf.d/good-nohosts/bad-nohosts/boot.toml b/t/conf.d/good-nohosts/bad-nohosts/boot.toml
new file mode 100644
index 000000000000..aad8ef162225
--- /dev/null
+++ b/t/conf.d/good-nohosts/bad-nohosts/boot.toml
@@ -0,0 +1,3 @@
+[bootstrap]
+
+# missing hosts array
diff --git a/t/conf.d/good-nohosts/boot.toml b/t/conf.d/good-nohosts/boot.toml
new file mode 100644
index 000000000000..0ae814e542c2
--- /dev/null
+++ b/t/conf.d/good-nohosts/boot.toml
@@ -0,0 +1,2 @@
+[bootstrap]
+
diff --git a/t/conf.d/good-tcp4/boot.toml b/t/conf.d/good-tcp4/boot.toml
new file mode 100644
index 000000000000..2e5577504179
--- /dev/null
+++ b/t/conf.d/good-tcp4/boot.toml
@@ -0,0 +1,13 @@
+[bootstrap]
+
+hosts = [
+    { host="fake0",
+      bind="tcp://127.0.0.1:5080",
+      connect="tcp://127.0.0.1:5080"
+    },
+    { host="fake1",
+      bind="tcp://127.0.0.1:5081",
+      connect="tcp://127.0.0.1:5081"
+    },
+    { host="fake[2-3]" }
+]
diff --git a/t/conf.d/priv2-0/boot.toml b/t/conf.d/priv2-0/boot.toml
deleted file mode 100644
index 724d2c2b0b18..000000000000
--- a/t/conf.d/priv2-0/boot.toml
+++ /dev/null
@@ -1,11 +0,0 @@
-[bootstrap]
-
-size = 2
-rank = 0
-
-# The @ prefix says use the "abstract namespace"
-# See unix(7), zmq_ipc(7)
-tbon-endpoints = [
-    "ipc://@flux-test-2-0",
-    "ipc://@flux-test-2-1",
-]
diff --git a/t/conf.d/priv2-1/boot.toml b/t/conf.d/priv2-1/boot.toml
deleted file mode 100644
index ee5ca087434e..000000000000
--- a/t/conf.d/priv2-1/boot.toml
+++ /dev/null
@@ -1,11 +0,0 @@
-[bootstrap]
-
-size = 2
-rank = 1
-
-# The @ prefix says use the "abstract namespace"
-# See unix(7), zmq_ipc(7)
-tbon-endpoints = [
-    "ipc://@flux-test-2-0",
-    "ipc://@flux-test-2-1",
-]
diff --git a/t/conf.d/private/boot.toml b/t/conf.d/private/boot.toml
deleted file mode 100644
index de90cfd01393..000000000000
--- a/t/conf.d/private/boot.toml
+++ /dev/null
@@ -1,8 +0,0 @@
-[bootstrap]
-
-size = 1
-rank = 0
-
-tbon-endpoints = [
-    "ipc://@flux-test-1-0",
-]
diff --git a/t/conf.d/shared/boot.toml b/t/conf.d/shared/boot.toml
deleted file mode 100644
index 685ae7efdb2a..000000000000
--- a/t/conf.d/shared/boot.toml
+++ /dev/null
@@ -1,5 +0,0 @@
-[bootstrap]
-
-tbon-endpoints = [
-    "tcp://127.0.0.1:8500",
-]
diff --git a/t/conf.d/shared_ipc/boot.toml b/t/conf.d/shared_ipc/boot.toml
deleted file mode 100644
index 52656020b7b9..000000000000
--- a/t/conf.d/shared_ipc/boot.toml
+++ /dev/null
@@ -1,5 +0,0 @@
-[bootstrap]
-
-tbon-endpoints = [
-    "ipc://@flux-testipc-1-0",
-]
diff --git a/t/conf.d/shared_none/boot.toml b/t/conf.d/shared_none/boot.toml
deleted file mode 100644
index 50c27517c57a..000000000000
--- a/t/conf.d/shared_none/boot.toml
+++ /dev/null
@@ -1,5 +0,0 @@
-[bootstrap]
-
-tbon-endpoints = [
-    "tcp://1.2.3.4:5",
-]
diff --git a/t/t0013-config-file.t b/t/t0013-config-file.t
index dd37ea5dedc1..eb5de09ff29f 100755
--- a/t/t0013-config-file.t
+++ b/t/t0013-config-file.t
@@ -36,84 +36,70 @@ test_expect_success 'broker startup with invalid TOML fails' "
 		flux broker ${ARGS} /bin/true
 "
 
-test_expect_success 'bootstrap config with missing bootstrap table fails' "
+test_expect_success '[bootstrap] config with missing bootstrap table fails' "
 	! FLUX_CONF_DIR=${TCONFDIR}/bad-nobootstrap \
 		flux broker ${ARGS} -Sboot.method=config /bin/true
 "
 
-test_expect_success 'bootstrap config with missing endpoints array fails' "
-	! FLUX_CONF_DIR=${TCONFDIR}/bad-noendpoints \
+test_expect_success '[bootstrap] config with bad hosts array' "
+	! FLUX_CONF_DIR=${TCONFDIR}/bad-hosts2 \
 		flux broker ${ARGS} -Sboot.method=config /bin/true
 "
 
-test_expect_success 'bootstrap config with bad endpoints array' "
-	! FLUX_CONF_DIR=${TCONFDIR}/bad-intendpoints2 \
+test_expect_success '[bootstrap] config with bad hosts array element' "
+	! FLUX_CONF_DIR=${TCONFDIR}/bad-hosts \
 		flux broker ${ARGS} -Sboot.method=config /bin/true
 "
 
-test_expect_success 'bootstrap config with bad endpoints array element' "
-	! FLUX_CONF_DIR=${TCONFDIR}/bad-intendpoints \
-		flux broker ${ARGS} -Sboot.method=config /bin/true
-"
-
-test_expect_success 'bootstrap config with negative rank fails' "
-	! FLUX_CONF_DIR=${TCONFDIR}/bad-rank \
-		flux broker ${ARGS} -Sboot.method=config /bin/true
-"
-
-test_expect_success 'bootstrap config with >= size rank fails' "
-	! FLUX_CONF_DIR=${TCONFDIR}/bad-rank2 \
+test_expect_success '[bootstrap] config with hostname not found' "
+	! FLUX_CONF_DIR=${TCONFDIR}/bad-nomatch \
 		flux broker ${ARGS} -Sboot.method=config /bin/true
 "
 
 # N.B. set short shutdown grace to speed up test, as in t0001-basic
 
-#
-# size=1 boot from config file
-# N.B. "private" config sets rank and optionally size, while "shared"
-# config requires broker to infer rank from position of a local interface's
-# IP address in the tbon-endpoints array (which only works for size=1 in
-# a single-node test).
-#
-
-test_expect_success 'start size=1 with shared config file, expected attrs set' "
-	FLUX_CONF_DIR=${TCONFDIR}/shared \
+test_expect_success 'start instance with missing hosts' "
+	FLUX_CONF_DIR=${TCONFDIR}/good-nohosts \
 		flux broker ${ARGS} -Sboot.method=config \
 		--shutdown-grace=0.1 \
-		flux lsattr -v >1s.out &&
-	grep -q 'tbon.endpoint[ ]*tcp://127.0.0.1:8500$' 1s.out
+		flux lsattr -v >attr.out &&
+	grep 'tbon.endpoint.*-$' attr.out
 "
 
-test_expect_success 'start size=1 with shared config file, ipc endpoint' "
-	FLUX_CONF_DIR=${TCONFDIR}/shared_ipc \
+test_expect_success 'start instance with empty hosts' "
+	FLUX_CONF_DIR=${TCONFDIR}/good-emptyhosts \
 		flux broker ${ARGS} -Sboot.method=config \
 		--shutdown-grace=0.1 \
-		/bin/true
+		flux lsattr -v >attr.out &&
+	grep 'tbon.endpoint.*-$' attr.out
 "
 
-test_expect_success 'start size=1 with shared config file, no endpoint' "
-	! FLUX_CONF_DIR=${TCONFDIR}/shared_none \
-		flux broker ${ARGS} -Sboot.method=config /bin/true
+# Usage: start_broker config hostname cmd ...
+start_broker() {
+	local dir=$1; shift
+	local host=$1; shift
+	FLUX_CONF_DIR=$dir FLUX_FAKE_HOSTNAME=$host \
+		flux broker ${ARGS} -Sboot.method=config "$@" &
+}
+
+test_expect_success 'start size=2 instance with ipc://' "
+	start_broker ${TCONFDIR}/good-ipc2 fake0 flux getattr size >ipc.out &&
+	start_broker ${TCONFDIR}/good-ipc2 fake1 &&
+	wait &&
+	wait &&
+	echo 2 >ipc.exp &&
+	test_cmp ipc.exp ipc.out
 "
 
-test_expect_success 'start size=1 with private config file' "
-	FLUX_CONF_DIR=${TCONFDIR}/private \
-		flux broker ${ARGS} -Sboot.method=config /bin/true
+test_expect_success 'start size=4 instance with tcp://' "
+	start_broker ${TCONFDIR}/good-tcp4 fake0 flux getattr size >tcp.out &&
+	start_broker ${TCONFDIR}/good-tcp4 fake1 &&
+	start_broker ${TCONFDIR}/good-tcp4 fake2 &&
+	start_broker ${TCONFDIR}/good-tcp4 fake3 &&
+	wait && wait && wait && wait &&
+	echo 4 >tcp.exp &&
+	test_cmp tcp.exp tcp.out
 "
 
-#
-# size=2 boot from config file
-#
-
-test_expect_success NO_CHAIN_LINT 'start size=2 with private config files' '
-	FLUX_CONF_DIR=${TCONFDIR}/priv2-1 \
-		flux broker ${ARGS} -Sboot.method=config &
-	FLUX_CONF_DIR=${TCONFDIR}/priv2-0 \
-		flux broker ${ARGS} -Sboot.method=config \
-		--shutdown-grace=0.1 \
-		flux getattr size >2p.out &&
-	echo 2 >2p.exp &&
-	test_cmp 2p.exp 2p.out
-'
 
 test_done

From e9447debb2a4331f0afffb5e60f8c0e05ada8639 Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Mon, 23 Dec 2019 12:35:06 -0800
Subject: [PATCH 08/16] broker/boot_config: add unit test

---
 src/broker/Makefile.am        |   7 +-
 src/broker/test/boot_config.c | 480 ++++++++++++++++++++++++++++++++++
 2 files changed, 486 insertions(+), 1 deletion(-)
 create mode 100644 src/broker/test/boot_config.c

diff --git a/src/broker/Makefile.am b/src/broker/Makefile.am
index 37f0be4880e7..60a237323a79 100644
--- a/src/broker/Makefile.am
+++ b/src/broker/Makefile.am
@@ -76,7 +76,8 @@ TESTS = test_shutdown.t \
 	test_service.t \
 	test_reduce.t \
 	test_liblist.t \
-	test_pmiutil.t
+	test_pmiutil.t \
+	test_boot_config.t
 
 test_ldadd = \
 	$(builddir)/libbroker.la \
@@ -127,3 +128,7 @@ test_liblist_t_LDADD = $(test_ldadd)
 test_pmiutil_t_SOURCES = test/pmiutil.c
 test_pmiutil_t_CPPFLAGS = $(test_cppflags)
 test_pmiutil_t_LDADD = $(test_ldadd)
+
+test_boot_config_t_SOURCES = test/boot_config.c
+test_boot_config_t_CPPFLAGS = $(test_cppflags)
+test_boot_config_t_LDADD = $(test_ldadd)
diff --git a/src/broker/test/boot_config.c b/src/broker/test/boot_config.c
new file mode 100644
index 000000000000..a14da6f6237e
--- /dev/null
+++ b/src/broker/test/boot_config.c
@@ -0,0 +1,480 @@
+/************************************************************\
+ * Copyright 2019 Lawrence Livermore National Security, LLC
+ * (c.f. AUTHORS, NOTICE.LLNS, COPYING)
+ *
+ * This file is part of the Flux resource manager framework.
+ * For details, see https://github.com/flux-framework.
+ *
+ * SPDX-License-Identifier: LGPL-3.0
+\************************************************************/
+
+#if HAVE_CONFIG_H
+#include "config.h"
+#endif
+#include <sys/param.h>
+#include <unistd.h>
+#include <jansson.h>
+#include <flux/core.h>
+
+#include "src/common/libtap/tap.h"
+#include "src/broker/boot_config.h"
+
+
+static void
+create_test_file (const char *dir, char *prefix, char *path, size_t pathlen,
+                  const char *contents)
+{
+    int fd;
+    if (snprintf (path,
+                  pathlen,
+                  "%s/%s.XXXXXX.toml",
+                  dir ? dir : "/tmp",
+                  prefix) >= pathlen)
+        BAIL_OUT ("snprintf overflow");
+    fd = mkstemps (path, 5);
+    if (fd < 0)
+        BAIL_OUT ("mkstemp %s: %s", path, strerror (errno));
+    if (write (fd, contents, strlen (contents)) != strlen (contents))
+        BAIL_OUT ("write %s: %s", path, strerror (errno));
+    if (close (fd) < 0)
+        BAIL_OUT ("close %s: %s", path, strerror (errno));
+}
+
+static void
+create_test_dir (char *dir, int dirlen)
+{
+    const char *tmpdir = getenv ("TMPDIR");
+    if (snprintf (dir,
+                  dirlen,
+                  "%s/cf.XXXXXXX",
+                  tmpdir ? tmpdir : "/tmp") >= dirlen)
+        BAIL_OUT ("snprintf overflow");
+    if (!mkdtemp (dir))
+        BAIL_OUT ("mkdtemp %s: %s", dir, strerror (errno));
+    setenv ("FLUX_CONF_DIR", dir, 1);
+}
+
+void test_parse (const char *dir)
+{
+    char path[PATH_MAX + 1];
+    flux_t *h;
+    json_t *hosts = NULL;
+    struct boot_conf conf;
+    uint32_t rank;
+    char uri[MAX_URI + 1];
+    int rc;
+    const char *input = \
+"[bootstrap]\n" \
+"default_port = 42\n" \
+"default_bind = \"tcp://en0:%p\"\n" \
+"default_connect = \"tcp://x%h:%p\"\n" \
+"hosts = [\n" \
+"  { host = \"foo0\" },\n" \
+"  { host = \"foo[1-62]\" },\n" \
+"  { host = \"foo63\" },\n" \
+"]\n";
+
+    if (!(h = flux_open ("loop://", 0)))
+        BAIL_OUT ("can't continue without loop handle");
+
+    create_test_file (dir, "boot", path, sizeof (path), input);
+    rc = boot_config_parse (h, &conf, &hosts);
+    ok (rc == 0,
+        "boot_conf_parse worked");
+    ok (hosts != NULL && json_array_size (hosts) == 64,
+        "got 64 hosts");
+    if (hosts == NULL)
+        BAIL_OUT ("cannot continue without hosts array");
+
+    ok (conf.default_port == 42,
+        "set default_port correctly");
+    ok (!strcmp (conf.default_bind, "tcp://en0:42"),
+        "and set default_bind correctly (with %%p substitution)");
+    ok (!strcmp (conf.default_connect, "tcp://x%h:42"),
+        "and set default_connect correctly (with %%p substitution)");
+
+    ok (boot_config_getrankbyname (hosts, "foo0", &rank) == 0
+        && rank == 0,
+       "boot_config_getrankbyname found rank 0");
+    ok (boot_config_getrankbyname (hosts, "foo1", &rank) == 0
+        && rank == 1,
+       "boot_config_getrankbyname found rank 1");
+    ok (boot_config_getrankbyname (hosts, "foo42", &rank) == 0
+        && rank == 42,
+       "boot_config_getrankbyname found rank 42");
+    ok (boot_config_getrankbyname (hosts, "notfound", &rank) < 0,
+       "boot_config_getrankbyname fails on unknown entry");
+
+    ok (boot_config_getbindbyrank (hosts, &conf, 0, uri, sizeof (uri)) == 0
+        && !strcmp (uri, "tcp://en0:42"),
+        "boot_config_getbindbyrank 0 works with expected value");
+    ok (boot_config_getbindbyrank (hosts, &conf, 1, uri, sizeof (uri)) == 0
+        && !strcmp (uri, "tcp://en0:42"),
+        "boot_config_getbindbyrank 1 works with expected value");
+    ok (boot_config_getbindbyrank (hosts, &conf, 63, uri, sizeof (uri)) == 0
+        && !strcmp (uri, "tcp://en0:42"),
+        "boot_config_getbindbyrank 63 works with expected value");
+    ok (boot_config_getbindbyrank (hosts, &conf, 64, uri, sizeof (uri))  < 0,
+        "boot_config_getbindbyrank 64 fails");
+
+    ok (boot_config_geturibyrank (hosts, &conf, 0, uri, sizeof (uri)) == 0
+        && !strcmp (uri, "tcp://xfoo0:42"),
+        "boot_config_geturibyrank 0 works with expected value");
+    ok (boot_config_geturibyrank (hosts, &conf, 1, uri, sizeof (uri)) == 0
+        && !strcmp (uri, "tcp://xfoo1:42"),
+        "boot_config_geturibyrank 1 works with expected value");
+    ok (boot_config_geturibyrank (hosts, &conf, 63, uri, sizeof (uri)) == 0
+        && !strcmp (uri, "tcp://xfoo63:42"),
+        "boot_config_geturibyrank 63 works with expected value");
+    ok (boot_config_geturibyrank (hosts, &conf, 64, uri, sizeof (uri))  < 0,
+        "boot_config_geturibyrank 64 fails");
+
+    json_decref (hosts);
+
+    if (unlink (path) < 0)
+        BAIL_OUT ("could not cleanup test file %s", path);
+
+    flux_close (h);
+}
+
+void test_overflow_bind (const char *dir)
+{
+    char path[PATH_MAX + 1];
+    flux_t *h;
+    struct boot_conf conf;
+    char t[MAX_URI*2];
+    json_t *hosts;
+
+    if (!(h = flux_open ("loop://", 0)))
+        BAIL_OUT ("can't continue without loop handle");
+
+    if (snprintf (t,
+                  sizeof (t),
+                  "[bootstrap]\ndefault_bind=\"%*s\"\nhosts=[\"foo\"]\n",
+                  MAX_URI+2, "foo") >= sizeof (t))
+        BAIL_OUT ("snprintf overflow");
+    create_test_file (dir, "boot", path, sizeof (path), t);
+
+    ok (boot_config_parse (h, &conf, &hosts) == -1,
+        "boot_conf_parse caught default_bind overflow");
+
+    if (unlink (path) < 0)
+        BAIL_OUT ("could not cleanup test file %s", path);
+
+    flux_close (h);
+}
+
+void test_overflow_connect (const char *dir)
+{
+    char path[PATH_MAX + 1];
+    flux_t *h;
+    struct boot_conf conf;
+    char t[MAX_URI*2];
+    json_t *hosts;
+
+    if (!(h = flux_open ("loop://", 0)))
+        BAIL_OUT ("can't continue without loop handle");
+
+    if (snprintf (t,
+                  sizeof (t),
+                  "[bootstrap]\ndefault_connect=\"%*s\"\nhosts=[\"foo\"]\n",
+                  MAX_URI+2, "foo") >= sizeof (t))
+        BAIL_OUT ("snprintf overflow");
+    create_test_file (dir, "boot", path, sizeof (path), t);
+
+    ok (boot_config_parse (h, &conf, &hosts) == -1,
+        "boot_conf_parse caught default_connect overflow");
+
+    if (unlink (path) < 0)
+        BAIL_OUT ("could not cleanup test file %s", path);
+
+    flux_close (h);
+}
+
+void test_bad_hosts_entry (const char *dir)
+{
+    char path[PATH_MAX + 1];
+    flux_t *h;
+    struct boot_conf conf;
+    json_t *hosts;
+    const char *input = \
+"[bootstrap]\n" \
+"hosts = [\n" \
+"  42,\n" \
+"]\n";
+
+    if (!(h = flux_open ("loop://", 0)))
+        BAIL_OUT ("can't continue without loop handle");
+
+    create_test_file (dir, "boot", path, sizeof (path), input);
+
+    ok (boot_config_parse (h, &conf, &hosts) == -1,
+        "boot_config_parse failed bad hosts entry");
+
+    if (unlink (path) < 0)
+        BAIL_OUT ("could not cleanup test file %s", path);
+
+    flux_close (h);
+}
+
+void test_missing_info (const char *dir)
+{
+    char path[PATH_MAX + 1];
+    flux_t *h;
+    json_t *hosts;
+    struct boot_conf conf;
+    char uri[MAX_URI + 1];
+    uint32_t rank;
+    const char *input = \
+"[bootstrap]\n" \
+"hosts = [\n" \
+"  { host = \"foo\" },\n" \
+"]\n";
+
+    if (!(h = flux_open ("loop://", 0)))
+        BAIL_OUT ("can't continue without loop handle");
+
+    create_test_file (dir, "boot", path, sizeof (path), input);
+
+    if (boot_config_parse (h, &conf, &hosts) < 0)
+        BAIL_OUT ("boot_config_parse unexpectedly failed");
+    if (!hosts)
+        BAIL_OUT ("cannot continue without hosts array");
+    ok (boot_config_getrankbyname (hosts, "foo", &rank) == 0
+        && rank == 0,
+        "boot_config_getrankbyname found entry");
+    ok (boot_config_getbindbyrank (hosts, &conf, 0, uri, sizeof(uri)) < 0,
+        "boot_config_getbindbyrank fails due to missing bind uri");
+    ok (boot_config_geturibyrank (hosts, &conf, 0, uri, sizeof(uri)) < 0,
+        "boot_config_geturibyrank fails due to missing connect uri");
+
+    json_decref (hosts);
+
+    if (unlink (path) < 0)
+        BAIL_OUT ("could not cleanup test file %s", path);
+
+    flux_close (h);
+}
+
+void test_bad_host_idset (const char *dir)
+{
+    char path[PATH_MAX + 1];
+    flux_t *h;
+    struct boot_conf conf;
+    json_t *hosts;
+    const char *input = \
+"[bootstrap]\n" \
+"hosts = [\n" \
+"  { host=\"foo[1-]\" },\n" \
+"]\n";
+
+    if (!(h = flux_open ("loop://", 0)))
+        BAIL_OUT ("can't continue without loop handle");
+
+    create_test_file (dir, "boot", path, sizeof (path), input);
+
+    ok (boot_config_parse (h, &conf, &hosts) == -1,
+        "boot_config_parse failed on host entry containing bad idset");
+
+    if (unlink (path) < 0)
+        BAIL_OUT ("could not cleanup test file %s", path);
+
+    flux_close (h);
+}
+
+void test_bad_host_bind (const char *dir)
+{
+    char path[PATH_MAX + 1];
+    flux_t *h;
+    struct boot_conf conf;
+    json_t *hosts;
+    char uri[MAX_URI + 1];
+    const char *input = \
+"[bootstrap]\n" \
+"hosts = [\n" \
+"  { host=\"foo\", bind=42 },\n" \
+"]\n";
+
+    if (!(h = flux_open ("loop://", 0)))
+        BAIL_OUT ("can't continue without loop handle");
+
+    create_test_file (dir, "boot", path, sizeof (path), input);
+
+    /* hosts will initially parse OK then fail in getbindbyrank */
+    if (boot_config_parse (h, &conf, &hosts) < 0)
+        BAIL_OUT ("boot_config_parse unexpectedly failed");
+    ok (boot_config_getbindbyrank (hosts, &conf, 0, uri, sizeof (uri)) < 0,
+        "boot_config_getbindbyrank failed on hoste entry wtih wrong bind type");
+
+    json_decref (hosts);
+
+    if (unlink (path) < 0)
+        BAIL_OUT ("could not cleanup test file %s", path);
+
+    flux_close (h);
+}
+
+
+/* Just double check that an array with mismatched types
+ * fails early with the expected libtomlc99 error.
+ */
+void test_toml_mixed_array (const char *dir)
+{
+    char path[PATH_MAX + 1];
+    flux_t *h;
+    const flux_conf_t *conf;
+    flux_conf_error_t error;
+    const char *input = \
+"[bootstrap]\n" \
+"hosts = [\n" \
+"  \"bar\",\n" \
+"  { host = \"foo\" },\n" \
+"]\n";
+
+    if (!(h = flux_open ("loop://", 0)))
+        BAIL_OUT ("can't continue without loop handle");
+
+    create_test_file (dir, "boot", path, sizeof (path), input);
+
+    conf = flux_get_conf (h, &error);
+    ok (conf == NULL && (strstr (error.errbuf, "array type mismatch")
+        || strstr (error.errbuf, "string array can only contain strings")),
+        "Mixed type hosts array fails with reasonable error");
+    diag ("%s: line %d: %s", error.filename, error.lineno, error.errbuf);
+
+    if (unlink (path) < 0)
+        BAIL_OUT ("could not cleanup test file %s", path);
+
+    flux_close (h);
+}
+
+void test_no_hosts (const char *dir)
+{
+    char path[PATH_MAX + 1];
+    flux_t *h;
+    json_t *hosts;
+    struct boot_conf conf;
+    const char *input = \
+"[bootstrap]\n";
+
+    if (!(h = flux_open ("loop://", 0)))
+        BAIL_OUT ("can't continue without loop handle");
+
+    create_test_file (dir, "boot", path, sizeof (path), input);
+
+    hosts = (json_t *)(uintptr_t)1;
+    ok (boot_config_parse (h, &conf, &hosts) == 0 && hosts == NULL,
+        "boot_config_parse works with missing hosts array");
+
+    if (unlink (path) < 0)
+        BAIL_OUT ("could not cleanup test file %s", path);
+
+    flux_close (h);
+}
+
+void test_empty_hosts (const char *dir)
+{
+    char path[PATH_MAX + 1];
+    flux_t *h;
+    json_t *hosts;
+    struct boot_conf conf;
+    const char *input = \
+"[bootstrap]\n" \
+"hosts = [\n" \
+"]\n";
+;
+
+    if (!(h = flux_open ("loop://", 0)))
+        BAIL_OUT ("can't continue without loop handle");
+
+    create_test_file (dir, "boot", path, sizeof (path), input);
+
+    hosts = (json_t *)(uintptr_t)1;
+    ok (boot_config_parse (h, &conf, &hosts) == 0 && hosts == NULL,
+        "boot_config_parse works with empty hosts array");
+
+    if (unlink (path) < 0)
+        BAIL_OUT ("could not cleanup test file %s", path);
+
+    flux_close (h);
+}
+
+void test_format (void)
+{
+    char buf[MAX_URI + 1];
+
+    ok (boot_config_format_uri (buf, sizeof (buf), "abcd", NULL, 0) == 0
+        && !strcmp (buf, "abcd"),
+        "format: plain string copy works");
+    ok (boot_config_format_uri (buf, sizeof (buf), "abcd:%p", NULL, 42) == 0
+        && !strcmp (buf, "abcd:42"),
+        "format: %%p substitution works end string");
+    ok (boot_config_format_uri (buf, sizeof (buf), "a%pb", NULL, 42) == 0
+        && !strcmp (buf, "a42b"),
+        "format: %%p substitution works mid string");
+    ok (boot_config_format_uri (buf, sizeof (buf), "%p:abcd", NULL, 42) == 0
+        && !strcmp (buf, "42:abcd"),
+        "format: %%p substitution works begin string");
+    ok (boot_config_format_uri (buf, sizeof (buf), "%h", NULL, 0) == 0
+        && !strcmp (buf, "%h"),
+        "format: %%h passes through when host=NULL");
+    ok (boot_config_format_uri (buf, sizeof (buf), "%h", "foo", 0) == 0
+        && !strcmp (buf, "foo"),
+        "format: %%h substitution works");
+    ok (boot_config_format_uri (buf, sizeof (buf), "%%", NULL, 0) == 0
+        && !strcmp (buf, "%"),
+        "format: %%%% literal works");
+    ok (boot_config_format_uri (buf, sizeof (buf), "a%X", NULL, 0) == 0
+        && !strcmp (buf, "a%X"),
+        "format: unknown token passes through");
+
+    ok (boot_config_format_uri (buf, 5, "abcd", NULL, 0) == 0
+        && !strcmp (buf, "abcd"),
+        "format: copy abcd to buf[5] works");
+    ok (boot_config_format_uri (buf, 4, "abcd", NULL, 0) < 0,
+        "format: copy abcd to buf[4] fails");
+
+    ok (boot_config_format_uri (buf, 5, "a%p", NULL, 123) == 0
+        && !strcmp (buf, "a123"),
+        "format: %%p substitution into exact size buf works");
+    ok (boot_config_format_uri (buf, 4, "a%p", NULL, 123) < 0,
+        "format: %%p substitution overflow detected");
+
+    ok (boot_config_format_uri (buf, 5, "a%h", "abc", 0) == 0
+        && !strcmp (buf, "aabc"),
+        "format: %%h substitution into exact size buf works");
+    ok (boot_config_format_uri (buf, 4, "a%h", "abc", 0) < 0,
+        "format: %%h substitution overflow detected");
+}
+
+int main (int argc, char **argv)
+{
+    char dir[PATH_MAX + 1];
+
+    plan (NO_PLAN);
+
+    test_format ();
+
+    create_test_dir (dir, sizeof (dir));
+
+    test_parse (dir);
+    test_overflow_bind (dir);
+    test_overflow_connect (dir);
+    test_bad_hosts_entry (dir);
+    test_bad_host_idset (dir);
+    test_bad_host_bind (dir);
+    test_no_hosts (dir);
+    test_empty_hosts (dir);
+    test_missing_info (dir);
+    test_toml_mixed_array (dir);
+
+    if (rmdir (dir) < 0)
+        BAIL_OUT ("could not cleanup test dir %s", dir);
+
+    done_testing ();
+    return 0;
+}
+
+/*
+ * vi:ts=4 sw=4 expandtab
+ */

From 0f491e4785203e036038590288684be3b2c09fba Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Sun, 22 Dec 2019 12:33:03 -0800
Subject: [PATCH 09/16] broker: skip overlay bind/connect if size==1

Avoid triggering the loading of curve keys and/or starting
of the zeromq service thread if the overlay is not going
to be used.
---
 src/broker/boot_config.c |  4 ++++
 src/broker/broker.c      | 16 +++++++---------
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/src/broker/boot_config.c b/src/broker/boot_config.c
index 985739caabfe..5a9bc3cb9cdd 100644
--- a/src/broker/boot_config.c
+++ b/src/broker/boot_config.c
@@ -424,6 +424,10 @@ int boot_config (flux_t *h, overlay_t *overlay, attr_t *attrs, int tbon_k)
             log_err ("overlay_set_child %s", bind_uri);
             goto error;
         }
+        if (overlay_bind (overlay) < 0) { /* idempotent */
+            log_err ("overlay_bind");
+            goto error;
+        }
         if (boot_config_geturibyrank (hosts,
                                       &conf,
                                       rank,
diff --git a/src/broker/broker.c b/src/broker/broker.c
index ef98d5b6d2cb..f983701d3851 100644
--- a/src/broker/broker.c
+++ b/src/broker/broker.c
@@ -595,15 +595,13 @@ int main (int argc, char *argv[])
 
     /* Wire up the overlay.
      */
-    if (ctx.verbose)
-        log_msg ("initializing overlay sockets");
-    if (overlay_bind (ctx.overlay) < 0) { /* idempotent */
-        log_err ("overlay_bind");
-        goto cleanup;
-    }
-    if (overlay_connect (ctx.overlay) < 0) {
-        log_err ("overlay_connect");
-        goto cleanup;
+    if (rank > 0) {
+        if (ctx.verbose)
+            log_msg ("initializing overlay connect");
+        if (overlay_connect (ctx.overlay) < 0) {
+            log_err ("overlay_connect");
+            goto cleanup;
+        }
     }
 
     if (shutdown_set_flux (ctx.shutdown, ctx.h) < 0) {

From 63eebbcb601671d03a3da97fa509154ec3c6df61 Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Sun, 22 Dec 2019 13:09:10 -0800
Subject: [PATCH 10/16] etc/flux.service.in: set local-uri

Problem: the broker sets the local-uri attribute to
local://${rundir}/${rank}/local, but if FLUX_URI is unset,
flux_open() tries to open local://${rundir}/local.

Set local-uri on the command line in the systemd unit file,
to match the flux_open() default.
---
 etc/flux.service.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/etc/flux.service.in b/etc/flux.service.in
index dde9abd81627..540d55301441 100644
--- a/etc/flux.service.in
+++ b/etc/flux.service.in
@@ -3,7 +3,7 @@ Description=Flux message broker
 
 [Service]
 Environment=FLUX_USERDB_OPTIONS=--default-rolemask=user
-ExecStart=@X_BINDIR@/flux broker -Srundir=@X_RUNSTATEDIR@/flux -Sboot.method=config sleep inf
+ExecStart=@X_BINDIR@/flux broker -Srundir=@X_RUNSTATEDIR@/flux -Slocal-uri=local://@X_RUNSTATEDIR@/flux/local -Sboot.method=config sleep inf
 User=flux
 Group=flux
 RuntimeDirectory=flux

From b3ff6e37469fbba0ab9f199b41356416b097610a Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Sat, 21 Dec 2019 07:48:10 -0800
Subject: [PATCH 11/16] testsuite/t3100-flux-in-flux: split up instance-level
 test

Problem: instance-level test tries to do too many things
in one test and is hard to debug.

Spit the test into several subtests, and write output to
the file system for post-mortem.
---
 t/t3100-flux-in-flux.t | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/t/t3100-flux-in-flux.t b/t/t3100-flux-in-flux.t
index da847c1386c9..f809956b87f6 100755
--- a/t/t3100-flux-in-flux.t
+++ b/t/t3100-flux-in-flux.t
@@ -51,16 +51,31 @@ test_expect_success "flux --parent --parent works in subinstance" '
 	test_cmp guest2.test.exp guest2.test
 '
 
-test_expect_success "flux sets instance-level attribute" '
-        level=$(flux mini run flux start ${ARGS} \
-                flux getattr instance-level) &&
-        level2=$(flux mini run flux start \
-                 flux mini run flux start ${ARGS} \
-                 flux getattr instance-level) &&
-	level0=$(flux start ${ARGS} flux getattr instance-level) &&
-        test "$level" = "1" &&
-        test "$level2" = "2" &&
-        test "$level0" = "0"
+test_expect_success "instance-level attribute = 0 in new stanalone instance" '
+	flux start ${ARGS} flux getattr instance-level >level_new.out &&
+	echo 0 >level_new.exp &&
+	test_cmp level_new.exp level_new.out
+'
+
+test_expect_success "instance-level attribute = 0 in test instance" '
+	flux getattr instance-level >level0.out &&
+	echo 0 >level0.exp &&
+	test_cmp level0.exp level0.out
+'
+
+test_expect_success "instance-level attribute = 1 in first subinstance" '
+        flux mini run flux start ${ARGS} \
+            flux getattr instance-level >level1.out &&
+	echo 1 >level1.exp &&
+	test_cmp level1.exp level1.out
+'
+
+test_expect_success "instance-level attribute = 2 in second subinstance" '
+        flux mini run flux start \
+	    flux mini run flux start ${ARGS} \
+		flux getattr instance-level >level2.out &&
+	echo 2 >level2.exp &&
+	test_cmp level2.exp level2.out
 '
 
 test_expect_success "flux sets jobid attribute" '

From 3b07605662efcaff14be40732c2a7b4770ddd300 Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Sat, 21 Dec 2019 07:49:40 -0800
Subject: [PATCH 12/16] testsuite/t0001-basic: relax attr assumptions

Problem: tests expect tbon.endpoint to be set in a size=1
instance when it need not be.

Check this attribute in rank 0 of a size=2 instance,
and relax expectations of content.

Also: [cleanup] use test_must_fail where appropriate.
---
 t/t0001-basic.t | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/t/t0001-basic.t b/t/t0001-basic.t
index 41b9755780dc..3382e1ee18f2 100755
--- a/t/t0001-basic.t
+++ b/t/t0001-basic.t
@@ -161,24 +161,28 @@ test_expect_success 'flux-start -o,--setattr ATTR=VAL can set broker attributes'
 	test $ATTR_VAL -eq 42
 '
 test_expect_success 'tbon.endpoint can be read' '
-	ATTR_VAL=`flux start flux getattr tbon.endpoint` &&
-        echo $ATTR_VAL | grep "^tcp"
+	ATTR_VAL=`flux start -s2 flux getattr tbon.endpoint` &&
+	echo $ATTR_VAL | grep "://"
 '
 test_expect_success 'tbon.endpoint can be set and %h works' '
-	ATTR_VAL=`flux start -o,--setattr=tbon.endpoint='tcp://%h:*' flux getattr tbon.endpoint` &&
-        echo $ATTR_VAL | grep "^tcp" &&
-        ! echo $ATTR_VAL | grep "%h"
+	flux start -s2 -o,--setattr=tbon.endpoint=tcp://%h:* \
+		flux getattr tbon.endpoint >pct_h.out &&
+	grep "^tcp" pct_h.out &&
+	test_must_fail grep "%h" pct_h.out
 '
 test_expect_success 'tbon.endpoint with %B works' '
-	ATTR_VAL=`flux start -o,--setattr=tbon.endpoint='ipc://%B/req' flux getattr tbon.endpoint` &&
-        echo $ATTR_VAL | grep "^ipc" &&
-        ! echo $ATTR_VAL | grep "%B"
+	flux start -s2 -o,--setattr=tbon.endpoint=ipc://%B/req \
+		flux getattr tbon.endpoint >pct_B.out &&
+	grep "^ipc" pct_B.out &&
+	test_must_fail grep "%B" pct_B.out
 '
+# N.B. rank 1 has to be killed in this test after rank 0 fails gracefully
+# so test_must_fail won't work here
 test_expect_success 'tbon.endpoint fails on bad endpoint' '
-	! flux start -o,--setattr=tbon.endpoint='foo://bar' flux getattr tbon.endpoint
+	! flux start -s2 -o,--setattr=tbon.endpoint=foo://bar /bin/true
 '
 test_expect_success 'tbon.parent-endpoint cannot be read on rank 0' '
-       ! flux start flux getattr tbon.parent-endpoint
+	test_must_fail flux start -s2 flux getattr tbon.parent-endpoint
 '
 test_expect_success 'tbon.parent-endpoint can be read on not rank 0' '
        NUM=`flux start --size 4 flux exec -n flux getattr tbon.parent-endpoint | grep ipc | wc -l` &&

From 2e3a0515d4f5c7ef87837dbcfe2426a7b8c193fb Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Sun, 22 Dec 2019 21:42:17 -0800
Subject: [PATCH 13/16] broker/boot_pmi: dont bind to URI if no downstream
 peers

When the bootstrap method is PMI, don't bind to a zeromq socket
if there are no downstream peers.

Change broker_response_sendmsg() to silently discard a response
if it has exhausted all other routing option and tries to send
it downstream.  This is zeromq's default behavior for unroutable
messages sent on a ROUTER socket anyway (drop).
---
 src/broker/boot_pmi.c | 79 ++++++++++++++++++++++++++-----------------
 src/broker/broker.c   | 18 +++++++---
 2 files changed, 61 insertions(+), 36 deletions(-)

diff --git a/src/broker/boot_pmi.c b/src/broker/boot_pmi.c
index 8c3ee78b3318..4a242fc46b32 100644
--- a/src/broker/boot_pmi.c
+++ b/src/broker/boot_pmi.c
@@ -207,33 +207,38 @@ int boot_pmi (overlay_t *overlay, attr_t *attrs, int tbon_k)
         log_err ("set_instance_level_attr");
         goto error;
     }
-    if (overlay_init (overlay,
-                      (uint32_t)pmi_params.size,
-                      (uint32_t)pmi_params.rank,
-                      tbon_k)< 0)
+    if (overlay_init (overlay, pmi_params.size, pmi_params.rank, tbon_k) < 0)
         goto error;
 
-    if (update_endpoint_attr (attrs, "tbon.endpoint", &tbonendpoint,
-                                                        "tcp://%h:*") < 0)
-        goto error;
-
-    if (overlay_set_child (overlay, tbonendpoint) < 0) {
-        log_err ("overlay_set_child");
-        goto error;
-    }
-
-    /* Bind to addresses to expand URI wildcards, so we can exchange
-     * the real addresses.
+    /* If there are to be downstream peers, then bind to socket and share the
+     * concretized URI with other ranks via PMI KVS key=cmbd.<rank>.uri.
+     * N.B. there are no downstream peers if the 0th child of this rank
+     * in k-ary tree does not exist.
      */
-    if (overlay_bind (overlay) < 0) {
-        log_err ("overlay_bind failed");   /* function is idempotent */
-        goto error;
-    }
-
-    /* Write the URI of downstream facing socket under the rank (if any).
-     */
-    if ((child_uri = overlay_get_child (overlay))) {
+    if (kary_childof (tbon_k,
+                      pmi_params.size,
+                      pmi_params.rank,
+                      0) != KARY_NONE) {
 
+        if (update_endpoint_attr (attrs,
+                                  "tbon.endpoint",
+                                  &tbonendpoint,
+                                  "tcp://%h:*") < 0) {
+            log_msg ("update_endpoint_attr failed");
+            goto error;
+        }
+        if (overlay_set_child (overlay, tbonendpoint) < 0) {
+            log_err ("overlay_set_child");
+            goto error;
+        }
+        if (overlay_bind (overlay) < 0) {
+            log_err ("overlay_bind failed");   /* function is idempotent */
+            goto error;
+        }
+        if (!(child_uri = overlay_get_child (overlay))) {
+            log_msg ("overlay_get_child returned NULL");
+            goto error;
+        }
         if (snprintf (key, sizeof (key),
                       "cmbd.%d.uri", pmi_params.rank) >= sizeof (key)) {
             log_msg ("pmi key string overflow");
@@ -248,25 +253,37 @@ int boot_pmi (overlay_t *overlay, attr_t *attrs, int tbon_k)
             log_msg ("broker_pmi_kvs_put: %s", pmi_strerror (result));
             goto error;
         }
+        result = broker_pmi_kvs_commit (pmi, pmi_params.kvsname);
+        if (result != PMI_SUCCESS) {
+            log_msg ("broker_pmi_kvs_commit: %s", pmi_strerror (result));
+            goto error;
+        }
+    }
+    else {
+        (void)attr_delete (attrs, "tbon.endpoint", true);
+        if (attr_add (attrs,
+                     "tbon.endpoint",
+                     NULL,
+                     FLUX_ATTRFLAG_IMMUTABLE) < 0) {
+            log_err ("setattr tbon.endpoint");
+            goto error;
+        }
     }
 
-    /* Puts are complete, now we synchronize and begin our gets.
+    /* The PMI barrier (which is implicitly over 'size' ranks) ensures that
+     * all KVS puts are complete before any PMI gets.
      */
-    result = broker_pmi_kvs_commit (pmi, pmi_params.kvsname);
-    if (result != PMI_SUCCESS) {
-        log_msg ("broker_pmi_kvs_commit: %s", pmi_strerror (result));
-        goto error;
-    }
     result = broker_pmi_barrier (pmi);
     if (result != PMI_SUCCESS) {
         log_msg ("broker_pmi_barrier: %s", pmi_strerror (result));
         goto error;
     }
 
-    /* Read the uri of our parent, after computing its rank
+    /* If there is to be an upstream peer, fetch its URI from PMI KVS.
+     * N.B. only rank 0 has no upstream peer.
      */
     if (pmi_params.rank > 0) {
-        parent_rank = kary_parentof (tbon_k, (uint32_t)pmi_params.rank);
+        parent_rank = kary_parentof (tbon_k, pmi_params.rank);
         if (snprintf (key, sizeof (key),
                       "cmbd.%d.uri", parent_rank) >= sizeof (key)) {
             log_msg ("pmi key string overflow");
diff --git a/src/broker/broker.c b/src/broker/broker.c
index f983701d3851..e35050919583 100644
--- a/src/broker/broker.c
+++ b/src/broker/broker.c
@@ -55,6 +55,7 @@
 #include "src/common/libpmi/pmi.h"
 #include "src/common/libpmi/pmi_strerror.h"
 #include "src/common/libutil/fsd.h"
+#include "src/common/libutil/errno_safe.h"
 
 #include "heartbeat.h"
 #include "module.h"
@@ -2001,6 +2002,8 @@ static int broker_response_sendmsg (broker_ctx_t *ctx, const flux_msg_t *msg)
     int rc = -1;
     char *uuid = NULL;
     uint32_t parent;
+    uint32_t rank;
+    uint32_t size;
     char puuid[16];
 
     if (flux_msg_get_route_last (msg, &uuid) < 0)
@@ -2013,7 +2016,9 @@ static int broker_response_sendmsg (broker_ctx_t *ctx, const flux_msg_t *msg)
         goto done;
     }
 
-    parent = kary_parentof (ctx->tbon_k, overlay_get_rank (ctx->overlay));
+    rank = overlay_get_rank (ctx->overlay);
+    size = overlay_get_size (ctx->overlay);
+    parent = kary_parentof (ctx->tbon_k, rank);
     snprintf (puuid, sizeof (puuid), "%"PRIu32, parent);
 
     /* See if it should go to the parent (backwards!)
@@ -2026,13 +2031,16 @@ static int broker_response_sendmsg (broker_ctx_t *ctx, const flux_msg_t *msg)
 
     /* Try to deliver to a module.
      * If modhash didn't match next hop, route to child.
+     * If no child, silently drop (module unloaded?)
      */
     rc = module_response_sendmsg (ctx->modhash, msg);
-    if (rc < 0 && errno == ENOSYS)
-        rc = overlay_sendmsg_child (ctx->overlay, msg);
+    if (rc < 0 && errno == ENOSYS) {
+        rc = 0;
+        if (kary_childof (ctx->tbon_k, size, rank, 0) != KARY_NONE)
+            rc = overlay_sendmsg_child (ctx->overlay, msg);
+    }
 done:
-    if (uuid)
-        free (uuid);
+    ERRNO_SAFE_WRAP (free, uuid);
     return rc;
 }
 

From b7d6010b634f6423fffa4c29dc4e14220170790a Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Sun, 22 Dec 2019 18:14:32 -0800
Subject: [PATCH 14/16] testsuite/t2004-hydra.t: drop unreliable tests

Problem: hydra tests fail occasionally

As noted in #1169, some versions of hydra might have a problem
capturing stdio, which makes these tests unreliable.  Drop the
tests that are just verifying hydra's PMI behavior, and make the
ones that remain not dependent on stdio.

Fixes #1169
---
 t/t2004-hydra.t | 30 ++----------------------------
 1 file changed, 2 insertions(+), 28 deletions(-)

diff --git a/t/t2004-hydra.t b/t/t2004-hydra.t
index 276bfa5e9948..4b0aef283b80 100755
--- a/t/t2004-hydra.t
+++ b/t/t2004-hydra.t
@@ -7,45 +7,19 @@ test_description='Test that MPICH Hydra can launch Flux'
 test -n "$FLUX_TESTS_LOGFILE" && set -- "$@" --logfile
 . `dirname $0`/sharness.sh
 PMI_INFO=${FLUX_BUILD_DIR}/src/common/libpmi/test_pmi_info
+ARGS="-o,-Sbroker.rc1_path=,-Sbroker.rc3_path=,--shutdown-grace=0.25"
 
 if ! which mpiexec.hydra 2>/dev/null; then
     skip_all='skipping hydra-launching-flux tests, mpiexec.hydra unavailable'
     test_done
 fi
 
-test_expect_success 'Hydra runs hello world' '
-	mpiexec.hydra -n 4 echo "Hello World"
-'
-
-count_uniq_lines() { sort $1 | uniq | wc -l; }
-
-test_expect_success 'Hydra sets PMI_FD to unique value' '
-	mpiexec.hydra -n 4 printenv PMI_FD > out &&
-	test_debug "cat out" &&
-	test $(count_uniq_lines out) -eq 4
-'
-
-test_expect_success 'Hydra sets PMI_RANK to unique value' '
-	mpiexec.hydra -n 4 printenv PMI_RANK > out2 &&
-	test_debug "cat out2" &&
-	test $(count_uniq_lines out2) -eq 4
-'
-
-test_expect_success 'Hydra sets PMI_SIZE to uniform value' '
-	mpiexec.hydra -n 4 printenv PMI_SIZE > out3 &&
-	test_debug "cat out3" &&
-	test $(count_uniq_lines out3) -eq 1
-'
-
 test_expect_success 'Flux libpmi-client wire protocol works with Hydra' '
 	mpiexec.hydra -n 4 ${PMI_INFO}
 '
 
 test_expect_success 'Hydra can launch Flux' '
-	mpiexec.hydra -n 4 flux start \
-		flux comms info >flux_out &&
-	test_debug "cat flux_out" &&
-	grep size=4 flux_out
+	mpiexec.hydra -n 4 flux start ${ARGS} flux getattr size
 '
 
 test_done

From 3e2acf0cdb9676b3a810f09f8bc561935fb2c386 Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Mon, 23 Dec 2019 08:25:51 -0800
Subject: [PATCH 15/16] doc/flux-config-bootstrap(5): add man page

---
 configure.ac                        |   1 +
 doc/Makefile.am                     |   2 +-
 doc/man5/COPYRIGHT.adoc             |   4 +
 doc/man5/Makefile.am                |  27 +++++++
 doc/man5/flux-config-bootstrap.adoc | 121 ++++++++++++++++++++++++++++
 doc/test/spell.en.pws               |   4 +
 6 files changed, 158 insertions(+), 1 deletion(-)
 create mode 100644 doc/man5/COPYRIGHT.adoc
 create mode 100644 doc/man5/Makefile.am
 create mode 100644 doc/man5/flux-config-bootstrap.adoc

diff --git a/configure.ac b/configure.ac
index e00365142d0e..1149158e0384 100644
--- a/configure.ac
+++ b/configure.ac
@@ -489,6 +489,7 @@ AC_CONFIG_FILES( \
   doc/Makefile \
   doc/man1/Makefile \
   doc/man3/Makefile \
+  doc/man5/Makefile \
   doc/man7/Makefile \
   doc/test/Makefile \
   t/Makefile \
diff --git a/doc/Makefile.am b/doc/Makefile.am
index f37d27ef64f5..18751b9cb017 100644
--- a/doc/Makefile.am
+++ b/doc/Makefile.am
@@ -1 +1 @@
-SUBDIRS = man1 man3 man7 test
+SUBDIRS = man1 man3 man5 man7 test
diff --git a/doc/man5/COPYRIGHT.adoc b/doc/man5/COPYRIGHT.adoc
new file mode 100644
index 000000000000..6b2304f95801
--- /dev/null
+++ b/doc/man5/COPYRIGHT.adoc
@@ -0,0 +1,4 @@
+Copyright 2014 Lawrence Livermore National Security, LLC
+and Flux developers.
+
+SPDX-License-Identifier: LGPL-3.0
diff --git a/doc/man5/Makefile.am b/doc/man5/Makefile.am
new file mode 100644
index 000000000000..a0438917ae10
--- /dev/null
+++ b/doc/man5/Makefile.am
@@ -0,0 +1,27 @@
+MAN5_FILES = \
+	flux-config-bootstrap.5
+
+ADOC_FILES  = $(MAN5_FILES:%.7=%.adoc)
+XML_FILES   = $(MAN5_FILES:%.7=%.xml)
+
+if ENABLE_DOCS
+dist_man_MANS = $(MAN5_FILES)
+$(MAN5_FILES): COPYRIGHT.adoc
+endif
+
+SUFFIXES = .adoc .5
+
+STDERR_DEVNULL = $(stderr_devnull_$(V))
+stderr_devnull_ =  $(stderr_devnull_$(AM_DEFAULT_VERBOSITY))
+stderr_devnull_0 = 2>/dev/null
+
+.adoc.5:
+	$(AM_V_GEN)$(ADOC) --attribute mansource=$(PACKAGE_NAME) \
+	    --attribute manversion=$(PACKAGE_VERSION) \
+	    --attribute manmanual="Flux Miscellaneous Reference" \
+	    --destination-dir $(builddir) \
+	    --doctype manpage $(ADOC_FORMAT_OPT) manpage $< $(STDERR_DEVNULL)
+
+EXTRA_DIST = $(ADOC_FILES) COPYRIGHT.adoc
+
+CLEANFILES = $(MAN5_FILES) $(XML_FILES)
diff --git a/doc/man5/flux-config-bootstrap.adoc b/doc/man5/flux-config-bootstrap.adoc
new file mode 100644
index 000000000000..6d7bf54bc23d
--- /dev/null
+++ b/doc/man5/flux-config-bootstrap.adoc
@@ -0,0 +1,121 @@
+flux-config-bootstrap(5)
+========================
+:doctype: manpage
+
+
+NAME
+----
+flux-config-bootstrap - configure Flux instance bootstrap
+
+
+DESCRIPTION
+-----------
+
+The broker discovers the size of the Flux instance, the broker's rank,
+and overlay network wireup information either dynamically using a PMI
+service, such as when being launched by Flux or another resource manager,
+or statically using the `bootstrap` section of the Flux configuration,
+such as when being launched by systemd.
+
+The default bootstrap mode is PMI.  The Flux systemd unit file forces the
+broker to use the config method by specifying `--setattr=boot.method=config`
+on the broker command line.
+
+CONFIG FILES
+------------
+
+Flux uses the TOML configuration file format.  The broker normally
+parses `/etc/flux/conf.d/*.toml`.  The actual path is dependent on
+`configure` command line options used to build flux, and can be
+overridden with the FLUX_CONF_DIR environment variable.
+
+The `bootstrap` section is a TOML table containing the following
+keys.  Each node in a cluster is expected to bootstrap from an
+identical config file.
+
+KEYWORDS
+--------
+
+default_port::
+(optional) The value is an integer port number that is substituted
+for the token `%p` in the other keys.
+
+default_bind::
+(optional) The value is a ZeroMQ endpoint URI that is used for host
+entries that do not explicitly set a bind address.  The tokens
+`%p` and `%h` are replaced with the default port and the host
+for the current host entry.
+
+default_connect::
+(optional) The value is a ZeroMQ endpoint URI that is used for host
+entries that do not explicitly set a connect address.  The tokens
+`%p` and `%h` are replaced with the default port and the host
+for the current host entry.
+
+hosts::
+(optional) A rank-ordered array of host entries.  Each host entry is
+a TOML table containing at minimum the `host` key.  The broker determines
+its rank by matching its hostname in the hosts array and taking the array
+index.  An empty or missing hosts array implies a standalone (single
+broker) instance.  The entry for a broker with downstream peers must
+either assign the `bind` key to a ZeroMQ endpoint URI, or the `default_bind`
+URI described above is used.  The entry for a broker with downstream peers
+must also either assign the `connect` key to a ZeroMQ endpoint URI, or
+the `default_connect` URI described above is used.  The same `%h` and `%p`
+substitutions work here as well.
+
+COMPACT HOSTS
+-------------
+
+Since it would be tedious to repeat host entries for every compute node in
+a large cluster, the `hosts` array may be abbreviated using bracketed
+"idset" notation in `host` keys.
+
+An idset is an unordered set of non-negative integers that may be expressed
+as a comma-separated list including hyphenated ranges.  For example
+the set 0, 1, 2, 3, 4, 18, 20 may be represented as "0-4,18,20".
+
+A `host` key may include one or more bracketed idsets.  For example,
+"foo[0-1023]" represents the hosts "foo0, foo1, ..., foo1023", or
+"rack[0-1]node[0-1]" represents the hosts "rack0node0, rack0node1,
+rack1node0, rack1node1".
+
+EXAMPLE
+-------
+
+....
+[bootstrap]
+
+default_port = 8050
+default_bind = "tcp://en0:%p"
+default_connect = "tcp://e%h:%p"
+
+hosts = [
+    {
+        host="fluke0",
+        bind="tcp://en4:9001",
+        connect="tcp://fluke-mgmt:9001"
+    },
+    { host = "fluke[1-1023]" },
+]
+....
+
+
+AUTHOR
+------
+This page is maintained by the Flux community.
+
+
+RESOURCES
+---------
+Github: <http://github.com/flux-framework>
+
+
+COPYRIGHT
+---------
+include::COPYRIGHT.adoc[]
+
+
+SEE ALSO
+--------
+flux-getattr(1), flux_attr_get(3)
diff --git a/doc/test/spell.en.pws b/doc/test/spell.en.pws
index 64126a9ef3b5..e28ab1f934df 100644
--- a/doc/test/spell.en.pws
+++ b/doc/test/spell.en.pws
@@ -466,3 +466,7 @@ FSD
 enqueues
 cpus
 gpu
+idsets
+systemd
+toml
+unordered

From 54ff0c1f7a8fbb73a48497c3306238ffa4c545c7 Mon Sep 17 00:00:00 2001
From: Jim Garlick <garlick.jim@gmail.com>
Date: Mon, 23 Dec 2019 14:05:01 -0800
Subject: [PATCH 16/16] testsuite/t2601-job-shell.t: don't redirect stderr

Problem: t2601-job-shell.t (test 8 - PMI cliques are correct
for 2 ppn) just failed in travis with no useful output.

This test, and a few tests surrounding it, redirect the stderr
of flux job attach to a file, then ignore the file.

Allow flux job attach's stderr to be included in the verbose
test output
---
 t/t2602-job-shell.t | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t2602-job-shell.t b/t/t2602-job-shell.t
index 84b16f947020..d1c34eb70173 100755
--- a/t/t2602-job-shell.t
+++ b/t/t2602-job-shell.t
@@ -72,7 +72,7 @@ test_expect_success 'job-shell: PMI works' '
 '
 test_expect_success 'pmi-shell: PMI cliques are correct for 1 ppn' '
         id=$(flux jobspec srun -N4 -n4 ${PMI_INFO} -c | flux job submit) &&
-	flux job attach $id >pmi_clique1.raw 2>pmi_clique1.err &&
+	flux job attach $id >pmi_clique1.raw &&
 	sort -snk1 <pmi_clique1.raw >pmi_clique1.out &&
 	sort >pmi_clique1.exp <<-EOT &&
 	0: clique=0
@@ -84,7 +84,7 @@ test_expect_success 'pmi-shell: PMI cliques are correct for 1 ppn' '
 '
 test_expect_success 'pmi-shell: PMI cliques are correct for 2 ppn' '
         id=$(flux jobspec srun -N2 -n4 ${PMI_INFO} -c | flux job submit) &&
-	flux job attach $id >pmi_clique2.raw 2>pmi_clique2.err &&
+	flux job attach $id >pmi_clique2.raw &&
 	sort -snk1 <pmi_clique2.raw >pmi_clique2.out &&
 	sort >pmi_clique2.exp <<-EOT &&
 	0: clique=0,1
@@ -96,7 +96,7 @@ test_expect_success 'pmi-shell: PMI cliques are correct for 2 ppn' '
 '
 test_expect_success 'pmi-shell: PMI cliques are correct for irregular ppn' '
         id=$(flux jobspec srun -N4 -n5 ${PMI_INFO} -c | flux job submit) &&
-	flux job attach $id >pmi_cliquex.raw 2>pmi_cliquex.err &&
+	flux job attach $id >pmi_cliquex.raw &&
 	sort -snk1 <pmi_cliquex.raw >pmi_cliquex.out &&
 	sort >pmi_cliquex.exp <<-EOT &&
 	0: clique=0,1
@@ -109,7 +109,7 @@ test_expect_success 'pmi-shell: PMI cliques are correct for irregular ppn' '
 '
 test_expect_success 'job-shell: PMI KVS works' '
         id=$(flux jobspec srun -N4 ${KVSTEST} | flux job submit) &&
-	flux job attach $id >kvstest.out 2>kvstest.err &&
+	flux job attach $id >kvstest.out &&
 	grep "t phase" kvstest.out
 '
 test_expect_success 'job-exec: decrease kill timeout for tests' '