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

query: Add readiness probe to query #1534

Merged
merged 2 commits into from
Sep 18, 2019
Merged

query: Add readiness probe to query #1534

merged 2 commits into from
Sep 18, 2019

Conversation

kakkoyun
Copy link
Member

@kakkoyun kakkoyun commented Sep 17, 2019 •
edited
Loading

This PR,

  • Adds /-/ready endpoint for readiness checks.

Changes

  • Adds /-/ready endpoint for readiness checks.
  • Uses prober.Prober for readiness and liveness endpoints.
  • Changes defaultHTTPListener scheduleHTTPServer to consume an existing http.Handler

Verification

  1. make test

  2. Started thanos query and made a request to related endpoints.

➜ curl http://0.0.0.0:10902/-/healthy
thanos query is healthy
➜ curl http://0.0.0.0:10902/-/ready
thanos query is not ready. Reason: thanos query is initializing
➜ curl http://0.0.0.0:10902/-/ready
thanos query is ready

@kakkoyun kakkoyun changed the title query: Use prober in query query: Add readiness probe to query Sep 17, 2019
@kakkoyun
Add prober for querier
4ed4f09 
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
@kakkoyun
Copy link
Member Author

cc @FUSAKLA @metalmatze @bwplotka

@kakkoyun kakkoyun mentioned this pull request Sep 17, 2019
Merged
1 task
bwplotka
bwplotka approved these changes Sep 17, 2019
View reviewed changes
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Nice, some suggestions around sprintf otherwise LGTM! 👍

cmd/thanos/main.go Show resolved Hide resolved
cmd/thanos/main.go Outdated Show resolved Hide resolved
cmd/thanos/main.go Outdated Show resolved Hide resolved
cmd/thanos/main.go Outdated Show resolved Hide resolved
cmd/thanos/main.go Outdated Show resolved Hide resolved
cmd/thanos/query.go Show resolved Hide resolved
@kakkoyun kakkoyun marked this pull request as ready for review September 17, 2019 17:47
GiedriusS
GiedriusS reviewed Sep 18, 2019
View reviewed changes
cmd/thanos/main.go Outdated Show resolved Hide resolved
@kakkoyun
Fix minor issues
d44ffa2 
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
@kakkoyun
Copy link
Member Author

@GiedriusS @bwplotka This is ready for another round. If this looks good I'll send similar PR's for other components today.

bwplotka
bwplotka approved these changes Sep 18, 2019
View reviewed changes
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

LGTM

@bwplotka bwplotka merged commit a07e91e into thanos-io:master Sep 18, 2019
@kakkoyun kakkoyun mentioned this pull request Sep 19, 2019
Merged
@FUSAKLA FUSAKLA mentioned this pull request Sep 23, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GiedriusS GiedriusS GiedriusS left review comments

@bwplotka bwplotka bwplotka approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kakkoyun @GiedriusS @bwplotka