-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Feature/publish-ingress #5714
Feature/publish-ingress #5714
Conversation
Welcome @Revolution1! |
Hi @Revolution1. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Revolution1 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/assign @bowei BTW, I saw some code related to |
@Revolution1 thank you for the pull request. That said I am against this feature. Why not just use the ALB ingress controller? |
@aledbf
Q: Why not just use the ALB ingress controller Q: Why not do some work around, like deploy alb first then put the address to |
This is not comparable. The flag publish-service only allows you to specify a kubernetes service, not an external dependency.
Yes, it is bad.
You should use this approach, not only requires no code changes but you can get this working in less than five minutes. |
Also, if you really want to follow the approach proposed in the pull request, you don't need to upstream the change to use it. |
For public cloud service, service load balancer and ingress both need the cloud provider to provision the external loaadbalancer for you. Service and ingress both are built-in api resources of kubernetes. Why?
Why? Why another ingress controller here is called external dependency and it is so bad? |
Okay, great! |
What this PR does / why we need it:
This is an old painful problem, you can't easily get perfect web app loadbalancing with ingress-nginx on AWS.
ingress-nginx only supports been put behined a Netowrk/Classic Elastic Loadbalancer,
but Classic ELB does not support websocket,
Network ELB does not pass a l7 protocol header to let nginx do the force-ssl-redirection.
So I decided to add a feature called "publish ingress".
Like "publish service", "publish ingress" is to let the controller automatically get lb address from another ingress provisioned by another ingress controller, which, in this scinario, is the alb-ingress-controller.
Then you can get perfect HTTP+HTTPS(terminated by ALB)+Websocket support.
Types of changes
Which issue/s this PR fixes
some of them:
#5231 #3378 #5051 #69 #2000
How Has This Been Tested?
Wrote unit test and passed.
Built a image and deployed on my own test cluster(EKS+alb-ingress-controller). Works well.
Checklist: