Destinations service doesn't discover services outside the local cluster

[briansmith]

If the DNS name sent in a query to the Destinations service has a suffix of "cluster.local" or whatever the current cluster domain name is, the Destination service should use the local k8s API server's Services API to discover the service. There are two cases:

• If the service is not an "external" service then the Destinations service should use the EndPoints API to watch the addresses for the service. This works now.

• If the service is an "external" service, then the Destination service should use DNS to discover the addresses for the service, as kube-proxy would. It should periodically poll DNS to get a similar effect toa watch on the EndPoints API for internal services. This doesn't work now.

If the DNS name sent in the query to the Destinations service doesn't end with "cluster.local" or the local cluster domain name, then the Destinations service should skip using the Services API and assume that it is an "external" service. This doesn't work now.

[briansmith]

FYI, I identified the most likely place where we need to kick off the DNS stuff in the Destinations service at https://github.com/runconduit/conduit/blob/5628d3c8f400ed4f3879dde7f4c1e0578432233b/controller/destination/server.go#L129. That's the point where we've ruled out the possibility that it is a local service.

[briansmith]

Basically the idea here is to replace that "// TODO: ..." comment with a service that polls DNS for the given name at regular intervals (no larger than the shortest TTL from previously-received A/AAAA records for that DNS name, and probably much shorter) and implements the controller side of the Destination service:

• When new (not currently in cache) A/AAAA records are received, send "add" notifications to the proxies.
• When A/AAAA records expire (based on their TTLs), send a "remove" notification to the proxies.

[seanmonstar]

Do we really want to build our own DNS polling service? Does Kubernetes not include something that consolidates it all?

Looking at DNS APIs in Go, I notice the main net package doesn't have a way to query records, only IP addresses. Google searches suggest https://github.com/miekg/dns to do record lookups...

[briansmith]

AFAICT, Kubernetes assumes that the clients themselves are going to do their own DNS polling. Basically the Destination service acts like a recursive resolver where the outbound queries are over DNS and the inbound queries are using the Conduit Destinations API, and so we have to implement something that bridges that gap.

[seanmonstar]

OK, here's my original plan:

1. Replace that TODO line with a return resolvePublicDns(host, stream).
2. Use https://github.com/miekg/dns for DNS queries.
3. Start with an empty map[string]IP in resolvePublicDns.
4. Build a dns.Msg for A and AAAA records for provided host string.
5. Any IP not in map, push on map and on stream. Any IP in map but not in answer is removed and also notified on stream.
6. Keep an interval variable that is based on the smallest TTL from all the answers.
7. After interval expires, repeat from step 4.
8. All of this is joined on <-stream.Context.Done().

Does this seem right? Is it gopherish? @siggy or @adleong?

Also:

• What should happen if the dns queries see an error? Have some retries? Or just send the error back on the stream and close?
• This doesn't try to consolidate if multiple proxies all are asking about the same hostname. It'd probably be best to do so, but we also don't consolidate asking the Kubernetes API either. Perhaps a future optimization?

[briansmith]

What should happen if the dns queries see an error? Have some retries? Or just send the error back on the stream and close?

See #335, where I ask the same question for the currently-implemented stuff.

This doesn't try to consolidate if multiple proxies all are asking about the same hostname. It'd probably be best to do so, but we also don't consolidate asking the Kubernetes API either. Perhaps a future optimization?

I think this optimization is pretty essential at least for cluster-internal services. I filed #334 to track implementing it.

What should happen if the dns queries see an error?

There are lots of exception cases. Two important ones to care about are:

• You don't get a response and time out. In that case, I would requery, and I would make the timeout short.
• You get a response that doesn't have any A or AAA records. You'll have to reschedule the query as though you got a response with a short TTL, because in a healthy system this probably means that the target services aren't quite ready yet but will be ready soon.

[seanmonstar]

The amount of work to query for records is quite a lot, compared to the default net.LookupHost call. The only thing gained from all of that is a list of TTLs. To reduce the scope of this, should we just use net.LookupHost and treat all records with a relatively short default TTL?

[briansmith]

1ms is probably not a reasonable interval for us to use as the default fixed TTL. However, there will be services that have a 1ms TTL and expect to have DNS polled that often. Thus, I think we will need to do things in a way that respects TTLs that are shorter than whatever short fixed TTL we start with.

So, if it doesn't seem like a huge amount of work to do it in terms of net.LookupHost and then do it in a TTL-sensitive manner then I think that that would be a reasonable progression.

[seanmonstar]

However, there will be services that have a 1ms TTL and expect to have DNS polled that often.

Is this case common enough that it's worth us writing several hundreds of lines to manage a dns client manually (rotate nameservers, try TCP and UDP, poll /etc/resolv.conf for updates, handle DNSSEC, etc)?

Probably eventually, but as something that we hope to ship this week in 0.3?

if it doesn't seem like a huge amount of work to do it in terms of net.LookupHost and then do it in a TTL-sensitive manner then I think that that would be a reasonable progression.

I don't quite get what this means. It's not a lot of work to set up a loop using just net.LookupHost on a specified interval (probably 10ish lines). However, it is impossible to get a TTL using the default net package, so we can't make it TTL-sensitive at all.

Unless you're saying do it first with just net.LookupHost, and then scope out all the work needed to run our own proper resolver for another milestone.

[briansmith]

Unless you're saying do it first with just net.LookupHost, and then scope out all the work needed to run our own proper resolver for another milestone.

I'm saying that it's OK to start out with net.LookupHost and then fix it to honor TTLs later. Whether one or both of those need to happen by 0.3 is up to the release drivers.

[briansmith]

Is this case common enough that it's worth us writing several hundreds of lines to manage a dns client manually

yes>

(rotate nameservers, try TCP and UDP, poll /etc/resolv.conf for updates,

We can assume we're talking to the default Kubernetes internal DNS server (Kube DNS or CoreDNS) and that UDP is supported. We can assume a fixed /etc/resolv.conf, I think.

handle DNSSEC, etc)?

We don't need to support DNSSEC.