teamhephy / router

MIT License
4 stars 10 forks source link

Suggest dumping the router #7

Closed krancour closed 2 years ago

krancour commented 6 years ago

Hey guys. I was the original author of the router over at Deis and the maintainer responsible for it up until Deis' acquisition by MS.

I want to suggest to you guys that rather than committing to the ongoing care and feeding of this component, you might want to consider dumping it in favor of using an Ingress controller. Speaking to the history here, ingress was alpha when we first started work on Workflow and this router emerged as "poor man's ingress controller." Indeed, the annotated services are just a poor (but stable, at the time) substitute for ingress resources.

Ingress is still beta, but Kubernetes makes pretty good guarantees re: ongoing backwards compatibility of anything they have declared beta. At the time of the acquisition, we were already starting to think of dumping the router in favor of "bring your own ingress controller."

imo, you should consider the same. Or if you want to commit to including a default, consider bundling Traefik or the Nginx ingress controller (+ kube-lego probably).

kingdonb commented 6 years ago

Thank you for the suggestion!

Personally (and I don't have production deployments, so take with a grain of salt) I have only ever used ingress since I started using Workflow/DeisV2

So what do you think were the outstanding issues with experimental native ingress (why it would have still been called Experimental as of v2.18.0) ? I know for one, it still required the admin to do some manual service configuration in order to expose the builder.

Other than that, and Ingress still being marked "beta" (and so was the deployment resource until recently, right) is there anything else you can think of?

kingdonb commented 6 years ago

Actually, I may have been telling people this is a problem when it's not.

Just tried experimental native ingress on a cluster with a modern ingress-nginx and followed cloud provider instructions for Azure, where I'm running my test cluster now... and actually, it handles builder just fine, if you follow the cloud provider instructions so that it can get an External-IP, actually looks like this is handled by the cloud provider's support for LoadBalancer service type.

(It works fine, except that the builder and the ingress are not at the same address... but clients are trying to reach deis-builder.[platform_domain] so even that is not really even mildly inconvenient.)

krancour commented 6 years ago

@kingdonb, the main reason "native ingress" was experimental was simply because we hid it behind a feature flag so that we weren't forcing a breaking change on everyone. The transition from Deis Workflow to Hephy Workflow seems like a fine time to make breaking changes and dump legacy stuff. Re: Ingress being a beta API in k8s, that should hardly matter. Beta vis a vis k8s carries strong guarantees of future compatibility. As for the builder-- ingress controllers only handle routing at L7 (i.e. HTTP/S). This means for the builder to work, its service must be one of type LoadBalander instead. As you said-- that means a different public IP. Also, as you said-- that should hardly be a big deal.