resolvconf: use /run/systemd/resolve/resolv.conf if systemd-resolved manages DNS #2385

tiborvass · 2019-05-31T19:18:08Z

Signed-off-by: Tibor Vass [email protected]

procfs was copied from https://github.com/moby/moby/blob/e353e7e/daemon/daemon_linux.go#L142-L167. It was a package that was only used for this purpose. It is being removed in moby/moby#39295. It was added in moby in moby/moby#37485

This PR is updated according to the comment below #2385 (comment)

internal/procfs/procfs_linux.go

tiborvass · 2019-05-31T21:42:19Z

Fixed ineffassign linter issue. And added testutils import in the procfs test, because apparently that's needed...

arkodg · 2019-05-31T21:45:55Z

resolvconf/resolvconf.go

 	"regexp"
 	"strings"
 	"sync"

 	"github.com/docker/docker/pkg/ioutils"
+	"github.com/docker/libnetwork/procfs"


Can we use https://godoc.org/k8s.io/kubernetes/pkg/util/procfs instead

It is a copy of that small package, which I'd rather have than depend on k8s.io/kubernetes.

Yes, it was copied at the time to prevent having to pull the whole kubernetes repo on each vendor update.

arkodg · 2019-05-31T22:42:28Z

Dockerfile

@@ -11,8 +11,7 @@ RUN go get golang.org/x/lint/golint \
 		golang.org/x/tools/cmd/cover \
 		github.com/mattn/goveralls \
 		github.com/gordonklaus/ineffassign \
-		github.com/client9/misspell/cmd/misspell \
-		honnef.co/go/tools/cmd/gosimple


Is this cmd failing consistently ?

Yes, try to go get honnef.co/go/tools/cmd/gosimple

resolvconf/resolvconf.go

AkihiroSuda · 2019-06-01T14:42:54Z

resolvconf/resolvconf.go

+		if len(pids) > 0 && pids[0] > 0 {
+			_, err := os.Stat(alternatePath)
+			if err == nil {
+				logrus.Infof("systemd-resolved is running, so using resolvconf: %s", alternatePath)


Probably we should use systemd-resolved config only if /etc/resolv.conf points to a loopback address?

e.g. when running with RootlessKit+slirp4netns/VPNKit, systemd-resolved would be visible (unless PIDNS is unshared) but /etc/resolv.conf would point to slirp4netns/VPNKit-builtin DNS.
Even in that case, using systemd DNS should be fine for almost all usecases, but not 100% sure.

I'm not sure what the correct behavior is. After reading a bit about systemd-resolved i was surprised of the current behavior in moby, but this PR is about bringing whatever we have in moby to libnetwork in order to fix a difference between buildkit and non-buildkit builds.
If you think this won't work with rootlesskit, I suggest we fix it in a follow up.

tiborvass · 2019-06-01T19:20:26Z

@arkodg @selansen updated, PTAL

selansen

LGTM

justincormack · 2019-06-03T15:14:21Z

This seems totally the wrong way to do this.

You can identify whether systemd-resolved is being used by whether /etc/resolv.conf is a symlink to /run/systemd/resolve/stub-resolv.conf or if it contains 127.0.0.53 as the DNS server, full details in https://www.freedesktop.org/software/systemd/man/systemd-resolved.service.html - this should be sanely detectable.

It is never going to work properly to just see if it is running as it could be running in a container or not activated or anything.

thaJeztah · 2019-06-03T16:19:26Z

I seem to recall that the symlink is created only in upgrade scenarios, but not in fresh installs (or the other way round).

tiborvass · 2019-06-03T18:29:25Z

@justincormack I agree with you that the logic is super weird and I think we should fix it. This PR is not about fixing that, it's about moving code here so it can be used by both buildkit and moby to fix a discrepancy between legacy and buildkit builder.

So I suggest we merge this and open up a new one to fix the behavior for all.

tiborvass · 2019-06-03T22:28:58Z

For the record this was the original PR: moby/moby#37485 I'm adding a reference in the description of this PR as well.

tiborvass · 2019-06-04T04:25:50Z

@mavenugo @justincormack ok I read https://www.freedesktop.org/software/systemd/man/systemd-resolved.service.html#/etc/resolv.conf

I'm trying to understand the proper logic here. We can obviously not use 127.0.0.53 inside the container.

So question Q1: do you agree that IF it is detected that systemd-resolved handles DNS, THEN, /run/systemd/resolve/resolv.conf should be read to find out what are the actual nameservers, despite the note in the docs:

Note that /run/systemd/resolve/resolv.conf should not be used directly by applications, but only through a symlink from /etc/resolv.conf.

Question Q2: do you agree that in order to detect whether systemd-resolved handles DNS, I would need to check if EITHER /etc/resolv.conf is a symlink to any of /run/systemd/resolve/stub-resolv.conf, /usr/lib/systemd/resolv.conf, /run/systemd/resolve/resolv.conf, OR /etc/resolv.conf ONLY contains 127.0.0.53 as a nameserver.

If you answered YES for Q1 and Q2, then Q3: do you agree that we don't need to check symlinks at all, we could simplify the logic to: if /etc/resolv.conf ONLY contains 127.0.0.53 as a nameserver, then read /run/systemd/resolve/resolv.conf, otherwise read /etc/resolv.conf.

In particular Q3 implies that if /etc/resolv.conf does not contain 127.0.0.53 as the sole nameserver then it could still be a symlink to /run/systemd/resolve/resolv.conf, in which case reading /etc/resolv.conf is the intended logic.

tiborvass · 2019-06-04T04:45:58Z

I updated the PR assuming you agree with my comment above @justincormack @mavenugo @arkodg

…manages DNS Signed-off-by: Tibor Vass <[email protected]>

Also fixes issue reported by ineffassign Signed-off-by: Tibor Vass <[email protected]>

justincormack · 2019-06-04T11:53:58Z

Yes this looks way better as it is much simpler! There are still configs in which DNS will not work, eg /etc/resolv.conf points to a caching nameserver on 127.0.0.1 but they didn't work before anyway so this is no worse than the current situation.

arkodg

LGTM, thanks for rewriting the path logic !

thaJeztah · 2019-06-09T10:29:36Z

Should we backport this to 18.09 (or would that not be useful without the buildkit changes?)

tiborvass · 2019-06-10T19:06:12Z

@thaJeztah yeah it's not worth the complications I think.

superseed · 2019-06-25T13:05:39Z

@tiborvass I opened #moby/moby#38243 when 18.09 was released because this update included "support for systemd-resolved" and ended up being a regression on my system as I suddenly lost the ability to resolve names available only on one of my interfaces' DNS. See the issue for more details, but the gist of it is that using /run/systemd/resolve/resolv.conf as a "resolvconf style source for DNS servers" hides the complexity that can be expressed by systemd-resolved: in my case, per-interface DNS servers. Queries should be sent to 127.0.0.53 when resolved manages the system, not to the servers listed in /run/systemd/resolve/resolv.conf.

I don't know if this PR is exactly related to the issue but given that it talks of using /run/systemd/resolve/resolv.conf, it seems like it is. In my case, it's for non-default networks (where the containers contain a 127.0.0.11 nameserver in their resolv.conf which from what I get, is an internal resolver in Docker)

I've had to pin Docker to version 18.06 ever since, which is a situation I'd like to get out of if possible.

thaJeztah · 2019-06-25T13:31:25Z

Resolving using 127.0.0.53 from within the container would only be possible in the "custom network" situation, in which case the docker embedded DNS (127.0.0.11) can forward the requests to 127.0.0.53

In the default bridge network, the embedded DNS is not used (for historic reasons / backward compatibility); in that case using 127.0.0.53 won't be possible (so I guess "best alternative" would be to try the DNS servers listed in /run/systemd/resolve/resolv.conf - even if that means you won't be able to make use of the advanced functionality of resolved.

thaJeztah · 2019-06-25T13:33:35Z

That said; perhaps we should revisit the "default bridge network doesn't use the embedded DNS"; also see moby/moby#22295

superseed · 2019-06-25T14:30:47Z

@thaJeztah I am indeed in a situation where I use custom networks (type bridge, but not the default one: it's created by docker-compose) and resolution fails since the host' 127.0.0.53 ends up not getting the queries from the Docker embedded DNS as it bypasses it since 18.09 when its behavior seems to have been changed to "use the servers in /run/systemd/resolve/resolv.conf when systemd-resolved exists" from "use the resolver in /etc/resolv.conf". Only systemd-resolved knows what DNS server to use for what domains and that can't be found in /run/systemd/resolve/resolv.conf as it's not translatable to resolvconf logic.

Also huge +1 for the idea of revisiting the default bridge network behavior, or at least letting it be opt-in through configuration :)

thaJeztah · 2019-06-25T16:38:45Z

@superseed could you perhaps open a ticket in moby/moby that captures the conversation above? (just so that it doesn't get lost)?

tiborvass · 2019-06-25T18:13:33Z

@superseed thanks for bringing this up to our attention. This PR was to bring the behavior that was in master to libnetwork. But indeed it is worth discussing any improvements or regression fixes. My understanding is that moby/moby#38243 is the issue for a regression in 18.09 from 18.06.

superseed · 2019-06-26T08:22:36Z

@thaJeztah Oh, I created the issue I mentioned above (#moby/moby#38243) for it. Do you want me to make changes to it/open another one on another project, or is it enough?

@tiborvass Yep, exactly. Sorry for discussing the issue in the wrong place, my search was pretty fuzzy and I don't know exactly how moby/docker components projects are related so I thought this PR might be it.

tonistiigi reviewed May 31, 2019

View reviewed changes

internal/procfs/procfs_linux.go Outdated Show resolved Hide resolved

tiborvass mentioned this pull request May 31, 2019

executor: use systemd resolvconf if enabled moby/buildkit#1033

Merged

tiborvass force-pushed the resolvconf-systemd branch 2 times, most recently from ba7ef83 to 6c67387 Compare May 31, 2019 20:46

tiborvass mentioned this pull request May 31, 2019

build: buildkit now also uses systemd's resolv.conf moby/moby#39295

Merged

2 tasks

tiborvass force-pushed the resolvconf-systemd branch 3 times, most recently from 62f1e5f to cb850c2 Compare May 31, 2019 21:42

arkodg reviewed May 31, 2019

View reviewed changes

selansen reviewed Jun 1, 2019

View reviewed changes

resolvconf/resolvconf.go Outdated Show resolved Hide resolved

selansen reviewed Jun 1, 2019

View reviewed changes

resolvconf/resolvconf.go Outdated Show resolved Hide resolved

AkihiroSuda reviewed Jun 1, 2019

View reviewed changes

tiborvass force-pushed the resolvconf-systemd branch from cb850c2 to 4dd2f01 Compare June 1, 2019 19:19

tiborvass force-pushed the resolvconf-systemd branch 2 times, most recently from 14d5651 to 4a75b1b Compare June 1, 2019 19:24

AkihiroSuda approved these changes Jun 1, 2019

View reviewed changes

tiborvass force-pushed the resolvconf-systemd branch from 4a75b1b to 083e39b Compare June 1, 2019 19:25

selansen approved these changes Jun 2, 2019

View reviewed changes

tiborvass force-pushed the resolvconf-systemd branch from 083e39b to 449d384 Compare June 4, 2019 04:43

tiborvass changed the title ~~resolvconf: use /run/systemd/resolve/resolv.conf if systemd-resolved is running~~ resolvconf: use /run/systemd/resolve/resolv.conf if systemd-resolved manages DNS Jun 4, 2019

tiborvass force-pushed the resolvconf-systemd branch 2 times, most recently from 330a3fc to bea9524 Compare June 4, 2019 04:50

Tibor Vass added 2 commits June 4, 2019 04:50

resolvconf: use /run/systemd/resolve/resolv.conf if systemd-resolved …

164a77e

…manages DNS Signed-off-by: Tibor Vass <[email protected]>

remove gosimple - package is gone and it's not important

23fc2c9

Also fixes issue reported by ineffassign Signed-off-by: Tibor Vass <[email protected]>

tiborvass force-pushed the resolvconf-systemd branch from bea9524 to 23fc2c9 Compare June 4, 2019 04:51

arkodg approved these changes Jun 4, 2019

View reviewed changes

tiborvass merged commit 3c26b4e into moby:master Jun 4, 2019

thaJeztah mentioned this pull request Sep 24, 2019

Wrong resolv.conf used on Ubuntu 19 (systemd-resolved enabled) moby/moby#39978

Closed

thaJeztah mentioned this pull request Jan 9, 2020

LCOW: Cannot resolve container hostname on default network moby/moby#40357

Closed

thaJeztah mentioned this pull request May 25, 2020

Better selection of DNS server moby/moby#41022

Merged

thaJeztah mentioned this pull request Aug 19, 2020

[19.03 backport] Better selection of DNS server moby/moby#41374

Closed

scalpel4k mentioned this pull request Jun 12, 2023

Support different DNS resolver if host network is used moby/buildkit#2404

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

resolvconf: use /run/systemd/resolve/resolv.conf if systemd-resolved manages DNS #2385

resolvconf: use /run/systemd/resolve/resolv.conf if systemd-resolved manages DNS #2385

tiborvass commented May 31, 2019 •

edited

Loading

tiborvass commented May 31, 2019

arkodg May 31, 2019

tiborvass Jun 1, 2019

thaJeztah Jun 3, 2019

arkodg May 31, 2019

tiborvass Jun 1, 2019

AkihiroSuda Jun 1, 2019

AkihiroSuda Jun 1, 2019

tiborvass Jun 1, 2019

tiborvass commented Jun 1, 2019 •

edited

Loading

selansen left a comment

justincormack commented Jun 3, 2019

thaJeztah commented Jun 3, 2019

tiborvass commented Jun 3, 2019

tiborvass commented Jun 3, 2019

tiborvass commented Jun 4, 2019

tiborvass commented Jun 4, 2019 •

edited

Loading

justincormack commented Jun 4, 2019

arkodg left a comment

thaJeztah commented Jun 9, 2019

tiborvass commented Jun 10, 2019

superseed commented Jun 25, 2019 •

edited

Loading

thaJeztah commented Jun 25, 2019

thaJeztah commented Jun 25, 2019

superseed commented Jun 25, 2019

thaJeztah commented Jun 25, 2019

tiborvass commented Jun 25, 2019

superseed commented Jun 26, 2019

resolvconf: use /run/systemd/resolve/resolv.conf if systemd-resolved manages DNS #2385

resolvconf: use /run/systemd/resolve/resolv.conf if systemd-resolved manages DNS #2385

Conversation

tiborvass commented May 31, 2019 • edited Loading

tiborvass commented May 31, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tiborvass commented Jun 1, 2019 • edited Loading

selansen left a comment

Choose a reason for hiding this comment

justincormack commented Jun 3, 2019

thaJeztah commented Jun 3, 2019

tiborvass commented Jun 3, 2019

tiborvass commented Jun 3, 2019

tiborvass commented Jun 4, 2019

tiborvass commented Jun 4, 2019 • edited Loading

justincormack commented Jun 4, 2019

arkodg left a comment

Choose a reason for hiding this comment

thaJeztah commented Jun 9, 2019

tiborvass commented Jun 10, 2019

superseed commented Jun 25, 2019 • edited Loading

thaJeztah commented Jun 25, 2019

thaJeztah commented Jun 25, 2019

superseed commented Jun 25, 2019

thaJeztah commented Jun 25, 2019

tiborvass commented Jun 25, 2019

superseed commented Jun 26, 2019

tiborvass commented May 31, 2019 •

edited

Loading

tiborvass commented Jun 1, 2019 •

edited

Loading

tiborvass commented Jun 4, 2019 •

edited

Loading

superseed commented Jun 25, 2019 •

edited

Loading