Add --add-host for docker build #30383

tabboud · 2017-01-23T23:45:13Z

Signed-off-by: Tony Abboud tdabboud@hotmail.com

- What I did
Added the --add-host flag to the docker build command. This flag allows you to add extra hosts to a containers /etc/hosts file upon building an image.

- How I did it
I added another flag to the build handler and added server side code to accept and set the extra hosts when the /build endpoint is hit.

- How to verify it
I have added an integration test TestBuildWithExtraHost which adds hosts whose ip is set to the localhost, but then it tries to ping based off of the hostname that was set.

- Description for the changelog
Add the --add-host flag to docker build to add entries to a containers /etc/hosts file when building an image

Implements #30096

LK4D4 · 2017-01-31T22:08:27Z

Can't this be achieved with ARG or whatever?
ping @docker/core-engine-maintainers

vdemeester

ping @tiborvass @tonistiigi @duglin @vieux

duglin · 2017-02-07T16:09:37Z

@LK4D4 can you elaborate? Are you imagining some kind of "cat $ARG >> /etc/hosts" type of thing appearing in the Dockerfile?

duglin · 2017-02-07T16:10:55Z

cli/command/image/build.go

@@ -107,6 +109,7 @@ func NewBuildCommand(dockerCli *command.DockerCli) *cobra.Command {
 	flags.BoolVar(&options.compress, "compress", false, "Compress the build context using gzip")
 	flags.StringSliceVar(&options.securityOpt, "security-opt", []string{}, "Security options")
 	flags.StringVar(&options.networkMode, "network", "default", "Set the networking mode for the RUN instructions during build")
+	flags.Var(&options.extraHosts, "add-host", "Add a custom host-to-IP mapping (host:ip)")


thank you for adding the "form" to the help text - I can never remember that stuff.

👍 me neither

duglin · 2017-02-07T16:13:01Z

integration-cli/docker_cli_build_test.go

+	buildImageSuccessfully(c, name,
+		withBuildFlags(
+			"--add-host", "foo:127.0.0.1",
+			"--add-host", "bar:127.0.0.1",


I know we're in the design phase and this is premature, but if I don't ask now I'll probably forget....
can you add some negative tests? For example --add-host myhost: or --add-host myhost and ones with bad IPs. And ones with ipv6

That makes total sense. I was thinking about this when I wrote the initial test. I will add some more to this PR and push it again.

duglin · 2017-02-07T16:13:28Z

In your testing, does the /etc/hosts file get saved in the resulting image?

duglin · 2017-02-07T16:14:33Z

as long as the changes are not saved in the resulting image, this SGTM

tabboud · 2017-02-09T18:39:39Z

The /etc/hosts file is not saved in the resulting image. I actually expressed concern about this in #30096

thaJeztah · 2017-02-09T19:35:35Z

We discussed this in the maintainers meeting, and although we still think setting up a DNS for these purposes, we already implemented --network for docker build, so let's go ahead and do the same for --add-host

vdemeester · 2017-02-13T09:40:30Z

@TDAbboud needs a rebase 👼

tabboud · 2017-02-14T00:58:02Z

@vdemeester I just re-based and pushed the branch again. I also added another test for some invalid extra hosts.
There currently is no test for a valid IPv6 address, as I was not sure how exactly to implement this. The default /etc/hosts file already contains entries for localhost i.e. ::1 localhost so adding another entry will not work. I think it would be good to test this case as well though, if anyone has a good idea on how to test it.

vdemeester

Small nit on the API changes, but otherwise LGTM 🦁

vdemeester · 2017-02-14T08:36:07Z

docs/api/version-history.md

@@ -34,6 +34,7 @@ keywords: "API, Docker, rcli, REST, documentation"
 * The API version is now required in all API calls. Instead of just requesting, for example, the URL `/containers/json`, you must now request `/v1.25/containers/json`.
 * `GET /version` now returns `MinAPIVersion`.
 * `POST /build` accepts `networkmode` parameter to specify network used during build.
+* `POST /build` now accepts `extrahosts` parameter to specify a host to ip mapping to use during the build.


This is not at the right place, should be in v1.27 API changes 👼

Good catch! I just fixed this and re-pushed

cpuguy83

LGTM

We are currently having issues with CI, so we need to make sure to wait for CI to run against this PR.

Thanks!

Signed-off-by: Tony Abboud <tdabboud@hotmail.com>

tabboud · 2017-02-23T22:38:02Z

Just pinging to verify all tests have passed and the branch has been rebased

thaJeztah

LGTM, thanks!

thaJeztah · 2017-02-27T11:42:47Z

/cc @albers @sdurrheimer for completion scripts

Add --add-host for docker build

arehmandev · 2017-07-12T14:55:37Z

Can we confirm this is working? I have just recently tried:

docker build --add-host=docker:10.180.0.1 -t abs
docker exec -it "containerid" sh
cat /etc/hosts - it doesn't appear there

thaJeztah · 2017-07-12T15:01:30Z

the host that's added with this flag is only used during the build; it deliberately should not persist in the image

arehmandev · 2017-07-12T15:08:25Z

Is this really as intended? I mean other than at runtime, how can you persist host entries to the image?

perhaps the best way to approach this would be something like "HOSTENTRY" in the Dockerfile

cpuguy83 · 2017-07-12T15:13:10Z

@arehmandev You don't. These are generally host specific settings and are not persisted into the image.

arehmandev · 2017-07-12T15:25:43Z

So I guess its up to the container orchestration system now to plug in these values. Cool thanks for clarification

GordonTheTurtle added the status/0-triage label Jan 23, 2017

vdemeester reviewed Feb 7, 2017

View reviewed changes

duglin reviewed Feb 7, 2017

View reviewed changes

thaJeztah added status/2-code-review and removed status/1-design-review labels Feb 9, 2017

GordonTheTurtle assigned thaJeztah Feb 10, 2017

cpuguy83 mentioned this pull request Feb 13, 2017

when using wget in a dockerfile the /etc/hosts of the server is not used only dns #30913

Closed

tabboud force-pushed the 30096-add-host-docker-build branch from 8b10e11 to 0b5b724 Compare February 14, 2017 00:53

vdemeester approved these changes Feb 14, 2017

View reviewed changes

tabboud force-pushed the 30096-add-host-docker-build branch from 0b5b724 to c21c5a6 Compare February 14, 2017 13:15

cpuguy83 approved these changes Feb 14, 2017

View reviewed changes

tabboud force-pushed the 30096-add-host-docker-build branch from c21c5a6 to ad3cfcf Compare February 20, 2017 20:00

Add --add-host for docker build

7a962e4

Signed-off-by: Tony Abboud <tdabboud@hotmail.com>

tabboud force-pushed the 30096-add-host-docker-build branch from ad3cfcf to 7a962e4 Compare February 20, 2017 22:32

thaJeztah added impact/api impact/cli impact/documentation labels Feb 27, 2017

thaJeztah approved these changes Feb 27, 2017

View reviewed changes

thaJeztah merged commit a64ea37 into moby:master Feb 27, 2017

GordonTheTurtle added this to the 17.04.0 milestone Feb 27, 2017

albers mentioned this pull request Feb 27, 2017

Add bash completion for docker build --add-host #31382

Merged

sdurrheimer mentioned this pull request Mar 22, 2017

Add zsh completion for 'docker build --add-host' #32017

Merged

dnephin pushed a commit to dnephin/docker that referenced this pull request Apr 17, 2017

Merge pull request moby#30383 from TDAbboud/30096-add-host-docker-build

a3adb78

Add --add-host for docker build

dnephin pushed a commit to dnephin/docker that referenced this pull request Apr 17, 2017

Merge pull request moby#30383 from TDAbboud/30096-add-host-docker-build

7665cb5

Add --add-host for docker build

This was referenced Jun 14, 2017

Feature Request: add --add-host=[] option to docker build #30096

Closed

Please add --add-host=[], --net options to docker build #10324

Closed

thaJeztah mentioned this pull request Jun 14, 2017

Support --dns or --addn-hosts for 'docker build' like 'docker run' #24928

Closed

thaJeztah mentioned this pull request Jul 12, 2017

docker build --add-host is not working #34078

Closed

thaJeztah mentioned this pull request Aug 18, 2017

Support --dns or --addn-hosts for docker build #5779

Closed

veyalla mentioned this pull request Dec 8, 2019

Unable to load the service index for source https://api.nuget.org/v3/index.json Azure/iotedge#2054

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add --add-host for docker build #30383

Add --add-host for docker build #30383

tabboud commented Jan 23, 2017

LK4D4 commented Jan 31, 2017

vdemeester left a comment

duglin commented Feb 7, 2017

duglin Feb 7, 2017

tabboud Feb 9, 2017

duglin Feb 7, 2017

tabboud Feb 9, 2017 •

edited

Loading

duglin commented Feb 7, 2017

duglin commented Feb 7, 2017

tabboud commented Feb 9, 2017

thaJeztah commented Feb 9, 2017

vdemeester commented Feb 13, 2017

tabboud commented Feb 14, 2017

vdemeester left a comment

vdemeester Feb 14, 2017

tabboud Feb 14, 2017

cpuguy83 left a comment

tabboud commented Feb 23, 2017

thaJeztah left a comment

thaJeztah commented Feb 27, 2017

arehmandev commented Jul 12, 2017

thaJeztah commented Jul 12, 2017

arehmandev commented Jul 12, 2017

cpuguy83 commented Jul 12, 2017

arehmandev commented Jul 12, 2017

Add --add-host for docker build #30383

Add --add-host for docker build #30383

Conversation

tabboud commented Jan 23, 2017

LK4D4 commented Jan 31, 2017

vdemeester left a comment

Choose a reason for hiding this comment

duglin commented Feb 7, 2017

duglin Feb 7, 2017

Choose a reason for hiding this comment

tabboud Feb 9, 2017

Choose a reason for hiding this comment

duglin Feb 7, 2017

Choose a reason for hiding this comment

tabboud Feb 9, 2017 • edited Loading

Choose a reason for hiding this comment

duglin commented Feb 7, 2017

duglin commented Feb 7, 2017

tabboud commented Feb 9, 2017

thaJeztah commented Feb 9, 2017

vdemeester commented Feb 13, 2017

tabboud commented Feb 14, 2017

vdemeester left a comment

Choose a reason for hiding this comment

vdemeester Feb 14, 2017

Choose a reason for hiding this comment

tabboud Feb 14, 2017

Choose a reason for hiding this comment

cpuguy83 left a comment

Choose a reason for hiding this comment

tabboud commented Feb 23, 2017

thaJeztah left a comment

Choose a reason for hiding this comment

thaJeztah commented Feb 27, 2017

arehmandev commented Jul 12, 2017

thaJeztah commented Jul 12, 2017

arehmandev commented Jul 12, 2017

cpuguy83 commented Jul 12, 2017

arehmandev commented Jul 12, 2017

tabboud Feb 9, 2017 •

edited

Loading