Automatic 'external' port collision correction. If a forwarded port c…

…ollides with any created VM and is marked to be fixed automatically, then vagrant will choose a new port automatically.

config/default.rb

@@ -18,7 +18,7 @@
   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

lib/vagrant/actions/vm/forward_ports.rb

@@ -3,17 +3,56 @@ module Actions
     module VM
       class ForwardPorts < Base
         def prepare
-          VirtualBox::VM.all.each do |vm|
-            next if !vm.running? || vm.uuid == @runner.uuid
-
-            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
+          external_collision_check
+        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!

lib/vagrant/config.rb

@@ -108,7 +108,8 @@ def forward_port(name, guestport, hostport, options=nil)
           :guestport  => guestport,
           :hostport   => hostport,
           :protocol   => "TCP",
-          :adapter    => 0
+          :adapter    => 0,
+          :auto       => false
         }.merge(options || {})
 
         forwarded_ports[name] = options

test/vagrant/actions/vm/forward_ports_test.rb

@@ -2,10 +2,18 @@
 
 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,39 +32,68 @@ 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)
-      assert_raises(Vagrant::Actions::ActionException) {
-        @action.prepare
+      @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 "convert ports to strings prior to checking" do
-      @forwarded_port.expects(:hostport).returns("2222")
+    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 "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
 
   context "execution" do
@@ -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