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

Support gRPC healthcheck #8583

Merged
merged 10 commits into from Sep 20, 2022
Merged

Support gRPC healthcheck #8583

merged 10 commits into from Sep 20, 2022

Conversation

jjacque
Copy link
Contributor

@jjacque jjacque commented Nov 15, 2021

What does this PR do?

Add gRPC support to healthcheck (using "grpc" scheme in the backend config option). It's based on GRPC Health Checking Protocol v1

Motivation

We need to deploy gRPC apps that will not be immediately ready to serve data (populating caches, ...). This will allow Traefik to not route request to a "unhealthy" container.

See also #6027

More

  • updated healthcheck tests: added TestSetGrpcBackendsConfiguration
  • updated routing/services documentation to reflect the changes (usage of the dedicated mode option in healthcheck)

Additional Notes

This is probably a naive implementation, any help / comment is welcome !
PS: most changes are related to tests (192+ / 2-)

@jjacque
Copy link
Contributor Author

jjacque commented Dec 5, 2021

Hello, just curious about when a review is planned for this PR ?
If needed, i'm ready to work on comments / improvements required to get this merged.
Thanks in advance !!

@kevinpollet
Copy link
Member

Hello @jjacque and thanks for your contribution,

As you can see in our contribution guide, needs-design-review means that we have to take a closer look at the scope of impact for that PR to see, how it would interact with the other parts of Traefik.

We will come back to you soon when we did the first design review iteration.

@jjacque
Copy link
Contributor Author

jjacque commented Dec 6, 2021

Thanks a lot @kevinpollet,
didn't mean to disturb !

@longquan0104
Copy link
Contributor

@kevinpollet I would like to know if supporting health checks for gRPC will be implemented in the future. Thanks 👯

Copy link
Member

@juliens juliens left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution.

docs/content/routing/services/index.md Outdated Show resolved Hide resolved
pkg/healthcheck/healthcheck.go Outdated Show resolved Hide resolved
@jjacque
Copy link
Contributor Author

jjacque commented Sep 16, 2022

Hi @juliens , thanks for your review, i've made the changes you suggested and rebased the branch.

@ldez ldez force-pushed the grpc/check branch 3 times, most recently from 18563a3 to c739847 Compare September 19, 2022 08:03
Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

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

LGTM

@ldez ldez added this to the next milestone Sep 19, 2022
Copy link
Member

@juliens juliens left a comment

Choose a reason for hiding this comment

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

LGTM

@ldez ldez force-pushed the grpc/check branch 4 times, most recently from 11afd2a to 766589a Compare September 19, 2022 08:24
@ldez ldez force-pushed the grpc/check branch 4 times, most recently from e4a8ac6 to b7973d6 Compare September 19, 2022 12:18
Copy link
Member

@kevinpollet kevinpollet left a comment

Choose a reason for hiding this comment

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

Thanks 👍

@kevinpollet kevinpollet changed the title feat: add gRPC support to healthcheck Add gRPC support to healthcheck Sep 20, 2022
@kevinpollet kevinpollet changed the title Add gRPC support to healthcheck Support gRPC healthcheck Sep 20, 2022
@traefiker traefiker merged commit 033fccc into traefik:master Sep 20, 2022
v2 automation moved this from To review to Done Sep 20, 2022
@jjacque
Copy link
Contributor Author

jjacque commented Sep 20, 2022

Thanks !!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
v2
Done
Development

Successfully merging this pull request may close these issues.

None yet

6 participants