-
Notifications
You must be signed in to change notification settings - Fork 41
proposal: Add optional healthcheck scripts to WBS images #755
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
base: main
Are you sure you want to change the base?
Conversation
ccd31f5 to
2e419f1
Compare
e17ecc8 to
58a3ece
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the late review. Nice effort. I like the cleanup and agree with the motivation.
Due to the long wait, merge conflicts appeared. Do you mind fixing them?
8ed8556 to
2e419f1
Compare
2e419f1 to
9004bdb
Compare
Got it, re-applied on the current main. |
Maybe we can now squash it again? |
* feat: setup wikibase manifest external service mappings * fix: tests * feat: SELinux compatibility, configure Traefik via file (#862) * Volume Flags * wip: static traefik config * wip * wip: static config * style: lint * fix: traefik config override in tests * chore: cleanup, remove ping for now * Privilege * fix: selinux label for runners workspace volume --------- Co-authored-by: ricki-jay-wmde <[email protected]> * chore: remove manifest config from deploy * feat: add IS_JOBRUNNER like in #755 * chore: move manifest external service setting to wikibase image * test: new manifest values * style: lint * chore: remove IS_JOBRUNNER again, not yet required --------- Co-authored-by: ricki-jay-wmde <[email protected]>
|
I like the idea to integrate the health checks in the images. I think this is the right place to put them |
Takes all in-line healthchecks specifics and puts them in a
healthcheck.shfor each image. Not applying by default in image (i.e. as aHEALTHCHECKin the Dockerfile) to keep things flexible, but gives the responsibility for how to healthcheck back to the image--which is what Docker recommends. Also removing this specific from Deploy removes one more possible change vector in the Deploy configuration, keeping things easier to maintain and upgrade for users over time. Also has the benefit of making a standard healthcheck option available for the images for use cases outside of Deploy.In a related move this adds a new
IS_JOBRUNNERoption the Wikibase image instead of changing the command/entrypoint script. This allows the same health check script to be pointed which now will also healthcheck jobrunner when used in that mode.This PR is setup with atomic conventional commits and would be best to do a regular merge vs squash.