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

(GH-2922) Attempt to restore Ruby environment on local #2956

Merged
merged 1 commit into from
Aug 2, 2021

Conversation

lucywyman
Copy link
Contributor

When running over the local transport with bundled-ruby set to
false, Bolt will attempt to restore the original Ruby environment
variables, specifically:

  • GEM_PATH
  • GEM_HOME
  • RUBYLIB
  • RUBYLIB_PREFIX
  • RUBYOPT
  • RUBYPATH
  • RUBYSHELL

The Bolt binary wrapper explicitly unsets these variables to provide a
clean environment for the Bolt process to run in, which affects running
Bolt from a system package, and Bundler can also modify these
environment variables, saving their original values to
BUNDLER_ORIG_<env_var>. Changes to the Ruby environment that Bolt runs
in are incidentally picked up by Ruby tasks and plans running over the
local transport, which opens a new subprocess to execute. The binary
wrapper now stores those environment variables in BOLT_ORIG_<env_var>
environment variables, and when executing with bundled-ruby explicitly set
to false (not unconfigured), Bolt will attempt to restore the
environment variables first from BOLT_ORIG_<env_var>, then from
BUNDLER_ORIG_<env_var>.

!bug

  • Attempt to restore Ruby environment on local with bundled-ruby false (#2922)
    When executing, Bolt will unset several Ruby environment variables in
    it's own environment to avoid unexpected behavior. This can interfere
    with running Ruby on the local transport with Bolt though. Bolt will now
    attempt to restore Ruby environment variables like GEM_HOME and
    GEM_PATH when running over the local transport with bundled-ruby set
    to false.

@lucywyman lucywyman force-pushed the GH-2922 branch 3 times, most recently from c266311 to dbc6aab Compare July 27, 2021 16:17
@lucywyman lucywyman force-pushed the GH-2922 branch 2 times, most recently from 5bfad64 to 9d41fbd Compare July 27, 2021 17:04
@lucywyman
Copy link
Contributor Author

Relies on puppetlabs/bolt-vanagon#170

@lucywyman lucywyman force-pushed the GH-2922 branch 5 times, most recently from 9487f48 to d06d70c Compare July 27, 2021 22:34
@lucywyman lucywyman marked this pull request as ready for review July 27, 2021 23:02
@lucywyman lucywyman requested a review from a team as a code owner July 27, 2021 23:02
@lucywyman lucywyman force-pushed the GH-2922 branch 3 times, most recently from 1098cd4 to a60f501 Compare July 28, 2021 15:53
@lucywyman
Copy link
Contributor Author

Phew, got a successful acceptance run (with an actual value for GEM_PATH):

  * execute `bolt task run test::ruby_env` on localhost via local transport
    
    numb-separation.delivery.puppetlabs.net (numb-separation.delivery.puppetlabs.net) 10:50:08$ GEM_PATH=$(gem env GEM_PATHS) bolt task run test::ruby_env --targets localhost --modulepath /tmp/local_task.5LiGPf/modules
      Started on localhost...
      Finished on localhost:
        /root/.gem/ruby/2.5.0:/var/lib/gems/2.5.0:/usr/lib/x86_64-linux-gnu/rubygems-integration/2.5.0:/usr/share/rubygems-integration/2.5.0:/usr/share/rubygems-integration/all
      Successful on 1 target: localhost
      Ran on 1 target in 0.08 sec
    
    numb-separation.delivery.puppetlabs.net (numb-separation.delivery.puppetlabs.net) executed in 1.77 seconds
    
    numb-separation.delivery.puppetlabs.net (numb-separation.delivery.puppetlabs.net) 10:50:10$ gem env GEM_PATHS
      /root/.gem/ruby/2.5.0:/var/lib/gems/2.5.0:/usr/lib/x86_64-linux-gnu/rubygems-integration/2.5.0:/usr/share/rubygems-integration/2.5.0:/usr/share/rubygems-integration/all
    
    numb-separation.delivery.puppetlabs.net (numb-separation.delivery.puppetlabs.net) executed in 0.21 seconds

lib/bolt/shell/bash.rb Outdated Show resolved Hide resolved
When running over the `local` transport with `bundled-ruby` set to
false, Bolt will attempt to restore the original Ruby environment
variables, specifically:
* GEM_PATH
* GEM_HOME
* RUBYLIB
* RUBYLIB_PREFIX
* RUBYOPT
* RUBYPATH
* RUBYSHELL

The Bolt binary wrapper explicitly unsets these variables to provide a
clean environment for the Bolt process to run in, which affects running
Bolt from a system package, and Bundler can also modify these
environment variables, saving their original values to
`BUNDLER_ORIG_<env_var>`. Changes to the Ruby environment that Bolt runs
in are incidentally picked up by Ruby tasks and plans running over the
local transport, which opens a new subprocess to execute. The binary
wrapper now stores those environment variables in `BOLT_ORIG_<env_var>`
environment variables, and when executing with `bundled-ruby` explicitly set
to false (not unconfigured), Bolt will attempt to restore the
environment variables first from `BOLT_ORIG_<env_var>`, then from
`BUNDLER_ORIG_<env_var>`.

!bug

* **Attempt to restore Ruby environment on local with bundled-ruby false** ([puppetlabs#2922](puppetlabs#2922))
  When executing, Bolt will unset several Ruby environment variables in
  it's own environment to avoid unexpected behavior. This can interfere
  with running Ruby on the local transport with Bolt though. Bolt will now
  attempt to restore Ruby environment variables like `GEM_HOME` and
  `GEM_PATH` when running over the local transport with `bundled-ruby` set
  to false.
Copy link
Contributor

@beechtom beechtom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. 👍

lib/bolt/transport/local/connection.rb Show resolved Hide resolved
@beechtom beechtom merged commit 2551ee1 into puppetlabs:main Aug 2, 2021
beechtom added a commit to beechtom/bolt that referenced this pull request Aug 3, 2021
The environment variable for setting the beaker hosts file was changed
from `BEAKER_HOSTS` to `BEAKER_HOSTS_FILE` in puppetlabs#2956, which caused Bolt's
acceptance tests to begin failing. Tests began failing because the
vanagon integration job in Jenkins generates the hosts file and sets the
path to the file in `BEAKER_HOSTS`. This results in Bolt's acceptance
tests generating a new hosts file, since `BEAKER_HOSTS_FILE` is unset.
Generating the hosts file fails since the rake task runs
`beaker-hostgenerator` instead of the correct `bundle exec
beaker-hostgenerator`.

This resets the environment variable to `BEAKER_HOSTS` as expected by
the Jenkins job and beaker itself, and updates the shell command
executed by the acceptance test rake task.

!no-release-note
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide a way to not leak bolt runner ruby environment to tasks run over local transport
3 participants