Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider having Route not become Ready until DNS resolves #1598

Closed
evankanderson opened this issue Jul 16, 2018 · 8 comments
Closed

Consider having Route not become Ready until DNS resolves #1598

evankanderson opened this issue Jul 16, 2018 · 8 comments
Assignees
Labels
area/networking kind/feature Well-understood/specified features, ready for coding. kind/question Further information is requested

Comments

@evankanderson
Copy link
Member

Expected Behavior

When a Route is Ready, I can reach it at some DNS name. (From places where I can resolve the DNS name*).

* if my cluster name is srv.cluster.local, I'd expect names to only resolve within the cluster.

Actual Behavior

Route will report that it is Ready even if the reported domainname doesn't exist.

Steps to Reproduce the Problem

  1. Deploy a cluster.
  2. Create a knative Service.
  3. Run a job which runs curl against the Service's domainname.

Additional Info

We should have the Route controller check that the DNS name resolves (possibly check that it's pointed at the Istio ingress?), and report the status in a new Condition type. I suggest the name DnsResolved.

We should also switch config-domain.yaml to point to srv.cluster.local so that DNS works (within the cluster) out of the box.

@google-prow-robot google-prow-robot added area/API API objects and controllers area/networking kind/bug Categorizes issue or PR as related to a bug. labels Jul 16, 2018
@evankanderson
Copy link
Member Author

/assign @evankanderson
/assign @mattmoor
/assign @tcnghia
/assign @scothis

PTAL and see if this makes sense.

@scothis
Copy link
Contributor

scothis commented Jul 16, 2018

Setting the default dns host to svc.cluster.local is a bit awkward since that value is hard coded into the Route's VirtualService for in cluster access. Assuming Istio doesn't fall over because of the duplicate value.

@tcnghia
Copy link
Contributor

tcnghia commented Jul 16, 2018

For svc.cluster.local domains, currently named routes aren't supported because we can't add a domain name of the form <target-name>.<route>.<ns>.cluster.local. I think CoreDNS may allow, but not Kube-DNS.

We could probably try something like <target-name>-dot-<route>.<ns>.cluster.local, but that will run into character limit issues. Another alternative is to replace named route with routes that uses the Revision name to sidestep this issue entirely. Since named route can only have only one target, this is functionally equivalent.

Or if we could limit the Route to only resolve cluster-internal names, and adds an [optional] Dispatch component to configure mappings from external domain names to cluster internal names, we could perform this check for Dispatch and not Route.

WDYT?

@tcnghia
Copy link
Contributor

tcnghia commented Jul 17, 2018

Also, I am not aware of a time-based trigger for a reconcilation, doing this probing in Route controller may be difficult.

WDYT about spinning up short-lived probers after a Route change? I proposed one in #1582 that can help with asserting effectiveness of changes in VirtualService, but we could generalize it to probe for a wider range of conditions.

@evankanderson
Copy link
Member Author

With respect to svc.cluster.local -- you're right that subdomains/subnames won't work. At this point, the number of (DNS, SSL, Istio) items that end up adding more work to support subdomains does lean me toward simply using multiple Routes to get different Service-level names, rather than trying to add a new type of DNS to kube-dns.

Controllers will perform a periodic resync; I think we have the resync period configured to be 5 minutes right now, though:

https://github.com/knative/serving/blob/master/cmd/controller/main.go#L105
https://github.com/kubernetes/client-go/blob/master/informers/factory.go#L101

The DNS lookup could also be done in-memory, though I'd be worried about clogging our reconciliation threads.

@mattmoor
Copy link
Member

I am receptive to this idea, but the assertion is somewhat context sensitive (e.g. it may resolve some places, not others, .svc.cluster.local is actually an interesting example of this, but not exactly what I was thinking).

/assign @tcnghia
/unassign @mattmoor

PS - our current resync period is 30s (IIRC, kubebuilder may have a longer cycle in eventing)

@grantr
Copy link
Contributor

grantr commented Aug 22, 2018

kubebuilder may have a longer cycle in eventing

Default controller-runtime cycle is 10 hours and that's what eventing is using. Resync period is now configurable as of kubernetes-sigs/controller-runtime#88.

@mattmoor mattmoor removed the kind/bug Categorizes issue or PR as related to a bug. label Nov 8, 2018
@mattmoor mattmoor added this to the Needs Triage milestone Nov 28, 2018
@mattmoor mattmoor added kind/question Further information is requested kind/feature Well-understood/specified features, ready for coding. and removed area/API API objects and controllers labels Feb 11, 2019
@mattmoor
Copy link
Member

tl;dr I think I'm inclined to close this in favor of other issues.

Users can use .svc.cluster.local for (mostly) functioning DNS, including making services cluster-local. We have a proposal to remove sub-Routes, which would make .svc.cluster.local totally work. We have an open issue to try and enable/use a service(s) like xip.io by default.

The above combined with the subjectivity of DNS "readiness" (e.g. given DNS caching settings), and a lurking desire to work even when DNS isn't functioning are motivating me to close this.

Feel free to /reopen if I missed something.

@eallred-google eallred-google modified the milestones: Needs Triage, Ice Box Oct 23, 2019
@dprotaso dprotaso removed this from the Ice Box milestone Oct 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/networking kind/feature Well-understood/specified features, ready for coding. kind/question Further information is requested
Projects
None yet
Development

No branches or pull requests

8 participants