Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

docker: generate /etc/hosts file for bridge network mode #10766

Merged
merged 1 commit into from
Jun 16, 2021
Merged

docker: generate /etc/hosts file for bridge network mode #10766

merged 1 commit into from
Jun 16, 2021

Conversation

tgross
Copy link
Member

@tgross tgross commented Jun 15, 2021 •
edited
Loading

Fixes #8900 and #7746. This is heavily based on @nickethier's earlier work on /etc/resolv.conf

When network.mode = "bridge", we create a pause container in Docker with no
networking so that we have a process to hold the network namespace we create
in Nomad. The default /etc/hosts file of that pause container is then used
for all the Docker tasks that share that network namespace. Some applications
rely on this file being populated.

This changeset generates a /etc/hosts file and bind-mounts it to the
container when Nomad owns the network, so that the container's hostname has an
IP in the file as expected. The hosts file will include the entries added by
the Docker driver's extra_hosts field.

In this changeset, only the Docker task driver will take advantage of this
option, as the exec/java drivers currently copy the host's /etc/hosts
file and this can't be changed without breaking backwards compatibility. But
the fields are available in the task driver protobuf for community task
drivers to use if they'd like.

With this changeset, and the following job:

jobspec 
job "example" {
  datacenters = ["dc1"]

  group "web" {

    network {
      mode = "bridge"
      port "www" {
        to = 8001
      }
    }

    task "httpd" {
      driver = "docker"

      config {
        image       = "busybox:1"
        command     = "httpd"
        args        = ["-v", "-f", "-p", "8001", "-h", "/local"]
        extra_hosts = ["wintermute:192.168.1.246"]
        ports       = ["www"]
      }

      template {
        data        = "<html>hello, world</html>"
        destination = "local/index.html"
      }

      resources {
        cpu    = 128
        memory = 128
      }
    }
  }
}

Results:

$ docker ps
CONTAINER ID   IMAGE                                      COMMAND                  CREATED         STATUS         PORTS     NAMES
34c0b3a0909c   busybox:1                                  "httpd -v -f -p 8001…"   7 seconds ago   Up 6 seconds             httpd-43714360-04ad-ff9c-d8b4-29d5c7961967
567fb409bebe   gcr.io/google_containers/pause-amd64:3.1   "/pause"                 8 seconds ago   Up 6 seconds             nomad_init_43714360-04ad-ff9c-d8b4-29d5c7961967

$ docker exec -it 34c hostname -f
567fb409bebe

$ docker exec -it 34c cat /etc/hosts
# this file was generated by Nomad
127.0.0.1 localhost
::1 localhost
::1 ip6-localhost ip6-loopback
fe00::0 ip6-localnet
ff00::0 ip6-mcastprefix
ff02::1 ip6-allnodes
ff02::2 ip6-allrouters
ff02::3 ip6-allhosts

# this entry is the IP address and hostname of the allocation
# shared with tasks in the task group's network
172.26.64.185 567fb409bebe

# these entries are extra hosts added by the task config
192.168.1.246 wintermute

I've tested this out in non-bridge networking modes and we're correctly leaving everything untouched as expected.

@tgross tgross mentioned this pull request Jun 15, 2021
Closed
@tgross tgross force-pushed the b-8900-etc-hosts branch from f5e57c3 to bb96190 Compare June 16, 2021 12:43
@vercel vercel bot temporarily deployed to Preview – nomad June 16, 2021 12:43 Inactive
@tgross tgross force-pushed the b-8900-etc-hosts branch from bb96190 to 95ff1d7 Compare June 16, 2021 12:49
@vercel vercel bot temporarily deployed to Preview – nomad June 16, 2021 12:49 Inactive
@tgross tgross linked an issue Jun 16, 2021 that may be closed by this pull request
Unable to set extra_hosts when using consul-connect (bridged networking) #7746
Closed
@tgross tgross force-pushed the b-8900-etc-hosts branch from 95ff1d7 to 541b89f Compare June 16, 2021 13:09
@vercel vercel bot temporarily deployed to Preview – nomad June 16, 2021 13:09 Inactive
@tgross tgross linked an issue Jun 16, 2021 that may be closed by this pull request
Allow configuration of network namespace container "extra_hosts" #6322
Open
@tgross tgross removed a link to an issue Jun 16, 2021
Allow configuration of network namespace container "extra_hosts" #6322
Open
@tgross tgross mentioned this pull request Jun 16, 2021
Open
@tgross tgross force-pushed the b-8900-etc-hosts branch from 541b89f to 6ac570a Compare June 16, 2021 13:28
@tgross tgross force-pushed the b-8900-etc-hosts branch from 6ac570a to 4348742 Compare June 16, 2021 14:01
@tgross tgross requested review from notnoop and shoenig June 16, 2021 14:04
@tgross tgross marked this pull request as ready for review June 16, 2021 14:04
@tgross tgross mentioned this pull request Jun 16, 2021
Open
@tgross tgross force-pushed the b-8900-etc-hosts branch from 4348742 to cabfdaf Compare June 16, 2021 14:11
@tgross tgross force-pushed the b-8900-etc-hosts branch from cabfdaf to 23b0326 Compare June 16, 2021 14:14
@tgross tgross added this to the 1.1.2 milestone Jun 16, 2021
@tgross tgross mentioned this pull request Jun 16, 2021
Closed
notnoop
notnoop reviewed Jun 16, 2021
View reviewed changes
Copy link
Contributor

@notnoop notnoop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a quick read and the approach seems good. A couple of minor comments you can consider, and plan to do another round of review later today.

client/allocrunner/network_hook.go Show resolved Hide resolved
client/allocrunner/network_hook.go Outdated Show resolved Hide resolved
drivers/shared/hostnames/mount.go Outdated Show resolved Hide resolved
@vercel vercel bot temporarily deployed to Preview – nomad June 16, 2021 15:04 Inactive
@tgross tgross force-pushed the b-8900-etc-hosts branch from aeca42e to 40b1545 Compare June 16, 2021 15:05
@vercel vercel bot temporarily deployed to Preview – nomad June 16, 2021 15:05 Inactive
shoenig
shoenig approved these changes Jun 16, 2021
View reviewed changes
Copy link
Member

@shoenig shoenig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM; I like the thought of doing the same to exec/java in #10768 in the next feature release

notnoop
notnoop approved these changes Jun 16, 2021
View reviewed changes
Copy link
Contributor

@notnoop notnoop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Great - thanks!

website/content/docs/drivers/docker.mdx Outdated Show resolved Hide resolved
@tgross
docker: generate /etc/hosts file for bridge network mode
74c45bd 
When `network.mode = "bridge"`, we create a pause container in Docker with no
networking so that we have a process to hold the network namespace we create
in Nomad. The default `/etc/hosts` file of that pause container is then used
for all the Docker tasks that share that network namespace. Some applications
rely on this file being populated.

This changeset generates a `/etc/hosts` file and bind-mounts it to the
container when Nomad owns the network, so that the container's hostname has an
IP in the file as expected. The hosts file will include the entries added by
the Docker driver's `extra_hosts` field.

In this changeset, only the Docker task driver will take advantage of this
option, as the `exec`/`java` drivers currently copy the host's `/etc/hosts`
file and this can't be changed without breaking backwards compatibility. But
the fields are available in the task driver protobuf for community task
drivers to use if they'd like.
@tgross tgross force-pushed the b-8900-etc-hosts branch from 40b1545 to 74c45bd Compare June 16, 2021 18:07
@tgross tgross merged commit 2a640f0 into main Jun 16, 2021
@tgross tgross deleted the b-8900-etc-hosts branch June 16, 2021 18:55
This was referenced Jun 16, 2021
Merged
Closed
@kainoaseto kainoaseto mentioned this pull request Jun 24, 2021
Closed
@Oloremo Oloremo mentioned this pull request Jun 29, 2021
Closed
@zh4n7wm zh4n7wm mentioned this pull request Jul 14, 2021
Open
@shishir-a412ed shishir-a412ed mentioned this pull request Jul 28, 2021
Open
@jrasell jrasell mentioned this pull request Aug 10, 2021
Closed
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@shoenig shoenig shoenig approved these changes

@notnoop notnoop notnoop approved these changes

Assignees
No one assigned
Labels
theme/driver/docker theme/networking
Projects
None yet
Milestone
1.1.2
3 participants
@tgross @notnoop @shoenig