From 4d70856b8a8a9ab6fab1636823d36ddc814fd427 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 19 Nov 2019 10:59:28 -0800 Subject: [PATCH] Enhance docker build matching for determining built container ID Prior to this commit, docker would look for a container ID based on "Successfully built" string. This output does not exist if a user has enabled the experimental feature buildkit. This commit updates the build behavior to match against both kinds of outputs, and instead of using `scan`, it uses MatchData and groups the container id with match group name `:id` instead of making hard assumptions with the matches being contained inside arrays from scan. --- plugins/providers/docker/driver.rb | 23 ++++++++++++------- plugins/providers/docker/errors.rb | 4 ++++ plugins/providers/docker/executor/local.rb | 1 - templates/locales/providers_docker.yml | 2 ++ .../plugins/providers/docker/driver_test.rb | 21 +++++++++++++++++ 5 files changed, 42 insertions(+), 9 deletions(-) diff --git a/plugins/providers/docker/driver.rb b/plugins/providers/docker/driver.rb index 6dfd7a438..f2b382fc3 100644 --- a/plugins/providers/docker/driver.rb +++ b/plugins/providers/docker/driver.rb @@ -15,23 +15,30 @@ module VagrantPlugins @executor = Executor::Local.new end + # Returns the id for a new container built from `docker build`. Raises + # an exception if the id was unable to be captured from the output + # + # @return [String] id - ID matched from the docker build output. def build(dir, **opts, &block) args = Array(opts[:extra_args]) args << dir - result = execute('docker', 'build', '--progress', 'plain', *args, &block) - matches = result.scan(/Successfully built (.+)$/i) - if matches.empty? + result = execute('docker', 'build', *args, &block) + matches = result.match(/Successfully built (?.+)$/i) + if !matches # Check for the new output format 'writing image sha256...' - matches = result.scan(/writing image sha256:([0-9a-z]+) +$/i) - if matches.empty? + # In this case, docker builtkit is enabled. Its format is different + # from standard docker + @logger.warn("Could not determine docker container ID. Scanning for buildkit output instead") + matches = result.match(/writing image .+:(?[0-9a-z]+) done/i) + if !matches # This will cause a stack trace in Vagrant, but it is a bug # if this happens anyways. - raise "UNKNOWN OUTPUT: #{result}" + raise Errors::BuildError, result: result end end - # Return the last match, and the capture of it - matches[-1][0] + # Return the matched group `id` + matches[:id] end def create(params, **opts, &block) diff --git a/plugins/providers/docker/errors.rb b/plugins/providers/docker/errors.rb index 11ae6fd68..6569f6a3a 100644 --- a/plugins/providers/docker/errors.rb +++ b/plugins/providers/docker/errors.rb @@ -5,6 +5,10 @@ module VagrantPlugins error_namespace("docker_provider.errors") end + class BuildError < DockerError + error_key(:build_error) + end + class CommunicatorNonDocker < DockerError error_key(:communicator_non_docker) end diff --git a/plugins/providers/docker/executor/local.rb b/plugins/providers/docker/executor/local.rb index 6a71d52d5..f99c4f929 100644 --- a/plugins/providers/docker/executor/local.rb +++ b/plugins/providers/docker/executor/local.rb @@ -27,7 +27,6 @@ module VagrantPlugins stdout: result.stdout end - # If the new buildkit-based docker build is used, stdout is empty, and the output is in stderr if result.stdout.to_s.strip.length == 0 result.stderr else diff --git a/templates/locales/providers_docker.yml b/templates/locales/providers_docker.yml index 3c23b905c..4b6795c4c 100644 --- a/templates/locales/providers_docker.yml +++ b/templates/locales/providers_docker.yml @@ -159,6 +159,8 @@ en: run exits and doesn't keep running. errors: + build_error: |- + Vagrant received unknown output from docker: %{result} compose_lock_timeout: |- Vagrant encountered a timeout waiting for the docker compose driver to become available. Please try to run your command again. If you diff --git a/test/unit/plugins/providers/docker/driver_test.rb b/test/unit/plugins/providers/docker/driver_test.rb index 6504876cb..7ae6524ef 100644 --- a/test/unit/plugins/providers/docker/driver_test.rb +++ b/test/unit/plugins/providers/docker/driver_test.rb @@ -152,6 +152,27 @@ describe VagrantPlugins::DockerProvider::Driver do ].to_json } + describe '#build' do + let(:result) { "Successfully built 1a2b3c4d" } + let(:buildkit_result) { "writing image sha256:1a2b3c4d done" } + let(:cid) { "1a2b3c4d" } + + it "builds a container with standard docker" do + allow(subject).to receive(:execute).and_return(result) + + container_id = subject.build("/tmp/fakedir") + + expect(container_id).to eq(cid) + end + + it "builds a container with buildkit docker" do + allow(subject).to receive(:execute).and_return(buildkit_result) + + container_id = subject.build("/tmp/fakedir") + + expect(container_id).to eq(cid) + end + end describe '#create' do let(:params) { {