Merge pull request #13146 from allisonlarson/fix-docker-port-collisio…

…n-error Ignore inactive docker containers when assigning ports
hashicorp · May 10, 2023 · 5c97e9a · 5c97e9a
2 parents 773f942 + 10e45f1
commit 5c97e9a
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 7 deletions.
diff --git a/plugins/providers/docker/driver.rb b/plugins/providers/docker/driver.rb
@@ -140,6 +140,9 @@ def read_used_ports
         all_containers.each do |c|
           container_info = inspect_container(c)
 
+          active = container_info["State"]["Running"]
+          next unless active # Ignore used ports on inactive containers
+
           if container_info["HostConfig"]["PortBindings"]
             port_bindings = container_info["HostConfig"]["PortBindings"]
             next if port_bindings.empty? # Nothing defined, but not nil either

diff --git a/test/unit/plugins/providers/docker/driver_test.rb b/test/unit/plugins/providers/docker/driver_test.rb
@@ -346,19 +346,33 @@
 
   describe '#read_used_ports' do
     let(:all_containers) { ["container1\ncontainer2"] }
-    let(:container_info) { {"Name"=>"/container", "HostConfig"=>{"PortBindings"=>{}}} }
+    let(:container_info) { {"Name"=>"/container", "State"=>{"Running"=>true}, "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(:container_info) { {"Name"=>"/container","State"=>{"Running"=>true}, "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)
+      context "with active containers" do
+        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)
+          used_ports = subject.read_used_ports
+          expect(used_ports).to eq(used_ports_set)
+        end
+      end
+
+      context "with inactive containers" do
+        let(:container_info) { {"Name"=>"/container", "State"=>{"Running"=>false}, "HostConfig"=>{"PortBindings"=>{"22/tcp"=>[{"HostIp"=>"127.0.0.1","HostPort"=>"2222"}] }}} }
+
+        it 'returns empty' 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
     end