Merge pull request flux-framework#4865 from garlick/upmi_plugin

enable broker bootstrap methods to be provided by dso plugins, and drop compiled-in pmix support

Author: mergify[bot]

config/x_ac_pmix.m4

@@ -416,7 +416,6 @@ AS_IF([test "x$enable_content_s3" = "xyes"], [
 
 AM_CONDITIONAL([ENABLE_CONTENT_S3], [test "x$enable_content_s3" = "xyes"])
 
-X_AC_PMIX
 
 ##
 # Check for systemd

configure.ac

@@ -96,6 +96,13 @@ broker.critical-ranks [Updates: C]
    is raised when a failing node or other error occurs affecting any rank
    in this set. Default: rank 0 plus any other overlay network routers.
 
+broker.boot-method [Updates: C]
+   A URI representing the method used to bootstrap Flux.  Valid values are
+   ``config`` (boot via TOML config file), ``simple`` (use the PMI-1 simple
+   wire protocol), ``libpmi[:path]`` (use a PMI-1 shared library), or
+   ``single`` (standalone size=1).  Additional boot methods may be provided
+   by plugins.
+
 broker.pid
    The process id of the local broker.
 

doc/man7/flux-broker-attributes.rst

@@ -9,7 +9,6 @@ AM_CPPFLAGS = \
 	-I$(top_srcdir)/src/include \
 	-I$(top_srcdir)/src/common/libccan \
 	-I$(top_builddir)/src/common/libflux \
-	$(PMIX_CFLAGS) \
 	$(ZMQ_CFLAGS) \
 	$(LIBUUID_CFLAGS) \
 	$(JANSSON_CFLAGS) \
@@ -74,12 +73,13 @@ flux_broker_LDADD = \
 	$(top_builddir)/src/common/libflux-core.la \
 	$(top_builddir)/src/common/libzmqutil/libzmqutil.la \
 	$(top_builddir)/src/common/libpmi/libupmi.la \
+	$(top_builddir)/src/common/libpmi/libpmi_client.la \
+	$(top_builddir)/src/common/libpmi/libpmi_common.la \
 	$(top_builddir)/src/common/libflux-internal.la \
 	$(top_builddir)/src/common/libflux-optparse.la \
 	$(ZMQ_LIBS) \
 	$(LIBUUID_LIBS) \
 	$(JANSSON_LIBS) \
-	$(PMIX_LIBS) \
 	$(LIBDL)
 
 flux_broker_LDFLAGS =
@@ -96,12 +96,10 @@ test_ldadd = \
 	$(top_builddir)/src/common/libtestutil/libtestutil.la \
 	$(top_builddir)/src/common/libflux-core.la \
 	$(top_builddir)/src/common/libzmqutil/libzmqutil.la \
-	$(top_builddir)/src/common/libpmi/libpmi_client.la \
 	$(top_builddir)/src/common/libflux-internal.la \
 	$(top_builddir)/src/common/libtap/libtap.la \
 	$(ZMQ_LIBS) \
-	$(JANSSON_LIBS) \
-	$(PMIX_LIBS)
+	$(JANSSON_LIBS)
 
 test_ldflags = \
 	-no-install

src/broker/Makefile.am

@@ -481,6 +481,19 @@ int boot_config_geturibyrank (json_t *hosts,
     return 0;
 }
 
+static int set_broker_boot_method_attr (attr_t *attrs, const char *value)
+{
+    (void)attr_delete (attrs, "broker.boot-method", true);
+    if (attr_add (attrs,
+                  "broker.boot-method",
+                  value,
+                  ATTR_IMMUTABLE) < 0) {
+        log_err ("setattr broker.boot-method");
+        return -1;
+    }
+    return 0;
+}
+
 int boot_config (flux_t *h, struct overlay *overlay, attr_t *attrs)
 {
     struct boot_conf conf;
@@ -629,6 +642,8 @@ int boot_config (flux_t *h, struct overlay *overlay, attr_t *attrs)
         log_err ("setattr instance-level 0");
         goto error;
     }
+    if (set_broker_boot_method_attr (attrs, "config") < 0)
+        goto error;
     json_decref (hosts);
     topology_decref (topo);
     return 0;

src/broker/boot_config.c

@@ -186,6 +186,19 @@ static int set_hostlist_attr (attr_t *attrs, struct hostlist *hl)
     return rc;
 }
 
+static int set_broker_boot_method_attr (attr_t *attrs, const char *value)
+{
+    (void)attr_delete (attrs, "broker.boot-method", true);
+    if (attr_add (attrs,
+                  "broker.boot-method",
+                  value,
+                  ATTR_IMMUTABLE) < 0) {
+        log_err ("setattr broker.boot-method");
+        return -1;
+    }
+    return 0;
+}
+
 static void trace_upmi (void *arg, const char *text)
 {
     fprintf (stderr, "boot_pmi: %s\n", text);
@@ -209,15 +222,18 @@ int boot_pmi (struct overlay *overlay, attr_t *attrs)
     const char *uri;
     int i;
     int upmi_flags = UPMI_LIBPMI_NOFLUX;
+    const char *upmi_method;
 
     // N.B. overlay_create() sets the tbon.topo attribute
     if (attr_get (attrs, "tbon.topo", &topo_uri, NULL) < 0) {
         log_msg ("error fetching tbon.topo attribute");
         return -1;
     }
+    if (attr_get (attrs, "broker.boot-method", &upmi_method, NULL) < 0)
+        upmi_method = NULL;
     if (getenv ("FLUX_PMI_DEBUG"))
         upmi_flags |= UPMI_TRACE;
-    if (!(upmi = upmi_create (NULL,
+    if (!(upmi = upmi_create (upmi_method,
                               upmi_flags,
                               trace_upmi,
                               NULL,
@@ -434,12 +450,14 @@ int boot_pmi (struct overlay *overlay, attr_t *attrs)
     }
 
 done:
-    if (upmi_finalize (upmi, &error) < 0) {
-        log_msg ("%s: finalize: %s", upmi_describe (upmi), error.text);
+    if (set_hostlist_attr (attrs, hl) < 0) {
+        log_err ("setattr hostlist");
         goto error;
     }
-    if (set_hostlist_attr (attrs, hl) < 0) {
-        log_err ("failed to set hostlist attribute to PMI-derived value");
+    if (set_broker_boot_method_attr (attrs, upmi_describe (upmi)) < 0)
+        goto error;
+    if (upmi_finalize (upmi, &error) < 0) {
+        log_msg ("%s: finalize: %s", upmi_describe (upmi), error.text);
         goto error;
     }
     free (bizcard);

src/broker/boot_pmi.c

@@ -197,6 +197,7 @@ int main (int argc, char *argv[])
     struct sigaction old_sigact_term;
     flux_msg_handler_t **handlers = NULL;
     const flux_conf_t *conf;
+    const char *method;
 
     memset (&ctx, 0, sizeof (ctx));
     log_init (argv[0]);
@@ -325,14 +326,20 @@ int main (int argc, char *argv[])
      * Default method is pmi.
      * If [bootstrap] is defined in configuration, use static configuration.
      */
-    if (flux_conf_unpack (conf, NULL, "{s:{}}", "bootstrap") == 0) {
-        if (boot_config (ctx.h, ctx.overlay, ctx.attrs) < 0) {
+    if (attr_get (ctx.attrs, "broker.boot-method", &method, NULL) < 0) {
+        if (flux_conf_unpack (conf, NULL, "{s:{}}", "bootstrap") == 0)
+            method = "config";
+        else
+            method = NULL;
+    }
+    if (!method || !streq (method, "config")) {
+        if (boot_pmi (ctx.overlay, ctx.attrs) < 0) {
             log_msg ("bootstrap failed");
             goto cleanup;
         }
     }
-    else { // PMI
-        if (boot_pmi (ctx.overlay, ctx.attrs) < 0) {
+    else {
+        if (boot_config (ctx.h, ctx.overlay, ctx.attrs) < 0) {
             log_msg ("bootstrap failed");
             goto cleanup;
         }

src/broker/broker.c

@@ -68,22 +68,12 @@ trace on stderr from the start command.
 
 #### Booting Flux as a job in a foreign resource manager
 
-The code in `pmiutil.c` attempts to adapt the broker's PMI client to different
-situations if the PMI-1 wire protocol is not available.  Its logic is roughly:
-1. if `PMI_FD` is set, use the PMI-1 wire protocol (for bootstrap under Flux)
-2. if configured with `--enable-pmix-bootstrap`, and `PMIX_SERVER_URI`
-or `PMIX_SERVER_URI2` is set, use PMIx
-3. if `libpmi.so` can be dynamically loaded, use PMI-1 (a specific library
-name/path may be forced by setting `PMI_LIBRARY`)
-4. assume singleton (rank = 0, size = 1)
-
-The following symbols must be defined in the PMI library for option 3 to be
-successful:
-* `PMI_Init()`, `PMI_Finalize()`
-* `PMI_Get_size()`, `PMI_Get_rank()`
-* `PMI_Barrier()`
-* `PMI_KVS_Get_my_name()`,
-* `PMI_KVS_Put()`, `PMI_KVS_Commit()`, `PMI_KVS_Get()`
+The code in `boot_pmi.c` (using libpmi/upmi.c) attempts to adapt the broker's
+PMI client to different situations if the PMI-1 wire protocol is not available.
+It tries the following in order (unless configured otherwise):
+1. simple PMI wire protocol
+2. find a PMI-1 library
+3. assume singleton (rank = 0, size = 1)
 
 ### Config File
 

src/broker/doc/bootstrap.md

@@ -75,24 +75,6 @@ static const char *env_blocklist[] = {
     "PMI_FD",
     "PMI_RANK",
     "PMI_SIZE",
-#ifdef HAVE_LIBPMIX
-    "PMIX_NAMESPACE",
-    "PMIX_VERSION",
-    "PMIX_SERVER_URI41",
-    "PMIX_RANK",
-    "PMIX_SERVER_URI21",
-    "PMIX_GDS_MODULE",
-    "PMIX_SECURITY_MODE",
-    "PMIX_HOSTNAME",
-    "PMIX_BFROP_BUFFER_TYPE",
-    "PMIX_SERVER_TMPDIR",
-    "PMIX_SYSTEM_TMPDIR",
-    "PMIX_SERVER_URI4",
-    "PMIX_MCA_psec",
-    "PMIX_SERVER_URI3",
-    "PMIX_SERVER_URI2",
-    "PMIX_SERVER_URI",
-#endif
     NULL,
 };
 

src/broker/runat.c

@@ -21,6 +21,8 @@ fluxcmd_ldadd = \
 	$(top_builddir)/src/common/libkvs/libkvs.la \
 	$(top_builddir)/src/common/librlist/librlist.la \
 	$(top_builddir)/src/common/libpmi/libupmi.la \
+	$(top_builddir)/src/common/libpmi/libpmi_client.la \
+	$(top_builddir)/src/common/libpmi/libpmi_common.la \
 	$(top_builddir)/src/common/libflux-internal.la \
 	$(top_builddir)/src/common/libflux-core.la \
 	$(top_builddir)/src/common/libflux-optparse.la \
@@ -122,6 +124,7 @@ fluxcmd_PROGRAMS = \
 flux_start_LDADD = \
 	$(fluxcmd_ldadd) \
 	$(top_builddir)/src/common/libpmi/libpmi_server.la \
+	$(top_builddir)/src/common/libpmi/libpmi_common.la \
 	$(top_builddir)/src/common/libczmqcontainers/libczmqcontainers.la
 
 flux_job_LDADD = \

src/cmd/Makefile.am

@@ -11,9 +11,6 @@
 #if HAVE_CONFIG_H
 # include <config.h>
 #endif
-#if HAVE_LIBPMIX
-# include <pmix.h>
-#endif
 #include <hwloc.h>
 #include <zmq.h>
 
@@ -66,12 +63,6 @@ static int cmd_version (optparse_t *p, int ac, char *av[])
 #endif
 #if HAVE_LIBSYSTEMD
     printf ("+systemd");
-#endif
-#if HAVE_LIBPMIX
-    printf ("+pmix-bootstrap==%ld.%ld.%ld",
-            PMIX_VERSION_MAJOR,
-            PMIX_VERSION_MINOR,
-            PMIX_VERSION_RELEASE);
 #endif
     printf ("+hwloc==%d.%d.%d",
             HWLOC_API_VERSION >> 16 & 0x000000ff,

src/cmd/builtin/version.c

@@ -168,6 +168,7 @@ flux_libpmi_la_LIBADD = \
 	libflux-taskmap.la \
 	libflux-idset.la \
 	$(builddir)/libpmi/libpmi_client.la \
+	$(builddir)/libpmi/libpmi_common.la \
 	$(builddir)/libutil/aux.lo
 flux_libpmi_la_LDFLAGS = \
 	-Wl,--version-script=$(srcdir)/libpmi.map \
@@ -181,6 +182,7 @@ flux_libpmi2_la_LIBADD = \
 	libflux-taskmap.la \
 	libflux-idset.la \
 	$(builddir)/libpmi/libpmi_client.la \
+	$(builddir)/libpmi/libpmi_common.la \
 	$(builddir)/libutil/aux.lo
 flux_libpmi2_la_LDFLAGS = \
 	-Wl,--version-script=$(srcdir)/libpmi2.map \

src/common/Makefile.am

@@ -31,6 +31,7 @@ installed_conf_cppflags = \
 	-DINSTALLED_SHELL_PLUGINPATH=\"$(fluxlibdir)/shell/plugins\" \
 	-DINSTALLED_SHELL_INITRC=\"$(fluxrcdir)/shell/initrc.lua\" \
 	-DINSTALLED_JOBTAP_PLUGINPATH=\"$(fluxlibdir)/job-manager/plugins\" \
+	-DINSTALLED_UPMI_PLUGINPATH=\"$(fluxlibdir)/upmi/plugins\" \
 	-DINSTALLED_CMDHELP_PATTERN=\"${datadir}/flux/help.d/*.json\" \
 	-DINSTALLED_NO_DOCS_PATH=\"${datadir}/flux/.nodocs\" \
 	-DINSTALLED_RUNDIR=\"${runstatedir}/flux\" \
@@ -52,6 +53,7 @@ intree_conf_cppflags = \
 	-DINTREE_SHELL_PLUGINPATH=\"$(abs_top_builddir)/src/shell/plugins\" \
 	-DINTREE_SHELL_INITRC=\"$(abs_top_srcdir)/src/shell/initrc.lua\" \
 	-DINTREE_JOBTAP_PLUGINPATH=\"$(abs_top_builddir)/src/modules/job-manager/plugins/.libs\" \
+	-DINTREE_UPMI_PLUGINPATH=\"$(abs_top_builddir)/src/common/libpmi/plugins/.libs\" \
 	-DINTREE_CMDHELP_PATTERN=\"${abs_top_builddir}/etc/flux/help.d/*.json\" \
 	-DINTREE_NO_DOCS_PATH=\"${abs_top_builddir}/etc/flux/.nodocs\" \
 	-DINTREE_BINDIR=\"${abs_top_builddir}/src/cmd\"

src/common/libflux/Makefile.am

@@ -62,6 +62,7 @@ static struct builtin builtin_tab[] = {
     { "shell_initrc",   INSTALLED_SHELL_INITRC,     INTREE_SHELL_INITRC },
     { "jobtap_pluginpath",
                         INSTALLED_JOBTAP_PLUGINPATH,INTREE_JOBTAP_PLUGINPATH },
+    { "upmi_pluginpath",INSTALLED_UPMI_PLUGINPATH,  INTREE_UPMI_PLUGINPATH },
     { "no_docs_path",   INSTALLED_NO_DOCS_PATH,     INTREE_NO_DOCS_PATH },
     { "rundir",         INSTALLED_RUNDIR,           NULL },
     { "bindir",         INSTALLED_BINDIR,           INTREE_BINDIR },