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

kubernetes healthcheck #98

Closed
chabmed opened this issue Nov 5, 2018 · 9 comments
Closed

kubernetes healthcheck #98

chabmed opened this issue Nov 5, 2018 · 9 comments
Labels
enhancement Auto-generates notes
Milestone

Comments

@chabmed
Copy link

chabmed commented Nov 5, 2018

Hi,
I'm using the starter and i'm seeing that it actually exposes the grpc.health.v1.Health.Check. So in this case should we implement in our own a general server status. Because for now i can check only when i specify the name of the service in healthcheck, but when i use an empty service name it returns an error of unknown service,
"The server should use an empty string as the key for server's overall health status, so that a client not interested in a specific service can query the server's status with an empty request." https://github.com/grpc/grpc/blob/v1.15.0/doc/health-checking.md,
i'm actually deploying in kubernetes and i'm trying to use this grpc-health-probe
https://github.com/grpc-ecosystem/grpc-health-probe

Does it mean that i have to implement my own implementation, i'm not able to find the right answer for that.

Thanks

@jvmlet
Copy link
Collaborator

jvmlet commented Nov 8, 2018

Hi @chabmed
Have you tried to enable the reflection as described here , get the names of all services and query the health one by one ? There is also unit test DemoAppTest::testReflection to help you to get the idea.

@chabmed
Copy link
Author

chabmed commented Nov 8, 2018

Hi, thanks for the hit, but that' not really what i'm looking for. I'm not sure but i think we are missing something here, or maybe we should implement it. Because it says "https://github.com/grpc/grpc/blob/master/doc/health-checking.md"
"The server should register all the services manually and set the individual status, including an empty service name and its status."
The starter is already registering all the service manually with the status, but i think we are missing the empty service, to return the general health of the server,

@jvmlet
Copy link
Collaborator

jvmlet commented Nov 8, 2018

Do you have the idea what they mean by 'general server health'? Have you seen any examples?

@chabmed
Copy link
Author

chabmed commented Nov 8, 2018

according to the code directly it's suppose to check all the grpc service, but it's still experimental, I also try to look into there code i didn't find any specific implementation of that so i'm wondering if the server had to implement or not.
`@io.grpc.ExperimentalApi("grpc/grpc-java#4696")
public final class HealthStatusManager {
/**

  • The special "service name" that represent all services on a GRPC server. It is an empty
  • string.
    */`

@ST-DDT
Copy link

ST-DDT commented Nov 12, 2018

The main issue here is that grpc-java's HealthService does not implement the "empty string for all services" feature. So there isn't much this library can do about it. (Unless you always also set the health state for "" yourself.)

See also: HealthServiceImpl#check(...)

Maybe you should open an issue in the grpc-java repo for this.

@chabmed
Copy link
Author

chabmed commented Nov 15, 2018

Hi thanks, for the answer. I asked the question here:
https://groups.google.com/forum/#!msg/grpc-io/mfEc57yGLr0/mSjyz8p7BgAJ

To use health-checking service, the application should create a HealthStatusManager and register getHealthService() to the server. The application can then set the serving status on the HealthStatusManager whenever desired. Because the serving status is always associated with a service, designated by a string, we define the empty string as the special service name to indicate the "whole server". This is a convention meant to communicate the health status of a whole server between the client and the server. The SERVICE_NAME_ALL_SERVICES constant is for the application server to use to set it, and for the application client to query it through RPC.

@balchua
Copy link

balchua commented Jan 15, 2019

I've implemented this in our micro service. We also added into our base image grpc_health_probe https://github.com/grpc-ecosystem/grpc-health-probe binary. And then we use the kubernetes exec in livenessProbe and readinessProbe.

But yes you need to implement the Check method.

@jvmlet jvmlet closed this as completed in 93c37e5 Nov 4, 2021
@jvmlet jvmlet added this to the 4.5.10 milestone Nov 4, 2021
@jvmlet jvmlet added the enhancement Auto-generates notes label Nov 4, 2021
@jvmlet
Copy link
Collaborator

jvmlet commented Nov 4, 2021

FYI, Supported via custom ManagedHealthStatusService.
The demo test that runs slyncio/grpc-health-probe docker is here ( uses grpc reflection to check each service separately)

@jvmlet
Copy link
Collaborator

jvmlet commented Nov 9, 2021

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Auto-generates notes
Projects
None yet
Development

No branches or pull requests

4 participants