CadolesKube/sso-kustom
15
0
Fork 0
Code Issues 21 Pull Requests 6 Projects Releases Wiki Activity

fix(hydra): use same liveness URL as ory's helm #63

Merged
pcaseiro merged 3 commits from fix/hydra_liveness_probe_url into unstable 2025-03-07 13:52:31 +01:00
Conversation 8 Commits 3 Files Changed 2 +23 -1
lgourvenec commented 2025-03-07 11:18:45 +01:00
Owner
Copy Link

Le fetch du .well-known/openid-configuration peut peut-être impliquer un accès à la BDD. Dans le doute, on peut utiliser /health/alive comme le helm de chez Ory.

Le fetch du .well-known/openid-configuration peut peut-être impliquer un accès à la BDD. Dans le doute, on peut utiliser /health/alive comme le helm de chez Ory.
lgourvenec added 1 commit 2025-03-07 11:18:46 +01:00
vfebvre approved these changes 2025-03-07 11:21:27 +01:00
wpetit commented 2025-03-07 11:31:55 +01:00
Owner
Copy Link

J'ai un petit doute sur l'usage du alive ?

Si j'en crois le code, il va toujours répondre 200 à partir du moment où le service est démarré (cf. https://github.com/ory/x/blob/master/healthx/handler.go#L129-L133). Ça ne donne pas d'information sur le fait que le service est "prêt" à être utilisé (au sens Kubernetes ready).

Il faudrait peut être vérifier également dans la chart Helm si il n'y a pas également une vérification sur le endpoint health/ready ? (cf. https://github.com/ory/x/blob/master/healthx/handler.go#L165-L190) ?

J'ai un petit doute sur l'usage du `alive` ? Si j'en crois le code, il va toujours répondre `200` à partir du moment où le service est démarré (cf. https://github.com/ory/x/blob/master/healthx/handler.go#L129-L133). Ça ne donne pas d'information sur le fait que le service est "prêt" à être utilisé (au sens Kubernetes `ready`). Il faudrait peut être vérifier également dans la chart Helm si il n'y a pas également une vérification sur le endpoint `health/ready` ? (cf. https://github.com/ory/x/blob/master/healthx/handler.go#L165-L190) ?
pcaseiro commented 2025-03-07 11:37:15 +01:00
Owner
Copy Link

Le endpoint "/health/ready" me semble également plus approprié.
Attention aux logs quand même :

https://github.com/ory/hydra/issues/1839

https://www.ory.sh/docs/hydra/reference/api#tag/metadata/operation/isReady

Le endpoint "/health/ready" me semble également plus approprié. Attention aux logs quand même : https://github.com/ory/hydra/issues/1839 https://www.ory.sh/docs/hydra/reference/api#tag/metadata/operation/isReady
lgourvenec commented 2025-03-07 11:42:48 +01:00
Author
Owner
Copy Link

Ce qui est fait dans le helm d'ory :

livenessProbe -> /health/alive
readinessProbe -> /health/ready
startupProbe -> /health/ready

Je pense qu'il est nécessaire de vérifier /health/ready. Dans le doute, je pense qu'il vaut mieux faire comme ory et laisser le livenessProbe avec /health/alive et ajouter les 2 autres sondes vers /health/ready

Ce qui est fait dans le helm d'ory : ``` livenessProbe -> /health/alive readinessProbe -> /health/ready startupProbe -> /health/ready ``` Je pense qu'il est nécessaire de vérifier `/health/ready`. Dans le doute, je pense qu'il vaut mieux faire comme ory et laisser le livenessProbe avec `/health/alive` et ajouter les 2 autres sondes vers `/health/ready`
👍 1
lgourvenec added 1 commit 2025-03-07 11:52:50 +01:00
lgourvenec requested review from vfebvre 2025-03-07 11:57:36 +01:00
wpetit approved these changes 2025-03-07 11:59:44 +01:00
Dismissed
lgourvenec added 1 commit 2025-03-07 12:30:09 +01:00
lgourvenec commented 2025-03-07 12:37:07 +01:00
Author
Owner
Copy Link

Par rapport aux logs générés par les appels à ces 2 endpoints : il est vrai qu'hydra spam 4 lignes toutes les 10s par pod.
Mais d'un autre coté, ça montre que c'est vivant. Encore de l'autre coté, est-ce que ce n'est pas à kubernetes de gérer ça et finalement on pourrait se passer de ces logs même en cas de failure d'hydra ?

Dans le doute, j'ai poussé un commit pour ajouter une variable d'environnement qui désactive par défaut ces logs.

Par rapport aux logs générés par les appels à ces 2 endpoints : il est vrai qu'hydra spam 4 lignes toutes les 10s par pod. Mais d'un autre coté, ça montre que c'est vivant. Encore de l'autre coté, est-ce que ce n'est pas à kubernetes de gérer ça et finalement on pourrait se passer de ces logs même en cas de failure d'hydra ? Dans le doute, j'ai poussé un commit pour ajouter une variable d'environnement qui désactive par défaut ces logs.
wpetit approved these changes 2025-03-07 12:37:15 +01:00
pcaseiro approved these changes 2025-03-07 13:52:23 +01:00
pcaseiro merged commit a26b8aafe1 into unstable 2025-03-07 13:52:31 +01:00
pcaseiro deleted branch fix/hydra_liveness_probe_url 2025-03-07 13:52:32 +01:00
Sign in to join this conversation.
Reviewers
No Reviewers
vfebvre
wpetit
pcaseiro
Labels
Clear labels
bug
Something is not working
duplicate
This issue or pull request already exists
enhancement
New feature
help wanted
Need some help
invalid
Something is wrong
question
More information is needed
wontfix
This won't be fixed
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
4 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: CadolesKube/sso-kustom#63
No description provided.
Delete Branch "fix/hydra_liveness_probe_url"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?