Skip to content
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

Initiate drain based on health check result #2322

Closed
dcowden opened this issue Apr 9, 2018 · 5 comments
Closed

Initiate drain based on health check result #2322

dcowden opened this issue Apr 9, 2018 · 5 comments

Comments

@dcowden
Copy link

dcowden commented Apr 9, 2018

Is this a request for help? (If yes, you should use our troubleshooting guide and community support channels, see https://kubernetes.io/docs/tasks/debug-application-cluster/troubleshooting/.): NO

What keywords did you search in NGINX Ingress controller issues before filing this one? (If you have found any duplicates, you should instead reply there.): drain affinity


Is this a BUG REPORT or FEATURE REQUEST? (choose one): FEATURE REQUEST

We have a legacy application ( tomcat/java), which needs sticky sessions. When we deploy new versions of our applications, we need to stop sending new connections to a server, while sending bound sessions to the old server. Please note: this is not referring to in-flight requests, we're needing the active tomcat sessions to expire, which normally takes a few hours.

This is possible using nginx drain command. This will send bound connections to the old server, but send new ones elsewhere. But in kubernetes, calling a command on the ingress controller is not part of the deployment flow. To do it with current tools, we would need to add a preStop hook to our application. In that hook, we'd need to access the ingress controller, and ask it to drain with an api call. We'd rather not introduce the ability for applications to call apis on the ingress controller.

It would be ideal if the health checker could be configured to read the result of the health check, and change the server status to drain based on its response. Currently, it only checks the result code, with no possibility to change the upstream server to 'drain'.

We are willing to do the development and submit a PR, but we're not really sure how to get started with this. It doesn't seem like it should be too difficult, but we don't have much experience with the design of the controller. Are we missing a better way to do it?

NGINX Ingress controller version:
0.12.0

Kubernetes version (use kubectl version):
1.8.4

Environment:
kops on AWS

Anything else we need to know:

@aledbf
Copy link
Member

aledbf commented Apr 9, 2018

It would be ideal if the health checker could be configured to read the result of the health check, and change the server status to drain based on its response. Currently, it only checks the result code, with no possibility to change the upstream server to 'drain'.

This health checker is for the ingress controller itself, not the exposed applications through Ingress.

The probes you mention are from Kubernetes itself.

@dcowden
Copy link
Author

dcowden commented Apr 9, 2018

@aledbf oh ok thank you! So, nginx-ingress really only knows how to use k8s service/pod state to understand what upstreams are available.

I'm really trying to avoid a pod hook that needs to reach out to the ingress controller to tell it to drain. It would be better if the controller itself could detect this.

An alternate implementation would be to automatically put the upstream server in drain mode if the pod appears in TERMINATING status. what about that?

@aledbf
Copy link
Member

aledbf commented Apr 9, 2018

in TERMINATING status

I am not sure the endpoint contains this information. This is what we have at runtime EndpointAddress.
Reading the definition TargetRef (in this case) contains information about the pod so I think we can add another informer to get the details and check the state.

That said drain is only available in the commercial version of nginx.

@dcowden
Copy link
Author

dcowden commented Apr 9, 2018

Ah yes, you're right about the fact that drain is only commercial. We'd be willing to pay that, but I can see that it would probably not be of interest to the community, because it would be a lot more work to then integrate an option to use the commercial nginx with this image.

I'm probably better off posting this issue on the commercial nginx repo instead.

Thank you very much for the help!

@dcowden dcowden closed this as completed Apr 9, 2018
@dcowden
Copy link
Author

dcowden commented Apr 9, 2018

For documentation purposes, I've posted a version of this issue here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants