eclipse / packages

IoT Packages project
https://eclipse.org/packages
Eclipse Public License 2.0
46 stars 66 forks source link

Update hawkBit to Application Version 0.5.0 #549

Closed lharzenetter closed 1 month ago

lharzenetter commented 2 months ago

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.

calohmn commented 2 months ago

@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 https://github.com/eclipse/hawkbit/issues/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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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

Cheers

strailov commented 2 months ago

It looks good for me. Could be merged.