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

Add healthcheck in components dockerfile #104

Merged
merged 5 commits into from
Nov 16, 2023

Conversation

mhrimaz
Copy link
Contributor

@mhrimaz mhrimaz commented Oct 5, 2023

With the integrated healthcheck, it is easier to determine container health.

image

As further addition it would be nice to bump the jdk base image to eclipse temurin jdk 17.

What do you think? @FrankSchnicke

@FrankSchnicke
Copy link
Contributor

Integrating the healthcheck directly in the container is a good idea. However, polling the /shells etc. endpoint might create a high load on backend etc. Can you update the PR to use the explicit health endpoint?

@mhrimaz
Copy link
Contributor Author

mhrimaz commented Oct 5, 2023

Integrating the healthcheck directly in the container is a good idea. However, polling the /shells etc. endpoint might create a high load on backend etc. Can you update the PR to use the explicit health endpoint?

I will do that, but before that, I need to know if management path should be configurable or not.

The latest aas environment docker digest: 3471dea2cf82 does not have /health endpoint. It is reachable via /actuator/health. This is because of commenting out the following config:

# Base Path for Spring Boot Actuator
management.endpoints.web.base-path=/

So it seems /health can change and if someone changes that (accidentally/intentionally), then the whole logic would break. So I need to find a more dynamic way if this case is intended.

@FrankSchnicke
Copy link
Contributor

Thanks for pointing this out, we missed updating the documentation for this. I'm currently not aware of a use case where reconfiguring the health endpoint's location would make sense in our contexts. Thus, you can assume it to always be at /actuator/health

@FrankSchnicke
Copy link
Contributor

Hi @mhrimaz, any update on this? Is this still of relevance for you?

use `/actuator/health` as health endpoint
set actuator config to default to keep docker images compatible
Healthcheck for AASXFileServer
@mhrimaz mhrimaz force-pushed the feature/dockerhealth branch from 8f146e5 to 8ae315f Compare November 15, 2023 14:23
@mhrimaz
Copy link
Contributor Author

mhrimaz commented Nov 15, 2023

@FrankSchnicke should be fine now. Also check #138 if you need base image bump.

@FrankSchnicke
Copy link
Contributor

Thanks a lot for this PR. If you mark it as "ready for review", I will merge it.

@mhrimaz mhrimaz marked this pull request as ready for review November 15, 2023 15:37
@FrankSchnicke FrankSchnicke merged commit 2f6781a into eclipse-basyx:main Nov 16, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants