diff --git a/lib/kitchen/docker/helpers/container_helper.rb b/lib/kitchen/docker/helpers/container_helper.rb index 3f0f4dd9..9f61d00d 100644 --- a/lib/kitchen/docker/helpers/container_helper.rb +++ b/lib/kitchen/docker/helpers/container_helper.rb @@ -12,6 +12,7 @@ # limitations under the License. require "erb" unless defined?(Erb) +require "ipaddr" unless defined?(IPAddr) require "json" unless defined?(JSON) require "shellwords" unless defined?(Shellwords) require "tempfile" unless defined?(Tempfile) @@ -253,15 +254,47 @@ def run_container(state, transport_port = nil) parse_container_id(output) end + # The container's address on the Docker network. + # + # Read from +NetworkSettings.Networks+ rather than the top-level + # +NetworkSettings.IPAddress+. That field was only ever populated for + # the default bridge, and Docker 29 removed it altogether -- asking for + # it there fails the whole `docker inspect` with "map has no entry for + # key \"IPAddress\"", which took +use_internal_docker_network+ with it. + # +Networks+ has been present since Docker 1.9, so reading it works on + # both. + # + # A container attached to several networks has an address on each; the + # first is used, which is the only one for the single-network case this + # option is for. + # # @param state [Hash] instance state naming the container # @return [String] the container's address on the Docker network # @raise [Kitchen::ActionFailed] if it cannot be determined def container_ip_address(state) - cmd = "inspect --format '{{ .NetworkSettings.IPAddress }}'" + cmd = "inspect --format '{{range .NetworkSettings.Networks}}{{.IPAddress}} {{end}}'" cmd << " #{state[:container_id]}" - docker_command(cmd).strip - rescue - raise ActionFailed, "Error getting internal IP of Docker container" + output = docker_command(cmd, suppress_output: !logger.debug?) + + # Picked by parsing rather than by taking the first word, so that a + # warning docker writes to stderr is not returned as an address. + address = output.split(/\s+/).find { |token| ip_address?(token) } + raise ActionFailed, "Docker reports no IP address for the container" if address.nil? + + address + rescue => e + raise ActionFailed, "Error getting internal IP of Docker container. #{e}" + end + + # @param token [String] a candidate address + # @return [Boolean] whether it parses as an IPv4 or IPv6 address + def ip_address?(token) + return false if token.nil? || token.empty? + + IPAddr.new(token) + true + rescue IPAddr::Error + false end # Stops and removes the container. diff --git a/spec/container_helper_spec.rb b/spec/container_helper_spec.rb index 258d3836..27062fd8 100644 --- a/spec/container_helper_spec.rb +++ b/spec/container_helper_spec.rb @@ -270,6 +270,58 @@ def helper_answering(inspect_ok:, running: "false") end end + describe "#container_ip_address" do + def helper_inspecting(output) + h = helper + @asked = nil + allow(h).to receive(:docker_command) { |cmd, _opts = {}| @asked = cmd; output } + h + end + + let(:state) { { container_id: "abc123abc123" } } + + it "returns the address docker reports" do + expect(helper_inspecting("172.17.0.7 \n").container_ip_address(state)).to eq "172.17.0.7" + end + + it "reads Networks rather than the removed top-level IPAddress" do + # Docker 29 dropped NetworkSettings.IPAddress. Asking for it does not + # return empty -- it fails the whole inspect with a template error, which + # took use_internal_docker_network down with it. + helper_inspecting("172.17.0.7\n").container_ip_address(state) + expect(@asked).to include(".NetworkSettings.Networks") + expect(@asked).not_to include(".NetworkSettings.IPAddress") + end + + it "takes the first address when the container is on several networks" do + expect(helper_inspecting("172.17.0.7 172.19.0.2 \n").container_ip_address(state)) + .to eq "172.17.0.7" + end + + it "handles an IPv6 address" do + expect(helper_inspecting("2001:db8::2 \n").container_ip_address(state)).to eq "2001:db8::2" + end + + it "ignores a warning docker wrote to stderr" do + expect(helper_inspecting("WARNING: something happened\n172.17.0.7 \n").container_ip_address(state)) + .to eq "172.17.0.7" + end + + it "fails loudly when docker reports no address" do + # Returning "" here would be recorded as the instance hostname, and the + # connection would fail somewhere far from the cause. + expect { helper_inspecting(" \n").container_ip_address(state) } + .to raise_error(Kitchen::ActionFailed, /no IP address/) + end + + it "fails loudly when the inspect itself fails" do + h = helper + allow(h).to receive(:docker_command).and_raise(Kitchen::ShellOut::ShellCommandFailed, "boom") + expect { h.container_ip_address(state) } + .to raise_error(Kitchen::ActionFailed, /Error getting internal IP/) + end + end + describe "#remove_container" do it "stops the container before removing it" do # `docker rm` refuses to remove a running container, so the order matters.