Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

simplify Flux environment handling, drop config file support #674

Merged
merged 7 commits into from May 16, 2016

Conversation

Projects
None yet
3 participants
@garlick
Copy link
Member

garlick commented May 16, 2016

This PR is an attempt to further simplify the "flux configuration":

  • drop the ZPL config file and compile in the "in-tree" paths
  • turn the flux_conf class into an accessor for compiled-in paths (flags select in-tree) rather than defining them in Makefile.am's all over the place
  • flux(1) sets the configuration in the outbound environment based on inbound environment, in-tree vs installed (determined heuristically), and compiled-in paths
  • broker mirrors configuration as broker attributes
  • other components do not reprocess config; they get it from environment or from broker attributes

The initial motivation for this PR was just to provide "fully cooked" configuration values via broker attributes to wrexec lua plugisn; however, once inside the config machinery, it became a cleanup effort.

Think of it as removing a lot of half-baked gunk that clouds our thinking about what a config file should be.

This PR will need to go in before pr #671.

Note: this PR stops setting LD_LIBRARY_PATH and I_MPI_PMI_LIBRARY_PATH in the initial program environment. That will temporarily break the ability to launch mvapich and Intel MPI with Flux, until #671 is merged. However, it doesn't break any Flux tests. #671 should go in fairly soon after this one though.

garlick added some commits May 10, 2016

libflux/conf: make flux_conf def comply with RFC 7
Make flux_conf_t a struct rather than a pointer to one per RFC 7.

Update users.
libutil/environment: extract class from flux_conf
The environment class can stand alone and does not need
to be tied to the beleaguered flux_conf_t class.  It also
doesn't need to be exposed in the Flux API.
cmd/flux: rework env/config usage
Give flux(1) the exclusive role of detecting whether flux is running
in installed or in-tree modes, and combining compiled-in paths with
incoming environment to determine outgoing environment.
The flux(1) outgoing environment contains the entire configuration,
which can then be taken up by the broker and mirrored in broker
attributes.

Flux components that have a broker handle can access the broker
attribitues with flux_attr_get().  Sub-commands are also free
to use environment the variables set by flux(1) directly.

Search paths that are modified by flux(1) are de-duplicated so
environment modification should be more or less idempotent,
and running flux(1) within the environment of a Flux instance
should not result in duplicate entries.

Drop flux --uri and --trace options and anticipate that FLUX_URI
or FLUX_HANDLE_TRACE will be set in the inbound environment
if those settings need to be influenced.  Drop --config option
since there is no longer a config file.

Update the following subcommands:
flux-env:
    Use environment rather than flux_conf to print the outgoing
    environment settings.

flux-help:
    Assume MANPATH is already set to include flux man pages.

flux-keygen:
    Add --secdir option to compensate for loss of flux(1) --secdir
    option, needed in tests.

flux-module:
    Assume FLUX_MODULE_PATH is set in the environment.
    Don't fallback to compiled-in default.

Update tests for new flux(1) options:
    t/t0001-basic
    t/t1005-cmddriver.t
    t/t0001-basic.t
    t/t2002-pmi
libflux/conf: make accessor for compiled-in paths
Demote flux_conf class to a mere accessor for compiled-in paths.
If flux_conf_get flags argument is zero, install paths are returned.
If flags is CONF_FLAG_INTREE, paths based on source/build tree are
returned.

Drop the in-tree config file (and support for ZPL config files
generally), as the in-tree config file reflected only the compiled-in
paths set by autoconf.

Drop the special compiled-in paths sprinkled around Makefile.am's,
and have users call the accessor.  In this commit, primarily
tests that behaved that way are updated, and also the fall-back
connector path is dropped from libflux/handle.c.
broker: get config from environment
Rely on flux(1) to set up environment to reflect configuration.
Mirror key environment variables as flux attributes for use by
other flux components.
wreck: don't use compiled-in paths directly
wrexec: obtain wrexecd path from broker attr
wrexecd: obtain lua pattern from broker attr

@garlick garlick added the review label May 16, 2016

@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented May 16, 2016

Looks good to me. Once Travis is green we can merge this one!

@coveralls

This comment has been minimized.

Copy link

coveralls commented May 16, 2016

Coverage Status

Coverage decreased (-0.2%) to 74.165% when pulling a8e67e3 on garlick:conf_emasculate into 39f2ccf on flux-framework:master.

@grondo grondo merged commit 1897035 into flux-framework:master May 16, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.2%) to 74.165%
Details

@grondo grondo removed the review label May 16, 2016

@grondo grondo referenced this pull request May 16, 2016

Merged

environment cleanup #671

0 of 2 tasks complete

@garlick garlick deleted the garlick:conf_emasculate branch May 17, 2016

@garlick garlick referenced this pull request Aug 2, 2016

Closed

0.4.0 release notes #739

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.