Only add 127.0.0.1 entry to /etc/hosts with --net=none #11605
Conversation
openshift-ci
bot
added
the
approved
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, Luap99 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 |
pkg/namespaces/namespaces.go
Outdated
| @@ -337,7 +337,7 @@ type NetworkMode string | |||
|
|
|||
| // IsNone indicates whether container isn't using a network stack. | |||
| func (n NetworkMode) IsNone() bool { | |||
| return n == noneType | |||
| return n == noneType || n == "" | |||
There was a problem hiding this comment.
Is this accurate? "" could also be the default (bridge), couldn't it?
There was a problem hiding this comment.
The thing is "none" is never set via the With... functions so all we have is an empty string for the NetMode.
Bridge and slirp4netns are set via WithNetNS which should set the mode correctly. There is a lot of code using NetMode.IsBridge() which only matches bridge so I think this is correct.
There was a problem hiding this comment.
Alright. We really ought to make this more reliable in 4.0, having some cases not assign this is iffy.
There was a problem hiding this comment.
OK, this might be a bigger problem than I thought. If a container is joined to a pod or another container it also has an empty NetworkMode.
|
Testing is not happy with this change. |
The check for net=none was wrong. It just assumed when we do not create the netns but have one set that we use the none mode. This however also applies to a container which joins the pod netns. To correctly check for the none mode use `config.NetMode.IsNone()`. Fixes containers#11596 Signed-off-by: Paul Holzinger <[email protected]>
| if ns.Type == spec.NetworkNamespace { | ||
| if ns.Path == "" && !c.config.CreateNetNS { | ||
| netNone = true | ||
| if c.config.NetNsCtr == "" && !c.config.CreateNetNS { |
There was a problem hiding this comment.
Honestly, this is probably safer...
There was a problem hiding this comment.
Yes we should fix this for the future so that we store the netmode in libpod. But now we are stucked with this forever if we want to support backwards compat.
|
LGTM |
|
/lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
The check for net=none was wrong. It just assumed when we do not create
the netns but have one set that we use the none mode. This however also
applies to a container which joins the pod netns.
To correctly check for the none mode use
config.NetMode.IsNone().Fixes #11596