-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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 healtcheck.sh scripts for jigasi, jicofo, jvb and prosody, add cu… #1845
Conversation
…rl and mod_http_health to prosody
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.
Left some comments, PTAL! Also @aaronkvanmeerten WDYT?
…/bin, use 127.0.0.1 instead of localhost
@saghul i think i've considered every suggestion, thanks for the input! :) |
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.
LGTM! LEt's wait for @aaronkvanmeerten 's feedback.
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.
LGTM, curious about the difference in hash for the mercurial source fetch, but not enough to block merging.
@@ -57,6 +57,7 @@ RUN wget -qO /etc/apt/trusted.gpg.d/prosody.gpg https://prosody.im/files/prosody | |||
rm -rf /usr/share/lua/{5.1,5.2,5.3} && \ | |||
wget -qO /prosody-plugins/mod_auth_cyrus.lua https://hg.prosody.im/prosody-modules/raw-file/65438e4ba563/mod_auth_cyrus/mod_auth_cyrus.lua && \ | |||
wget -qO /prosody-plugins/sasl_cyrus.lua https://hg.prosody.im/prosody-modules/raw-file/65438e4ba563/mod_auth_cyrus/sasl_cyrus.lua && \ | |||
wget -qO /prosody-plugins/mod_http_health.lua https://hg.prosody.im/prosody-modules/raw-file/2b80188448d1/mod_http_health/mod_http_health.lua && \ |
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.
This one has a different hash than the rest, not urgent but I'm curious if we can either have the same hash or update the others to a newer one? Doesn't have to be in this PR.
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.
Thanks for merging! :)
Let me try to satisfy your curiosity.. ;)
2b80188448d1/5924 refers to the current tip (June 2024) of the hg repository.
65438e4ba563/4929 refers to a commit from Apr 18 2022.
mod_http_health was added in 5667:9bcd257dea4e (Oct 2023).
That is why 65438e4ba563 was not feasible for mod_http_health.
After diffing 65438e4ba563 and 2b80188448d1 i can confirm, there are no differences in mod_auth_cyrus..
So there is nothing to be said against using the current tip 2b80188448d1 for mod_auth_cyrus too.
Want a PR? :)
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.
If that's the case, I'd say there is no need!
coming from jitsi/jigasi#539
We are using the container images with podman and are going to leverage the integrated healthcheck features..
This PR adds some basic healtcheck.sh scripts for jigasi, jicofo, jvb and prosody but does not add them as healthcheck in the Dockerfiles - because we are not using docker and i cant estimate if there are any sideeffects.
Anyhow i think having general healtcheck scripts available in the images would be a good thing :)