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.

Sign up for GitHub

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

Jump to bottom

*: add /health endpoint to grpc-proxy #8322

Merged
merged 3 commits into from
Jul 31, 2017
Merged

*: add /health endpoint to grpc-proxy #8322

merged 3 commits into from
Jul 31, 2017

Conversation

gyuho
Copy link
Contributor

@gyuho gyuho commented Jul 27, 2017

No description provided.

@gyuho gyuho force-pushed the health-grpc-proxy branch 3 times, most recently from 50ccd8d to 8a02e1f Compare July 28, 2017 16:34
@gyuho gyuho requested a review from heyitsanthony July 28, 2017 16:36
@gyuho gyuho force-pushed the health-grpc-proxy branch 2 times, most recently from 393b236 to c9a4ec9 Compare July 28, 2017 21:58
@heyitsanthony
Copy link
Contributor

can grpcproxy just have its own health.go that has implements a /health http handler using the v3 client instead? a separate package + special interface + adding a client API + having the client API know about etcdserver JSON seems like overkill for this

@gyuho gyuho force-pushed the health-grpc-proxy branch 2 times, most recently from 5d3713a to 39c8ff0 Compare July 28, 2017 22:35
@gyuho
Copy link
Contributor Author

gyuho commented Jul 28, 2017

@heyitsanthony PTAL.

Instead exposed Health with NewHealthHandler.

heyitsanthony
heyitsanthony approved these changes Jul 31, 2017
View reviewed changes
Copy link
Contributor

@heyitsanthony heyitsanthony left a comment

Choose a reason for hiding this comment

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

lgtm after fixing the permission denied case. Thanks!

proxy/grpcproxy/health.go Outdated
ctx, cancel := context.WithTimeout(c.Ctx(), time.Second)
_, err := c.Get(ctx, "a")
cancel()
h.Health = err == nil
Copy link
Contributor

Choose a reason for hiding this comment

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

err == nil || err == rpctypes.ErrPermissionDenied in case a is locked down

@gyuho
*: expose etcdhttp.Health, define proxy health handler
cd37ef2 
Signed-off-by: Gyu-Ho Lee <gyuhox@gmail.com>
@gyuho
etcdmain: add '/health' endpoint to grpc-proxy
b8fd5c3 
Signed-off-by: Gyu-Ho Lee <gyuhox@gmail.com>
@gyuho
e2e: test /metrics, /health endpoint in grpc-proxy
661da1e 
Signed-off-by: Gyu-Ho Lee <gyuhox@gmail.com>
@gyuho gyuho merged commit e0843c6 into etcd-io:master Jul 31, 2017
@gyuho gyuho deleted the health-grpc-proxy branch July 31, 2017 22:45
@gyuho gyuho mentioned this pull request Jul 12, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@heyitsanthony heyitsanthony heyitsanthony approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@gyuho @heyitsanthony