diff --git a/lib/vagrant/action/builtin/handle_forwarded_port_collisions.rb b/lib/vagrant/action/builtin/handle_forwarded_port_collisions.rb index 3c90f4a2c..917cbde8d 100644 --- a/lib/vagrant/action/builtin/handle_forwarded_port_collisions.rb +++ b/lib/vagrant/action/builtin/handle_forwarded_port_collisions.rb @@ -4,6 +4,7 @@ require "log4r" require "socket" require "vagrant/util/is_port_open" +require "vagrant/util/ipv4_interfaces" module Vagrant module Action @@ -26,6 +27,7 @@ module Vagrant # class HandleForwardedPortCollisions include Util::IsPortOpen + include Util::IPv4Interfaces def initialize(app, env) @app = app @@ -121,6 +123,7 @@ module Vagrant in_use = is_forwarded_already(extra_in_use, host_port, host_ip) || call_port_checker(port_checker, host_ip, host_port) || lease_check(host_ip, host_port) + if in_use if !repair || !options[:auto_correct] raise Errors::ForwardPortCollision, @@ -243,22 +246,19 @@ module Vagrant return extra_in_use.fetch(hostport).include?(hostip) end - def ipv4_interfaces - Socket.getifaddrs.select do |ifaddr| - ifaddr.addr && ifaddr.addr.ipv4? - end.map do |ifaddr| - [ifaddr.name, ifaddr.addr.ip_address] - end + def port_check(host_ip, host_port) + self.class.port_check(@machine, host_ip, host_port) end - def port_check(host_ip, host_port) + def self.port_check(machine, host_ip, host_port) + @logger = Log4r::Logger.new("vagrant::action::builtin::handle_port_collisions") # If no host_ip is specified, intention taken to be listen on all interfaces. test_host_ip = host_ip || "0.0.0.0" if Util::Platform.windows? && test_host_ip == "0.0.0.0" @logger.debug("Testing port #{host_port} on all IPv4 interfaces...") - available_interfaces = ipv4_interfaces.select do |interface| + available_interfaces = Vagrant::Util::IPv4Interfaces.ipv4_interfaces.select do |interface| @logger.debug("Testing #{interface[0]} with IP address #{interface[1]}") - !is_port_open?(interface[1], host_port) + !Vagrant::Util::IsPortOpen.is_port_open?(interface[1], host_port) end if available_interfaces.empty? @logger.debug("Cannot forward port #{host_port} on any interfaces.") @@ -269,10 +269,10 @@ module Vagrant end else # Do a regular check - if test_host_ip == "0.0.0.0" || ipv4_interfaces.detect { |iface| iface[1] == test_host_ip } - is_port_open?(test_host_ip, host_port) + if test_host_ip == "0.0.0.0" || Vagrant::Util::IPv4Interfaces.ipv4_interfaces.detect { |iface| iface[1] == test_host_ip } + Vagrant::Util::IsPortOpen.is_port_open?(test_host_ip, host_port) else - raise Errors::ForwardPortHostIPNotFound, name: @machine.name, host_ip: host_ip + raise Errors::ForwardPortHostIPNotFound, name: machine.name, host_ip: host_ip end end end diff --git a/lib/vagrant/util/ipv4_interfaces.rb b/lib/vagrant/util/ipv4_interfaces.rb new file mode 100644 index 000000000..7ae486704 --- /dev/null +++ b/lib/vagrant/util/ipv4_interfaces.rb @@ -0,0 +1,15 @@ +module Vagrant + module Util + module IPv4Interfaces + def ipv4_interfaces + Socket.getifaddrs.select do |ifaddr| + ifaddr.addr && ifaddr.addr.ipv4? + end.map do |ifaddr| + [ifaddr.name, ifaddr.addr.ip_address] + end + end + + extend IPv4Interfaces + end + end +end diff --git a/lib/vagrant/util/is_port_open.rb b/lib/vagrant/util/is_port_open.rb index b51a01ecd..1ae5f8208 100644 --- a/lib/vagrant/util/is_port_open.rb +++ b/lib/vagrant/util/is_port_open.rb @@ -34,6 +34,8 @@ module Vagrant # Any of the above exceptions signal that the port is closed. return false end + + extend IsPortOpen end end end diff --git a/plugins/providers/docker/action.rb b/plugins/providers/docker/action.rb index b512da387..f0a2d7a05 100644 --- a/plugins/providers/docker/action.rb +++ b/plugins/providers/docker/action.rb @@ -251,14 +251,16 @@ module VagrantPlugins if env[:machine_action] != :run_command # If the container is NOT created yet, then do some setup steps # necessary for creating it. + b2.use Call, IsState, :preparing do |env2, b3| if env2[:result] b3.use EnvSet, port_collision_repair: true b3.use HostMachinePortWarning b3.use HostMachinePortChecker + b3.use ForwardedPorts # This action converts the `ports` param into proper network configs + b3.use PrepareForwardedPortCollisionParams b3.use HandleForwardedPortCollisions b3.use SyncedFolders - b3.use ForwardedPorts b3.use Pull b3.use Create b3.use WaitForRunning @@ -313,6 +315,7 @@ module VagrantPlugins autoload :IsBuild, action_root.join("is_build") autoload :IsHostMachineCreated, action_root.join("is_host_machine_created") autoload :Login, action_root.join("login") + autoload :PrepareForwardedPortCollisionParams, action_root.join("prepare_forwarded_port_collision_params") autoload :PrepareNetworks, action_root.join("prepare_networks") autoload :PrepareNFSValidIds, action_root.join("prepare_nfs_valid_ids") autoload :PrepareNFSSettings, action_root.join("prepare_nfs_settings") diff --git a/plugins/providers/docker/action/forwarded_ports.rb b/plugins/providers/docker/action/forwarded_ports.rb index ed51b086d..728eead59 100644 --- a/plugins/providers/docker/action/forwarded_ports.rb +++ b/plugins/providers/docker/action/forwarded_ports.rb @@ -6,6 +6,8 @@ module VagrantPlugins @app = app end + # Converts the `ports` docker provider param into proper network configs + # of type :forwarded_port def call(env) env[:machine].provider_config.ports.each do |p| host_ip = nil diff --git a/plugins/providers/docker/action/prepare_forwarded_port_collision_params.rb b/plugins/providers/docker/action/prepare_forwarded_port_collision_params.rb new file mode 100644 index 000000000..2a838a51b --- /dev/null +++ b/plugins/providers/docker/action/prepare_forwarded_port_collision_params.rb @@ -0,0 +1,61 @@ +module VagrantPlugins + module DockerProvider + module Action + class PrepareForwardedPortCollisionParams + def initialize(app, env) + @app = app + end + + def call(env) + machine = env[:machine] + + # Get the forwarded ports used by other containers and + # consider those in use as well. + other_used_ports = machine.provider.driver.read_used_ports + env[:port_collision_extra_in_use] = other_used_ports + + # Build the remap for any existing collision detections + # + # Note: This remap might not be required yet (as it is with the virtualbox provider) + # so for now we leave the remap hash empty. + remap = {} + env[:port_collision_remap] = remap + + # This port checker method calls the custom port_check method + # defined below. If its false, it will go ahead and use the built-in + # port_check method to see if there are any live containers with bound + # ports + docker_port_check = proc { |host_ip, host_port| + result = port_check(env, host_port) + if !result + result = Vagrant::Action::Builtin::HandleForwardedPortCollisions.port_check(machine, host_ip, host_port) + end + result} + env[:port_collision_port_check] = docker_port_check + + @app.call(env) + end + + protected + + # This check is required the docker provider. Containers + # can bind ports but be halted. We don't want new containers to + # grab these bound ports, so this check is here for that since + # the checks above won't detect it + # + # @param [Vagrant::Environment] env + # @param [String] host_port + # @returns [Bool] + def port_check(env, host_port) + extra_in_use = env[:port_collision_extra_in_use] + + if extra_in_use + return extra_in_use.include?(host_port.to_s) + else + return false + end + end + end + end + end +end diff --git a/plugins/providers/docker/communicator.rb b/plugins/providers/docker/communicator.rb index 20b0df912..e36a2f366 100644 --- a/plugins/providers/docker/communicator.rb +++ b/plugins/providers/docker/communicator.rb @@ -151,7 +151,7 @@ module VagrantPlugins "-i #{path}" end - # Use ad-hoc SSH options for the hop on the docker proxy + # Use ad-hoc SSH options for the hop on the docker proxy if info[:forward_agent] ssh_args << "-o ForwardAgent=yes" end diff --git a/plugins/providers/docker/driver.rb b/plugins/providers/docker/driver.rb index 506df5900..2693883ac 100644 --- a/plugins/providers/docker/driver.rb +++ b/plugins/providers/docker/driver.rb @@ -126,6 +126,41 @@ module VagrantPlugins result =~ /^#{Regexp.escape(id)}$/ end + # Reads all current docker containers and determines what ports + # are currently registered to be forwarded + # {2222=>#, 8080=>#, 9090=>#} + # + # Note: This is this format because of what the builtin action for resolving colliding + # port forwards expects. + # + # @return [Hash[Set]] used_ports - {forward_port: #} + def read_used_ports + used_ports = Hash.new{|hash,key| hash[key] = Set.new} + + all_containers.each do |c| + container_info = inspect_container(c) + + if container_info["HostConfig"]["PortBindings"] + port_bindings = container_info["HostConfig"]["PortBindings"] + next if port_bindings.empty? # Nothing defined, but not nil either + + port_bindings.each do |guest_port,host_mapping| + host_mapping.each do |h| + if h["HostIp"] == "" + hostip = "*" + else + hostip = h["HostIp"] + end + hostport = h["HostPort"] + used_ports[hostport].add(hostip) + end + end + end + end + + used_ports + end + def running?(cid) result = execute('docker', 'ps', '-q', '--no-trunc') result =~ /^#{Regexp.escape cid}$/m diff --git a/test/unit/plugins/providers/docker/driver_test.rb b/test/unit/plugins/providers/docker/driver_test.rb index 326b0945f..7235ee9b6 100644 --- a/test/unit/plugins/providers/docker/driver_test.rb +++ b/test/unit/plugins/providers/docker/driver_test.rb @@ -309,6 +309,33 @@ describe VagrantPlugins::DockerProvider::Driver do end end + describe '#read_used_ports' do + let(:all_containers) { ["container1\ncontainer2"] } + let(:container_info) { {"Name"=>"/container", "HostConfig"=>{"PortBindings"=>{}}} } + let(:empty_used_ports) { {} } + + context "with existing port forwards" do + let(:container_info) { {"Name"=>"/container", "HostConfig"=>{"PortBindings"=>{"22/tcp"=>[{"HostIp"=>"127.0.0.1","HostPort"=>"2222"}] }}} } + let(:used_ports_set) { {"2222"=>Set["127.0.0.1"]} } + + it 'should read all port bindings and return a hash of sets' do + allow(subject).to receive(:all_containers).and_return(all_containers) + allow(subject).to receive(:inspect_container).and_return(container_info) + + used_ports = subject.read_used_ports + expect(used_ports).to eq(used_ports_set) + end + end + + it 'returns empty if no ports are already bound' do + allow(subject).to receive(:all_containers).and_return(all_containers) + allow(subject).to receive(:inspect_container).and_return(container_info) + + used_ports = subject.read_used_ports + expect(used_ports).to eq(empty_used_ports) + end + end + describe '#running?' do let(:result) { subject.running?(cid) } diff --git a/test/unit/vagrant/action/builtin/handle_forwarded_port_collisions_test.rb b/test/unit/vagrant/action/builtin/handle_forwarded_port_collisions_test.rb index 48b7e3a11..8f16954af 100644 --- a/test/unit/vagrant/action/builtin/handle_forwarded_port_collisions_test.rb +++ b/test/unit/vagrant/action/builtin/handle_forwarded_port_collisions_test.rb @@ -11,6 +11,7 @@ describe Vagrant::Action::Builtin::HandleForwardedPortCollisions do port_collision_remap: collision_remap, port_collision_repair: collision_repair, port_collision_port_check: collision_port_check } } + let(:provider_name) { "default" } let(:extra_in_use){ nil } let(:collision_remap){ nil } let(:collision_repair){ nil } @@ -19,6 +20,7 @@ describe Vagrant::Action::Builtin::HandleForwardedPortCollisions do let(:machine) do double("machine").tap do |machine| + allow(machine).to receive(:provider_name).and_return(provider_name) allow(machine).to receive(:config).and_return(machine_config) allow(machine).to receive(:env).and_return(machine_env) end @@ -77,7 +79,7 @@ describe Vagrant::Action::Builtin::HandleForwardedPortCollisions do it "should check if host port is in use" do expect(instance).to receive(:is_forwarded_already).and_return(false) - expect(instance).to receive(:is_port_open?).and_return(false) + expect(Vagrant::Util::IsPortOpen).to receive(:is_port_open?).and_return(false) instance.call(env) end @@ -145,45 +147,6 @@ describe Vagrant::Action::Builtin::HandleForwardedPortCollisions do describe "#recover" do end - describe "#ipv4_interfaces" do - let(:name) { double("name") } - let(:address) { double("address") } - - let(:ipv4_ifaddr) do - double("ipv4_ifaddr").tap do |ifaddr| - allow(ifaddr).to receive(:name).and_return(name) - allow(ifaddr).to receive_message_chain(:addr, :ipv4?).and_return(true) - allow(ifaddr).to receive_message_chain(:addr, :ip_address).and_return(address) - end - end - - let(:ipv6_ifaddr) do - double("ipv6_ifaddr").tap do |ifaddr| - allow(ifaddr).to receive(:name) - allow(ifaddr).to receive_message_chain(:addr, :ipv4?).and_return(false) - end - end - - let(:ifaddrs) { [ ipv4_ifaddr, ipv6_ifaddr ] } - - before do - allow(Socket).to receive(:getifaddrs).and_return(ifaddrs) - end - - it "returns a list of IPv4 interfaces with their names and addresses" do - expect(instance.send(:ipv4_interfaces)).to eq([ [name, address] ]) - end - - context "with nil interface address" do - let(:nil_ifaddr) { double("nil_ifaddr", addr: nil ) } - let(:ifaddrs) { [ ipv4_ifaddr, ipv6_ifaddr, nil_ifaddr ] } - - it "filters out nil addr info" do - expect(instance.send(:ipv4_interfaces)).to eq([ [name, address] ]) - end - end - end - describe "#port_check" do let(:host_ip){ "127.0.0.1" } let(:host_port){ 8080 } @@ -191,11 +154,11 @@ describe Vagrant::Action::Builtin::HandleForwardedPortCollisions do before do instance.instance_variable_set(:@machine, machine) - allow(instance).to receive(:ipv4_interfaces).and_return(interfaces) + allow(Vagrant::Util::IPv4Interfaces).to receive(:ipv4_interfaces).and_return(interfaces) end it "should check if the port is open" do - expect(instance).to receive(:is_port_open?).with(host_ip, host_port).and_return(true) + expect(Vagrant::Util::IsPortOpen).to receive(:is_port_open?).with(host_ip, host_port).and_return(true) instance.send(:port_check, host_ip, host_port) end @@ -208,23 +171,23 @@ describe Vagrant::Action::Builtin::HandleForwardedPortCollisions do end it "should check the port on every IPv4 interface" do - expect(instance).to receive(:is_port_open?).with(interfaces[0][1], host_port) - expect(instance).to receive(:is_port_open?).with(interfaces[1][1], host_port) + expect(Vagrant::Util::IsPortOpen).to receive(:is_port_open?).with(interfaces[0][1], host_port) + expect(Vagrant::Util::IsPortOpen).to receive(:is_port_open?).with(interfaces[1][1], host_port) instance.send(:port_check, host_ip, host_port) end it "should return false if the port is closed on any IPv4 interfaces" do - expect(instance).to receive(:is_port_open?).with(interfaces[0][1], host_port). + expect(Vagrant::Util::IsPortOpen).to receive(:is_port_open?).with(interfaces[0][1], host_port). and_return(true) - expect(instance).to receive(:is_port_open?).with(interfaces[1][1], host_port). + expect(Vagrant::Util::IsPortOpen).to receive(:is_port_open?).with(interfaces[1][1], host_port). and_return(false) expect(instance.send(:port_check, host_ip, host_port)).to be(false) end it "should return true if the port is open on all IPv4 interfaces" do - expect(instance).to receive(:is_port_open?).with(interfaces[0][1], host_port). + expect(Vagrant::Util::IsPortOpen).to receive(:is_port_open?).with(interfaces[0][1], host_port). and_return(true) - expect(instance).to receive(:is_port_open?).with(interfaces[1][1], host_port). + expect(Vagrant::Util::IsPortOpen).to receive(:is_port_open?).with(interfaces[1][1], host_port). and_return(true) expect(instance.send(:port_check, host_ip, host_port)).to be(true) end diff --git a/test/unit/vagrant/util/ipv4_interfaces_test.rb b/test/unit/vagrant/util/ipv4_interfaces_test.rb new file mode 100644 index 000000000..eb907dff1 --- /dev/null +++ b/test/unit/vagrant/util/ipv4_interfaces_test.rb @@ -0,0 +1,47 @@ +require File.expand_path("../../../base", __FILE__) + +require "vagrant/util/ipv4_interfaces" + +describe Vagrant::Util::IPv4Interfaces do + subject { described_class } + + describe "#ipv4_interfaces" do + let(:name) { double("name") } + let(:address) { double("address") } + + let(:ipv4_ifaddr) do + double("ipv4_ifaddr").tap do |ifaddr| + allow(ifaddr).to receive(:name).and_return(name) + allow(ifaddr).to receive_message_chain(:addr, :ipv4?).and_return(true) + allow(ifaddr).to receive_message_chain(:addr, :ip_address).and_return(address) + end + end + + let(:ipv6_ifaddr) do + double("ipv6_ifaddr").tap do |ifaddr| + allow(ifaddr).to receive(:name) + allow(ifaddr).to receive_message_chain(:addr, :ipv4?).and_return(false) + end + end + + let(:ifaddrs) { [ ipv4_ifaddr, ipv6_ifaddr ] } + + before do + allow(Socket).to receive(:getifaddrs).and_return(ifaddrs) + end + + it "returns a list of IPv4 interfaces with their names and addresses" do + expect(subject.ipv4_interfaces).to eq([ [name, address] ]) + end + + context "with nil interface address" do + let(:nil_ifaddr) { double("nil_ifaddr", addr: nil ) } + let(:ifaddrs) { [ ipv4_ifaddr, ipv6_ifaddr, nil_ifaddr ] } + + it "filters out nil addr info" do + expect(subject.ipv4_interfaces).to eq([ [name, address] ]) + end + end + end +end + diff --git a/test/unit/vagrant/util/is_port_open_test.rb b/test/unit/vagrant/util/is_port_open_test.rb index d931a8790..e9c757d8d 100644 --- a/test/unit/vagrant/util/is_port_open_test.rb +++ b/test/unit/vagrant/util/is_port_open_test.rb @@ -5,11 +5,7 @@ require "socket" require "vagrant/util/is_port_open" describe Vagrant::Util::IsPortOpen do - let(:klass) do - Class.new do - extend Vagrant::Util::IsPortOpen - end - end + subject { described_class } let(:open_port) { 52811 } let(:closed_port) { 52811 } @@ -36,7 +32,7 @@ describe Vagrant::Util::IsPortOpen do end # Verify that we report the port is open - expect(klass.is_port_open?("127.0.0.1", open_port)).to be + expect(subject.is_port_open?("127.0.0.1", open_port)).to be # Kill the thread thr[:die] = true @@ -47,17 +43,17 @@ describe Vagrant::Util::IsPortOpen do # This CAN fail, since port 52811 might actually be in use, but I'm # not sure what to do except choose some random port and hope for the # best, really. - expect(klass.is_port_open?("127.0.0.1", closed_port)).not_to be + expect(subject.is_port_open?("127.0.0.1", closed_port)).not_to be end it "should handle connection refused" do expect(TCPSocket).to receive(:new).with("0.0.0.0", closed_port).and_raise(Errno::ECONNREFUSED) - expect(klass.is_port_open?("0.0.0.0", closed_port)).to be(false) + expect(subject.is_port_open?("0.0.0.0", closed_port)).to be(false) end it "should raise an error if cannot assign requested address" do expect(TCPSocket).to receive(:new).with("0.0.0.0", open_port).and_raise(Errno::EADDRNOTAVAIL) - expect { klass.is_port_open?("0.0.0.0", open_port) }.to raise_error(Errno::EADDRNOTAVAIL) + expect { subject.is_port_open?("0.0.0.0", open_port) }.to raise_error(Errno::EADDRNOTAVAIL) end end