diff --git a/lib/vagrant/errors.rb b/lib/vagrant/errors.rb index 7d333a39d..f6be7c111 100644 --- a/lib/vagrant/errors.rb +++ b/lib/vagrant/errors.rb @@ -88,6 +88,10 @@ module Vagrant end end + class ActiveMachineWithDifferentProvider < VagrantError + error_key(:active_machine_with_different_provider) + end + class BaseVMNotFound < VagrantError status_code(18) error_key(:base_vm_not_found) diff --git a/lib/vagrant/plugin/v2/command.rb b/lib/vagrant/plugin/v2/command.rb index 34c7af1bc..bc32b8b54 100644 --- a/lib/vagrant/plugin/v2/command.rb +++ b/lib/vagrant/plugin/v2/command.rb @@ -84,8 +84,51 @@ module Vagrant names ||= [] names = [names] if !names.is_a?(Array) - # The provider that we'll be loading up. - provider = (options[:provider] || @env.default_provider).to_sym + # This is a helper that gets a single machine with the proper + # provider. The "proper provider" in this case depends on what was + # given: + # + # * If a provider was explicitly specified, then use that provider. + # But if an active machine exists with a DIFFERENT provider, + # then throw an error (for now), since we don't yet support + # bringing up machines with different providers. + # + # * If no provider was specified, then use the active machine's + # provider if it exists, otherwise use the default provider. + # + get_machine = lambda do |name| + # Check for an active machine with the same name + provider_to_use = options[:provider] + + @env.active_machines.each do |active_name, active_provider| + if name == active_name + # We found an active machine with the same name + + if provider_to_use && provider_to_use != active_provider + # We found an active machine with a provider that doesn't + # match the requested provider. Show an error. + raise Errors::ActiveMachineWithDifferentProvider, + :name => active_name.to_s, + :active_provider => active_provider.to_s, + :requested_provider => provider_to_use.to_s + else + # Use this provider and exit out of the loop. One of the + # invariants [for now] is that there shouldn't be machines + # with multiple providers. + @logger.info("Active machine found with name #{active_name}. " + + "Using provider: #{active_provider}") + provider_to_use = active_provider + break + end + end + end + + # Use the default provider if nothing else + provider_to_use ||= @env.default_provider + + # Get the right machine with the right provider + @env.machine(name, provider_to_use) + end # First determine the proper array of VMs. machines = [] @@ -100,7 +143,7 @@ module Vagrant @env.machine_names.each do |machine_name| if machine_name =~ regex - machines << @env.machine(machine_name, provider) + machines << get_machine.call(machine_name) end end @@ -108,7 +151,7 @@ module Vagrant else # String name, just look for a specific VM @logger.debug("Finding machine that match name: #{name}") - machines << @env.machine(name.to_sym, provider) + machines << get_machine.call(name.to_sym) raise Errors::VMNotFoundError, :name => name if !machines[0] end end @@ -117,7 +160,7 @@ module Vagrant # configured. @logger.debug("Loading all machines...") machines = @env.machine_names.map do |machine_name| - @env.machine(machine_name, provider) + get_machine.call(machine_name) end end diff --git a/templates/locales/en.yml b/templates/locales/en.yml index de5be88be..2eb4f2c9c 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -25,6 +25,16 @@ en: # Translations for exception classes #------------------------------------------------------------------------------- errors: + active_machine_with_different_provider: |- + An active machine was found with a different provider. Vagrant + currently allows each machine to be brought up with only a single + provider at a time. A future version will remove this limitation. + Until then, please destroy the existing machine to up with a new + provider. + + Machine name: %{name} + Active provider: %{active_provider} + Requested provider: %{requested_provider} base_vm_not_found: The base VM with the name '%{name}' was not found. box_not_found: Box '%{name}' could not be found. box_provider_doesnt_match: |- diff --git a/test/unit/vagrant/plugin/v2/command_test.rb b/test/unit/vagrant/plugin/v2/command_test.rb index 82294177b..c7ed4c7b3 100644 --- a/test/unit/vagrant/plugin/v2/command_test.rb +++ b/test/unit/vagrant/plugin/v2/command_test.rb @@ -57,6 +57,7 @@ describe Vagrant::Plugin::V2::Command do let(:environment) do env = double("environment") + env.stub(:active_machines => []) env.stub(:default_provider => default_provider) env.stub(:root_path => "foo") env @@ -73,10 +74,10 @@ describe Vagrant::Plugin::V2::Command do it "should yield every VM in order is no name is given" do foo_vm = double("foo") - foo_vm.stub(:name).and_return("foo") + foo_vm.stub(:name => "foo", :provider => :foobarbaz) bar_vm = double("bar") - bar_vm.stub(:name).and_return("bar") + bar_vm.stub(:name => "bar", :provider => :foobarbaz) environment.stub(:machine_names => [:foo, :bar]) environment.stub(:machine).with(:foo, default_provider).and_return(foo_vm) @@ -100,6 +101,7 @@ describe Vagrant::Plugin::V2::Command do it "yields the given VM if a name is given" do foo_vm = double("foo") + foo_vm.stub(:name => "foo", :provider => :foobarbaz) environment.stub(:machine).with(:foo, default_provider).and_return(foo_vm) @@ -112,12 +114,49 @@ describe Vagrant::Plugin::V2::Command do foo_vm = double("foo") provider = :foobarbaz + foo_vm.stub(:name => "foo", :provider => provider) environment.stub(:machine).with(:foo, provider).and_return(foo_vm) vms = [] instance.with_target_vms("foo", :provider => provider) { |vm| vms << vm } vms.should == [foo_vm] end + + it "should raise an exception if an active machine exists with a different provider" do + name = :foo + + environment.stub(:active_machines => [[name, :vmware]]) + expect { instance.with_target_vms(name, :provider => :foo) }. + to raise_error Vagrant::Errors::ActiveMachineWithDifferentProvider + end + + it "should default to the active machine provider if no explicit provider requested" do + name = :foo + provider = :vmware + vmware_vm = double("vmware_vm") + + environment.stub(:active_machines => [[name, provider]]) + environment.stub(:machine).with(name, provider).and_return(vmware_vm) + vmware_vm.stub(:name => name, :provider => provider) + + vms = [] + instance.with_target_vms(name) { |vm| vms << vm } + vms.should == [vmware_vm] + end + + it "should use the explicit provider if it maches the active machine" do + name = :foo + provider = :vmware + vmware_vm = double("vmware_vm") + + environment.stub(:active_machines => [[name, provider]]) + environment.stub(:machine).with(name, provider).and_return(vmware_vm) + vmware_vm.stub(:name => name, :provider => provider) + + vms = [] + instance.with_target_vms(name, :provider => provider) { |vm| vms << vm } + vms.should == [vmware_vm] + end end describe "splitting the main and subcommand args" do