-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
sleep before shutdown #11799
sleep before shutdown #11799
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wuzhuoquan 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 |
This issue is currently awaiting triage. If Ingress contributors determines this is a relevant issue, they will accept it by applying the The 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-sigs/prow repository. |
Hi @wuzhuoquan. 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-sigs/prow repository. |
✅ Deploy Preview for kubernetes-ingress-nginx canceled.
|
We recently had a discussion about this for a different reason (AWS NLB) and came to the conclusion that a wait, at least such a static value, is not a good way to solve this and the actual value might vary from use-case to use-case. You can read more here: #11890. I therefore close this PR and ask you to implement this on your own in the according chart value. |
What this PR does / why we need it:
Types of changes
Which issue/s this PR fixes
How Has This Been Tested?
The
ingress-controller
deploy in k8s cluster,when deleting the ingress-controller pod, k8s controller will remove this pod from endpoint list, and then the container executewaitshutdown
to sent SIGTERM to thenginx-ingress-controller
process according thepreStop
setting.After remove the pod from endpoint list, kube-proxy listwatch the change of endpoint and update the iptables rule to remove the pod IP.
But sometimes kube-proxy and iptables remote the pod and ip not as fast as executing
waitshutdown
, so if at this time,a new connection request may be send to the deleting pod butwaitshutdown
is executed,will return connection refused to client.So I think before sent
SIGTERM
to thenginx-ingress-controller
process, it should sleep for a while to avoid this problem and make it more smooth.Checklist: