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

Detect chef provisioners by type instead of by name (multi-machine), fix #101 #103

Merged

Conversation

jperville
Copy link
Contributor

@tmatilai, here is my modest attempt at fixing #101 (where chef provisioner detection was broken in multi-machine Vagrantfiles). I have tested by hand that the PR works in both single-machine and multi-machine modes, with the following Vagrantfile:

# -*- mode: ruby -*-
# vi: set ft=ruby :

Vagrant.configure("2") do |config|
  # Every Vagrant virtual environment requires a box to build off of.
  config.vm.box      = 'opscode-ubuntu-14.04-chef'
  config.vm.box_url  = 'http://opscode-vm-bento.s3.amazonaws.com/vagrant/virtualbox/opscode_ubuntu-14.04_chef-provisionerless.box'

  provision = lambda { |chef|
    chef.add_recipe "apt::default"
  }

  if String(ENV['MULTIVM']) == 'true'
    config.vm.define "multi" do |machine|
      machine.vm.hostname = 'multi'
      config.vm.provision(:chef_solo, &provision)
    end
  else
    config.vm.hostname = 'single'
    config.vm.provision(:chef_solo, &provision)
  end
end

In single-machine mode:

jp440p:u $ MULTIVM=true vagrant ssh -- 'hostname && grep no_proxy /tmp/vagrant-chef-*/solo.rb'
multi
no_proxy "localhost,127.0.0.1,172.17.42.1,10.0.3.1,192.168.56.1,192.168.33.1,192.168.122.1,192.168.33.1,192.168.33.10.xip.io,192.168.33.10.xip.io:81"

In multi-machine mode:

jp440p:u $ MULTIVM=false vagrant ssh -- 'hostname && grep no_proxy /tmp/vagrant-chef-*/solo.rb'
single
no_proxy "localhost,127.0.0.1,172.17.42.1,10.0.3.1,192.168.56.1,192.168.33.1,192.168.122.1,192.168.33.1,192.168.33.10.xip.io,192.168.33.10.xip.io:81"

(without the PR, it would return nil as the value of the chef no_proxy in the multi-vm case).

Feel free to merge, close #101 and enjoy Christmas @tmatilai.

@tmatilai
Copy link
Owner

Unfortunately it seems that the type method came only in Vagrant 1.7, and we don't want to break compatibility with older versions yet.

Did the multi-vm setup work with Vagrant 1.6.5? If yes, we could maybe test if prov.respond_to?(:type) and use the correct attribute based on that.

@jperville
Copy link
Contributor Author

Well spotted, I will investigate more during the week-end.

@jperville
Copy link
Contributor Author

Here is my updated version of this PR, which is backward-compatible with Vagrant 1.6.5 (and probably older versions). After a quick check, it seems that the issue only applies to the chef proxy, so no related PR should be needed.

I have tested the following cases:

vagrant with fix without fix
1.6.5 pass pass
1.7.1 pass break

Note: results were identical in the single and multi machine cases.

@jperville
Copy link
Contributor Author

According to hashicorp/vagrant#5069 Vagrant won't provide a backward-compatibility (it would mess the internals too much); plugins must be updated to handle the pre and post vagrant-1.7 cases.

Can you merge this @tmatilai and (if possible) issue an official 1.4.1 release with the recent fixes?

tmatilai added a commit that referenced this pull request Jan 7, 2015
Detect chef provisioners by type instead of by name (multi-machine), fix #101
@tmatilai tmatilai merged commit b55b4a3 into tmatilai:master Jan 7, 2015
@tmatilai
Copy link
Owner

tmatilai commented Jan 7, 2015

Merged! Thanks!

I'll be mostly offline for the next couple of weeks, but I'll release v1.5 as soon as I have an hour of internet with the laptop. =)

@jperville jperville deleted the detect-chef-provisioners-by-type branch January 7, 2015 06:29
@jperville
Copy link
Contributor Author

Thank you so much @tmatilai, looking forward to using the new version.

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.

Chef proxy detection broken with multi-vm Vagrantfiles
2 participants