From 7fa0303925e212c4d0c506ef4f2e9b3e5b75f439 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 27 May 2010 17:31:36 -0700 Subject: [PATCH] Automatic 'external' port collision correction. If a forwarded port collides with any created VM and is marked to be fixed automatically, then vagrant will choose a new port automatically. --- config/default.rb | 2 +- lib/vagrant/actions/vm/forward_ports.rb | 53 ++++++++++++-- lib/vagrant/config.rb | 3 +- test/vagrant/actions/vm/forward_ports_test.rb | 69 ++++++++++++++----- 4 files changed, 102 insertions(+), 25 deletions(-) diff --git a/config/default.rb b/config/default.rb index d8559b894..c9eadce3b 100644 --- a/config/default.rb +++ b/config/default.rb @@ -18,7 +18,7 @@ Vagrant::Config.run do |config| config.vm.base_mac = "0800279C2E42" config.vm.project_directory = "/vagrant" config.vm.rsync_project_directory = false - config.vm.forward_port("ssh", 22, 2222) + config.vm.forward_port("ssh", 22, 2222, :auto => true) config.vm.disk_image_format = 'VMDK' config.vm.provisioner = nil config.vm.shared_folder_uid = nil diff --git a/lib/vagrant/actions/vm/forward_ports.rb b/lib/vagrant/actions/vm/forward_ports.rb index c0bd3accd..314eaba2a 100644 --- a/lib/vagrant/actions/vm/forward_ports.rb +++ b/lib/vagrant/actions/vm/forward_ports.rb @@ -3,17 +3,56 @@ module Vagrant module VM class ForwardPorts < Base def prepare - VirtualBox::VM.all.each do |vm| - next if !vm.running? || vm.uuid == @runner.uuid + external_collision_check + end - vm.forwarded_ports.each do |fp| - @runner.env.config.vm.forwarded_ports.each do |name, options| - if fp.hostport.to_s == options[:hostport].to_s - raise ActionException.new(:vm_port_collision, :name => name, :hostport => fp.hostport.to_s, :guestport => options[:guestport].to_s, :adapter => options[:adapter]) - end + # This method checks for any port collisions with any VMs + # which are already created (by Vagrant or otherwise). + # report the collisions detected or will attempt to fix them + # automatically if the port is configured to do so. + def external_collision_check + # Flatten all the already-created forwarded ports into a + # flat list. + used_ports = VirtualBox::VM.all.collect do |vm| + if vm.running? && vm.uuid != runner.uuid + vm.forwarded_ports.collect do |fp| + fp.hostport.to_s end end end + + used_ports.flatten! + used_ports.uniq! + + runner.env.config.vm.forwarded_ports.each do |name, options| + if used_ports.include?(options[:hostport].to_s) + handle_collision(name, options, used_ports) + end + end + end + + # Handles any collisions. This method will either attempt to + # fix the collision automatically or will raise an error if + # auto fixing is disabled. + def handle_collision(name, options, used_ports) + if !options[:auto] + # Auto fixing is disabled for this port forward, so we + # must throw an error so the user can fix it. + raise ActionException.new(:vm_port_collision, :name => name, :hostport => options[:hostport].to_s, :guestport => options[:guestport].to_s, :adapter => options[:adapter]) + end + + # Get the auto port range and get rid of the used ports so + # all we're left with is available ports + range = runner.env.config.vm.auto_port_range.to_a + range -= used_ports + + # Set the port up to be the first one and add that port to + # the used list. + options[:hostport] = range.shift + used_ports << options[:hostport] + + # Notify the user + logger.info "Fixed port collision: #{name} now on port #{options[:hostport]}" end def execute! diff --git a/lib/vagrant/config.rb b/lib/vagrant/config.rb index 845774b82..016caf120 100644 --- a/lib/vagrant/config.rb +++ b/lib/vagrant/config.rb @@ -108,7 +108,8 @@ module Vagrant :guestport => guestport, :hostport => hostport, :protocol => "TCP", - :adapter => 0 + :adapter => 0, + :auto => false }.merge(options || {}) forwarded_ports[name] = options diff --git a/test/vagrant/actions/vm/forward_ports_test.rb b/test/vagrant/actions/vm/forward_ports_test.rb index a9c6c0461..f21d38dc0 100644 --- a/test/vagrant/actions/vm/forward_ports_test.rb +++ b/test/vagrant/actions/vm/forward_ports_test.rb @@ -2,10 +2,18 @@ require File.join(File.dirname(__FILE__), '..', '..', '..', 'test_helper') class ForwardPortsActionTest < Test::Unit::TestCase setup do - @mock_vm, @vm, @action = mock_action(Vagrant::Actions::VM::ForwardPorts) + @runner, @vm, @action = mock_action(Vagrant::Actions::VM::ForwardPorts) end - context "checking for colliding ports" do + context "preparing" do + should "call proper sequence" do + prep_seq = sequence("prepare") + @action.expects(:external_collision_check).in_sequence(prep_seq) + @action.prepare + end + end + + context "checking for colliding external ports" do setup do @forwarded_port = mock("forwarded_port") @forwarded_port.stubs(:hostport) @@ -15,7 +23,7 @@ class ForwardPortsActionTest < Test::Unit::TestCase @vm.stubs(:forwarded_ports).returns(@forwarded_ports) @vm.stubs(:running?).returns(true) @vm.stubs(:uuid).returns("foo") - @mock_vm.stubs(:uuid).returns("bar") + @runner.stubs(:uuid).returns("bar") vms = [@vm] VirtualBox::VM.stubs(:all).returns(vms) @@ -24,38 +32,67 @@ class ForwardPortsActionTest < Test::Unit::TestCase config.vm.forward_port("ssh", 22, 2222) end - @mock_vm.stubs(:env).returns(@env) + @runner.stubs(:env).returns(@env) + + # So no exceptions are raised + @action.stubs(:handle_collision) end should "ignore vms which aren't running" do @vm.expects(:running?).returns(false) @vm.expects(:forwarded_ports).never - @action.prepare + @action.external_collision_check end should "ignore vms which are equivalent to ours" do - @mock_vm.expects(:uuid).returns(@vm.uuid) + @runner.expects(:uuid).returns(@vm.uuid) @vm.expects(:forwarded_ports).never - @action.prepare + @action.external_collision_check end should "not raise any errors if no forwarded ports collide" do @forwarded_port.expects(:hostport).returns(80) - assert_nothing_raised { @action.prepare } + assert_nothing_raised { @action.external_collision_check } end - should "raise an ActionException if a port collides" do + should "handle the collision if it happens" do @forwarded_port.expects(:hostport).returns(2222) + @action.expects(:handle_collision).with("ssh", anything, anything).once + @action.external_collision_check + end + end + + context "handling collisions" do + setup do + @name = :foo + @options = { + :hostport => 0, + :auto => true + } + @used_ports = [1,2,3] + + @runner.env.config.vm.auto_port_range = (1..5) + @auto_port_range = @runner.env.config.vm.auto_port_range.to_a + end + + should "raise an exception if auto forwarding is disabled" do + @options[:auto] = false + assert_raises(Vagrant::Actions::ActionException) { - @action.prepare + @action.handle_collision(@name, @options, @used_ports) } end - should "convert ports to strings prior to checking" do - @forwarded_port.expects(:hostport).returns("2222") - assert_raises(Vagrant::Actions::ActionException) { - @action.prepare - } + should "set the host port to the first available port" do + assert_equal 0, @options[:hostport] + @action.handle_collision(@name, @options, @used_ports) + assert_equal 4, @options[:hostport] + end + + should "add the newly used port to the list of used ports" do + assert !@used_ports.include?(4) + @action.handle_collision(@name, @options, @used_ports) + assert @used_ports.include?(4) end end @@ -76,7 +113,7 @@ class ForwardPortsActionTest < Test::Unit::TestCase @vm.expects(:network_adapters).returns([network_adapter]) network_adapter.expects(:attachment_type).returns(:nat) - @mock_vm.env.config.vm.forwarded_ports.each do |name, opts| + @runner.env.config.vm.forwarded_ports.each do |name, opts| forwarded_ports.expects(:<<).with do |port| assert_equal name, port.name assert_equal opts[:hostport], port.hostport