Merge pull request flux-framework#2866 from garlick/no_default_config

config: parse TOML once in broker, share with modules

configure.ac

@@ -391,9 +391,6 @@ AC_SUBST(fluxrc1dir)
 AS_VAR_SET(fluxrc3dir, $sysconfdir/flux/rc3.d)
 AC_SUBST(fluxrc3dir)
 
-AS_VAR_SET(fluxcfdir, $sysconfdir/flux/conf.d)
-AC_SUBST(fluxcfdir)
-
 AS_VAR_SET(fluxlibexecdir, $libexecdir/flux)
 AC_SUBST(fluxlibexecdir)
 

doc/man5/flux-config-bootstrap.adoc

@@ -17,21 +17,17 @@ 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.
+The default bootstrap mode is PMI.  To select config file bootstrap,
+specify the config directory with the `--config-path=PATH` broker command
+line option or set `FLUX_CONF_DIR` in the broker's environment.  Ensure that
+this directory contains a file that defines the `bootstrap` section.
 
 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.
+Flux uses the TOML configuration file format.  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
 --------

etc/Makefile.am

@@ -2,9 +2,6 @@
 systemdsystemunit_DATA = flux.service
 #endif
 
-dist_fluxcf_DATA = \
-	conf.d/bootstrap.toml
-
 noinst_DATA = \
 	flux/curve
 

etc/flux.service.in

@@ -6,9 +6,9 @@ Environment=FLUX_USERDB_OPTIONS=--default-rolemask=user
 TimeoutStopSec=90
 KillMode=mixed
 ExecStart=@X_BINDIR@/flux broker \
+  --config-path=@X_SYSCONFDIR@/flux/system/conf.d \
   -Srundir=@X_RUNSTATEDIR@/flux \
   -Slocal-uri=local://@X_RUNSTATEDIR@/flux/local \
-  -Sboot.method=config \
   -Slog-stderr-level=6 \
   -Scontent.backing-path=@X_LOCALSTATEDIR@/lib/flux/content.sqlite \
   -Sbroker.rc2_none

src/broker/Makefile.am

@@ -20,6 +20,8 @@ flux_broker_SOURCES = \
 	broker.c
 
 libbroker_la_SOURCES = \
+	brokercfg.c \
+	brokercfg.h \
 	module.c \
 	module.h \
 	modservice.c \

src/broker/boot_config.c

@@ -162,20 +162,21 @@ static json_t *boot_config_expand_hosts (json_t *hosts)
 }
 
 /* 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)
+int boot_config_parse (const flux_conf_t *cf,
+                       struct boot_conf *conf,
+                       json_t **hostsp)
 {
     flux_conf_error_t error;
     const char *default_bind = NULL;
     const char *default_connect = NULL;
     json_t *hosts = NULL;
 
     memset (conf, 0, sizeof (*conf));
-    if (flux_conf_unpack (flux_get_conf (h, NULL),
+    if (flux_conf_unpack (cf,
                           &error,
                           "{s:{s?:i s?:s s?:s s?:o}}",
                           "bootstrap",
@@ -367,13 +368,13 @@ int boot_config (flux_t *h, overlay_t *overlay, attr_t *attrs, int tbon_k)
      * config boot method as it would be overwritten below.
      */
     if (attr_get (attrs, "tbon.endpoint", NULL, NULL) == 0) {
-        log_msg ("attr tbon.endpoint may not be set with boot_method=config");
+        log_msg ("attr tbon.endpoint may not be set with [bootstrap] config");
         return -1;
     }
 
     /* Ingest the [bootstrap] stanza.
      */
-    if (boot_config_parse (h, &conf, &hosts) < 0)
+    if (boot_config_parse (flux_get_conf (h), &conf, &hosts) < 0)
         return -1;
 
     /* If hosts array was specified, match hostname to determine rank,

src/broker/boot_config.h

@@ -46,7 +46,9 @@ int boot_config_getbindbyrank (json_t *hosts,
 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_parse (const flux_conf_t *cf,
+                       struct boot_conf *conf,
+                       json_t **hosts);
 int boot_config_format_uri (char *buf,
                             int bufsz,
                             const char *fmt,

src/broker/broker.c

@@ -59,6 +59,7 @@
 #include "src/common/libutil/errno_safe.h"
 
 #include "heartbeat.h"
+#include "brokercfg.h"
 #include "module.h"
 #include "overlay.h"
 #include "service.h"
@@ -101,6 +102,8 @@ typedef struct {
     zlist_t *sigwatchers;
     struct service_switch *services;
     heartbeat_t *heartbeat;
+    struct brokercfg *config;
+    const char *config_path;
     struct shutdown *shutdown;
     double shutdown_grace;
     double heartbeat_rate;
@@ -165,11 +168,9 @@ static void init_attrs (attr_t *attrs, pid_t pid);
 
 static const struct flux_handle_ops broker_handle_ops;
 
-static int parse_config_files (flux_t *h);
-
 static int exit_rc = 1;
 
-#define OPTIONS "+vM:X:k:s:g:EIS:"
+#define OPTIONS "+vs:X:k:H:g:S:c:"
 static const struct option longopts[] = {
     {"verbose",         no_argument,        0, 'v'},
     {"security",        required_argument,  0, 's'},
@@ -178,6 +179,7 @@ static const struct option longopts[] = {
     {"heartrate",       required_argument,  0, 'H'},
     {"shutdown-grace",  required_argument,  0, 'g'},
     {"setattr",         required_argument,  0, 'S'},
+    {"config-path",     required_argument,  0, 'c'},
     {0, 0, 0, 0},
 };
 
@@ -192,6 +194,7 @@ static void usage (void)
 " -H,--heartrate SECS          Set heartrate in seconds (rank 0 only)\n"
 " -g,--shutdown-grace SECS     Set shutdown grace period in seconds\n"
 " -S,--setattr ATTR=VAL        Set broker attribute\n"
+" -c,--config-path PATH        Set broker config directory (default: none)\n"
 );
     exit (1);
 }
@@ -252,6 +255,9 @@ void parse_command_line_arguments (int argc, char *argv[], broker_ctx_t *ctx)
             free (attr);
             break;
         }
+        case 'c': /* --config-path PATH */
+            ctx->config_path = optarg;
+            break;
         default:
             usage ();
         }
@@ -305,7 +311,7 @@ int main (int argc, char *argv[])
     struct sigaction old_sigact_int;
     struct sigaction old_sigact_term;
     flux_msg_handler_t **handlers = NULL;
-    const char *boot_method;
+    const flux_conf_t *conf;
 
     memset (&ctx, 0, sizeof (ctx));
     log_init (argv[0]);
@@ -387,12 +393,13 @@ int main (int argc, char *argv[])
         goto cleanup;
     }
 
-    if (increase_rlimits () < 0)
+    /* Parse config.
+     */
+    if (!(ctx.config = brokercfg_create (ctx.h, ctx.config_path, ctx.attrs)))
         goto cleanup;
+    conf = flux_get_conf (ctx.h);
 
-    /* Parse config file(s).  The result is cached in ctx.h.
-     */
-    if (parse_config_files (ctx.h) < 0)
+    if (increase_rlimits () < 0)
         goto cleanup;
 
     /* Prepare signal handling
@@ -447,29 +454,17 @@ int main (int argc, char *argv[])
      */
     overlay_set_init_callback (ctx.overlay, create_broker_rundir, ctx.attrs);
 
-    /* Execute boot method selected by 'boot.method' attr.
-     * Default is pmi.
+    /* Execute broker network bootstrap.
+     * Default method is pmi.
+     * If [bootstrap] is defined in configuration, use static configuration.
      */
-    if (attr_get (ctx.attrs, "boot.method", &boot_method, NULL) < 0) {
-        boot_method = "pmi";
-        if (attr_add (ctx.attrs, "boot.method", boot_method, 0)) {
-            log_err ("setattr boot.method");
-            goto cleanup;
-        }
-    }
-    if (attr_set_flags (ctx.attrs,
-                        "boot.method",
-                        FLUX_ATTRFLAG_IMMUTABLE) < 0) {
-        log_err ("attr_set_flags boot.method");
-        goto cleanup;
-    }
-    if (!strcmp (boot_method, "config")) {
+    if (flux_conf_unpack (conf, NULL, "{s:{}}", "bootstrap") == 0) {
         if (boot_config (ctx.h, ctx.overlay, ctx.attrs, ctx.tbon_k) < 0) {
             log_msg ("bootstrap failed");
             goto cleanup;
         }
     }
-    else if (!strcmp (boot_method, "pmi")) {
+    else { // PMI
         double elapsed_sec;
         struct timespec start_time;
         monotime (&start_time);
@@ -481,10 +476,6 @@ int main (int argc, char *argv[])
         flux_log (ctx.h, LOG_INFO, "pmi: bootstrap time %.1fs", elapsed_sec);
 
     }
-    else {
-        log_err ("unknown boot method: %s", boot_method);
-        goto cleanup;
-    }
     uint32_t rank = overlay_get_rank (ctx.overlay);
     uint32_t size = overlay_get_size (ctx.overlay);
     char rank_str[16];
@@ -773,6 +764,7 @@ int main (int argc, char *argv[])
     shutdown_destroy (ctx.shutdown);
     broker_remove_services (handlers);
     publisher_destroy (ctx.publisher);
+    brokercfg_destroy (ctx.config);
     flux_close (ctx.h);
     flux_reactor_destroy (ctx.reactor);
     zlist_destroy (&ctx.subscriptions);
@@ -863,29 +855,6 @@ static void init_attrs (attr_t *attrs, pid_t pid)
         log_err_exit ("attr_add version");
 }
 
-/* Parse TOML config, emitting any parse error here.
- * This will fail if no configuration exists.
- */
-static int parse_config_files (flux_t *h)
-{
-    flux_conf_error_t error;
-
-    if (flux_get_conf (h, &error) == NULL) {
-        if (error.lineno == -1)
-            log_err ("Config file error: %s%s%s",
-                     error.filename,
-                     *error.filename ? ": " : "",
-                     error.errbuf);
-        else
-            log_err ("Config file error: %s:%d: %s",
-                     error.filename,
-                     error.lineno,
-                     error.errbuf);
-        return -1;
-    }
-    return 0;
-}
-
 static void hello_update_cb (hello_t *hello, void *arg)
 {
     broker_ctx_t *ctx = arg;