[WIP] Return slirp ip information from getHosts() #13219
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 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 |
openshift-ci
bot
added
the
approved
|
@Luap99 PTAL, I have added comment directed at you on why this is not working for rootfull containers. Basically the tests is And user expects to see test2 and test1 listed in /etc/hosts. |
libpod/container_internal_linux.go
Outdated
| @@ -2397,7 +2407,7 @@ func (c *Container) getHosts() string { | |||
| // if no bridge gw was found try to use a local ip | |||
| if host == "" { | |||
| if ip := getLocalIP(); ip != "" { | |||
| host = fmt.Sprintf("%s\t%s\n", ip, "host.containers.internal") | |||
| host = fmt.Sprintf("%s\t%s %s\n", ip, c.Hostname(), c.config.Name) | |||
There was a problem hiding this comment.
This should not be changed we add host.containers.internal here
There was a problem hiding this comment.
Ok I will remove it, but I think it ends up being added multiple times in certain situations. Bottom line is this does not work in rootfull mode. And I am not sure why.
| if depCtr.config.NetMode.IsSlirp4netns() { | ||
| // When using slirp4netns, the interface gets a static IP | ||
| if slirp4netnsIP, err := GetSlirp4netnsIP(depCtr.slirp4netnsSubnet); err != nil { | ||
| logrus.Warnf("Failed to determine slirp4netnsIP: %v", err.Error()) | ||
| } else { | ||
| return fmt.Sprintf("%s\t%s %s\n", slirp4netnsIP.String(), c.Hostname(), c.config.Name) | ||
| } | ||
| } |
There was a problem hiding this comment.
This block has to be moved to line 2318, you are changing the host.containers.internal code here.
There was a problem hiding this comment.
I did not find the depCtr until now.
| outer: | ||
| // Paul this is returning nothing, even though I know depctr is up and | ||
| // running with an IP Address in rootfull mode? |
There was a problem hiding this comment.
I think you want to change c.cniHosts() and not the code here
There was a problem hiding this comment.
Fine, how do I change it. It looks like depctr has no network in rootfull mode.
There was a problem hiding this comment.
seems to work for me, what are you expecting?
This code is for host.containers.internal, if you want to add the container/host names you need to do this at a different place before this.
There was a problem hiding this comment.
Where would you suggest?
When one container shares the network namespace with another container or with a Pod, there should be an entry added to the /etc/hosts file for the second container. Fixes: containers#12003 Signed-off-by: Daniel J Walsh <[email protected]>
|
Friendly ping. @rhatdan, do you still want this in? |
|
No this should be part of my /etc/hosts rewrite for c/common |
github-actions
bot
added
the
locked - please file new issue/PR
When one container shares the network namespace with another container
or with a Pod, there should be an entry added to the /etc/hosts file
for the second container.
Fixes: #12003
Signed-off-by: Daniel J Walsh [email protected]