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: + + +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 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/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 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/boot_config.c b/src/broker/boot_config.c index 6a31a7363341..5a9bc3cb9cdd 100644 --- a/src/broker/boot_config.c +++ b/src/broker/boot_config.c @@ -14,226 +14,474 @@ #include "config.h" #endif #include -#include #include -#include -#include +#include #include #include #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; + } } - if (rank < 0 || rank > size - 1) { - log_msg ("Config file error [bootstrap]: invalid rank %d for size %d", - rank, - size); + + *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; + } + /* 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 (parse_config_file (h, &rank, &size, &endpoints) < 0) + 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 (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) + 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 (parse_endpoint_by_rank (endpoints, rank, &this_endpoint) < 0) - 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 (overlay_bind (overlay) < 0) { /* idempotent */ + log_err ("overlay_bind"); + 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/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..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 ef98d5b6d2cb..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" @@ -595,15 +596,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) { @@ -2003,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) @@ -2015,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!) @@ -2028,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; } diff --git a/src/broker/log.c b/src/broker/log.c index 4982cb45ec4d..d0a1093256ba 100644 --- a/src/broker/log.c +++ b/src/broker/log.c @@ -668,28 +668,19 @@ 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 (); 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) - goto error; if (flux_msg_handler_addvec (h, htab, logbuf, &logbuf->handlers) < 0) 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: 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 +#include +#include +#include + +#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 + */ 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"); } diff --git a/src/common/libidset/Makefile.am b/src/common/libidset/Makefile.am index 5825b8e14a35..111e9e2b04bf 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) @@ -38,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/idset.h b/src/common/libidset/idset.h index b0ac43ce38b2..ffb9a67878ad 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. */ @@ -95,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_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 */ 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 +#include +#include +#include +#include +#include +#include + +#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 */ 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 #include #include +#include #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 (); 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/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` && 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 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 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.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.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.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' ' 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" '