fix: check the correct port for the container #275
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
We are always happy to welcome new contributors ❤️ To make things easier for everyone, please make sure to follow our contribution guidelines, check if you have already signed the ECA, and relate this pull request to an existing issue or discussion.
|
@JinsYin please follow the contribution guide and do not propose PR out-of-the-blue: https://eclipse-edc.github.io/documentation/for-contributors/guidelines/pr-checklist/ (small doc fix PRs are accepted tho, as the one you opened on IdentityHub) |
|
@ndr-brt I have made the necessary changes. |
launchers/catalog-dcp/Dockerfile
Outdated
| @@ -17,7 +17,7 @@ EXPOSE 8182 | |||
| ENV WEB_HTTP_PORT="8181" | |||
| ENV WEB_HTTP_PATH="/api" | |||
|
|
|||
| HEALTHCHECK --interval=5s --timeout=5s --retries=10 CMD curl --fail http://localhost:8181/api/check/health | |||
| HEALTHCHECK --interval=5s --timeout=5s --retries=10 CMD curl --fail http://localhost:$WEB_HTTP_PORT/api/check/health | |||
There was a problem hiding this comment.
it could worth also to replace the /api with WEB_HTTP_PATH in the url
launchers/catalog-mocked/Dockerfile
Outdated
| @@ -17,7 +17,7 @@ EXPOSE 8182 | |||
| ENV WEB_HTTP_PORT="8181" | |||
| ENV WEB_HTTP_PATH="/api" | |||
|
|
|||
| HEALTHCHECK --interval=5s --timeout=5s --retries=10 CMD curl --fail http://localhost:8181/api/check/health | |||
| HEALTHCHECK --interval=5s --timeout=5s --retries=10 CMD curl --fail http://localhost:$WEB_HTTP_PORT/api/check/health | |||
|
@ndr-brt Thank you, I have made a new commit. |
launchers/catalog-dcp/Dockerfile
Outdated
| @@ -17,7 +17,7 @@ EXPOSE 8182 | |||
| ENV WEB_HTTP_PORT="8181" | |||
| ENV WEB_HTTP_PATH="/api" | |||
|
|
|||
| HEALTHCHECK --interval=5s --timeout=5s --retries=10 CMD curl --fail http://localhost:8181/api/check/health | |||
| HEALTHCHECK --interval=5s --timeout=5s --retries=10 CMD curl --fail http://localhost:$WEB_HTTP_PORT/$WEB_HTTP_PATH/check/health | |||
There was a problem hiding this comment.
| HEALTHCHECK --interval=5s --timeout=5s --retries=10 CMD curl --fail http://localhost:$WEB_HTTP_PORT/$WEB_HTTP_PATH/check/health | |
| HEALTHCHECK --interval=5s --timeout=5s --retries=10 CMD curl --fail http://localhost:$WEB_HTTP_PORT$WEB_HTTP_PATH/check/health |
please note that the PATH starts with a slash, so it should not be reported in the url
launchers/catalog-mocked/Dockerfile
Outdated
| @@ -17,7 +17,7 @@ EXPOSE 8182 | |||
| ENV WEB_HTTP_PORT="8181" | |||
| ENV WEB_HTTP_PATH="/api" | |||
|
|
|||
| HEALTHCHECK --interval=5s --timeout=5s --retries=10 CMD curl --fail http://localhost:8181/api/check/health | |||
| HEALTHCHECK --interval=5s --timeout=5s --retries=10 CMD curl --fail http://localhost:$WEB_HTTP_PORT/$WEB_HTTP_PATH/check/health | |||
There was a problem hiding this comment.
ditto. plus, as I said in my previous comment could you add a test for each of these dockerfile that will assert that the containers will start correctly? (tests already exist, so...) change launcher tests so they will verify also the healthcheck with a different port/path?
|
@JinsYin updated a comment of mine because tests already exist, but they should be improved to cover the case when the default http port/path are configured |
|
@ndr-brt Thank you for your help. As I am not yet familiar with this code, I will need to spend some time. |
NP, it's just a call to the FederatedCatalog/.github/workflows/verify.yaml Lines 65 to 68 in 8f8faf4 |
|
This pull request is stale because it has been open for 7 days with no activity. |
|
This pull request was closed because it has been inactive for 7 days since being marked as stale. |
|
@JinsYin are your still committed to this pr? |
|
This pull request is stale because it has been open for 7 days with no activity. |
What this PR changes/adds
Set up the correct health check for the Dockerfile.
Why it does that
When starting the container and changing the WEB_HTTP_PORT environment, the container is unhealthy.
Further notes
List other areas of code that have changed but are not necessarily linked to the main feature. This could be method
signature changes, package declarations, bugs that were encountered and were fixed inline, etc.
Linked Issue(s)
Closes #276