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.

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

Ensure that hostname is added to hosts with net=host #8067

Merged
merged 1 commit into from
Oct 20, 2020

Conversation

mheon
Copy link
Member

@mheon mheon commented Oct 19, 2020

When a container uses --net=host the default hostname is set to the host's hostname. However, we were not creating any entries in /etc/hosts despite having a hostname, which is incorrect.

Fixes #8054

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mheon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 19, 2020
@zhangguanzhang
Copy link
Collaborator

seem docker don't have this?

[root@k8s-m1 ~]# docker run --hostname test111  --net host nginx:alpine cat /etc/hosts | grep test111
[root@k8s-m1 ~]# docker info
Client:
 Debug Mode: false

Server:
 Containers: 13
  Running: 9
  Paused: 0
  Stopped: 4
 Images: 22
 Server Version: 19.03.2
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Native Overlay Diff: true
 Logging Driver: json-file
 Cgroup Driver: systemd
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local logentries splunk syslog
 Swarm: inactive
 Runtimes: runc
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: b34a5c8af56e510852c35414db4c1f4fa6172339
 runc version: 3e425f80a8c931f88e6d94a8c831b9d5aa481657
 init version: fec3683
 Security Options:
  seccomp
   Profile: default
 Kernel Version: 5.8.9-1.el7.elrepo.x86_64
 Operating System: CentOS Linux 7 (Core)
 OSType: linux
 Architecture: x86_64
 CPUs: 1
 Total Memory: 1.943GiB
 Name: k8s-m1
 ID: EIUG:BTLZ:UK4M:P6C5:L2IW:UXEA:ZTL5:6CHJ:I3CL:G2UM:SGQ5:LG6S
 Docker Root Dir: /var/lib/docker
 Debug Mode: false
 Username: zhangguanzhang
 Registry: https://index.docker.io/v1/
 Labels:
 Experimental: false
 Insecure Registries:
  127.0.0.0/8
 Registry Mirrors:
  https://fz5yth0r.mirror.aliyuncs.com/
 Live Restore Enabled: false

@mheon
Copy link
Member Author

mheon commented Oct 20, 2020

Docker is unconditionally forcing the entry in /etc/hosts to use the hostname of the host system, not the container. I'll adjust to match this.

When a container uses --net=host the default hostname is set to
the host's hostname. However, we were not creating any entries
in `/etc/hosts` despite having a hostname, which is incorrect.
This hostname, for Docker compat, will always be the hostname of
the host system, not the container, and will be assigned to IP
127.0.1.1 (not the standard localhost address).

Also, when `--hostname` and `--net=host` are both passed, still
use the hostname from `--hostname`, not the host's hostname (we
still use the host's hostname by default in this case if the
`--hostname` flag is not passed).

Fixes containers#8054

Signed-off-by: Matthew Heon <[email protected]>
@mheon
Copy link
Member Author

mheon commented Oct 20, 2020

New changes: we now respect --hostname if passed, even if --net=host was given, and the hostname added to /etc/hosts when --net=host is set will always be the host system's hostname, not the container's hostname - both match Docker.

@TomSweeneyRedHat
Copy link
Member

code LGTM
any notes/changes to man pages needed?

@rhatdan
Copy link
Member

rhatdan commented Oct 20, 2020

Shouldn't --net=host and --hostname conflict?
I don't believe a container should be able to change the hosts namespace, unless it has NET_ADMIN capability?

@mheon
Copy link
Member Author

mheon commented Oct 20, 2020

@rhatdan I don't think we are changing the host, since the container does have a UTS namespace in this case - it's just missing the net namespace.

@rhatdan
Copy link
Member

rhatdan commented Oct 20, 2020

Ah OK, I guess they should conflict with the --uts=host, but no one ever does this.

@rhatdan
Copy link
Member

rhatdan commented Oct 20, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 20, 2020
@openshift-merge-robot openshift-merge-robot merged commit 6961b94 into containers:master Oct 20, 2020
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 24, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

hostname is (still) not added to /etc/hosts
6 participants