Skip to content

Commit

Permalink
Fixes hashicorp#9067: Ensure new containers don't grab existing bound…
Browse files Browse the repository at this point in the history
… ports

Prior to this commit, if a created but exited container bound a port,
and a new container grabed that same port (say for an ssh port forward),
when the initial container came back up it would fail because the port
also got bound to the second container. This commit fixes that behavior
by first looking at what containers are already bound prior to creating
a container.
  • Loading branch information
briancain committed May 8, 2020
1 parent 8dfaebb commit 45e87bd
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,13 @@ 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)

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,
Expand Down
5 changes: 4 additions & 1 deletion plugins/providers/docker/action.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down
2 changes: 2 additions & 0 deletions plugins/providers/docker/action/create.rb
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,8 @@ def forwarded_ports(include_ssh=false)
next
end

require 'pry'
binding.pry
mappings["#{options[:host]}/#{options[:protocol]}"] = options
end

Expand Down
1 change: 1 addition & 0 deletions plugins/providers/docker/action/forwarded_ports.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ def initialize(app, env)
@app = app
end

# TODO: Maybe rename this action
def call(env)
env[:machine].provider_config.ports.each do |p|
host_ip = nil
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
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

@app.call(env)
end
end
end
end
end
2 changes: 1 addition & 1 deletion plugins/providers/docker/communicator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
41 changes: 41 additions & 0 deletions plugins/providers/docker/driver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,47 @@ def image?(id)
result =~ /^#{Regexp.escape(id)}$/
end

# Reads all current docker containers and determines what ports
# are currently registered to be forwarded
# {2222=>#<Set: {"127.0.0.1"}>, 8080=>#<Set: {"*"}>, 9090=>#<Set: {"*"}>}
#
# Note: This is this format because of what the builtin action for resolving colliding
# port forwards expects.
#
# @return [Hash[Set]] used_ports - {forward_port: #<Set: {"host ip address"}>}
def read_used_ports
#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
Expand Down

0 comments on commit 45e87bd

Please sign in to comment.