Set up DNS names for Windows default network #47375
Conversation
DNS names were only set up for user-defined networks. On Linux, none of the built-in networks (bridge/host/none) have built-in DNS, so they don't need DNS names. But, on Windows, the default network is "nat" and it does need the DNS names. Signed-off-by: Rob Murray <rob.murray@docker.com>
|
Honestly, I wonder if this was ever intentional. As the design for the default network (at least for Preserving the existing behavior, at least for now, seems sane, but we should look when this "feature" was introduced, and if it was intentional or not. |
| // Set up DNS names for a user defined network, and for the default 'nat' | ||
| // network on Windows (IsBridge() returns true for nat). | ||
| if containertypes.NetworkMode(n.Name()).IsUserDefined() || | ||
| (serviceDiscoveryOnDefaultNetwork() && containertypes.NetworkMode(n.Name()).IsBridge()) { |
There was a problem hiding this comment.
Could we ever hit the default network here (so not "nat")?
moby/api/types/container/hostconfig.go
Lines 140 to 143 in a60546b
Wondering if we should include a link to the github discussion here for future reference 🤔
There was a problem hiding this comment.
I tried to get "default" as far as this, but couldn't - so, decided not to add it. I couldn't see any problem with adding it just-in-case but, because I couldn't test it, I left it out and made a change targeted as specifically as possible at the issue being fixed. (Maybe libnetwork has a surprise to spring with "default", it usually does!)
For the github link ... I think the comment probably covers it, and git-blame is there if needed. But, could do?
As noted in the description, we need a follow-up change to clean this up properly - probably by adding a better predicate and looking at where else it should be used (and what the consequences would be). For example, I think the test in setupPathsAndSandboxOptions for whether to use systemd's resolv.conf also uses IsUserDefined as a proxy for "has internal resolver", so it suffers a similar problem... swarm-scoped networks, which do have an internal resolver, end up using systemd's underlying nameservers instead of 127.0.0.53... I think they miss the IsUserDefined() case by having the name default when it matters. I think that's probably an oversight but, as-ever, changing it might break things.
There was a problem hiding this comment.
Yeah, maybe the default is not an issue (but I'm definitely not a fan of that magic value 😂 #19421)
Adding a link is not a real show-stopper for me; was mostly considering if we wanted to have a crumble-path to find back the discussion around this (basically; this is really a bit of a stop-gap, pending a more solid definition of all of this).
|
Issue to track improvement of the predicates used here and elsewhere ... #47378 |
- What I did
Fixes #47370
- How I did it
DNS names were only set up for user-defined networks. On Linux, none of the built-in networks (bridge/host/none) have built-in DNS, so they don't need DNS names.
But, on Windows, the default network is "nat" and it does need the DNS names.
This is meant as a minimal change that we can ship in a patch if we make one ... but we probably need a better predicate to determine whether a container's going to get an internal resolver, here and elsewhere.
- How to verify it
As described in #47370.
Also, without the fix, in
inspectoutput ...And, with the fix ...
Will also need a regression test.
- Description for the changelog
Restore DNS names for containers in the default "nat" network on Windows.