From 8d1b5fc1c95c87a9a8d726cf956771f044e8b231 Mon Sep 17 00:00:00 2001 From: Oleksiy Protas Date: Sat, 15 Oct 2016 08:56:31 +0300 Subject: [PATCH 1/8] Docu changes for building Docker images from git --- CHANGELOG.md | 1 + plugins/providers/docker/config.rb | 10 +++++++++- templates/locales/providers_docker.yml | 6 ++++-- website/source/docs/docker/configuration.html.md | 9 +++++++-- 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 981813cab..aeb892839 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -617,6 +617,7 @@ IMPROVEMENTS: - provisioners/puppet: Support custom environment variables [GH-7931, GH-7252, GH-2270] - util/safe_exec: Use subprocess for safe_exec on Windows [GH-7802] - util/subprocess: Allow closing STDIN [GH-7778] + - providers/docker: Support building containter images directly from git BUG FIXES: diff --git a/plugins/providers/docker/config.rb b/plugins/providers/docker/config.rb index b3b6993c6..7ad23908c 100644 --- a/plugins/providers/docker/config.rb +++ b/plugins/providers/docker/config.rb @@ -14,10 +14,18 @@ module VagrantPlugins attr_accessor :build_args # The directory with a Dockerfile to build and use as the basis - # for this container. If this is set, "image" should not be set. + # for this container. If this is set, neither "image" nor "git_repo" + # should be set. # # @return [String] attr_accessor :build_dir + + # The URL for a git repository with a Dockerfile to build and use + # as the basis for this container. If this is set, neither "image" + # nor "build_dir" should be set. + # + # @return [String] + attr_accessor :git_repo # Use docker-compose to manage the lifecycle and environment for # containers instead of using docker directly. diff --git a/templates/locales/providers_docker.yml b/templates/locales/providers_docker.yml index 3fb8ba403..3ad6820a7 100644 --- a/templates/locales/providers_docker.yml +++ b/templates/locales/providers_docker.yml @@ -138,15 +138,17 @@ en: "docker" provider. config: both_build_and_image: |- - Only one of "build_dir" or "image" can be set + Only one of "build_dir", "git_repo" or "image" can be set build_dir_invalid: |- "build_dir" must exist and contain a Dockerfile build_dir_or_image: |- - One of "build_dir" or "image" must be set + One of "build_dir", "git_repo" or "image" must be set compose_configuration_hash: |- "compose_configuration" must be a hash compose_force_vm: |- Docker compose is not currently supported from within proxy VM. + git_repo_invalid: |- + "git_repo" must be a valid git URL create_args_array: |- "create_args" must be an array invalid_link: |- diff --git a/website/source/docs/docker/configuration.html.md b/website/source/docs/docker/configuration.html.md index a72d6d6e0..955c62991 100644 --- a/website/source/docs/docker/configuration.html.md +++ b/website/source/docs/docker/configuration.html.md @@ -15,10 +15,15 @@ you may set. A complete reference is shown below. ### Required * `build_dir` (string) - The path to a directory containing a Dockerfile. - One of this or `image` is required. + One of this, `image` or `git_repo` is required. * `image` (string) - The image to launch, specified by the image ID or a name - such as `ubuntu:12.04`. One of this or `build_dir` is required. + such as `ubuntu:12.04`. One of this, `git_repo` or `build_dir` is required. + + * `git_repo` (string) - The URL of a git repository to build the image from. + Supports pulling specific tags, branches and revision, consult the + [docker documenation](https://docs.docker.com/engine/reference/commandline/build/#/git-repositories) + for more information. One of this, `image` or `build_dir` is required. ### Optional From de6a1794c75be4fadd3548728412b5df81019706 Mon Sep 17 00:00:00 2001 From: Oleksiy Protas Date: Mon, 17 Oct 2016 00:16:33 +0300 Subject: [PATCH 2/8] Config, validation and test --- plugins/providers/docker/config.rb | 53 +++++-- templates/locales/providers_docker.yml | 2 + .../plugins/providers/docker/config_test.rb | 147 ++++++++++++++++-- 3 files changed, 181 insertions(+), 21 deletions(-) diff --git a/plugins/providers/docker/config.rb b/plugins/providers/docker/config.rb index 7ad23908c..6abc997cc 100644 --- a/plugins/providers/docker/config.rb +++ b/plugins/providers/docker/config.rb @@ -157,6 +157,7 @@ module VagrantPlugins def initialize @build_args = [] @build_dir = UNSET_VALUE + @git_repo = UNSET_VALUE @cmd = UNSET_VALUE @compose = UNSET_VALUE @compose_configuration = {} @@ -193,14 +194,39 @@ module VagrantPlugins super.tap do |result| # This is a bit confusing. The tests explain the purpose of this # better than the code lets on, I believe. - if (other.image != UNSET_VALUE || other.build_dir != UNSET_VALUE) && - (other.image == UNSET_VALUE || other.build_dir == UNSET_VALUE) - if other.image != UNSET_VALUE && @build_dir != UNSET_VALUE - result.build_dir = nil + has_image = (other.image != UNSET_VALUE) + has_build_dir = (other.build_dir != UNSET_VALUE) + has_git_repo = (other.git_repo != UNSET_VALUE) + + if (has_image ^ has_build_dir ^ has_git_repo) && !(has_image && has_build_dir && has_git_repo) + # image + if has_image + if @build_dir != UNSET_VALUE + result.build_dir = nil + end + if @git_repo != UNSET_VALUE + result.git_repo = nil + end end - - if other.build_dir != UNSET_VALUE && @image != UNSET_VALUE - result.image = nil + + # build_dir + if has_build_dir + if @image != UNSET_VALUE + result.image = nil + end + if @git_repo != UNSET_VALUE + result.git_repo = nil + end + end + + # git_repo + if has_git_repo + if @build_dir != UNSET_VALUE + result.build_dir = nil + end + if @image != UNSET_VALUE + result.image = nil + end end end @@ -222,6 +248,7 @@ module VagrantPlugins def finalize! @build_args = [] if @build_args == UNSET_VALUE @build_dir = nil if @build_dir == UNSET_VALUE + @git_repo = nil if @git_repo == UNSET_VALUE @cmd = [] if @cmd == UNSET_VALUE @compose = false if @compose == UNSET_VALUE @create_args = [] if @create_args == UNSET_VALUE @@ -269,12 +296,13 @@ module VagrantPlugins def validate(machine) errors = _detected_errors - - if @build_dir && @image + + # FIXME: is there a more ruby-elegant way to write this? + if (@build_dir? 1 : 0) + (@git_repo? 1 : 0) + (@image? 1 : 0) > 1 errors << I18n.t("docker_provider.errors.config.both_build_and_image") end - if !@build_dir && !@image + if !@build_dir && !@git_repo && !@image errors << I18n.t("docker_provider.errors.config.build_dir_or_image") end @@ -284,6 +312,11 @@ module VagrantPlugins errors << I18n.t("docker_provider.errors.config.build_dir_invalid") end end + + # Comparison logic taken directly from docker's urlutil.go + if @git_repo && !( @git_repo =~ /^http(?:s)?:\/\/.*.git(?:#.+)?$/ || @git_repo =~ /^git(?:hub\.com|@|:\/\/)/) + errors << I18n.t("docker_provider.errors.config.git_repo_invalid") + end if !@compose_configuration.is_a?(Hash) errors << I18n.t("docker_provider.errors.config.compose_configuration_hash") diff --git a/templates/locales/providers_docker.yml b/templates/locales/providers_docker.yml index 3ad6820a7..df1fa2ceb 100644 --- a/templates/locales/providers_docker.yml +++ b/templates/locales/providers_docker.yml @@ -141,6 +141,8 @@ en: Only one of "build_dir", "git_repo" or "image" can be set build_dir_invalid: |- "build_dir" must exist and contain a Dockerfile + git_repo_invalid: |- + "git_repo" must be a valid repository URL build_dir_or_image: |- One of "build_dir", "git_repo" or "image" must be set compose_configuration_hash: |- diff --git a/test/unit/plugins/providers/docker/config_test.rb b/test/unit/plugins/providers/docker/config_test.rb index 063634872..ba9a0088a 100644 --- a/test/unit/plugins/providers/docker/config_test.rb +++ b/test/unit/plugins/providers/docker/config_test.rb @@ -43,6 +43,7 @@ describe VagrantPlugins::DockerProvider::Config do before { subject.finalize! } its(:build_dir) { should be_nil } + its(:git_repo) { should be_nil } its(:expose) { should eq([]) } its(:cmd) { should eq([]) } its(:env) { should eq({}) } @@ -67,16 +68,44 @@ describe VagrantPlugins::DockerProvider::Config do allow(Vagrant::Util::Platform).to receive(:linux?).and_return(true) end - it "should be invalid if both build dir and image are set" do - subject.build_dir = build_dir - subject.image = "foo" - subject.finalize! - assert_invalid + describe "should be invalid if any two or more of build dir, git repo and image are set" do + it "build dir and image" do + subject.build_dir = build_dir + subject.image = "foo" + subject.git_repo = nil + subject.finalize! + assert_invalid + end + + it "build dir and git repo" do + subject.build_dir = build_dir + subject.git_repo = "http://someone.com/something.git#branch:dir" + subject.image = nil + subject.finalize! + assert_invalid + end + + it "git repo dir and image" do + subject.build_dir = nil + subject.git_repo = "http://someone.com/something.git#branch:dir" + subject.image = "foo" + subject.finalize! + assert_invalid + end + + it "build dir, git repo and image" do + subject.build_dir = build_dir + subject.git_repo = "http://someone.com/something.git#branch:dir" + subject.image = "foo" + subject.finalize! + assert_invalid + end end describe "#build_dir" do - it "should be valid if not set with image" do + it "should be valid if not set with image or git repo" do subject.build_dir = nil + subject.git_repo = nil subject.image = "foo" subject.finalize! assert_valid @@ -88,6 +117,52 @@ describe VagrantPlugins::DockerProvider::Config do assert_valid end end + + describe "#git_repo" do + it "should be valid if not set with image or build dir" do + subject.build_dir = nil + subject.git_repo = "http://someone.com/something.git#branch:dir" + subject.image = nil + subject.finalize! + assert_valid + end + + it "should be valid with a http git url" do + subject.git_repo = "http://someone.com/something.git#branch:dir" + subject.finalize! + assert_valid + end + + it "should be valid with a git@ url" do + subject.git_repo = "git@someone.com:somebody/something" + subject.finalize! + assert_valid + end + + it "should be valid with a git:// url" do + subject.git_repo = "git://someone.com/something" + subject.finalize! + assert_valid + end + + it "should be valid with a short url beginning with github.com url" do + subject.git_repo = "github.com/somebody/something" + subject.finalize! + assert_valid + end + + it "should be invalid with an non-git url" do + subject.git_repo = "http://foo.bar.com" + subject.finalize! + assert_invalid + end + + it "should be invalid with an non url" do + subject.git_repo = "http||://foo.bar.com sdfs" + subject.finalize! + assert_invalid + end + end describe "#compose" do before do @@ -179,7 +254,7 @@ describe VagrantPlugins::DockerProvider::Config do subject { one.merge(two) } - context "#build_dir and #image" do + context "#build_dir, #git_repo and #image" do it "overrides image if build_dir is set previously" do one.build_dir = "foo" two.image = "bar" @@ -187,22 +262,72 @@ describe VagrantPlugins::DockerProvider::Config do expect(subject.build_dir).to be_nil expect(subject.image).to eq("bar") end + + it "overrides image if git_repo is set previously" do + one.git_repo = "foo" + two.image = "bar" - it "overrides image if build_dir is set previously" do + expect(subject.image).to eq("bar") + expect(subject.git_repo).to be_nil + end + + it "overrides build_dir if image is set previously" do one.image = "foo" two.build_dir = "bar" - expect(subject.image).to be_nil expect(subject.build_dir).to eq("bar") + expect(subject.image).to be_nil + end + + it "overrides build_dir if git_repo is set previously" do + one.git_repo = "foo" + two.build_dir = "bar" + + expect(subject.build_dir).to eq("bar") + expect(subject.git_repo).to be_nil + end + + it "overrides git_repo if build_dir is set previously" do + one.build_dir = "foo" + two.git_repo = "bar" + + expect(subject.build_dir).to be_nil + expect(subject.git_repo).to eq("bar") + end + + it "overrides git_repo if image is set previously" do + one.image = "foo" + two.git_repo = "bar" + + expect(subject.image).to be_nil + expect(subject.git_repo).to eq("bar") end - it "preserves if both set" do + it "preserves if both image and build_dir are set" do one.image = "foo" two.image = "baz" two.build_dir = "bar" - expect(subject.image).to eq("baz") expect(subject.build_dir).to eq("bar") + expect(subject.image).to eq("baz") + end + + it "preserves if both image and git_repo are set" do + one.image = "foo" + two.image = "baz" + two.git_repo = "bar" + + expect(subject.image).to eq("baz") + expect(subject.git_repo).to eq("bar") + end + + it "preserves if both build_dir and git_repo are set" do + one.build_dir = "foo" + two.build_dir = "baz" + two.git_repo = "bar" + + expect(subject.build_dir).to eq("baz") + expect(subject.git_repo).to eq("bar") end end From cbc69f51584219ec3f41a3735fc912c2b7badb9b Mon Sep 17 00:00:00 2001 From: Oleksiy Protas Date: Mon, 17 Oct 2016 00:51:33 +0300 Subject: [PATCH 3/8] Build from git operation for docker --- plugins/providers/docker/action/build.rb | 8 +++++--- plugins/providers/docker/action/is_build.rb | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/plugins/providers/docker/action/build.rb b/plugins/providers/docker/action/build.rb index f6c071fc0..34d25827e 100644 --- a/plugins/providers/docker/action/build.rb +++ b/plugins/providers/docker/action/build.rb @@ -17,9 +17,11 @@ module VagrantPlugins machine = env[:machine] build_dir = env[:build_dir] build_dir ||= machine.provider_config.build_dir - + git_repo = env[:git_repo] + git_repo ||= machine.provider_config.git_repo + # If we're not building a container, then just skip this step - return @app.call(env) if !build_dir + return @app.call(env) if (!build_dir && !git_repo) # Try to read the image ID from the cache file if we've # already built it. @@ -52,7 +54,7 @@ module VagrantPlugins end image = machine.provider.driver.build( - build_dir, + build_dir || git_repo, extra_args: args) do |type, data| data = remove_ansi_escape_codes(data.chomp).chomp env[:ui].detail(data) if data != "" diff --git a/plugins/providers/docker/action/is_build.rb b/plugins/providers/docker/action/is_build.rb index fc2c4be8d..882e294c8 100644 --- a/plugins/providers/docker/action/is_build.rb +++ b/plugins/providers/docker/action/is_build.rb @@ -7,7 +7,7 @@ module VagrantPlugins end def call(env) - env[:result] = !!env[:machine].provider_config.build_dir + env[:result] = (!!env[:machine].provider_config.build_dir || !!env[:machine].provider_config.git_repo) @app.call(env) end end From 166fe374b61eb901c44e5de2dd97ca7f02d69719 Mon Sep 17 00:00:00 2001 From: Oleksiy Protas Date: Mon, 17 Oct 2016 01:05:05 +0300 Subject: [PATCH 4/8] More explicit logging and 'dockerfile' option support --- plugins/providers/docker/action/build.rb | 23 ++++++++++++++++++----- templates/locales/providers_docker.yml | 4 ++++ 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/plugins/providers/docker/action/build.rb b/plugins/providers/docker/action/build.rb index 34d25827e..cb03f8ba0 100644 --- a/plugins/providers/docker/action/build.rb +++ b/plugins/providers/docker/action/build.rb @@ -43,14 +43,27 @@ module VagrantPlugins args = machine.provider_config.build_args.clone if machine.provider_config.dockerfile dockerfile = machine.provider_config.dockerfile - dockerfile_path = File.join(build_dir, dockerfile) + dockerfile_path = build_dir ? File.join(build_dir, dockerfile) : dockerfile args.push("--file").push(dockerfile_path) - machine.ui.output( - I18n.t("docker_provider.building_named_dockerfile", - file: machine.provider_config.dockerfile)) + if build_dir + machine.ui.output( + I18n.t("docker_provider.building_named_dockerfile", + file: machine.provider_config.dockerfile)) + else + machine.ui.output( + I18n.t("docker_provider.building_git_repo_named_dockerfile", + file: machine.provider_config.dockerfile, + repo: git_repo)) + end else - machine.ui.output(I18n.t("docker_provider.building")) + if build_dir + machine.ui.output(I18n.t("docker_provider.building")) + else + machine.ui.output( + I18n.t("docker_provider.building_git_repo", + repo: git_repo)) + end end image = machine.provider.driver.build( diff --git a/templates/locales/providers_docker.yml b/templates/locales/providers_docker.yml index df1fa2ceb..99aefd370 100644 --- a/templates/locales/providers_docker.yml +++ b/templates/locales/providers_docker.yml @@ -12,8 +12,12 @@ en: Build image no longer exists. Rebuilding... building: |- Building the container from a Dockerfile... + building_git_repo: |- + Building the container from the git repository: %{repo}... building_named_dockerfile: |- Building the container from the named Dockerfile: %{file}... + building_git_repo_named_dockerfile: |- + Building the container from the named Dockerfile: %{file} in the git repository: %{repo}... creating: |- Creating the container... created: |- From aecfc45eab0ce23415747832c9fc138a8620764c Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 19 Sep 2018 10:38:40 -0700 Subject: [PATCH 5/8] Add note about required docker config options --- website/source/docs/docker/configuration.html.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/website/source/docs/docker/configuration.html.md b/website/source/docs/docker/configuration.html.md index 955c62991..9dcf4f1ea 100644 --- a/website/source/docs/docker/configuration.html.md +++ b/website/source/docs/docker/configuration.html.md @@ -14,14 +14,16 @@ you may set. A complete reference is shown below. ### Required +One of the following settings is required when using the Docker provider: + * `build_dir` (string) - The path to a directory containing a Dockerfile. One of this, `image` or `git_repo` is required. * `image` (string) - The image to launch, specified by the image ID or a name such as `ubuntu:12.04`. One of this, `git_repo` or `build_dir` is required. - + * `git_repo` (string) - The URL of a git repository to build the image from. - Supports pulling specific tags, branches and revision, consult the + Supports pulling specific tags, branches and revision, consult the [docker documenation](https://docs.docker.com/engine/reference/commandline/build/#/git-repositories) for more information. One of this, `image` or `build_dir` is required. From 29128e73b954eb29dd89cfec8a52d7bbc4319133 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 19 Sep 2018 10:52:42 -0700 Subject: [PATCH 6/8] Remove old changelog addition --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aeb892839..981813cab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -617,7 +617,6 @@ IMPROVEMENTS: - provisioners/puppet: Support custom environment variables [GH-7931, GH-7252, GH-2270] - util/safe_exec: Use subprocess for safe_exec on Windows [GH-7802] - util/subprocess: Allow closing STDIN [GH-7778] - - providers/docker: Support building containter images directly from git BUG FIXES: From 4612619dc46e3273b1616b4732c605494d7882b1 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 19 Sep 2018 10:52:59 -0700 Subject: [PATCH 7/8] Fixup docker config update --- plugins/providers/docker/config.rb | 19 +++++++++---------- templates/locales/providers_docker.yml | 2 +- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/plugins/providers/docker/config.rb b/plugins/providers/docker/config.rb index 6abc997cc..07c4e5333 100644 --- a/plugins/providers/docker/config.rb +++ b/plugins/providers/docker/config.rb @@ -14,12 +14,12 @@ module VagrantPlugins attr_accessor :build_args # The directory with a Dockerfile to build and use as the basis - # for this container. If this is set, neither "image" nor "git_repo" + # for this container. If this is set, neither "image" nor "git_repo" # should be set. # # @return [String] attr_accessor :build_dir - + # The URL for a git repository with a Dockerfile to build and use # as the basis for this container. If this is set, neither "image" # nor "build_dir" should be set. @@ -197,7 +197,7 @@ module VagrantPlugins has_image = (other.image != UNSET_VALUE) has_build_dir = (other.build_dir != UNSET_VALUE) has_git_repo = (other.git_repo != UNSET_VALUE) - + if (has_image ^ has_build_dir ^ has_git_repo) && !(has_image && has_build_dir && has_git_repo) # image if has_image @@ -208,7 +208,7 @@ module VagrantPlugins result.git_repo = nil end end - + # build_dir if has_build_dir if @image != UNSET_VALUE @@ -218,7 +218,7 @@ module VagrantPlugins result.git_repo = nil end end - + # git_repo if has_git_repo if @build_dir != UNSET_VALUE @@ -296,10 +296,9 @@ module VagrantPlugins def validate(machine) errors = _detected_errors - - # FIXME: is there a more ruby-elegant way to write this? - if (@build_dir? 1 : 0) + (@git_repo? 1 : 0) + (@image? 1 : 0) > 1 - errors << I18n.t("docker_provider.errors.config.both_build_and_image") + + if [@build_dir, @git_repo, @image].compact.size > 1 + errors << I18n.t("docker_provider.errors.config.both_build_and_image_and_git") end if !@build_dir && !@git_repo && !@image @@ -312,7 +311,7 @@ module VagrantPlugins errors << I18n.t("docker_provider.errors.config.build_dir_invalid") end end - + # Comparison logic taken directly from docker's urlutil.go if @git_repo && !( @git_repo =~ /^http(?:s)?:\/\/.*.git(?:#.+)?$/ || @git_repo =~ /^git(?:hub\.com|@|:\/\/)/) errors << I18n.t("docker_provider.errors.config.git_repo_invalid") diff --git a/templates/locales/providers_docker.yml b/templates/locales/providers_docker.yml index 99aefd370..ba90473c6 100644 --- a/templates/locales/providers_docker.yml +++ b/templates/locales/providers_docker.yml @@ -141,7 +141,7 @@ en: and notify them to not use this communicator for anything except the "docker" provider. config: - both_build_and_image: |- + both_build_and_image_and_git: |- Only one of "build_dir", "git_repo" or "image" can be set build_dir_invalid: |- "build_dir" must exist and contain a Dockerfile From d471932a4ec5c3952fb4108b18f2c541e4d12734 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 19 Sep 2018 10:54:14 -0700 Subject: [PATCH 8/8] Update website about required docker params --- website/source/docs/docker/configuration.html.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/website/source/docs/docker/configuration.html.md b/website/source/docs/docker/configuration.html.md index 9dcf4f1ea..0a9f94746 100644 --- a/website/source/docs/docker/configuration.html.md +++ b/website/source/docs/docker/configuration.html.md @@ -17,15 +17,14 @@ you may set. A complete reference is shown below. One of the following settings is required when using the Docker provider: * `build_dir` (string) - The path to a directory containing a Dockerfile. - One of this, `image` or `git_repo` is required. * `image` (string) - The image to launch, specified by the image ID or a name - such as `ubuntu:12.04`. One of this, `git_repo` or `build_dir` is required. + such as `ubuntu:12.04`. * `git_repo` (string) - The URL of a git repository to build the image from. Supports pulling specific tags, branches and revision, consult the [docker documenation](https://docs.docker.com/engine/reference/commandline/build/#/git-repositories) - for more information. One of this, `image` or `build_dir` is required. + for more information. ### Optional