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

Handle /healthz for liveness checks. #1264

Merged
merged 2 commits into from
Apr 9, 2021
Merged

Conversation

stashist
Copy link
Contributor

@stashist stashist commented Apr 3, 2021

This is pre-authentication as it exposes no info and is far cheaper than
e.g. transmitting the login page.

This is pre-authentication as it exposes no info and is far cheaper than
e.g. transmitting the login page.
@Tweeticoats
Copy link
Contributor

If we are adding this we might as well update the docker container to run a check this url to see if the container is running.
ie run curl / wget and download the url and see if this returns a valid response.
To keep size down we could use busybox and the wget applet to add 743k to the size of the container.

You can then add the following to the Dockerfile:
HEALTHCHECK --interval=12s --timeout=12s --start-period=30s CMD busybox wget http://localhost:9999/healthz

@stashist
Copy link
Contributor Author

stashist commented Apr 4, 2021

Definitely. That's what I'm doing now internally.

That said, my intent is to propose a followup PR for discussion where I add this functionality to the stash binary itself which is comparatively cheaper (since we're already linking in HTTP get functionality). That would allow me to move to distroless images (and propose those if it works out).

If that get's rejected I'll likely propose the bundled wget/curl instead as you already suggested.

@bnkai
Copy link
Collaborator

bnkai commented Apr 5, 2021

This looks ok.
As for adding the functionality in stash itself i dont think it will be of any practical use (If i understood correctly what you meant).
When you are troubleshooting you want to test the access to stash from outside eg other pc, wan, outside the container, not from where stash is running.
As for the distro-less image while stash doesnt have any external dependencies helping scripts or scrapers usualy require python or node or something else. It's a lot easier to just use an ubuntu or debian image for example to add those deps instead of a distro-less image.

@WithoutPants WithoutPants added the improvement Something needed tweaking. label Apr 7, 2021
@WithoutPants WithoutPants added this to the Version 0.7.0 milestone Apr 7, 2021
@WithoutPants WithoutPants merged commit 4462b3c into stashapp:develop Apr 9, 2021
peolic pushed a commit to peolic/stash that referenced this pull request Apr 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Something needed tweaking.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants