8000 Fix runtime shutdown when triggered via signal handlers by dominikschubert · Pull Request #11018 · localstack/localstack · GitHub
[go: up one dir, main page]
Skip to content
8000

Fix runtime shutdown when triggered via signal handlers #11018

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 13, 2024
Merged

Fix runtime shutdown when triggered via signal handlers #11018

merged 1 commit into from
Jun 13, 2024

Conversation

dominikschubert
Copy link
Member
@dominikschubert dominikschubert commented Jun 13, 2024 •
edited
Loading

Motivation

After the merge of #10942 the CLI tests in Community started failing on the localstack restart command.

localstack restart calls POST /_localstack/health with {"action": "restart"}. The handler then sends a signal to the localstack supervisor (SIGUSR1) signaling a restart. The supervisor then sends a SIGTERM to localstack to shut it down.

The issue we observed was that the call was aborted and never returned with the expected "ok" response.

How we replicated the behavior without involving the supervisor was this:

  • Take some endpoint and add a sleep. We used the localstack.services.internal.HealthResource.on_post one and replaced it with
    def on_post(self, request: Request):
        import sys
        time.sleep(10)   # sleep added so we can send a signal while its running
        sys.stdout.write("SLEEP DONE\n")
        data = request.get_json(True, True)
        if not data:
            return Response("invalid request", 400)

        # backdoor API to support restarting the instance
        # if data.get("action") == "restart":
        #     signal_supervisor_restart()
        # elif data.get("action") == "kill":
        #     exit_infra(0)

        return Response("ok", 200)
  • Run http POST http://localhost:4566/_localstack/health action=restart
  • While the previous request is running run kill -SIGTERM <localstack-process-id>
  • Wait for the response of the http call. It should return "ok"

Changes

  • Spawns the runtime shutdown in a new thread instead of the main thread

Follow-ups

  • Create a proper bootstrap test for restarts
  • Fix SIGTERM handling when using LEGACY_RUNTIME=1. This should have the same behavior as SIGINT but currently it doesn't.

@dominikschubert dominikschubert self-assigned this Jun 13, 2024
@dominikschubert dominikschubert requested a review from thrau as a code owner June 13, 2024 10:21
@dominikschubert dominikschubert added the semver: patch Non-breaking changes which can be included in patch releases label Jun 13, 2024
@dominikschubert dominikschubert merged commit 84a643f into master Jun 13, 2024
11 of 18 checks passed
@dominikschubert dominikschubert deleted the fix-runtime-shutdown branch June 13, 2024 10:21
@dominikschubert dominikschubert changed the title Fix runtime shutdown when called triggered via signal handlers Fix runtime shutdown when triggered via signal handlers Jun 13, 2024
@alexrashed alexrashed mentioned this pull request Jun 13, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexrashed alexrashed alexrashed approved these changes

@dfangl dfangl Awaiting requested review from dfangl

@thrau thrau Awaiting requested review from thrau thrau is a code owner

Assignees

@dominikschubert dominikschubert

Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dominikschubert @alexrashed
0