Closed
Conversation
This is the first step to handle configuration reloading. With those changes, it is already possible to do so, by sending a SIGUSR2 signal to the Go server process. The next step would be to watch for changes to configuration files and call the Restart function of the GoUnicorn instance. SIGHUP is catched by the go server and forwarded as-is to gunicorn, which causes it to restart its workers. However, that does not trigger a reload of the Django settings, probably because they are already loaded in the master, before creating any of the worker instances. SIGUSR2, however, can be used to spawn a new gunicorn master process, but handling it is a bit trickier. Please refer to Gunicorn's documentation[0] for details, especially the "Upgrading to a new binary on the fly" section. As we are now effectively killing the gunicorn processed launched by the server, we need to handle some sort of check to make sure it is still running. That's done by using the already existing healthchecks, making them useful not only for the application start, but also for its lifetime. If a check is failed too many times in a given time period, the gunicorn processed is killed (if necessary) and then restarted. [0] https://docs.gunicorn.org/en/20.1.0/signals.html Other relevant links and documentation: Python library handling the processing swaping upon a SIGUSR2: https://github.com/flupke/rainbow-saddle/ Golang cannot easily check if a process exists on Unix systems: golang/go#34396 Signed-off-by: Marc 'risson' Schmitt <marc.schmitt@risson.space>
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## main #5415 +/- ##
==========================================
+ Coverage 90.37% 92.61% +2.25%
==========================================
Files 506 550 +44
Lines 25944 26487 +543
==========================================
+ Hits 23443 24528 +1085
+ Misses 2501 1959 -542
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Member
Author
|
I've started working on automatic configuration reloading, but currently the configuration discovery doesn't match between the go and python code, so I'm updating that first. |
rissson
commented
Apr 28, 2023
internal/gounicorn/gounicorn.go
Outdated
| return | ||
| } | ||
|
|
||
| newPidB, err := ioutil.ReadFile(newPidFile) |
Member
Author
There was a problem hiding this comment.
If anyone that knows go, knows how to do this without writing 1k lines, I'd be happy to know it too!
Signed-off-by: Marc 'risson' Schmitt <marc.schmitt@risson.space>
Signed-off-by: Marc 'risson' Schmitt <marc.schmitt@risson.space>
Signed-off-by: Marc 'risson' Schmitt <marc.schmitt@risson.space>
Signed-off-by: Marc 'risson' Schmitt <marc.schmitt@risson.space>
f347c36 to
d3ef158
Compare
rissson
commented
Apr 28, 2023
Comment on lines
38
to
39
| started: false, | ||
| killed: false, |
Member
Author
There was a problem hiding this comment.
Would an enum be better here?
7 tasks
Signed-off-by: Jens Langhammer <jens@goauthentik.io>
❌ Deploy Preview for authentik-storybook failed.
|
Signed-off-by: Jens Langhammer <jens@goauthentik.io>
✅ Deploy Preview for authentik ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Contributor
|
authentik PR Installation instructions Instructions for docker-composeAdd the following block to your AUTHENTIK_IMAGE=ghcr.io/goauthentik/dev-server
AUTHENTIK_TAG=gh-server-handle-sighup-1687539399-241059f
AUTHENTIK_OUTPOSTS__CONTAINER_IMAGE_BASE=ghcr.io/goauthentik/dev-%(type)s:gh-%(build_hash)sFor arm64, use these values: AUTHENTIK_IMAGE=ghcr.io/goauthentik/dev-server
AUTHENTIK_TAG=gh-server-handle-sighup-1687539399-241059f-arm64
AUTHENTIK_OUTPOSTS__CONTAINER_IMAGE_BASE=ghcr.io/goauthentik/dev-%(type)s:gh-%(build_hash)sAfterwards, run the upgrade commands from the latest release notes. Instructions for KubernetesAdd the following block to your authentik:
outposts:
container_image_base: ghcr.io/goauthentik/dev-%(type)s:gh-%(build_hash)s
image:
repository: ghcr.io/goauthentik/dev-server
tag: gh-server-handle-sighup-1687539399-241059fFor arm64, use these values: authentik:
outposts:
container_image_base: ghcr.io/goauthentik/dev-%(type)s:gh-%(build_hash)s
image:
repository: ghcr.io/goauthentik/dev-server
tag: gh-server-handle-sighup-1687539399-241059f-arm64Afterwards, run the upgrade commands from the latest release notes. |
BeryJu
reviewed
Aug 24, 2023
Member
BeryJu
left a comment
There was a problem hiding this comment.
- Configuration changes are good to keep all the defaults in one place
- Gunicorn health checking and reloading is great
- Configuration watching isn't really needed anymore since we can do that natively now
- In a future PR maybe generalise the process manager to be re-usable for Guacamole
- We also don't really want to have our own "init-system"
- We could consider using supervisord
- Also https://github.com/ochinchina/supervisord exists which we could maybe use as a library
This was referenced Aug 25, 2023
Member
Author
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
2 participants
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.
EDIT: Configuration watching is now implemented. This PR is best reviewed commit by commit, I'll update this description later.
Details
This is the first step to handle configuration reloading. With those changes, it is already possible to do so, by sending a SIGUSR2 signal to the Go server process. The next step would be to watch for changes to configuration files and call the Restart function of the GoUnicorn instance.
SIGHUP is catched by the go server and forwarded as-is to gunicorn, which causes it to restart its workers. However, that does not trigger a reload of the Django settings, probably because they are already loaded in the master, before creating any of the worker instances.
SIGUSR2, however, can be used to spawn a new gunicorn master process, but handling it is a bit trickier. Please refer to Gunicorn's documentation[0] for details, especially the "Upgrading to a new binary on the fly" section.
As we are now effectively killing the gunicorn processed launched by the server, we need to handle some sort of check to make sure it is still running. That's done by using the already existing healthchecks, making them useful not only for the application start, but also for its lifetime. If a check is failed too many times in a given time period, the gunicorn processed is killed (if necessary) and then restarted.
[0] https://docs.gunicorn.org/en/20.1.0/signals.html
Other relevant links and documentation:
Python library handling the processing swaping upon a SIGUSR2: https://github.com/flupke/rainbow-saddle/
Golang cannot easily check if a process exists on Unix systems: golang/go#34396
Changes
New Features
serverprocess.Breaking Changes
Not that I'm aware of.
Checklist
ak test authentik/)make lint-fix)If applicable
make website)