-
Notifications
You must be signed in to change notification settings - Fork 5
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
add health endpoint for healthchecks #27
Conversation
The healthcheck only works with the PORT env variable and I wanted to make the healthcheck built in inside invidious-companion but I couldn't find any way to make this code run synchronously to prevent the execution of the code that is bellow of this: Fijxu@6c40b8a |
Dockerfile
Outdated
@@ -13,6 +13,10 @@ RUN curl -fsSL https://github.com/krallin/tini/releases/download/v${TINI_VERSION | |||
--output /tini \ | |||
&& chmod +x /tini | |||
|
|||
RUN curl -fsSL https://github.com/dmikusa/tiny-health-checker/releases/download/v0.33.0/thc-x86_64-unknown-linux-musl \ |
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.
We support ARM too. And why not using a common tool like wget or curl?
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.
We support ARM too. And why not using a common tool like wget or curl?
I wanted to use something simple with a small attack surface, curl works but seems to be overkill IMO.
What about doing an ARM docker file with the thc
executable but for ARM?
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.
I have a better idea. Have the "healthcheck" command builtin into the invidious companion binary.
Add a new helper in the helper folder and if you run "./invidious companion healthcheck" then this will ping /health and confirm if it works or not.
This way, no additional binary!
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.
I have a better idea. Have the "healthcheck" command builtin into the invidious companion binary.
That is what I wanted to do in the first place! But as I said in #27 (comment), I couldn't find a way to make it run synchronously without executing none of the code that is bellow. It still works tho so I will add what I did in Fijxu@6c40b8a
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.
Ha yes indeed right! I think it's time I introduce some kind of args system. Like having --version
, --run
and so on. I have created a github issue: #33
This way we can add the arg healthcheck
which do not execute the main code but instead just check if a local install is running.
Could you please remove the modifications about the Dockerfile? We will see about these modifications in another PR which will add the args system.
Thanks.
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.
Done!
Dockerfile
Outdated
@@ -13,6 +13,10 @@ RUN curl -fsSL https://github.com/krallin/tini/releases/download/v${TINI_VERSION | |||
--output /tini \ | |||
&& chmod +x /tini | |||
|
|||
RUN curl -fsSL https://github.com/dmikusa/tiny-health-checker/releases/download/v0.33.0/thc-x86_64-unknown-linux-musl \ |
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.
I have a better idea. Have the "healthcheck" command builtin into the invidious companion binary.
Add a new helper in the helper folder and if you run "./invidious companion healthcheck" then this will ping /health and confirm if it works or not.
This way, no additional binary!
Partially solve #24