deis / controller

Deis Workflow Controller (API)
https://deis.com
MIT License
41 stars 53 forks source link

Consider different URL for builder #657

Closed krancour closed 8 years ago

krancour commented 8 years ago

Currently when you create an app, the generated deis git remote uses a URL like:

ssh://git@deis.example.com:2222/utmost-autoharp.git

Note that it uses the same fully qualified domain name as the controller. There are some circumstances where this can be a problem...

Consider someone running Workflow on GKE. If they care about obtaining end users' IPs, as most do, they need to manually create an HTTP/S load balancer in GCP to facilitate that since the TCP load balancer that k8s automatically provisions for the router does not support PROXY protocol. (Don't get too hung up on this detail though.) The TCP load balancer, however, needs to stick around because the HTTP load balancer cannot handle git pushes over ssh. (Again, don't get hung up on the details of why someone might have two load balancers. The cogent point is that there are legitimate scenarios where someone would.)

Back to the URLs. If the controller and builder both use the same fully-qualified domain name, there's no possibility of directing controller traffic to one load balancer and builder traffic to the other.

By contrast, if we transitioned to using a URL more like the following:

ssh://git@deis-builder.example.com:2222/utmost-autoharp.git

Then DNS can be configured such that *.example.com resolves to the IP of the HTTP/S load balancer whilst deis-buidler.example.com, specifically, resolves to the IP of the TCP load balancer.

If we don't do this, we're going to have trouble supporting any Workflow user who cares about end user IPs and wants to utilize the builder. If we do fix this, we should do it before v2 goes GA, because this is a somewhat disruptive change.

cc @gabrtv @slack @helgi

EDIT:

To fix:

helgi commented 8 years ago

Off hand I don't see a problem with this approach and separating the Builder from the Controller endpoint is probably a good idea going forward to have more explicit contract and separation of concerns.

Should be somewhat trivial to implement. Controller needs to add another reserved_name, Builder SVC manifest needs to add router annotations and I believe the router may have to understand / build the BuilderConfig struct slightly differently? Perhaps treating it like a real app with a stream ability and custom port (which you can discover from the SVC port anyway).

Only thing is we should talk about if we want to support people that already have deis.foo.com for git push still (as in beta users and migrations). The only people that would run into the scenario you described are the ones that really have to migrate their remotes.

krancour commented 8 years ago

The router handles builder differently than routable applications. The builder svc doesn't require any annotations for the router to be able to discover it. Also, the fqdn used to access the builder is inconsequential to the router iirc. Compared to an http block, a stream block isn't selecting traffic based on host name. It's just forwarding all traffic on the specified socket to the upstream (2222 -> builder in this case).

As far as continuing to support git push to the "old" URLs.... as a consequence of what I just described, as long as the user's DNS gets the requests to the right load balancer, the router will guatanteed get the traffic to the builder regardless of the fqdn used.