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

Graceful Shutdown not Implemented Properly #731

Closed
t0rr3sp3dr0 opened this issue Jul 14, 2023 · 0 comments · Fixed by #732
Closed

Graceful Shutdown not Implemented Properly #731

t0rr3sp3dr0 opened this issue Jul 14, 2023 · 0 comments · Fixed by #732
Labels
bug Something isn't working

Comments

@t0rr3sp3dr0
Copy link
Contributor

Report

The Interceptor and the Scaler have a graceful shutdown process triggered when the context is cancelled. But I noticed neither process handles SIGINT or SIGTERM, so they die instantly.

This is not a huge problem for the Scaler, but I can't say the same for the Interceptor. When rolling out the Interceptor, it will abruptly terminate all existing connections, creating problems for clients of services behind the proxy.

Even if we were handling the signals, the shutdown routine of both services is not implemented correctly, as I explain below. This also means that, in the current implementation, if we have an error or we cancel the context for any reason, it terminates the service in a way very similar to a SIGKILL.

For the Interceptor, we correctly invoke http.Server#Shutdown to close the active connections before terminating the process. The problem is that we always invoke that method with a cancelled context, making it fail instantly.

On the Scaler, we are invoking net.Listener#Close to shutdown the gRPC server, terminating connections on the TCP level. This is not the proper way of tearing down the server and grpc.Server#GracefulStop should be used instead.

Expected Behavior

Both the Interceptor and the Scaler should shutdown gracefully, just like the Operator does.

Actual Behavior

The services terminate connections in a bad way, resulting in error on clients connected to them.

Steps to Reproduce the Problem

  1. kubectl rollout restart -n keda deployment.apps/keda-http-add-on-interceptor deployment.apps/keda-http-add-on-scaler

Logs from KEDA HTTP operator

Given the processes die upon receiving SIGINT or SIGTERM, there is no log.

HTTP Add-on Version

0.5.0

Kubernetes Version

1.26

Platform

Microsoft Azure

Anything else?

No response

@t0rr3sp3dr0 t0rr3sp3dr0 added the bug Something isn't working label Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant