Use the full path to `chef-solo` and `chef-client` #381

sethvargo · 2014-03-11T18:40:53Z

This fixes a bug where a cookbook (like rbenv, rvm, chruby) sets the default system Ruby. Chruby and RBenv specifically use $PATH munging, making those binaries unavailable on subsequent Test Kitchen runs.

Since other people can "touch" /usr/local, assuming the symlinks in /usr/local/bin will point to the correct places isn't a valid assumption. I bit myself with this locally when cooking up some magic. I think specifying the full path is the right thing to do.

/cc @schisamo @yzl

portertech · 2014-04-08T20:13:55Z

lib/kitchen/provisioner/chef_zero.rb

            echo ">>>>>> Attempting to use chef-zero with old version of Chef"
            echo "-----> Installing chef zero dependencies"
-            #{sudo("#{ruby_bin}/gem")} install chef --no-ri --no-rdoc --conservative
+            #{sudo(chef_bindir.join('gem'))} install chef --no-ri --no-rdoc --conservative


The chef_bindir default value is not the same as the ruby_bin default, /opt/chef/bin vs /opt/chef/embedded/bin.

@portertech I think those are symlinks though. Or am I misunderstanding your comment?

@sethvargo So @portertech is correct on this. /opt/chef/bin is not a symlink to /opt/chef/embedded/bin the gem bin only exists under /opt/chef/embedded/bin:

vagrant@master-ubuntu-1204:~$ ls -la /opt/chef/ total 20 drwxr-xr-x 4 root root 4096 Jul 29 12:55 . drwxr-xr-x 5 root root 4096 Jul 29 13:00 .. drwxr-xr-x 2 root root 4096 Jul 29 12:55 bin drwxr-xr-x 7 root root 4096 Jul 29 12:55 embedded -rw-r--r-- 1 root root 1747 Apr 30 00:31 version-manifest.txt vagrant@master-ubuntu-1204:~$ ls -la /opt/chef/bin/ total 72 drwxr-xr-x 2 root root 4096 Jul 29 12:55 . drwxr-xr-x 4 root root 4096 Jul 29 12:55 .. -rwxr-xr-x 1 root root 466 Apr 30 00:30 chef-apply -rwxr-xr-x 1 root root 467 Apr 30 00:30 chef-client -rwxr-xr-x 1 root root 476 Apr 30 00:30 chef-service-manager -rwxr-xr-x 1 root root 466 Apr 30 00:30 chef-shell -rwxr-xr-x 1 root root 465 Apr 30 00:30 chef-solo -rwxr-xr-x 1 root root 480 Apr 30 00:30 chef-zero -rwxr-xr-x 1 root root 472 Apr 30 00:30 coderay -rwxr-xr-x 1 root root 468 Apr 30 00:30 erubis -rw-r--r-- 1 root root 0 Apr 30 00:26 .gitkeep -rwxr-xr-x 1 root root 479 Apr 30 00:30 htmldiff -rwxr-xr-x 1 root root 461 Apr 30 00:30 knife -rwxr-xr-x 1 root root 476 Apr 30 00:30 ldiff -rwxr-xr-x 1 root root 460 Apr 30 00:30 ohai -rwxr-xr-x 1 root root 456 Apr 30 00:30 pry -rwxr-xr-x 1 root root 462 Apr 30 00:30 rackup -rwxr-xr-x 1 root root 487 Apr 30 00:30 restclient -rwxr-xr-x 1 root root 460 Apr 30 00:30 shef vagrant@master-ubuntu-1204:~$ ls -la /opt/chef/embedded/ total 28 drwxr-xr-x 7 root root 4096 Jul 29 12:55 . drwxr-xr-x 4 root root 4096 Jul 29 12:55 .. drwxr-xr-x 2 root root 4096 Jul 29 12:55 bin drwxr-xr-x 6 root root 4096 Jul 29 12:55 include drwxr-xr-x 5 root root 4096 Jul 29 12:55 lib drwxr-xr-x 8 root root 4096 Jul 29 12:55 share drwxr-xr-x 4 root root 4096 Jul 29 12:55 ssl vagrant@master-ubuntu-1204:~$ ls -la /opt/chef/embedded/bin/ total 1636 drwxr-xr-x 2 root root 4096 Jul 29 12:55 . drwxr-xr-x 7 root root 4096 Jul 29 12:55 .. -rwxr-xr-x 1 root root 472 Apr 30 00:31 autospec -rwxr-xr-x 1 root root 471 Apr 30 00:26 bundle -rwxr-xr-x 1 root root 472 Apr 30 00:26 bundler lrwxrwxrwx 1 root root 3 Apr 30 00:26 captoinfo -> tic -rwxr-xr-x 1 root root 8573 Apr 30 00:26 clear -rwxr-xr-x 1 root root 4273 Apr 30 00:26 c_rehash -rwxr-xr-x 1 root root 4407 Apr 30 00:26 erb -rwxr-xr-x 1 root root 897 Apr 30 00:26 gem -rw-r--r-- 1 root root 0 Apr 30 00:26 .gitkeep -rwxr-xr-x 1 root root 72961 Apr 30 00:26 infocmp lrwxrwxrwx 1 root root 3 Apr 30 00:26 infotocap -> tic -rwxr-xr-x 1 root root 328 Apr 30 00:26 irb -rwxr-xr-x 1 root root 46587 Apr 30 00:26 makedepend -rwxr-xr-x 1 root root 5326 Apr 30 00:26 ncurses5-config -rwxr-xr-x 1 root root 5330 Apr 30 00:26 ncursesw5-config -rwxr-xr-x 1 root root 594438 Apr 30 00:26 openssl -rwxr-xr-x 1 root root 593744 Apr 30 00:26 pkg-config -rwxr-xr-x 1 root root 450 Apr 30 00:30 rake -rwxr-xr-x 1 root root 798 Apr 30 00:26 rdoc lrwxrwxrwx 1 root root 4 Apr 30 00:26 reset -> tset -rwxr-xr-x 1 root root 198 Apr 30 00:26 ri -rwxr-xr-x 1 root root 469 Apr 30 00:31 rspec -rwxr-xr-x 1 root root 11959 Apr 30 00:26 ruby -rwxr-xr-x 1 root root 18277 Apr 30 00:26 tabs -rwxr-xr-x 1 root root 77602 Apr 30 00:26 testgdbm -rwxr-xr-x 1 root root 308 Apr 30 00:26 testrb -rwxr-xr-x 1 root root 77897 Apr 30 00:26 tic -rwxr-xr-x 1 root root 18880 Apr 30 00:26 toe -rwxr-xr-x 1 root root 18842 Apr 30 00:26 tput -rwxr-xr-x 1 root root 29028 Apr 30 00:26 tset

tobiaspanknin · 2014-07-29T12:25:45Z

I'd love to see this branch integrated, too. Making the full path to chef configurable would enable me to use omnibus chef 11 in /opt/chef on an image containing an already installed chef 10 under /usr/local/bin.

This fixes a bug where a cookbook (like rbenv, rvm, chruby) sets the default system Ruby. Chruby and RBenv specifically use $PATH munging, making those binaries unavailable on subsequent Test Kitchen runs.

sethvargo · 2014-07-29T14:43:29Z

@portertech updated. I have a failing test locally on ssh_base, but I don't see how it's related. A second set of eyes would be appreciated /cc @fnichol @schisamo

sethvargo · 2014-07-29T14:46:19Z

Interesting... the unit tests pass on Travis, but mine are failing locally with:

Kitchen::SSH::#upload_path!#test_0002_logs upload progress to debug [/Users/sethvargo/Development/test-kitchen/spec/kitchen/ssh_spec.rb:423]:
Expected /^D, .* : Uploaded\ \/tmp\/local20140729\-76469\-1u6xj7s\/alpha\ \(15\ bytes\)$/ to match "D, [2014-07-29T10:39:41.131543 #76469] DEBUG -- : [SSH] opening connection to me@foo:22<{}>\nD, [2014-07-29T10:39:41.132374 #76469] DEBUG -- : Uploaded /var/folders/qg/njz75n6s7732p84cl5sndv580000gn/T/local20140729-76469-1u6xj7s/alpha (15 bytes)\nD, [2014-07-29T10:39:41.132888 #76469] DEBUG -- : Uploaded /var/folders/qg/njz75n6s7732p84cl5sndv580000gn/T/local20140729-76469-1u6xj7s/subdir/beta (14 bytes)\nD, [2014-07-29T10:39:41.133291 #76469] DEBUG -- : Uploaded /var/folders/qg/njz75n6s7732p84cl5sndv580000gn/T/local20140729-76469-1u6xj7s/zulu (14 bytes)\n".

fnichol · 2014-07-29T19:36:33Z

@sethvargo looks like one failure on the Travis side, trying to resubmit that job/build now

sethvargo · 2014-07-29T19:43:31Z

@fnichol yea, but the tests are failing locally (for a different reason). It looks like one of the tests assumes tempfiles will be in /tmp, but they are /var/ on OSX

tobiaspanknin · 2014-07-29T20:01:34Z

I had that problem in my fork too, turns out DIR.mktmpdir() and Dir.tmpdir() resolve to /var/folders under MacOS X

Here's a diff of the changes I made to the test to get it running again:

--- a/spec/kitchen/ssh_spec.rb
+++ b/spec/kitchen/ssh_spec.rb
@@ -318,7 +318,7 @@ describe Kitchen::SSH do
     end

     before do
-      expect_scp_session("-t /tmp/remote") do |channel|
+      expect_scp_session("-t #{Dir.tmpdir}/remote") do |channel|
         channel.gets_data("\0")
         channel.sends_data("C0755 1234 #{File.basename(src.path)}\n")
         channel.gets_data("\0")
@@ -334,13 +334,13 @@ describe Kitchen::SSH do

     it "uploads a file to remote over scp" do
       assert_scripted do
-        ssh.upload!(src.path, "/tmp/remote")
+        ssh.upload!(src.path, "#{Dir.tmpdir}/remote")
       end
     end

     it "logs upload progress to debug" do
       assert_scripted do
-        ssh.upload!(src.path, "/tmp/remote")
+        ssh.upload!(src.path, "#{Dir.tmpdir}/remote")
       end

       logged_output.string.must_match debug_line(
@@ -366,7 +366,7 @@ describe Kitchen::SSH do
       File.open("#{@dir}/zulu", "wb") { |f| f.write("zulu-contents\n") }
       FileUtils.chmod(0444, "#{@dir}/zulu")

-      expect_scp_session("-t -r /tmp/remote") do |channel|
+      expect_scp_session("-t -r #{Dir.tmpdir}/remote") do |channel|
         channel.gets_data("\0")
         channel.sends_data("D0700 0 #{File.basename(@dir)}\n")
         channel.gets_data("\0")
@@ -400,15 +400,15 @@ describe Kitchen::SSH do

     it "uploads a file to remote over scp" do
       with_sorted_dir_entries do
-        assert_scripted { ssh.upload_path!(@dir, "/tmp/remote") }
+        assert_scripted { ssh.upload_path!(@dir, "#{Dir.tmpdir}/remote") }
       end
     end

     it "logs upload progress to debug" do
-      remote_base = "/tmp/#{File.basename(@dir)}"
+      remote_base = "#{Dir.tmpdir}/#{File.basename(@dir)}"

       with_sorted_dir_entries do
-        assert_scripted { ssh.upload_path!(@dir, "/tmp/remote") }
+        assert_scripted { ssh.upload_path!(@dir, "#{Dir.tmpdir}/remote") }
       end

hth

sethvargo · 2014-07-29T21:32:20Z

@fnichol this is green now 😄. Can I merge plz 😄

sethvargo · 2014-09-15T16:01:47Z

Ping @fnichol @portertech

@sethvargo

This commit adds 3 new tunables: 1 for all Chef provisioners, 1 for `chef_solo`, and 1 for `chef_zero` and builds on @sethvargo's previous commit. **:chef_omnibus_root** which defaults to `"/opt/chef"` is the location in which Kitchen will check when determining if a Chef Omnibus package is installed. Modifying this attribute will allow users to use alternative Omnibus packages and is most likely always reasonable with its default value. **:chef_solo_path** is used with the `chef_solo` provisioner, and is the full path on the remote instance to the `chef-solo` binary, and by default is calculated using the `:chef_omnibus_root` attribute as its base. For example, if the Omnibus root is `"/opt/chef"`, then the `:chef_solo_path` value will be calculated as `"/opt/chef/bin/chef-solo"`. If the `chef-solo` command is located elsewhere, this value can be set in a provisioner block to override the default. For example, setting this attribute to `"chef-solo"` will invoke the command without an absolute path and rely on `$PATH` lookup. **:chef_client_path** is used with the `chef_zero` provisioner, and is the full path on the remote instance to the `chef-client` binary, and by default is calculated using the `:chef_omnibus_root` attribute as its base. For example, if the Omnibus root is `"/opt/chef"`, then the `:chef_client_path` value will be calculated as `"/opt/chef/bin/chef-client"`. If the `chef-client` command is located elsewhere, this value can be set in a provisioner block to override the default. For example, setting this attribute to `"chef-client"` will invoke the command without an absolute path an rely on `$PATH` lookup. Here is a concrete example of overriding `:chef_solo_path` and `chef_client_path` in a `.kitchen.yml` file: --- driver: name: vagrant platforms: - name: ubuntu-14.04-solo provisioner: name: chef_solo chef_solo_path: /mnt/alt/bin/chef-solo - name: ubuntu-14.04-client provisioner: name: chef_zero chef_client_path: /bin/chef-client suites: - name: default **Warning:** previous logic used a non-absolute path to invoke both `chef-solo` and `chef-zero` which means that this commit has the potential to cause some Kitchen runs to fail, assuming that the Chef being invoked before was not actually `/opt/chef/bin/chef-solo` or `/opt/chef/bin/chef-client`. In this case, users can override the new computed default with a value of `"chef-solo"` or `"chef-client"` to exactly preserve previous behavior.

fnichol · 2014-10-17T18:46:04Z

@sethvargo I did some thinking on this & hopefully tweaked this slightly so as to allow for more customizing down the road. Namely, splitting out the path to chef-solo and chef-client to be first class tunables. What do you think?

sethvargo · 2014-10-17T19:35:10Z

This looks great to me 😄 👍

fnichol · 2014-10-17T20:02:29Z

Use the full path to `chef-solo` and `chef-client`

portertech reviewed Apr 8, 2014
View reviewed changes

Use the full path to chef-solo and chef-client

e2a508e

This fixes a bug where a cookbook (like rbenv, rvm, chruby) sets the default system Ruby. Chruby and RBenv specifically use $PATH munging, making those binaries unavailable on subsequent Test Kitchen runs.

fnichol added a commit that referenced this pull request Oct 17, 2014

Merge pull request #381 from test-kitchen/sethvargo/full_chef_path

a83398e

Use the full path to `chef-solo` and `chef-client`

fnichol merged commit a83398e into master Oct 17, 2014

fnichol deleted the sethvargo/full_chef_path branch October 17, 2014 20:02

fnichol mentioned this pull request Oct 17, 2014

Added chef_solo_command config option #456

Closed

test-kitchen locked and limited conversation to collaborators Nov 16, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use the full path to `chef-solo` and `chef-client` #381

Use the full path to `chef-solo` and `chef-client` #381

sethvargo commented Mar 11, 2014

portertech Apr 8, 2014

sethvargo Apr 8, 2014

schisamo Jul 29, 2014

tobiaspanknin commented Jul 29, 2014

sethvargo commented Jul 29, 2014

sethvargo commented Jul 29, 2014

fnichol commented Jul 29, 2014

sethvargo commented Jul 29, 2014

tobiaspanknin commented Jul 29, 2014

sethvargo commented Jul 29, 2014

sethvargo commented Sep 15, 2014

fnichol commented Oct 17, 2014

sethvargo commented Oct 17, 2014

fnichol commented Oct 17, 2014

Use the full path to chef-solo and chef-client #381

Use the full path to chef-solo and chef-client #381

Conversation

sethvargo commented Mar 11, 2014

portertech Apr 8, 2014

Choose a reason for hiding this comment

sethvargo Apr 8, 2014

Choose a reason for hiding this comment

schisamo Jul 29, 2014

Choose a reason for hiding this comment

tobiaspanknin commented Jul 29, 2014

sethvargo commented Jul 29, 2014

sethvargo commented Jul 29, 2014

fnichol commented Jul 29, 2014

sethvargo commented Jul 29, 2014

tobiaspanknin commented Jul 29, 2014

sethvargo commented Jul 29, 2014

sethvargo commented Sep 15, 2014

fnichol commented Oct 17, 2014

sethvargo commented Oct 17, 2014

fnichol commented Oct 17, 2014

Use the full path to `chef-solo` and `chef-client` #381

Use the full path to `chef-solo` and `chef-client` #381