use etchosts package from c/common #3917
Conversation
|
@Luap99 needs a no new tests tag, or a new test. |
|
I don't think we are docker compatible here. Docker does not add the container name, we do this in buildah and podman (currently) and I think this makes more sense. Also docker does not have the duplicate entries detection logic so this can result in problems for users when they manually add them with --add-host and podman/buildah could add another entry with this name. Overall I think this is much better and more robust compared to docker. I also don't think users would rely on such "undefined" behaviour but we never know until they are complaining to us. |
|
@rhatdan 's call, but I'm very leery to not match Docker's default behavior. If we add a new option that handles this difference with additional options like we did with |
|
I am fine with this change, Having the /etc/hosts work the same in builds versus regular containers in podman run and podman build, makes sense to me. |
|
@TomSweeneyRedHat Well we match mostly docker, we just add the container name as well. This is done currently in both podman and buildah and I see no reason to change this. This should also not cause any issues for docker users since it is just an extra name. |
openshift-ci
bot
added
the
needs-rebase
Signed-off-by: Paul Holzinger <[email protected]>
openshift-ci
bot
removed
the
needs-rebase
|
LGTM |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Luap99, rhatdan 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 |
|
/lgtm |
Use the new etchosts package to generate the hosts file. This will ensure that we use the same logic in podman and buildah. New features are: - no duplicated entries - adds entries for the network/slirp4netns ips - configure the host.containers.internal entry in containers.conf - configure the base hosts file in containers.conf Signed-off-by: Paul Holzinger <[email protected]>
|
tests are green |
|
/lgtm |
|
/hold cancel |
use etchosts package from c/common Signed-off-by: Mike Lawrenceit.com <[email protected]>
What type of PR is this?
/kind feature
What this PR does / why we need it:
Use the new etchosts package to generate the hosts file.
This will ensure that we use the same logic in podman and buildah.
New features are:
How to verify it
Which issue(s) this PR fixes:
Special notes for your reviewer:
Does this PR introduce a user-facing change?