From 10e45f1df198a258d903f829dedc0ae0981bd81d Mon Sep 17 00:00:00 2001 From: Allison Larson Date: Tue, 9 May 2023 16:17:23 -0700 Subject: [PATCH] Ignore inactive docker containers when assigning ports Checks to make sure that a docker container is running before determining whether or not the port is in use. This prevents the a port on an inactive container from being treated as if it is use. Fixes https://github.com/hashicorp/vagrant/issues/13110 --- plugins/providers/docker/driver.rb | 3 ++ .../plugins/providers/docker/driver_test.rb | 28 ++++++++++++++----- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/plugins/providers/docker/driver.rb b/plugins/providers/docker/driver.rb index 659d8c3cc26..9ed009be26d 100644 --- 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 index 35003bd9478..c193a21d76d 100644 --- 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