rework init system that starts localstack#7970
Merged
Conversation
b88d23b to
d800794
Compare
thrau
commented
Mar 28, 2023
Member
Author
|
just going to wait for @dfangl to approve/request changes :-) |
dfangl
approved these changes
Mar 28, 2023
Member
dfangl
left a comment
There was a problem hiding this comment.
LGTM! We can test my remaining "possible issues" after this is merged, as they are not critical in any way.
This was referenced Mar 29, 2023
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR completely reworks the way the localstack process is started in the container.
The original motivation was to implement a way for localstack to actually exit with a proper exit code from inside the container. It turned out to be non-trivial, given several challenges:
exitcodesandautorestart=unexpectedconfiguration in supervisor, but then actually terminating supervisor correctly turned out to also be non-trivial and involve hackingSolution
The localstack supervisor is a python program that behaves as follows (from the doc)
bin/localstack-supervisor.pyis now used indocker-entrypoint.shtogether withexec, which can forward the exit code of localstack without complicated traps and can properly terminate the container.The PR also adds:
LocalstackExitexception that indiciates during the startup procedure that localstack shouldlocalstack.runtime.main.Effects / Implications
docker-entrypoint.shand signal handlingSIGHUPto cleanly restart localstack. This also works outside docker now./var/lib/logs/localstack_infra.log(and the stderr equivalent). The log was introduced in Allow a init.d like dir to be used to execute scripts upon startup #1018 with thedocker-entrypointy.sh, but seemed unecessary with the new implementation. If people want this behavior back (i.e., the extra log file), we can introduce it inlocalstack-supervisor.pyto write to stdout and a logfile. When starting from the CLI, we still have the main container log.On automatically restarting localstack
The original behavior I had implemented was:
but i decided to remove it because it seems to me that, in most cases, when localstack terminates, there's no obvious motivation to try to restart it. in fact, i've seen instance be in an endless loop of trying to start up localstack when re-trying makes no sense (e.g., import errors).
if people need this behavior, then we can easily re-introduce it. it's a few lines of code in the localstack-supervisor.py script.
How to test
From the host:
start the supervisor:
to restart localstack you can:
pkill -HUP -f "bin/localstack-supervisor.py"curl -X POST -H 'Content-Type: application/json' http://localhost:4566/_localstack/health -d '{"action":"restart"}'to terminate localstack you can:
pkill -f "bin/localstack-supervisor.py"curl -X POST -H 'Content-Type: application/json' http://localhost:4566/_localstack/health -d '{"action":"kill"}'infra.exit_infra(0)LocalstackExitexception. (in the future, we can write handlers that can exit localstack from an HTTP request - but there seems to be no use case for this yet)Using docker
You can check that the subprocesses of localstack terminate correctly:
run
in a new terminal window, you can observe the process tree:
You can start up some processes by running:
awslocal dynamodb list-tables awslocal opensearch create-domain --domain-name "foobar"Then, you can HUP the docker process, or follow any of the steps from earlier:
docker kill --signal=HUP localstack_mainTODO
In the future, I would like to further separate
bootstrap.pyandinfra.py. Whereinfra.pyis tasked with starting the runtime itself, andbootstrap.pyis associated with starting localstack from the CLI.