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

Livez/Readyz #16007

Open
logicalhan opened this issue Jun 5, 2023 · 24 comments
Open

Livez/Readyz #16007

logicalhan opened this issue Jun 5, 2023 · 24 comments
Labels
priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. type/feature
Milestone

Comments

@logicalhan
Copy link

What would you like to be added?

We currently have a single health endpoint for etcd /health which is used in Kubernetes distros as both liveness and readiness checking. In order to be fully api-compliant, we should have both a liveness check (i.e. /livez) which checks that this individual etcd member is "alive" and does not need to be restarted and a readiness check (i.e. /readyz) which signals that the etcd member is ready to accept traffic.

Why is this needed?

There is a difference between "please restart me I'm that unhealthy" vs "please send me all sorts of traffic, I'm ready for it".

@chaochn47
Copy link
Member

Please check this #13340 (comment)

@logicalhan
Copy link
Author

Please check this #13340 (comment)

Yeah I don't buy it. No one is going to dig up an obscure github issue in order to properly configure their etcd configurations for Kubernetes.

@chaochn47
Copy link
Member

Yeah that makes sense. We should rethink and document it properly, for example it applies only which etcd version, etc.

@logicalhan
Copy link
Author

Yeah that makes sense. We should rethink and document it properly, for example it applies only which etcd version, etc.

As long as you don't touch the existing health endpoint, it's completely backwards compatible and therefore can even be backported.

@logicalhan
Copy link
Author

for ref: #16008

@ahrtr
Copy link
Member

ahrtr commented Jun 5, 2023

Thanks @logicalhan for raising this request. I am supportive on it. /health/serializable=<true|false> isn't an explicit API, and it also requires people to understand what's serializable.

/livez and /readyz are more explicit and easy to understand & use.

/livez is similar to (or a syntax sugar ) /health/serializable=true; It just checks local etcd instance's health status, it should return true/healthy as long as local etcd instance is running & healthy. We shouldn't restart the etcd instance when the cluster isn't healthy (e.g. the quorum isn't satisfied) because it will make the situation even worse.

While /readyz should require quorum, and actually check the health of the cluster. Each etcd instance isn't ready to receive traffic until the cluster is healthy. It's similar to (or a syntax sugar of) /health/serializable=false or /health.

@ahrtr
Copy link
Member

ahrtr commented Jun 6, 2023

cc @neolit123

@neolit123
Copy link

+1

@serathius
Copy link
Member

Don't want to rush into adding livez/readyz probe. Main problem with existing health probe we just added it to have it without proper consideration.

I want livez to properly reflect fact that etcd needs restart, for example etcd is stuck on stalled storage https://docs.google.com/document/d/1U9hAcZQp3Y36q_JFiw2VBJXVAo2dK2a-8Rsbqv3GgDo/edit?usp=sharing.

Readyz should properly reflect fact that etcd is ready to serve traffic. Don't think alarms matter here. It's a degradation, however it doesn't mean we shouldn't serve reads.

TLDR; I would like to have a design written that will do a proper analysis etcd failure modes and propose matching probes to detect them. Example kubernetes-sigs/metrics-server#542

@wenjiaswe
Copy link
Contributor

Thanks for bringing this up @logicalhan.

I will continue work on this.

@ahrtr
Copy link
Member

ahrtr commented Jul 24, 2023

Link to #15440

@chaochn47
Copy link
Member

chaochn47 commented Sep 19, 2023

Reached out to @wenjiaswe for collaboration of the latest updated version of the design doc etcd livez and readyz. Updates resolve the comments / feedback mentioned in the issue and PoC #16008.

/cc @dims

@wenjiaswe
Copy link
Contributor

wenjiaswe commented Sep 19, 2023

Thank you @chaochn47, could you please use a google doc so we could comment? Thanks!

@wenjiaswe
Copy link
Contributor

cc @marukozh who is also working on this.

@chaochn47
Copy link
Member

chaochn47 commented Sep 19, 2023

Thank you @chaochn47, could you please use a google doc so we could comment? Thanks!

Done. Anyone in etcd-dev@googlegroups.com should have access to it etcd livez and readyz and can comment.

@ahrtr
Copy link
Member

ahrtr commented Sep 29, 2023

Various discussions are scattered in various places, so I raise my comment under this ticket.

liveness probe

A node is live when both below are satisfied:

  • It can serve serializable request. But note that if the node is in progress of defragmentation, it can't serve any requests; in such case, we should NOT consider the node as not live due to this rule. Link to gRPC health server sets serving status to NOT_SERVING on defrag #16278
  • The raft loop isn't blocked. There is one exception, for one-node cluster, there raft loop will be indeed blocked if there is no any client request. One possible way to resolve this is to intentionally trigger events periodically for the liveness check

readyness probe

Basically it shares the same logic as the existing health check (see below), and

  • A node should be considered to be ready even the alarm NOSPACE is activated.
  • Should we differentiate local member ready and the cluster ready?

func HandleHealth(lg *zap.Logger, mux *http.ServeMux, srv ServerHealth) {
mux.Handle(PathHealth, NewHealthHandler(lg, func(excludedAlarms AlarmSet, serializable bool) Health {
if h := checkAlarms(lg, srv, excludedAlarms); h.Health != "true" {
return h
}
if h := checkLeader(lg, srv, serializable); h.Health != "true" {
return h
}
return checkAPI(lg, srv, serializable)
}))
}

Compatiblity

Do not break the existing /health endpoint!

@serathius
Copy link
Member

serathius commented Sep 29, 2023

@siyuanfoundation
Copy link
Contributor

Created a k/k issue to track this kubernetes/kubernetes#120970

@siyuanfoundation
Copy link
Contributor

siyuanfoundation commented Oct 2, 2023

Tracking work

  • add livez/readyz endpoint with basic structure for checkers
  • add checker for defrag
  • add checker for readIndex
  • add checker for local file read
  • raise a issue and fix existing health probe not checking defrag
  • raise a issue and fix existing health probe does not respect context from http request

@scuzhanglei
Copy link

is there any plan to add a endpoint live command to etcdctl.
my situation is I run etcd in a docker compose container, docker compose's healthcheck command is running in the container ,but etcd's base image doesn't contain curl, so can't use curl localhost:2379/livez directly to check it, if there is a etcdctl endpoint live command would be useful.

@serathius
Copy link
Member

etcdctl uses GRPC only, we would need to make equivalent of /livez and /readyz in GRPC.

@tjungblu
Copy link
Contributor

@scuzhanglei I think we have that filed under #16276, just needs a cmdline in etcdctl

since we're bumping that thread again already, is there anything left to pick up? I've just "saved" one PR #16959 from @siyuanfoundation from being stale reaped, I think #16858 is also going to fall prey to the evil bot soon.

@siyuanfoundation
Copy link
Contributor

is there any plan to add a endpoint live command to etcdctl. my situation is I run etcd in a docker compose container, docker compose's healthcheck command is running in the container ,but etcd's base image doesn't contain curl, so can't use curl localhost:2379/livez directly to check it, if there is a etcdctl endpoint live command would be useful.

I have tried to add the commands before 293f087#diff-ab6fb0684315e16355f6ebe0f4b3cf860b9b2ff5a0fe1b4e4308a680b19f1b0c. Currently I don't have time to rebase it to the most recent implementation of livez/readyz.

Hope someone can pick it up.

@henrybear327
Copy link
Contributor

is there any plan to add a endpoint live command to etcdctl. my situation is I run etcd in a docker compose container, docker compose's healthcheck command is running in the container ,but etcd's base image doesn't contain curl, so can't use curl localhost:2379/livez directly to check it, if there is a etcdctl endpoint live command would be useful.

I have tried to add the commands before 293f087#diff-ab6fb0684315e16355f6ebe0f4b3cf860b9b2ff5a0fe1b4e4308a680b19f1b0c. Currently I don't have time to rebase it to the most recent implementation of livez/readyz.

Hope someone can pick it up.

Hey @siyuanfoundation, I will pick this issue up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. type/feature
Development

No branches or pull requests

10 participants