From b2d9abe344cde538604f2ca96bf020430319b3f1 Mon Sep 17 00:00:00 2001
From: Brian Cain <bcain@hashicorp.com>
Date: Wed, 6 May 2020 16:01:05 -0700
Subject: [PATCH 1/5] Fixes #9067: Ensure new containers don't grab existing
 bound 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.
---
 .../handle_forwarded_port_collisions.rb       | 11 +++++
 plugins/providers/docker/action.rb            |  5 ++-
 .../docker/action/forwarded_ports.rb          |  2 +
 ...prepare_forwarded_port_collision_params.rb | 29 ++++++++++++++
 plugins/providers/docker/communicator.rb      |  2 +-
 plugins/providers/docker/driver.rb            | 40 +++++++++++++++++++
 .../plugins/providers/docker/driver_test.rb   | 27 +++++++++++++
 .../handle_forwarded_port_collisions_test.rb  |  2 +
 8 files changed, 116 insertions(+), 2 deletions(-)
 create mode 100644 plugins/providers/docker/action/prepare_forwarded_port_collision_params.rb

diff --git a/lib/vagrant/action/builtin/handle_forwarded_port_collisions.rb b/lib/vagrant/action/builtin/handle_forwarded_port_collisions.rb
index 3c90f4a2c..23c40ed59 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 @@ 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)
+
+            # 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 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..cf4548378
--- /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 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..11462da21 100644
--- a/plugins/providers/docker/driver.rb
+++ b/plugins/providers/docker/driver.rb
@@ -126,6 +126,46 @@ module VagrantPlugins
         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 = 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 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..aecba56ee 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

From 5aff6660fbd9c8c9adbf0403715cef95a3a73a60 Mon Sep 17 00:00:00 2001
From: Brian Cain <bcain@hashicorp.com>
Date: Tue, 12 May 2020 10:54:23 -0700
Subject: [PATCH 2/5] Move port checker method to class rather than instance

This commit also moves out the ipv4_interfaces method to be a util
method, so that the port checker can also access it as a class mehtod
---
 .../handle_forwarded_port_collisions.rb       | 23 ++++----
 lib/vagrant/util/ipv4_interfaces.rb           | 13 +++++
 lib/vagrant/util/is_port_open.rb              |  2 +-
 .../handle_forwarded_port_collisions_test.rb  | 57 +++----------------
 .../unit/vagrant/util/ipv4_interfaces_test.rb | 47 +++++++++++++++
 test/unit/vagrant/util/is_port_open_test.rb   | 14 ++---
 6 files changed, 86 insertions(+), 70 deletions(-)
 create mode 100644 lib/vagrant/util/ipv4_interfaces.rb
 create mode 100644 test/unit/vagrant/util/ipv4_interfaces_test.rb

diff --git a/lib/vagrant/action/builtin/handle_forwarded_port_collisions.rb b/lib/vagrant/action/builtin/handle_forwarded_port_collisions.rb
index 23c40ed59..61acdf914 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
@@ -254,22 +256,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.")
@@ -280,10 +279,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..1d2af5d05
--- /dev/null
+++ b/lib/vagrant/util/ipv4_interfaces.rb
@@ -0,0 +1,13 @@
+module Vagrant
+  module Util
+    module IPv4Interfaces
+      def self.ipv4_interfaces
+        Socket.getifaddrs.select do |ifaddr|
+          ifaddr.addr && ifaddr.addr.ipv4?
+        end.map do |ifaddr|
+          [ifaddr.name, ifaddr.addr.ip_address]
+        end
+      end
+    end
+  end
+end
diff --git a/lib/vagrant/util/is_port_open.rb b/lib/vagrant/util/is_port_open.rb
index b51a01ecd..f4b0f8f6d 100644
--- a/lib/vagrant/util/is_port_open.rb
+++ b/lib/vagrant/util/is_port_open.rb
@@ -13,7 +13,7 @@ module Vagrant
       # @param [Integer] port Port to check.
       # @return [Boolean] `true` if the port is open (listening), `false`
       #   otherwise.
-      def is_port_open?(host, port)
+      def self.is_port_open?(host, port)
         # We wrap this in a timeout because once in awhile the TCPSocket
         # _will_ hang, but this signals that the port is closed.
         Timeout.timeout(1) do
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 aecba56ee..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
@@ -79,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
 
@@ -147,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 }
@@ -193,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
 
@@ -210,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
 

From c18ceb20bc0ddeec67a2fcfab718758f55413006 Mon Sep 17 00:00:00 2001
From: Brian Cain <bcain@hashicorp.com>
Date: Tue, 12 May 2020 11:17:15 -0700
Subject: [PATCH 3/5] Define custom port_check method for halted docker
 containers

---
 .../handle_forwarded_port_collisions.rb       | 10 ------
 ...prepare_forwarded_port_collision_params.rb | 36 +++++++++++++++++--
 2 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/lib/vagrant/action/builtin/handle_forwarded_port_collisions.rb b/lib/vagrant/action/builtin/handle_forwarded_port_collisions.rb
index 61acdf914..917cbde8d 100644
--- a/lib/vagrant/action/builtin/handle_forwarded_port_collisions.rb
+++ b/lib/vagrant/action/builtin/handle_forwarded_port_collisions.rb
@@ -124,16 +124,6 @@ module Vagrant
               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/prepare_forwarded_port_collision_params.rb b/plugins/providers/docker/action/prepare_forwarded_port_collision_params.rb
index cf4548378..2a838a51b 100644
--- a/plugins/providers/docker/action/prepare_forwarded_port_collision_params.rb
+++ b/plugins/providers/docker/action/prepare_forwarded_port_collision_params.rb
@@ -15,14 +15,46 @@ module VagrantPlugins
           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
 
-          # Note: This might not be required yet (as it is with the virtualbox provider)
-          # so for now we leave the remap hash empty.
+          # 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

From cfb9a6457e8bf0e8399699233982960dc3a2f375 Mon Sep 17 00:00:00 2001
From: Brian Cain <bcain@hashicorp.com>
Date: Tue, 12 May 2020 11:26:48 -0700
Subject: [PATCH 4/5] Remove obtaining name from docker container info

---
 plugins/providers/docker/driver.rb | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/plugins/providers/docker/driver.rb b/plugins/providers/docker/driver.rb
index 11462da21..2693883ac 100644
--- a/plugins/providers/docker/driver.rb
+++ b/plugins/providers/docker/driver.rb
@@ -141,11 +141,6 @@ module VagrantPlugins
           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
 

From 64ed950fd8b7eaf2872f1911d1a7774ad3b7145f Mon Sep 17 00:00:00 2001
From: Brian Cain <bcain@hashicorp.com>
Date: Mon, 18 May 2020 10:39:21 -0700
Subject: [PATCH 5/5] Put back methods as instance methods

---
 lib/vagrant/util/ipv4_interfaces.rb | 4 +++-
 lib/vagrant/util/is_port_open.rb    | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/lib/vagrant/util/ipv4_interfaces.rb b/lib/vagrant/util/ipv4_interfaces.rb
index 1d2af5d05..7ae486704 100644
--- a/lib/vagrant/util/ipv4_interfaces.rb
+++ b/lib/vagrant/util/ipv4_interfaces.rb
@@ -1,13 +1,15 @@
 module Vagrant
   module Util
     module IPv4Interfaces
-      def self.ipv4_interfaces
+      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 f4b0f8f6d..1ae5f8208 100644
--- a/lib/vagrant/util/is_port_open.rb
+++ b/lib/vagrant/util/is_port_open.rb
@@ -13,7 +13,7 @@ module Vagrant
       # @param [Integer] port Port to check.
       # @return [Boolean] `true` if the port is open (listening), `false`
       #   otherwise.
-      def self.is_port_open?(host, port)
+      def is_port_open?(host, port)
         # We wrap this in a timeout because once in awhile the TCPSocket
         # _will_ hang, but this signals that the port is closed.
         Timeout.timeout(1) do
@@ -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