keep pod hostname when a container is stopped#28494
Conversation
Luap99
left a comment
There was a problem hiding this comment.
Thanks, we need a test case at least.
But I don't believe fixing this like that is correct, you have just sidestep your personal problem. If a user would have the container name and ip in the hosts file it would still be the same problem.
Isn't the real problem that the etchosts.Remove() only matches one name instead of matching all names? I guess this is because Add() does not actually add all names when the hostname or container name is already present.
Since the main pod infra will always have the hostname as entry with the main ip the localhost entries for the container will never receive the hostname and thus we cannot use the full match like that.
But then if we never add the hostname in the entry that we might as well just drop it from getLocalhostHostEntry() and then it would work already for you?
Practically there is the question of why do you even set --add-host hostname:127.0.0.1 when there is a hostname entry by default?
I have applications running in multiple pods that need a FQDN, so the --hostname argument is not sufficient. To get the FQDN for the pod hostname, there has to be an entry
That sounds great, I'll add a test case and refactor it. |
c139e84 to
08004a4
Compare
|
[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore. |
1 similar comment
|
[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore. |
Honny1
left a comment
There was a problem hiding this comment.
Additionally to @Luap99's comments: I would like to ask you to squash your commits and remove the merge commit (please rebase onto main instead). Also, if there is an existing issue related to this fix, please reference it in the description and commit message using Fixes: #<issue-number>.
08004a4 to
3e97ccd
Compare
|
PTAL @containers/podman-maintainers |
|
there is a lot of information in the PR but that is not present in the git commit message itself, could you add it there? |
|
and also please use your real name for the git commit and signed-off-by line |
When a container in a pod is stopped, its container name is removed from /etc/hosts. etchosts.Remove() filters for any entry matching the container name or the pod hostname. A pod with additional host entries like --add-host FQDN;pod-hostname:127.0.0.1 is affected by this deletion, too. Only the container name needs to be removed when a container is stopped. Signed-off-by: Clemens Klug <git@agp8x.org>
3e97ccd to
24130e2
Compare
|
Added more details to commit message and updated to use the real name |
Checklist
Ensure you have completed the following checklist for your pull request to be reviewed:
commits. (
git commit -s). (If needed, usegit commit -s --amend). The author email must matchthe sign-off email address. See CONTRIBUTING.md
for more information.
Fixes: #00000in commit message (if applicable)make validatepr(format/lint checks)Noneif no user-facing changes)Does this PR introduce a user-facing change?
Problem
When a Pod is configured with additional hosts containing an FQDN for the pod hostname, the FQDN entry is removed when any container is stopped.
Root Cause
etchosts.Remove()is called with an array containing the container hostname. In Pods, the container hostname is configured on pod level.Fix
Omit the hostname during remove
Test
Preparations
mkdir -p /my/run/Create pod
Start containers
Verify
podman exec my-pod-contA hostname -f->myhost--my-pod.my.domainpodman stop my-pod-contBpodman exec my-pod-contA hostname -f->hostname: myhost--my-pod: Host not found