Uploaded image for project: 'Puppet Server'
  1. Puppet Server
  2. SERVER-297

Consolidate environment variable handling behaviors



    • Type: Improvement
    • Status: Closed
    • Priority: Normal
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: SERVER 2.1.1
    • Component/s: Puppet Server
    • Labels:
    • Template:
    • Sub-team:
    • Story Points:
    • Sprint:
      Server Emerald 2015-05-13, Server Emerald 2015-05-27, Server Emerald 2015-06-10
    • CS Priority:
    • Release Notes:
      Bug Fix


      The way puppet server handles environment variables should be consolidated and normalized across the various use cases of the production service and all subcommand invocations.

      This came up in the discussion on https://github.com/puppetlabs/puppet-server/pull/354

      Also, interested in hashing out the whole discussion on clearing/overriding/preserving of different environment variables. You have some great points there. Wanted to get some other opinions to see where others are on that.

      Yeah, maybe we should just pull that commit out of this PR and reconsider our environment support both for the CLI and the production scripting containers as a separate topic?

      I'd be good with separating them, too, although I think the tests would need to be rewritten a bit to work for the code as currently implemented.

      I think it also would be good to cover the environment rework stuff under a new JIRA ticket and expand the scope to include reworking the production ScriptingContainer code to be consistent with that of the subcommand tools.

      Some work had been done in SERVER-262 – and committed for the SERVER 1.0.2 release – to fix the "puppetserver gem env" command. Prior to SERVER-262, running the "puppetserver gem env" command would result in an error:

      [vagrant@localhost ~]$ puppetserver gem env
      ERROR:  While executing gem ... (NoMethodError)
          undefined method `split' for nil:NilClass

      SERVER-262 was later reverted in SERVER-522 – SERVER 1.0.3 release – because of some concerns about the new behavior which would have allowed the GEM_PATH variable to flow through from the user's environment into the JRuby environment in a "puppetserver gem" command execution. In addition to the overall work to consolidate environment handling behavior, this ticket should also cover restoring "puppetserver gem env" to working order.

      UPDATE - One of the pieces we'll need to reconcile as part of this work is how the ruby-load-path gets set. In the irb.clj subcommand, the load path is front-loaded with some embedded JRuby lib paths as it is set onto the ScriptingContainer. See: https://github.com/puppetlabs/puppet-server/blob/puppet-server-1.0.8/src/clj/puppetlabs/puppetserver/cli/irb.clj#L20-L24. This was done in order to avoid some errors that were otherwise being produced by the system rubygems. For ScriptingContainer initialization in production puppetserver code, however, this front-loading was not being done. In SERVER-538, we found that the hipchat gem could not be loaded correctly unless the JRuby paths were front-loaded onto the load path on CentOS 6. The front-loading approach may, therefore, be something we should do for production code as well. We'd have to consider the best way to do that, though. For example, should the JRuby lib paths just be frontloaded onto the beginning of the list of load paths or could the "ruby-load-path" configuration variable provide a way for the user to designate a location in the list of paths into which the JRuby libs are expanded?

      UPDATE (2) - To avoid the risk of loading too much work into this one ticket, I'd propose that we limit the scope of this ticket to just consolidating the ScriptingContainer initialization behaviors for now and defer related work out to the implementation for linked tickets:

      • SERVER-377 - proxy support for puppetserver gem
      • SERVER-156 - proxy support for http client in production puppetserver master code
      • SERVER-584 - ability to specify arbitrary values for environment variables

      The work for SERVER-297, then, would be about ensuring that all of the puppetserver CLI tools and production puppetserver code stack use the same ScriptingContainer initialization logic. The work to be done for the proxy and arbitrary environment variable tickets may alter the original implementation but for now the implementation would just include the following with respect to the JRubyPuppet environment initialization:

      1) Set GEM_HOME from gem-home in the jruby-puppet config as before.
      2) Set JARS_NO_REQUIRE to true.
      3) Set HOME and PATH from the corresponding environment variables in the shell. Production Puppet Server code whitelists these for Facter resolution. This is also needed to allow the irb subcommand to continue working and to restore puppetserver gem env to working order. Note that as we do SERVER-584 later that we may decide to derive this only from a configuration setting instead.
      4) Set load path from the ruby-load-path setting in os-settings/jruby-puppet as appropriate per branch. For Server 1.X only, add in the "front-loading" of the load path with embedded JRuby path libs, as is done for the irb subcommand today - see https://github.com/puppetlabs/puppet-server/blob/puppet-server-1.0.8/src/clj/puppetlabs/puppetserver/cli/irb.clj#L20-L24. This is needed for the irb command to continue working without errors and to address SERVER-538. The use case outlined in SERVER-538 of being able to properly load the hipchat gem should be validated against this work on Server 1.x to confirm that the issue has been addressed as expected. Note that this is only strictly needed for Server 1.X and not Server 2.X because for Server 2.x, core Ruby Puppet code is loaded into a separate directory from the system one.

      Note that I'm not entirely sure if the above changes will be sufficient for the "ruby" subcommand to continue working properly. It may have dependencies on flowing through other environment variables which would need to be discovered as this work is undertaken.


          Issue Links



              erik Erik Dasher
              jeff Jeff McCune
              QA Contact:
              Erik Dasher Erik Dasher
              1 Vote for this issue
              8 Start watching this issue



                  Zendesk Support