diff --git a/lib/vagrant/action/builtin/handle_forwarded_port_collisions.rb b/lib/vagrant/action/builtin/handle_forwarded_port_collisions.rb index 3c90f4a2cc4..23c40ed59d4 100644 --- a/lib/vagrant/action/builtin/handle_forwarded_port_collisions.rb +++ b/lib/vagrant/action/builtin/handle_forwarded_port_collisions.rb @@ -121,6 +121,17 @@ def handle(env) 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) + + # This check is required only for 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 + if !in_use && env[:machine].provider_name == :docker + if extra_in_use.keys.include?(host_port.to_s) + in_use = true + end + end + if in_use if !repair || !options[:auto_correct] raise Errors::ForwardPortCollision, diff --git a/plugins/providers/docker/action.rb b/plugins/providers/docker/action.rb index b512da387dd..f0a2d7a0506 100644 --- a/plugins/providers/docker/action.rb +++ b/plugins/providers/docker/action.rb @@ -251,14 +251,16 @@ def self.action_start 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 @@ def self.action_suspend 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 ed51b086d71..728eead5957 100644 --- a/plugins/providers/docker/action/forwarded_ports.rb +++ b/plugins/providers/docker/action/forwarded_ports.rb @@ -6,6 +6,8 @@ def initialize(app, env) @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 00000000000..cf45483788b --- /dev/null +++ b/plugins/providers/docker/action/prepare_forwarded_port_collision_params.rb @@ -0,0 +1,29 @@ +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 + remap = {} + env[:port_collision_remap] = remap + + # Note: This might not be required yet (as it is with the virtualbox provider) + # so for now we leave the remap hash empty. + + @app.call(env) + end + end + end + end +end diff --git a/plugins/providers/docker/communicator.rb b/plugins/providers/docker/communicator.rb index 20b0df9122e..e36a2f366fa 100644 --- a/plugins/providers/docker/communicator.rb +++ b/plugins/providers/docker/communicator.rb @@ -151,7 +151,7 @@ def container_ssh_command "-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 506df5900eb..11462da215e 100644 --- a/plugins/providers/docker/driver.rb +++ b/plugins/providers/docker/driver.rb @@ -126,6 +126,46 @@ def image?(id) 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"] + # We remove the first character because docker inspect adds a '/' to + # the beginning every container name to match the "internal" + # implementation of docker: + # https://github.com/moby/moby/issues/6705 + container_name = container_info["Name"][1..-1] + 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 326b0945fbf..7235ee9b678 100644 --- a/test/unit/plugins/providers/docker/driver_test.rb +++ b/test/unit/plugins/providers/docker/driver_test.rb @@ -309,6 +309,33 @@ 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 48b7e3a1164..aecba56eeed 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 @@ 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 @@ 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