-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Add hostname to /etc/hosts for --net=none #8101
Add hostname to /etc/hosts for --net=none #8101
Conversation
This does not match Docker, which does not add hostname in this case, but it seems harmless enough. Fixes containers#8095 Signed-off-by: Matthew Heon <[email protected]>
[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 |
Changes LGTM |
/lgtm |
/hold cancel |
run := podmanTest.Podman([]string{"run", "--net=none", "--hostname", hostname, ALPINE, "hostname"}) | ||
run.WaitWithDefaultTimeout() | ||
Expect(run.ExitCode()).To(BeZero()) | ||
Expect(strings.Contains(run.OutputToString(), hostname)).To(BeTrue()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What exactly is this test supposed to accomplish? Actually, what exactly is this PR supposed to accomplish? As I read the description and the code -- and of course I may just be confused -- I expected the given hostname to appear in the container's /etc/hosts
file. That is not what I'm seeing:
$ ./bin/podman run --net=none --hostname abcde alpine cat /etc/hosts
127.0.0.1 localhost localhost.localdomain
::1 localhost localhost.localdomain
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds like a bug, because it should be in there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, none of the code added in this PR is ever actually invoked, because of:
podman/libpod/container_internal_linux.go
Line 1243 in 7774f63
if !netDisabled { |
(This is what calls
generateHosts()
, and it's not called if netDisabled
).
This does not match Docker, which does not add hostname in this case, but it seems harmless enough.
Fixes #8095