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

Create liveliness endpoint #1506

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

Create liveliness endpoint #1506

wants to merge 37 commits into from

Conversation

asalan316
Copy link
Contributor

@asalan316 asalan316 commented Mar 2, 2023

This change introduces new /health/liveness endpoint, allowing users to check the health of the individual services
The response

{"overall_status":"UP","checks":[]}

Also, the following existing endpoints are now moved to the new paths

Golang Microservices

Exisiting Routes New Health Routes
/ /health/prometheus
/debug/pprof /health/debug/pprof
/debug/pprof /health/debug/pprof
- /health/liveness (new)
- /health (respond to /health/liveness )

Scheduler Microservice

Exisiting Routes New Health Routes
/health /health/prometheus
/prometheus /health/prometheus
- /health/liveness (new)
- /health (respond to /health/liveness )

By default, all health routes are protected with basic auth.

However, routes can be configured to be unprotected by using the configuration
autoscaler.apiserver.health.unprotected_endpoints

@asalan316 asalan316 force-pushed the create-liveliness-endpoint branch 2 times, most recently from 04bc7db to 445429d Compare March 8, 2023 15:21
@joergdw joergdw added the 🚧🛠️ WIP📝 Work in progress label Mar 8, 2023
@joergdw joergdw requested a review from a team March 9, 2023 13:42
@joergdw joergdw added the allow-acceptance-tests This label needs to be added to enable the acceptance tests to run. label Mar 14, 2023
@joergdw joergdw force-pushed the create-liveliness-endpoint branch 12 times, most recently from d8fabe2 to d484f7b Compare March 14, 2023 14:29
@joergdw joergdw removed the 🚧🛠️ WIP📝 Work in progress label Mar 14, 2023
@joergdw joergdw force-pushed the create-liveliness-endpoint branch 5 times, most recently from de0e65d to 8498b9b Compare March 17, 2023 15:28
@asalan316 asalan316 force-pushed the create-liveliness-endpoint branch 4 times, most recently from 0270ec1 to 2865f60 Compare March 28, 2023 14:58
@asalan316 asalan316 force-pushed the create-liveliness-endpoint branch 6 times, most recently from 12aec5f to 7aff4d1 Compare May 24, 2023 11:38
start health server on defined ports (Port 0 is not allowed)
if (ObjectUtils.isEmpty(unprotectedEndpointsConfig)) { // health endpoints are authorized
isUserAuthenticatedOrSendError(chain, httpRequest, httpResponse);
} else if (!ObjectUtils.isEmpty(unprotectedEndpointsConfig)) {
Map<String, Boolean> validateMap = checkValidEndpoints(unprotectedEndpointsConfig);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I think a config-check should rather go into the config-class itself (see former comments).
  2. The name may be misleading. I rather understand it as a nonExistingEndpoints.
  3. Why do we need a boolean? Isn't pure existence enough? Then a set (as mentioned above) would be more appropriate.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an additional note, when I think about what we want to do: It would simplify a lot to have a function with the following signature in the class HealthServerConfiguration:

public boolean endpointIsUnprotected(<Some type> endpoint) {…

It is then reusable here! And as it will be quite performant, this simplification would then make as well obvious that we do not need the special case treatment in the lines 58-59 of the current implementation.

@asalan316 asalan316 force-pushed the create-liveliness-endpoint branch 5 times, most recently from 6a0c834 to 73cf734 Compare May 30, 2023 14:22
@sonarcloud
Copy link

sonarcloud bot commented Jun 15, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
allow-acceptance-tests This label needs to be added to enable the acceptance tests to run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants