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

[FR] Add HEALTHCHECK to Dockerfile #149

Open
mogul opened this issue Dec 4, 2020 · 2 comments
Open

[FR] Add HEALTHCHECK to Dockerfile #149

mogul opened this issue Dec 4, 2020 · 2 comments
Labels
core broker enhancement New feature or request good first issue Good for newcomers help wanted The team has de-prioritized this and could use your help!

Comments

@mogul
Copy link
Contributor

mogul commented Dec 4, 2020

Is your feature request related to a problem? Please describe.

When using the Docker image in automated testing, after we start the CSB and then run tests (with the eden OSBAPI client), we see:

Could not find service in catalog: Could not fetch catalog: Failed doing HTTP request: Get http://127.0.0.1:8080/v2/catalog: read tcp 127.0.0.1:54568->127.0.0.1:8080: read: connection reset by peer

This is because after startup of the CSB, there is a period where the broker is still reading files and not yet ready to serve requests, and there's no good way to know when the CSB is actually ready.

Describe the solution you'd like

The Dockerfile should include a HEALTHCHECK directive so that scripts and Makefiles can explicitly wait for the container to be healthy before proceeding.

Describe alternatives you've considered

We previously added in a delay to wait at least X seconds after starting the image before proceeding to run tests, where X was picked by trial and error. This felt like a pretty poor workaround, given that the performance between the GitHub Actions runner and our local machines varies widely, and we didn't want to inject any more delay than was necessary into our manual iteration time.

Additional Context

I worked out what the healthcheck should be over here (though you may want to change the interval and retry count). With the healthcheck in place, here's a one-liner that waits for the container to be healthy.

Priority

Low

Priority Context

I'd like to remove as much complexity from my Makefile as possible, and having a built-in healthcheck will benefit other users of the CSB in various contexts. However, I'm clearly able to work around the problem now by supplying the docker run arguments.

Platform

N/A

Applicable Services

N/A

@mogul mogul added the enhancement New feature or request label Dec 4, 2020
@mogul mogul changed the title [FR] Add healthcheck to Dockerfile [FR] Add HEALTHCHECK to Dockerfile Dec 4, 2020
@erniebilling
Copy link

HEALTHCHECK seems to be global to the container, but is only necessary in one use case for the container - when running the server in a test mode. The other use cases like building brokerpaks, generating docs, running the test examples; a global health check may not work.

Happy to have someone do some investigation here and offer a PR to the dockerfile

@mogul
Copy link
Contributor Author

mogul commented Dec 19, 2020

Note this may be a good reason for specifying the HEALTHCHECK via CLI params as I did in the example... If so then this should be documented for folks to find... even if only in the form of specifying those parameters in your own Makefile for them to copy from!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core broker enhancement New feature or request good first issue Good for newcomers help wanted The team has de-prioritized this and could use your help!
Projects
Status: Waiting for Changes | Open for Contributions
Development

No branches or pull requests

3 participants