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

adding healthcheck to docker-compose #3557

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

Conversation

Tokesh
Copy link
Contributor

@Tokesh Tokesh commented May 24, 2023

Description

Here few ways how we can add healthcheck, but i add some settings to docker-compose files based on their ports.
I test it in docker ps. Statuses added healthy or unhealthy (problems with SSL certification)
I'm ready to listen all your suggestions and rework this.

Issues Resolved

[#3010]

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Tokesh <tokesh789@gmail.com>
@codecov
Copy link

codecov bot commented May 24, 2023

Codecov Report

Merging #3557 (0729b25) into main (fb29661) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #3557   +/-   ##
=======================================
  Coverage   91.48%   91.48%           
=======================================
  Files         181      181           
  Lines        5369     5369           
=======================================
  Hits         4912     4912           
  Misses        457      457           

@peterzhuamazon
Copy link
Member

Thanks @Tokesh on the contribution here.

@prudhvigodithi I am not sure if using curl on docker-compose file is a good way to do healthcheck?
Since this is only applying to docker-compose file.

@Tokesh
Copy link
Contributor Author

Tokesh commented May 25, 2023

Thanks @Tokesh on the contribution here.

@prudhvigodithi I am not sure if using curl on docker-compose file is a good way to do healthcheck? Since this is only applying to docker-compose file.

Thank you for reviewing of pr!
I have looked at different implementations of dockerhealth and often use either curl or ping.

But really, i'm not sure about docker-compose, because i can add it everywhere, like in dockerfiles.
This I wanted to ask you, maybe there is a more efficient option where you can add it? I just thought that all the images that are needed will be raised through the docker compose and you can add them there and that will be enough. But I'll still be happy to hear what I can improve in my code. I am so glad that I can learn from you!

@Tokesh Tokesh mentioned this pull request May 29, 2023
@prudhvigodithi
Copy link
Collaborator

Thanks @Tokesh can you check this comment ?
Thank you

Signed-off-by: Tokesh <tokesh789@gmail.com>
@prudhvigodithi
Copy link
Collaborator

Hey @Tokesh thanks for the update the HEALTHCHECK CMD added looks good to me, however I assume to invoke the URL http://localhost:9201/_cat/health?v when security plugin is enabled we need the authentication.
@peterzhuamazon @gaiksaya can you please add your thoughts?
Thank you

@peterzhuamazon
Copy link
Member

Hey @Tokesh thanks for the update the HEALTHCHECK CMD added looks good to me, however I assume to invoke the URL http://localhost:9201/_cat/health?v when security plugin is enabled we need the authentication. @peterzhuamazon @gaiksaya can you please add your thoughts? Thank you

I think so, tho I have not tested on that yet.
Probably need some checks to ensure different behaviors when security is enabled vs disabled.
Thanks.

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

Successfully merging this pull request may close these issues.

3 participants