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

add graceful shutdown for node18 functions #306

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

NikhilSharmaWe
Copy link

@NikhilSharmaWe NikhilSharmaWe commented May 26, 2023

Description

This PR adds graceful shutdown for template node18 functions.

Motivation and Context

  • I have raised an issue to propose this change (required)

Which issue(s) this PR fixes

Fixes #305

How Has This Been Tested?

  1. First create a function for the update node18 template.
  2. After the function is created, delete the function pod running with signal SIGTERM, like : kubectl -n openfaas-fn exec testdrainfunc-b44d49bb6-9kzzh -- kill -s SIGTERM 1.
  3. The logs shows the added logic is being implemented :
2023-05-26T09:18:38Z 2023/05/26 09:18:38 SIGTERM: no new connections in 15s
2023-05-26T09:18:38Z 2023/05/26 09:18:38 Removing lock-file : /tmp/.lock
2023-05-26T09:18:38Z Function got SIGTERM event, draining up to: 15s
2023-05-26T09:18:38Z Server gracefully shut down
2023-05-26T09:18:53Z 2023/05/26 09:18:53 No new connections allowed, draining: 0 requests
2023-05-26T09:18:53Z 2023/05/26 09:18:53 Exiting. Active connections: 0

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Version change (see: Impact to existing users)

Impact to existing users

There will be no significant impact on how users use openfaas functions.

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@alexellis
Copy link
Member

This looks very similar to what I was excepting. I think you'll need to test it and also need to wait for the health check duration before calling close on the server. See how we do that in the Go template.

@NikhilSharmaWe NikhilSharmaWe force-pushed the drainNode18Func branch 4 times, most recently from 13eba1c to 7508b33 Compare May 26, 2023 10:41
Signed-off-by: Nikhil Sharma <nikhilsharma230303@gmail.com>
@alexellis
Copy link
Member

I've sent you a trial license for OpenFaaS Pro/Standard.

Would you like to try testing it with a 10min shutdown time?

Installation -> https://docs.openfaas.com/deployment/pro/

Testing with long timeouts -> https://www.openfaas.com/blog/long-running-jobs/

I simulate it like this:

Deploy function... invoke it and watch the logs. (with a 10m sleep)

Then I update the code and image tag, and redeploy it.

If it's all working right, the Pod should go to Terminating but stay around until the invocation has completed successfully.

Here's my test function for Go/Python/Node for a longer timeout if you need it - https://github.com/alexellis/go-long

@NikhilSharmaWe
Copy link
Author

@alexellis

I test the update node18 template for

environment:
  write_timeout: 10m2s
  healthcheck_interval: 5s

Then the logs for the node18 func after kubectl scale -n openfaas-fn deploy/node18 --replicas=0 are:

2023-05-27T19:03:50Z 2023/05/27 19:03:50 SIGTERM: no new connections in 5s
2023-05-27T19:03:50Z 2023/05/27 19:03:50 Removing lock-file : /tmp/.lock
2023-05-27T19:03:50Z Function got SIGTERM event, draining up to: 10m2s
2023-05-27T19:03:50Z Server gracefully shut down
2023-05-27T19:03:55Z 2023/05/27 19:03:55 No new connections allowed, draining: 0 requests
2023-05-27T19:03:55Z 2023/05/27 19:03:55 Exiting. Active connections: 0

@alexellis
Copy link
Member

Thanks for working on this and for testing the change. What I'd like to see is a curl statement - followed by you scaling to zero replicas. Show that "time curl ..." completes despite you scaling down. You'll need the Pro/Standard license that I sent you separately.

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

Successfully merging this pull request may close these issues.

Graceful Shutdown for template node18 functions
2 participants