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
Add a Hostname field to the EndpointAddress object. #11838
Conversation
This will be used for DNS CName services.
GCE e2e build/test passed for commit cde577e. |
IP string `json:"ip" description:"IP address of the endpoint"` | ||
|
||
// Hostname for this endpoint, you can specify IP or Hostname, but not both. | ||
Hostname string `json:"hostname" description:"Hostname for this endpoint"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can sorta see what you're trying to do here, but the details seem very murky. Do you anticipate the endpoints controller ever setting this field? If so, how? Don't we need to solve external DNS before we'll have something reasonable to put here? If not, should this really go here?
Is Hostname the right field name? How about "DNSName" or "DNS"?
Why not combine this with the IP field? Do we really need two fields?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding one field vs two, this is a question we need to solve in a bunch of places. We were originally going to make IP fields use a net.IP-derived type for strong validation, but we backed off when we realized some places really do want host or IP. So in my mind this is still an open question. This sets precedent - we either use twin fields all over or we document these all as being "ip or hostname" and stick with 'string'. I have not put much thought into this, but I guess it doesn't matter much either way.
@bgrant0607 for API call
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would we set both IP and Hostname, or just OneOf? If OneOf, then this is a breaking API change. If we overloaded the existing field, that would also be a breaking API change.
We do have both IP and Hostname in LoadBalancerIngress. If users won't typically care about the difference and we wouldn't provide both, then a single Host field would be better. However, if we would provide both IP and DNS name and/or clients would need to use them differently or in different situations, then two fields would be best.
With respect to the use of "Host" in the API: We use Host in a number of places: HostPort, HostIP, HostNetwork, HostPath, Host (in HTTPGetAction and EventSource), and Hostname. More often than Node (not counting the Node API itself): NodeSelector, NodeName, NodePort. Users have complained about the inconsistency. However, I agree that Hostname makes sense in this case, and is consistent with the other use in the API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would we set both IP and Hostname, or just OneOf?
In most places we really want one-of. Yeah, making a previously required field suddenly be optional will potentially screw up clients who expect a field to be populated. Changing it to be a hostname or IP is probably less likely to do damage, but it is sketchy to change.
LoadBalancerIngress was thrown together quickly.
HostPort and HostNet are named in equivalence with Docker. One could argue it's a matter of perspective - Node is what we call a machine from outside that machine, Host is what we call it from within a container. That's a stretch.
In this context though "hostname" (and HTTPGet.Host) means the name-or-IP of a host somewhere, and has nothing to do with kubernetes nodes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs omitempty. Otherwise this field is required.
This needs validation to enforce the one-of property, if we keep twin fields. This must never have been run at all because current validation checks We need to define the use case for this:
Does kube-proxy just work with this? I just took a look, and I guess it does. This will break when we switch to pure iptables, which is O(soon) @BenTheElder Needs some sort of e2e testing |
@brendandburns What exactly did you intend to use this for? |
@thockin BTW: we have a few things to sort out with the iptables version but it's been mostly working fine seemingly for a while now. I have one possible problem reported here and am still working on this (deterministic rule names and reconciling against I have summer finals for my two classes Monday and Tuesday so I am and will be studying, but Tuesday afternoon I will be back to resolving the remaining issues. I'll keep an eye of this, and when/if it has been clarified & merged I'll update kube-proxy to handle it. That said, looking at it now: I would also like to know as you asked if we are going to allow mixing hostnames and IPs in single Endpoints, if the records will be 1:1, etc, so I know what kind of changes will need to be made to endpoint tracking and rule generation. Further, will we allow updating of hostname records in the DNS service and if so: if the DNS service is updated will there be an endpoints event, or will we need to periodically re-resolve hostnames in the proxy or...? |
That reminds me: We'd need to resolve the hostnames to ensure they didn't resolve to link-local addresses. |
If we intend to enforce non-link-localness of these, we'll need to query On Sun, Jul 26, 2015 at 8:08 PM, Brian Grant notifications@github.com
|
@thockin can you take this one? |
ACK On Mon, Jul 27, 2015 at 2:43 PM, Daniel Smith notifications@github.com
|
@bgrant0607 I want to enable CName based services. So that I can have a headless service that maps to a hostname, and results in a CName in the Kube DNS server. I can lay that in at a higher level, if we'd prefer, by introducing a new "DNSDelegate" ServiceType. That is less breaking, I guess. --brendan |
You want a Kubernetes service to target a public CNAME, or you want a public CNAME to target a Kubernetes service? A "headless" service is one with no ClusterIP. It exposes the pod IPs directly. An imported service is one with no selector that targets a manually populated Endpoints resource. |
I guess it's "imported service" then. (I can never seem to keep the different names straight) I want a service that targets a public CNAME. As a concrete example of this, AWS ELB load balancers are only exposed as hostnames, never as reliable IP addresses. --brendan |
@thockin @bgrant0607 |
If the target were specified as a hostname, but the endpoints controller did the DNS lookup and updated the IP field, then clients that expected the IP field would still work (except for maybe the short duration where that field would be unset). Do we need to support hostnames that resolve to multiple IP addresses? Or would it be acceptable for clients not aware of the hostname field to just target one IP? |
I think we can actually do this on admissions control, so IP will never not be set. And yes, only using a single IP for hosts with multiple IP seems reasonable to me. |
I think what we agreed then is that if Hostname is set, it will overwrite IP with a resolved address, regardless whether that field was set or not. Can we do this resolution synchronously upon creation? Then consumers could rely on IP to always be set. |
@bgrant0607 |
Sorry, I somehow missed that comment. Admission control is not the right place for this. That's where optional, pluggable policy goes. Resolution would need to go into the Endpoints registry. |
@bgrant0607 |
What timeout would we use for synchronous resolution? Has the potential to increase latency of endpoint updates significantly. Does it only update when IP is unset? |
@smarterclayton, yeah it would only update when IP is unset. And it wouldn't attempt to re-resolve, it's really just to make legacy tools continue to work (ish) future tools should be re-tooled to prefer hostname. |
Sgtm. What about resolution timeout? On Aug 4, 2015, at 6:34 PM, Brendan Burns notifications@github.com wrote: @smarterclayton https://github.com/smarterclayton, yeah it would only — |
Seems like the timeout would have to be less than the overall API timeout so that we'd have the opportunity to fail the API request in the case that resolution failed. |
Go ahead and put the implementation in this PR as a separate commit. We don't have a good way of splitting API changes across multiple PRs such that we can ensure the changes aren't visible and aren't persisted to etcd. That's discussed here: #4855 (comment) |
Labelling this PR as size/M |
Labelling this PR as size/S |
I didn't follow the final negotiation on this, but I'm not keen on never re-resolving. I'm not sure where this PR stands |
I really wish we could implement this as a distinct object - ExternalDNSEndpoints type that a controller reads. |
I'd prefer to implement #13748 which is think is strictly superior to this proposal (in that it has a wider solution space). |
ref #13358 |
GCE e2e build/test failed for commit cde577e. |
Are there any updates on this Pull Request? We really need it or a similar solution to be able to point a mysql service at an AWS RDS instance. The IP address changes, so we can't just set the IP address of the endpoint. We have to use the DNS name. I tried to follow the conversation, but I apologize that a lot of it was over my head. If this is infeasible for some reason, is there a good work around? |
I think the status is that we should either have a way of exposing arbitrary CNAMEs into kube-dns or we should allow a new form of Service that does not have a selector but instead a hostname, which kube-dns would act on by creating a CNAME record. Or both. The alias-service seems simple but its another step down an ugly path of making Service more complicated and polymorphic. Neither solution is in for v1.1, sadly. |
As soon as we expose arbitrary Cnames we have to worry about collisions and On Oct 16, 2015, at 1:49 PM, Tim Hockin notifications@github.com wrote: I think the status is that we should either have a way of exposing The alias-service seems simple but its another step down an ugly path of Neither solution is in for v1.1, sadly. — |
GCE e2e build/test failed for commit cde577e. |
@thockin I agree with @smarterclayton can we please re-activate this PR? |
and I'm also ok with implementing this in the style that @smarterclayton suggests, by having kube-dns load in a CNAME for the service, rather than resolving this at create time. |
GCE e2e build/test failed for commit cde577e. |
+1 As an end user of Kubernetes we are eagerly awaiting something like what was proposed. We use AWS resources that are often represented by a DNS entry. My first thought was adding this manually to an endpoint as the author suggests but I'm game for any solution that works. |
Closing this in favor of #13748 - we don't need 2 places to discuss. |
This will be used for DNS CName services, and also using DNS to lookup addresses for Headless services.
Related to #4447
@thockin @bgrant0607 @lavalamp