Add proposal for controller readiness probe based on KRaft raft state - #225
Add proposal for controller readiness probe based on KRaft raft state#225mly-zju wants to merge 1 commit into
Conversation
Adds a new /v1/controller-ready endpoint to the Kafka Agent that reports whether a KRaft controller is attached to the metadata quorum, by reading the current-state attribute of the kafka.server:type=raft-metrics JMX MBean. The controller-only branch of kafka_readiness.sh is updated to call this endpoint so the existing readiness probe reflects raft attachment state rather than only whether the controller listener port is bound. Motivated by strimzi-kafka-operator#12760: a real-world cold-boot CoreDNS race that wedged a controller for 8 hours despite the pod remaining 1/1 Ready under the current netstat-based probe. A reference implementation is at strimzi-kafka-operator#12768 (closed pending this proposal). Signed-off-by: lingyangma <lingyang.ma@enterprisedb.com>
scholzj
left a comment
There was a problem hiding this comment.
Thanks for the proposal. I left some comments.
Out of curiosity, are you actually using this check in your clusters? Or how did you come up with it? Any references to Kafka documentation with regards to this?
Note for future proposals: If you write them with sentence per line, it would be much easier to comment on them as it would be possible to comment on individual sentences and not on whole paragraphs.
|
|
||
| This succeeds the moment the listener binds, which happens before `ControllerRegistrationManager` registers the node with the rest of the quorum and well before the local raft state machine has converged to `leader` or `follower`. | ||
|
|
||
| A controller that wedges during startup — for example because its `NodeToController` channel manager times out while peer FQDNs are temporarily unresolvable (the cold-boot CoreDNS race observed in [issue #12760](https://github.com/strimzi/strimzi-kafka-operator/issues/12760)) — therefore stays at `1/1 Ready`. The kubelet never restarts it, the cluster operator's `KafkaRoller` reconciles indefinitely with `"An error while trying to determine the active controller"` because `describeMetadataQuorum` can't complete, and recovery requires a manual roll of each controller one-at-a-time with the leader last. |
There was a problem hiding this comment.
Have you raised this in Kafka? While I agree that the health checks should be improved in Strimzi, this seems like a Kafka issue int he first place if the DNS recovers but the node remains stuck. It should either exit or recover.
There was a problem hiding this comment.
I agree with Jakub. As already discussed in the thread, I think there could be something not working properly in Kafka which doesn't resolve DNS names anymore after some time.
|
|
||
| A controller that wedges during startup — for example because its `NodeToController` channel manager times out while peer FQDNs are temporarily unresolvable (the cold-boot CoreDNS race observed in [issue #12760](https://github.com/strimzi/strimzi-kafka-operator/issues/12760)) — therefore stays at `1/1 Ready`. The kubelet never restarts it, the cluster operator's `KafkaRoller` reconciles indefinitely with `"An error while trying to determine the active controller"` because `describeMetadataQuorum` can't complete, and recovery requires a manual roll of each controller one-at-a-time with the leader last. | ||
|
|
||
| The Strimzi Cluster Operator already consumes JMX raft metrics elsewhere: the KRaft Grafana dashboards shipped under `packaging/examples/metrics/strimzi-metrics-reporter/grafana-dashboards/strimzi-kraft.json` graph `kafka_server_raft_metrics_current_state_info`, `kafka_server_raft_metrics_current_leader`, `kafka_server_raft_metrics_high_watermark`, etc. The data the probe needs to do better is already exposed and stable; it just isn't read by the agent. |
There was a problem hiding this comment.
I do not think this is comparable as the metrics in Grafana dashboards are just for information only. So I do not think this has any real meaning.
There was a problem hiding this comment.
I would just delete this sentence.
There was a problem hiding this comment.
Yes, agree this should be removed.
|
|
||
| ## Motivation | ||
|
|
||
| A reliable controller readiness signal: |
There was a problem hiding this comment.
How does readiness help with this? If the Pod reports not ready, that will on its own not help much with anything.
|
|
||
| Add a new endpoint to the Kafka Agent: | ||
|
|
||
| * `GET http://localhost:8080/v1/controller-ready` |
There was a problem hiding this comment.
Why do we need a new endpoint rather then use the existing? How would this fit mixed nodes?
| * **204 No Content** when the value of the `current-state` attribute on the `kafka.server:type=raft-metrics` JMX MBean is one of `{leader, follower}`. The local node is attached to the metadata quorum. | ||
| * **503 Service Unavailable** for any other `current-state` value (`unattached`, `candidate`, `prospective`, `voted`, `resigned`, `observer`). The response body includes the actual state name as a JSON object for diagnostics, e.g. `{"error":"controller not ready, current raft state: prospective"}`. | ||
| * **404 Not Found** when the `kafka.server:type=raft-metrics` MBean is not registered on the local JVM. This is expected on broker-only nodes (no controller process), and possible during the early-startup window before the raft layer initializes. Returning 404 (rather than 503) lets the calling script preserve the existing port-listening behavior in those cases. |
There was a problem hiding this comment.
Does this mirror the existing endpoints? Worth mentioning in the proposal.
| * **No new probe timing changes** — kubelet's default `initialDelaySeconds=15, periodSeconds=10, failureThreshold=3` is sufficient for the steady-state contract. Cluster operators that wish to tune may already do so via `spec.kafka.readinessProbe`. | ||
| * **Forward-compatibility with newer Kafka raft states.** Any value emitted by Kafka other than `leader`/`follower` is treated as not-ready, with the literal state name surfaced in the 503 body. If a future Kafka version adds, say, `bootstrap` as an additional valid steady state, that requires a follow-up here to extend `RAFT_READY_STATES`. The safe direction (default-not-ready) preserves correctness in the meantime. | ||
|
|
||
| ## Open question — `ControllerRegistrationManager`-layer wedges |
There was a problem hiding this comment.
Sorry, but I do not think it is really clear what the open question is here..
ppatierno
left a comment
There was a problem hiding this comment.
Thanks for the proposal, I left some comments. At the same time I was wondering, as already mentioned by Jakub, if this should be raised to the Kafka community because it seems something is not working properly on that side. Then we can, of course, thinking of improving our readiness check.
| netstat -lnt | grep -Eq 'tcp6?[[:space:]]+[0-9]+[[:space:]]+[0-9]+[[:space:]]+[^ ]+:9090.*LISTEN[[:space:]]*' | ||
| ``` | ||
|
|
||
| This succeeds the moment the listener binds, which happens before `ControllerRegistrationManager` registers the node with the rest of the quorum and well before the local raft state machine has converged to `leader` or `follower`. |
There was a problem hiding this comment.
I guess you are referring to some Kafka internals. Can you point to some documentation or source code related to this?
|
|
||
| This succeeds the moment the listener binds, which happens before `ControllerRegistrationManager` registers the node with the rest of the quorum and well before the local raft state machine has converged to `leader` or `follower`. | ||
|
|
||
| A controller that wedges during startup — for example because its `NodeToController` channel manager times out while peer FQDNs are temporarily unresolvable (the cold-boot CoreDNS race observed in [issue #12760](https://github.com/strimzi/strimzi-kafka-operator/issues/12760)) — therefore stays at `1/1 Ready`. The kubelet never restarts it, the cluster operator's `KafkaRoller` reconciles indefinitely with `"An error while trying to determine the active controller"` because `describeMetadataQuorum` can't complete, and recovery requires a manual roll of each controller one-at-a-time with the leader last. |
There was a problem hiding this comment.
It's not clear what NodeToController is within this proposal. Something within Kafka I assume.
|
|
||
| This succeeds the moment the listener binds, which happens before `ControllerRegistrationManager` registers the node with the rest of the quorum and well before the local raft state machine has converged to `leader` or `follower`. | ||
|
|
||
| A controller that wedges during startup — for example because its `NodeToController` channel manager times out while peer FQDNs are temporarily unresolvable (the cold-boot CoreDNS race observed in [issue #12760](https://github.com/strimzi/strimzi-kafka-operator/issues/12760)) — therefore stays at `1/1 Ready`. The kubelet never restarts it, the cluster operator's `KafkaRoller` reconciles indefinitely with `"An error while trying to determine the active controller"` because `describeMetadataQuorum` can't complete, and recovery requires a manual roll of each controller one-at-a-time with the leader last. |
There was a problem hiding this comment.
I agree with Jakub. As already discussed in the thread, I think there could be something not working properly in Kafka which doesn't resolve DNS names anymore after some time.
|
|
||
| A controller that wedges during startup — for example because its `NodeToController` channel manager times out while peer FQDNs are temporarily unresolvable (the cold-boot CoreDNS race observed in [issue #12760](https://github.com/strimzi/strimzi-kafka-operator/issues/12760)) — therefore stays at `1/1 Ready`. The kubelet never restarts it, the cluster operator's `KafkaRoller` reconciles indefinitely with `"An error while trying to determine the active controller"` because `describeMetadataQuorum` can't complete, and recovery requires a manual roll of each controller one-at-a-time with the leader last. | ||
|
|
||
| The Strimzi Cluster Operator already consumes JMX raft metrics elsewhere: the KRaft Grafana dashboards shipped under `packaging/examples/metrics/strimzi-metrics-reporter/grafana-dashboards/strimzi-kraft.json` graph `kafka_server_raft_metrics_current_state_info`, `kafka_server_raft_metrics_current_leader`, `kafka_server_raft_metrics_high_watermark`, etc. The data the probe needs to do better is already exposed and stable; it just isn't read by the agent. |
There was a problem hiding this comment.
I would just delete this sentence.
|
|
||
| Add a new endpoint to the Kafka Agent: | ||
|
|
||
| * `GET http://localhost:8080/v1/controller-ready` |
There was a problem hiding this comment.
I was wondering if instead of having a dedicated endpoint indicating if controller is ready or not we could return the current state of the node, whatever it is broker or controller. Maybe it could be useful in the future. So as per Kafka documentation the kafka.server:type=raft-metrics would be:
The current state of this member; possible values are leader, candidate, voted, follower, unattached, observer.
As stated here https://kafka.apache.org/43/operations/monitoring/#kraft-quorum-monitoring-metrics
| * **503 Service Unavailable** for any other `current-state` value (`unattached`, `candidate`, `prospective`, `voted`, `resigned`, `observer`). The response body includes the actual state name as a JSON object for diagnostics, e.g. `{"error":"controller not ready, current raft state: prospective"}`. | ||
| * **404 Not Found** when the `kafka.server:type=raft-metrics` MBean is not registered on the local JVM. This is expected on broker-only nodes (no controller process), and possible during the early-startup window before the raft layer initializes. Returning 404 (rather than 503) lets the calling script preserve the existing port-listening behavior in those cases. | ||
|
|
||
| The endpoint listens on the existing localhost HTTP port (`8080`) the Kafka Agent already uses for `/v1/ready` and `/v1/broker-state`. No new ports, no new RBAC, no new authentication surface — the MBean is read on the local JVM via `ManagementFactory.getPlatformMBeanServer()`. |
There was a problem hiding this comment.
Referring to my latest comment maybe we could have a /v1/raft-state endpoint.
|
Hi @mly-zju, thanks for the proposal. Before I dive into the review, I would like to kindly ask you: have you tested these changes on your PoC [1] against the scenario described in [2]? Did the new endpoint actually detect the problem in your testing? [1] - strimzi/strimzi-kafka-operator#12768 |
There was a problem hiding this comment.
Thanks for the proposal. I've left a few comments. I think 2 out of the 3 motivations for this proposal could be resolved by an improvement in the KafkaRoller. So the remaining motivation is to align the readiness check with the broker.
In the original proposal for KRaft readiness has intended this behaviour:
A controller is:
"alive" if it has a process running.
"ready" when it is ready to join the controller quorum.
not when it has joined the controller quorum. So this proposal would fundamentally change the behaviour by introducing an additional requirement for a controller to be ready which has an impact on KafkaRoller. I think this proposal needs to carefully consider why we went with this approach in the first place and what kind of impact this behaviour change can introduce. There were edge cases that had to be considered during the original proposal, such as how to recover quorum quickly in situations such as controller nodes are misconfigured or in Pending state etc., to avoid scenarios where none of the controllers can join the quorum. The proposal is missing the potential impacts of this change and justifications for them.
|
|
||
| A reliable controller readiness signal: | ||
|
|
||
| * Closes the silent-wedge failure mode in [strimzi-kafka-operator#12760](https://github.com/strimzi/strimzi-kafka-operator/issues/12760), so a controller that can't attach to the quorum is detected by the kubelet and restarted within `initialDelaySeconds + failureThreshold * periodSeconds` — by which point a transient cause (DNS propagation, network blip, restart of a peer) has typically resolved itself. |
There was a problem hiding this comment.
I agree that this should be investigated in Kafka but I think there is an improvement we can make to the KafkaRoller to fix this issue. I described it in this comment in the issue.
| A reliable controller readiness signal: | ||
|
|
||
| * Closes the silent-wedge failure mode in [strimzi-kafka-operator#12760](https://github.com/strimzi/strimzi-kafka-operator/issues/12760), so a controller that can't attach to the quorum is detected by the kubelet and restarted within `initialDelaySeconds + failureThreshold * periodSeconds` — by which point a transient cause (DNS propagation, network blip, restart of a peer) has typically resolved itself. | ||
| * Catches a broader class of unattached-controller failures than the original incident, including misconfigured TLS, peer partition, future Kafka regressions, and any other path that prevents the local raft layer from staying in `leader`/`follower`. The cost of the existing probe is paid not only on the specific incident filed. |
There was a problem hiding this comment.
Again if we improve KafkaRoller to catch non functioning quorum correctly and restart controller nodes when needed, we can resolve some of these issues.
| Add a new endpoint to the Kafka Agent: | ||
|
|
||
| * `GET http://localhost:8080/v1/controller-ready` | ||
| * **204 No Content** when the value of the `current-state` attribute on the `kafka.server:type=raft-metrics` JMX MBean is one of `{leader, follower}`. The local node is attached to the metadata quorum. |
There was a problem hiding this comment.
Controller can join the quorum as an observer. So do we mark it unready in that case? (I know dynamic quorum is not supported with Strimzi but still worth considering).
|
|
||
| The proposal as written catches the broad class of failures where the local raft layer is not attached to the quorum. The on-disk evidence captured during the [strimzi-kafka-operator#12760](https://github.com/strimzi/strimzi-kafka-operator/issues/12760) incident — `quorum-state` with `leaderId: 3, leaderEpoch: 1` persistent for 8 hours — suggests raft *did* elect a leader during that wedge, and the actual failure was one layer above (in `ControllerRegistrationManager`). If that interpretation is correct, `current-state` would have been `leader` on the active controller and `follower` on the others during the wedge, and this proposal's check would have returned 204. | ||
|
|
||
| A second check covering this layer is possible — for example, "if `current-state` is `leader`/`follower` but the metadata-apply offset (`broker-metadata-metrics.last-applied-record-offset` on the broker side, or the equivalent controller-side metric) hasn't advanced in the last N seconds, return 503". That introduces stateful checking (compare-to-previous-probe) and a tuning parameter (N), and is best discussed as a follow-up to this proposal once the basic JMX-state probe lands. |
There was a problem hiding this comment.
are you suggesting to mark a controller unready based on broker metrics from brokers? I'm not sure if that makes sense.
|
Discussed on community call on 23.7.26: It seems like there isn't agreement that this proposal is a good idea and we haven't heard back from the contributor in a month so closing this proposal. @mly-zju if you have time to get back to this and want to continue discussions then feel free to reopen this, or perhaps continue the discussion on the issue to better scope the issue. |
Opening per maintainer request on strimzi-kafka-operator#12768.
This proposal adds a new
/v1/controller-readyendpoint to the Kafka Agent that reports whether a KRaft controller is attached to the metadata quorum, by reading thecurrent-stateattribute of thekafka.server:type=raft-metricsJMX MBean. The controller-only branch ofkafka_readiness.shis updated to call this endpoint so the existing readiness probe reflects raft attachment state rather than only whether the controller listener port is bound.Motivated by the cold-boot CoreDNS race documented in strimzi-kafka-operator#12760: a real production incident where a controller pod stayed
1/1 Readyfor 8 hours despite the metadata quorum being wedged, because the existingnetstat-based probe could not distinguish "listener bound" from "actually serving the quorum."A reference implementation that includes the agent endpoint, the script update, a unit-test matrix covering every raft state observed in Kafka 4.x, and end-to-end validation on both kind and AKS (including a
hostAliases-induced wedge that capturedHTTP 503 {"error":"controller not ready, current raft state: prospective"}from the new endpoint) is at strimzi-kafka-operator#12768 — closed pending acceptance of this proposal.I deliberately did not pre-assign a proposal number to the filename (matching the recent convention from PRs #220, #216, #211, #204, #203, #191, #159) or update the README index, since the number is typically finalized at merge. Happy to add both in a follow-up commit once a number is assigned.
One section worth particular attention is the Open question at the bottom: the proposal as written catches the broad class of "raft is not attached" failures, but the original #12760 incident's on-disk
quorum-state(leaderId: 3, leaderEpoch: 1persistent for 8 h) suggests raft did elect a leader and the wedge was one layer above, inControllerRegistrationManager. A second check covering that layer (e.g.last-applied-record-offsetadvance) is possible but introduces stateful checking and a tuning parameter — I'd appreciate steer on whether to include it here or treat as a follow-up.