Use --network=host to use local dns in resolv.conf #3244
Conversation
Signed-off-by: Chee Hau Lim <ch33hau@gmail.com>
8ccd9bd to
837978d
Compare
| hostNetMode = true | ||
| } | ||
|
|
||
| resolvConf, err := oci.GetResolvConf(ctx, w.root, nil, w.dnsConfig, hostNetMode) |
There was a problem hiding this comment.
Move this block here so that the if block in line 143 can be reused.
| genFileHasLocalDNS = resolvconf.HasLocal(gen.Content[:]) | ||
| } | ||
|
|
||
| if hostNetMode && oriFileHasLocalDNS && !genFileHasLocalDNS { |
There was a problem hiding this comment.
After comparing all possible 8 combinations, only these 2 combinations need to re-generate resolv.conf.
|
|
||
| for i := 0; i < tc.execution; i++ { | ||
| if i > 0 { | ||
| time.Sleep(100 * time.Millisecond) |
There was a problem hiding this comment.
The "TestRegenerateResolvconf*" tests are flaky without time.Sleep due to the next run might still getting old content... any ideas for not using sleep?
| @@ -139,6 +143,11 @@ func FilterResolvDNS(resolvConf []byte, ipv6Enabled bool) (*File, error) { | |||
| return &File{Content: cleanedResolvConf, Hash: hash}, nil | |||
| } | |||
|
|
|||
| // HasLocal returns true if localhost nameserver entries are found | |||
There was a problem hiding this comment.
This is needed for the generate logic.
| @@ -115,8 +115,12 @@ func GetSpecific(path string) (*File, error) { | |||
| // cleaned config has no defined nameservers left, adds default DNS entries | |||
| // 2. Given the caller provides the enable/disable state of IPv6, the filter | |||
| // code will remove all IPv6 nameservers if it is not enabled for containers | |||
| func FilterResolvDNS(resolvConf []byte, ipv6Enabled bool) (*File, error) { | |||
| cleanedResolvConf := localhostNSRegexp.ReplaceAll(resolvConf, []byte{}) | |||
| func FilterResolvDNS(resolvConf []byte, ipv6Enabled bool, removeLocalDNS bool) (*File, error) { | |||
There was a problem hiding this comment.
It seems like this is the right place to fix the issue (not this vendor file, but the upstream codebase)... but the upstream code might be used by other projects as well, how should we deal with this?
There was a problem hiding this comment.
You would need to make the change in upstream. But in this case I think you can just skip calling this function and add default nameservers with a function on buildkit side if needed.
| @@ -152,8 +139,23 @@ func (w *containerdExecutor) Run(ctx context.Context, id string, root executor.M | |||
| } | |||
| defer namespace.Close() | |||
|
|
|||
| hostNetMode := false | |||
| if meta.NetMode == pb.NetMode_HOST { | |||
There was a problem hiding this comment.
I think this check in here does not cover all cases. Default net provider can also be in host for certain daemon configuration.
I think instead we should extend the network.Provider interface with LocalhostAllowed() method
| @@ -115,8 +115,12 @@ func GetSpecific(path string) (*File, error) { | |||
| // cleaned config has no defined nameservers left, adds default DNS entries | |||
| // 2. Given the caller provides the enable/disable state of IPv6, the filter | |||
| // code will remove all IPv6 nameservers if it is not enabled for containers | |||
| func FilterResolvDNS(resolvConf []byte, ipv6Enabled bool) (*File, error) { | |||
| cleanedResolvConf := localhostNSRegexp.ReplaceAll(resolvConf, []byte{}) | |||
| func FilterResolvDNS(resolvConf []byte, ipv6Enabled bool, removeLocalDNS bool) (*File, error) { | |||
There was a problem hiding this comment.
You would need to make the change in upstream. But in this case I think you can just skip calling this function and add default nameservers with a function on buildkit side if needed.
| @@ -51,6 +51,22 @@ func GetResolvConf(ctx context.Context, stateDir string, idmap *idtools.Identity | |||
| } else { | |||
| if fi.ModTime().Before(fiMain.ModTime()) { | |||
| generate = true | |||
| } else { | |||
| var oriFileHasLocalDNS bool | |||
There was a problem hiding this comment.
This does not look safe if this function is called in parallel with different hostNetMode value. I think the only way would be to return a different path for these cases.
There is also this issue that is related #3096 (but you don't have to fix this as part of this PR)
|
Is there a chance to revive this PR? What are the things left to do? Can I help? |
fix #3210
This is still a draft, there are some parts that need to be clarified.
github.com/docker/docker/libnetwork/resolvconf/resolvconf.goin order to keep the localhost DNS. Are these changes (see below temp changes for this vendor) make sense for the upstream repo?generatelogic so that when users update the--networkoption,resolv.confwill be updated accordingly.@tonistiigi could you please take a look? Thanks!