-
Notifications
You must be signed in to change notification settings - Fork 272
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
Docker Healthcheck #3010
Comments
[Triage] We will look into this. Looks like a good thing to have. Thanks for creating the issue @markusa380 |
@markusa380 Thanks for opening the issue. We are looking in to this issue. In the meantime, feel free to contribute this enhancement. |
Hello! I'm newbie at Open Source Community. If it still relevant issue, can you provide information about the dockerfiles in which the heartbeat status needs to be added? |
Is opensearch-project/opensearch-go#315 relevant? |
@prudhvigodithi @peterzhuamazon @rishabh6788 @gaiksaya Do you have some inputs for @Tokesh ? |
Hi @Tokesh These are the relevant docker files which would need health check: Associated docker images: |
Thank you all for the details! I am currently taking my college exams. I will try to send the PR for consideration within a week. I don't seem to be blocking anyone. |
All the best for your exam @Tokesh. |
Sure @Tokesh , all the best to your exam and feel free to ask any questions regarding this issue whenever you need. |
Now I'm thinking that maybe I made the wrong approach for implementation in my PR. I just added a health check to each service in the docker compose. |
The best practice to add health check in docker is to use Example (using https://opensearch.org/docs/2.7/api-reference/cat/cat-health/)
The above once the image is built Also FYI this will not impact when running docker in Kubernetes as Kubernetes doesn't use Docker Healthchecks. They are explicitly disabled within Kubernetes. @Tokesh can you please explore Thank you |
Okkie, i got it! I am very glad that I can learn the best practices. Thank you very much! Made a PR to check. Im ready to change or add if that necessary. |
@Tokesh Any update on this issue? |
Hello! As I remember, we got a little stuck on the implementation itself. The question was that we need authentication. |
@prudhvigodithi @peterzhuamazon Can you help @Tokesh here? |
Is your feature request related to a problem? Please describe.
It's harder than necessary to detect when OpenSearch Docker containers are ready to accept requests, for example when using testcontainers.
Describe the solution you'd like
I'd like to see the official Docker images use the
HEALTHCHECK
instruction (which should probably request/_cat/health?v=status
) so other systems can depend on it.Describe alternatives you've considered
Alternative solutions revolve around the systems using the image, e.g. by implementing custom health check loops or, like testcontainers with ElasticSearch, by using a regex on STDOUT.
Additional context
n/a
The text was updated successfully, but these errors were encountered: