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

Update hawkBit to Application Version 0.5.0 #549

Merged
merged 10 commits into from
Oct 7, 2024

Conversation

lharzenetter
Copy link
Member

In the current version, the helm chart does not support the latest version of hawkBit that is 0.5.0.

Because of the removal of the vaadin UI, the readyness and liveness probes are broken.
Additionally, by default, the version 0.5.0 uses the MariaDB Connector instead of the MySQL Connector.

Thus, I updated the readiness and liveness probes to use the Swagger-UI and the MariaDB connection string by default.

fix readiness probes
fix default jdbc driver

Signed-off-by: Lukas Harzenetter <[email protected]>
@calohmn
Copy link
Contributor

calohmn commented Sep 6, 2024

@lharzenetter The CI checks are currently failing here because of a failing helm test [-n <NS>] eclipse-hawkbit check.
The issue is that in charts/hawkbit/templates/tests/test-connection.yaml there is an HTTP request to the root resource on the hawkBit pod being made, which results in a status 400 response (see eclipse-hawkbit/hawkbit#1832).
Using

args:  ['{{ include "hawkbit.fullname" . }}:{{ .Values.service.port }}/swagger-ui/index.html']

instead lets the test succeed.
However, as the same endpoint is already used in the liveness/readiness checks, this test wouldn't look very useful. Maybe another endpoint could be used (using the admin credentials).

@strailov @avgustinmm Could you have a look at this PR?

@avgustinmm
Copy link

However, as the same endpoint is already used in the liveness/readiness checks, this test wouldn't look very useful. Maybe another endpoint could be used (using the admin credentials).

Using admin credentials you could call:

curl -X 'GET' \
  'http://localhost:8080/rest/v1/userinfo' \
  -H 'accept: application/json' \
  -H 'Authorization: Basic ...'

This shall return info about the tenant and user, e.g.:

{
  "tenant": "DEFAULT",
  "username": "admin"
}

This could be a useful liveness/readiness probe since it indicates that the management API is ready to be called up.

@lharzenetter
Copy link
Member Author

This could be a useful liveness/readiness probe since it indicates that the management API is ready to be called up.

I agree.
However, IMHO, we should not use an admin resource as a liveness/readiness probe as you must update it if you use "real" credentials.

@lharzenetter
Copy link
Member Author

Do you have any comments on the code? Could you please merge it?

Cheers

@strailov
Copy link

It looks good for me. Could be merged.

charts/hawkbit/values.yaml Outdated Show resolved Hide resolved
@calohmn calohmn merged commit f766511 into eclipse:master Oct 7, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants