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

Fixed: DockerHealthcheck.sh #4432

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fixed: DockerHealthcheck.sh #4432

wants to merge 2 commits into from

Conversation

tilltmk
Copy link

@tilltmk tilltmk commented Nov 12, 2023

Fixed "unhealthy" container status in docker

@zerebos
Copy link
Collaborator

zerebos commented Nov 12, 2023

But Trilium already has a healthcheck that works? Here's how it looks for me

image

image

And here is where Trilium sets up the Healthcheck https://github.com/zadam/trilium/blob/master/Dockerfile#L42 and the actual implementation here https://github.com/zadam/trilium/blob/master/docker_healthcheck.js

@tilltmk
Copy link
Author

tilltmk commented Nov 12, 2023

yes, i know it has a js Healthcheck; but it didn't work, in portainer the container status was shown as 'unhealthy'. i searched online for the problem and some people had issues with it as well. maybe it is because of environmental configurations, but the DockerHealthcheck.sh is necessary for the container in some peoples env to be shown as healthy.

@tilltmk
Copy link
Author

tilltmk commented Nov 12, 2023

referencing: #3665

@zerebos
Copy link
Collaborator

zerebos commented Nov 12, 2023

I'm just saying it feels unnecessary to have two separate healthchecks especially when the one is already working for the majority of cases. Obviously it's not an incompatibility with docker or portainer in general as I've just shown both, so there's some other issue that could/should be fixed, or removed in favor of a different health check.

@zadam
Copy link
Owner

zadam commented Nov 13, 2023

As @rauenzi said, it's much better to figure out why the existing health check doesn't work and fix that.

The health check you're adding is kind not integrated into the app at all - it refers to non existing TRILIUM_URL env. variable etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants