Allow macOS to resolve service FQDNs during 'minikube tunnel' #3464

ceason · 2018-12-17T16:23:59Z

minikube tunnel is an excellent command, but it still has a particularly rough edge; users must manually look up a service's ClusterIP and refer to that IP directly, rather than referencing the service's DNS name (ie <service>.<namespace>.svc.<clusterDomain>).

This PR adds the ability to resolve service FQDNs from host during minikube tunnel.

Currently this is only implemented for macOS, (via its resolver which allows easily overriding nameservers for a particular domain). Some potential implementations for other platforms are:

Linux

An appropriately configured systemd network unit, which would operate similarly to macOS's resolver (though would require the host be using systemd-resolved's stub resolver)
mDNS "proxy" which responds to host requests for *.svc.<clusterDomain> by querying the cluster's DNS service

Windows

mDNS "proxy", as above
???

Does the macOS implementation seem reasonable? If so, does adding support for macOS now and Windows/Linux later seem reasonable?

k8s-ci-robot · 2018-12-17T16:24:02Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
If you have done the above and are still having issues with the CLA being reported as unsigned, please email the CNCF helpdesk: [email protected]

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

minikube-bot · 2018-12-17T16:24:03Z

Can one of the admins verify this patch?

ceason · 2018-12-17T16:27:04Z

I signed it

…l' (MacOS only for now)

balopat · 2018-12-19T23:28:34Z

@minikube-bot OK to test

balopat · 2018-12-19T23:32:26Z

This is nice, I've been doing the exact same thing manually.
I'm okay to implement DNS support piecemeal.
Do you mind modifying the integration test as well to test for DNS resolution on Mac?

balopat · 2018-12-21T23:04:33Z

@minikube-bot OK to test

ceason · 2018-12-22T01:04:35Z

Will do. I'm traveling for the holidays but will get to this first week of the new year.

ceason · 2019-01-02T20:46:31Z

@balopat I've updated the integration test to include the DNS resolver config.

There is one test failure at pv_test.go:109, though it appears unrelated to the changes in this PR.

tstromberg · 2019-01-16T01:13:47Z

@minikube-bot OK to test

tstromberg · 2019-01-25T00:00:44Z

@minikube-bot OK to test

tstromberg

Looks great - thank you for contributing this! Minor nits:

tstromberg · 2019-02-08T23:22:42Z

pkg/minikube/tunnel/route_darwin.go

@@ -34,6 +36,9 @@ func (router *osRouter) EnsureRouteIsAdded(route *Route) error {
 	if exists {
 		return nil
 	}
+	if err := writeResolverFile(route); err != nil {
+		return fmt.Errorf("could not write /etc/resolver/{cluster_domain} file: %s", err)


Don't include the path name in the message, as EnsureRouteIsAdded doesn't authoritatively know what it is. My suggestion is: return fmt.Errorf("write resolver file: %v", err)

tstromberg · 2019-02-08T23:23:43Z

pkg/minikube/tunnel/route_darwin.go

@@ -162,5 +167,37 @@ func (router *osRouter) Cleanup(route *Route) error {
 	if !re.MatchString(message) {
 		return fmt.Errorf("error deleting route: %s, %d", message, len(strings.Split(message, "\n")))
 	}
+	// idempotent removal of cluster domain dns
+	resolverFile := fmt.Sprintf("/etc/resolver/%s", route.ClusterDomain)


Make '/etc/resolver' a constant to share between functions, and use filepath.Join to create paths from it.

Also, I prefer that the variable name uses path instead of file, since the latter references something specific in Go (file.File)

tstromberg · 2019-02-08T23:26:38Z

pkg/minikube/tunnel/route_darwin_test.go

 	command := exec.Command("sudo", "route", "-n", "delete", cidr)
 	_, err := command.CombinedOutput()
 	if err != nil {
 		t.Logf("cleanup error (should be ok): %s", err)
 	}
+	command = exec.Command("sudo", "rm", "-f", fmt.Sprintf("/etc/resolver/%s", r.ClusterDomain))
+	_, err = command.CombinedOutput()


Since we're not using the output:

if err := command.Run(); err != nil { return fmt.Errorf("rm: %v", err) }

andsens · 2019-02-12T12:36:21Z

May I suggest an alternative? By using the ingress addon (or e.g. Traefik) you can leverage external-dns to populate CoreDNS and use that to resolve any ingresses, just like you would in a production setup.

Right now I've got it running with a CoreDNS & etcd deployment that are separate from kube-system, but it should be possible to simply use the kube-system etcd without any changes and add some config to the kube-system etcd. Meaning only external-dns would need to be added.

I have all my ingresses running on the .k8s TLD and it works like a charm and seems to be very robust.

In order to get the host to query the right resolver for the .k8s TLD you need to set it up once.
For MacOS you just run

sudo mkdir /etc/resolvers
sudo tee /etc/resolver/k8s <<<$(printf "nameserver %s" $(kubectl --namespace localdns get svc coredns -o=jsonpath='{.status.loadBalancer.ingress[0].ip}

Though I'm not quite sure how to achieve the same for Windows or Linux. Linux I'm sure is trivial, Windows maybe not so much.

Check out the gist: https://gist.github.com/andsens/2c8c67cf72346c2c0df02614d6386d0a

svenvanheugten · 2019-02-13T21:23:39Z

May I suggest an alternative? By using the ingress addon (or e.g. Traefik) you can leverage external-dns to populate CoreDNS and use that to resolve any ingresses, just like you would in a production setup.

Right now I've got it running with a CoreDNS & etcd deployment that are separate from kube-system, but it should be possible to simply use the kube-system etcd without any changes and add some config to the kube-system etcd. Meaning only external-dns would need to be added.

I have all my ingresses running on the .k8s TLD and it works like a charm and seems to be very robust.

In order to get the host to query the right resolver for the .k8s TLD you need to set it up once.
For MacOS you just run
sudo mkdir /etc/resolvers
sudo tee /etc/resolver/k8s <<<$(printf "nameserver %s" $(kubectl --namespace localdns get svc coredns -o=jsonpath='{.status.loadBalancer.ingress[0].ip}
Though I'm not quite sure how to achieve the same for Windows or Linux. Linux I'm sure is trivial, Windows maybe not so much.

Check out the gist: https://gist.github.com/andsens/2c8c67cf72346c2c0df02614d6386d0a

I don't think this is so much an alternative as it is a really nice addition.

I use the exact same commands as you, but I point it at the kube-system CoreDNS server rather than a separate one managed by external-dns. Glancing at this PR, it does the exact same.

This gives me the experience of being "inside" the cluster during development, and allows me to access services from the host machine that are usually only accessible from containers running inside the cluster (i.e. ClusterIP services). In order to minimize the differences between development and production, I'd rather not expose every service through an ingress (or, in the case of non-HTTP services, change the service type to LoadBalancer). By using the kube-system CoreDNS server, I can use the same DNS entries from my host machine as I'd use in containers running inside the cluster (i.e. <service>.<namespace>.svc.cluster.local).

However, the kube-system CoreDNS server doesn't contain DNS entries for ingresses, so combining this with external-dns seems like a good addition. I currently put ingress DNS entries in /etc/hosts manually, but I think I'll try and see if it is indeed possible to get external-dns to populate the kube-system CoreDNS.

tstromberg · 2019-04-11T18:27:07Z

Marking as WIP as there are still outstanding review comments to be addressed.

andsens · 2019-04-11T18:41:15Z

I just wanted to give a heads up regarding the mDNS option. I have have done a bit of research on this before I arrived at my resolver setup and there is one big mean hurdle: mDNS is made for link-local adresses. So either the broadcaster needs to sit directly on the tunnel interface (i.e. in the virtual machine) or you set up a reflector in its place and move the discovery mechanism into a container, avahi has an enable-reflector switch or you can use something like https://github.com/Gandem/bonjour-reflector.

That said, mDNS is a really nice cross-platform way to do this. The pain it takes to get it working properly is repaid a thousandfold once everything "just works", on every machine, with zero configuration.

tstromberg · 2019-04-30T19:39:19Z

Interesting test failure on the last run:

18:16:13 | ! error execution phase addon/coredns: unable to create deployment: 0-length response

@minikube-bot OK to test

tstromberg · 2019-05-14T20:13:42Z

@minikube-bot OK to test

tstromberg · 2019-05-17T02:11:52Z

Merging! Thank you for dealing with this PR being open extraordinarily long.

k8s-ci-robot · 2019-05-17T02:12:25Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ceason
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: balopat

If they are not already assigned, you can assign the PR to them by writing /assign @balopat in a comment when ready.

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:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 17, 2018

k8s-ci-robot requested review from balopat and dlorenc December 17, 2018 16:24

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Dec 17, 2018

ceason force-pushed the tunnel-dns branch from 84db53e to f2c58e4 Compare December 18, 2018 18:00

Add ability to resolve service FQDNs from host during 'minikube tunne…

bd15020

…l' (MacOS only for now)

ceason force-pushed the tunnel-dns branch from f2c58e4 to bd15020 Compare December 18, 2018 18:22

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 2, 2019

Test for DNS resolution on Mac

335da39

ceason force-pushed the tunnel-dns branch from 17a5def to 335da39 Compare January 2, 2019 19:46

Merge remote-tracking branch 'origin/master' into tunnel-dns

082fe50

tstromberg suggested changes Feb 8, 2019

View reviewed changes

tstromberg changed the title ~~Resolve service FQDNs from host during 'minikube tunnel' (macOS)~~ WIP: Resolve service FQDNs from host during 'minikube tunnel' (macOS) Apr 11, 2019

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 11, 2019

tstromberg closed this May 17, 2019

tstromberg reopened this May 17, 2019

tstromberg merged commit b65d602 into kubernetes:master May 17, 2019

tstromberg changed the title ~~WIP: Resolve service FQDNs from host during 'minikube tunnel' (macOS)~~ Allow macOS to resolve service FQDNs during 'minikube tunnel' May 17, 2019

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 17, 2019

tstromberg added a commit to tstromberg/minikube that referenced this pull request May 17, 2019

Clean up new lint issues brought in by the kubernetes#3464 merge

8445f78

tstromberg mentioned this pull request May 17, 2019

Lint cleanup after merging tunnel DNS resolution #4278

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow macOS to resolve service FQDNs during 'minikube tunnel' #3464

Allow macOS to resolve service FQDNs during 'minikube tunnel' #3464

ceason commented Dec 17, 2018

k8s-ci-robot commented Dec 17, 2018

minikube-bot commented Dec 17, 2018

ceason commented Dec 17, 2018

balopat commented Dec 19, 2018

balopat commented Dec 19, 2018

balopat commented Dec 21, 2018

ceason commented Dec 22, 2018

ceason commented Jan 2, 2019

tstromberg commented Jan 16, 2019

tstromberg commented Jan 25, 2019

tstromberg left a comment

tstromberg Feb 8, 2019

tstromberg Feb 8, 2019

tstromberg Feb 8, 2019

andsens commented Feb 12, 2019

svenvanheugten commented Feb 13, 2019 •

edited

Loading

tstromberg commented Apr 11, 2019

andsens commented Apr 11, 2019

tstromberg commented Apr 30, 2019

tstromberg commented May 14, 2019

tstromberg commented May 17, 2019

k8s-ci-robot commented May 17, 2019

Allow macOS to resolve service FQDNs during 'minikube tunnel' #3464

Allow macOS to resolve service FQDNs during 'minikube tunnel' #3464

Conversation

ceason commented Dec 17, 2018

k8s-ci-robot commented Dec 17, 2018

minikube-bot commented Dec 17, 2018

ceason commented Dec 17, 2018

balopat commented Dec 19, 2018

balopat commented Dec 19, 2018

balopat commented Dec 21, 2018

ceason commented Dec 22, 2018

ceason commented Jan 2, 2019

tstromberg commented Jan 16, 2019

tstromberg commented Jan 25, 2019

tstromberg left a comment

Choose a reason for hiding this comment

tstromberg Feb 8, 2019

Choose a reason for hiding this comment

tstromberg Feb 8, 2019

Choose a reason for hiding this comment

tstromberg Feb 8, 2019

Choose a reason for hiding this comment

andsens commented Feb 12, 2019

svenvanheugten commented Feb 13, 2019 • edited Loading

tstromberg commented Apr 11, 2019

andsens commented Apr 11, 2019

tstromberg commented Apr 30, 2019

tstromberg commented May 14, 2019

tstromberg commented May 17, 2019

k8s-ci-robot commented May 17, 2019

svenvanheugten commented Feb 13, 2019 •

edited

Loading