feat: add support for dynamic KRaft quorum scaling - #1010
Conversation
Controllers now run a quorum-manager sidecar that admits itself into the KRaft voter set on startup (add-controller) and removes itself before termination (remove-controller via preStop), so controller role groups can be scaled up/down on a running cluster without a full rolling restart. controller.quorum.bootstrap.servers now points at each controller role group's headless Service DNS name instead of individual pod addresses, keeping container commands stable across replica changes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Controllers get a startupProbe (plain TCP, generous failure threshold for slow metadata-log replay on boot), a plain-TCP livenessProbe, and a readinessProbe that checks the node's Raft state via its metrics endpoint instead of a bare TCP check, so a controller stuck rejoining the quorum is correctly reported as not ready. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Controller pods now start/scale sequentially (OrderedReady) instead of in parallel, since the quorum-manager sidecar's admission flow assumes one voter joins at a time. Brokers are unaffected and keep Parallel. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Scaling controllers to 0 replicas while brokers keep running is now rejected at validation time with an actionable error, instead of failing much later and confusingly while building the broker's ConfigMap. Scaling controllers and brokers to 0 together (a coordinated whole-cluster stop) is still allowed and now actually builds, since downstream resource builders no longer assume a non-empty controller quorum whenever KRaft mode is active. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ns tests Enable the previously version-gated scale-up/down steps (Kafka 3.7 no longer needs special-casing), assert quorum voter counts via kafka-metadata-quorum.sh after each scale, and add a final step scaling both controllers and brokers to 0 to exercise the whole-cluster-stop path before namespace teardown. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Update the KRaft controller usage guide for scale-up/down support, record the design spec and implementation plan, add the CHANGELOG entries for this branch's changes, and ignore .worktrees/ for local worktree checkouts. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
The failed test was due to namespace deletion timeout |
| /// Only adding or removing a whole role group changes this list. | ||
| pub(crate) fn kraft_controllers(pod_descriptors: &[KafkaPodDescriptor]) -> Vec<String> { | ||
| pod_descriptors | ||
| let mut role_group_addresses: Vec<String> = pod_descriptors |
There was a problem hiding this comment.
Why not use a BTreeSet here? No dedup or sort required in the end?
| with `kafka-storage.sh format --standalone`, bootstrapping a single-node quorum by itself. | ||
| Every other controller formats with `--no-initial-controllers` and joins purely through the sidecar's `add-controller` call. | ||
| Brokers always format with `--no-initial-controllers` too; they are never voters. | ||
| the current replica count. |
There was a problem hiding this comment.
Seems lost!
| the current replica count. |
| The property `controller.quorum.bootstrap.servers` now contains the headless service names | ||
| of all controller role groups instead of individual peer host names. This prevevents the | ||
| restart controller from restarting all pods in the quorum when a new one is added/deleted. | ||
| The controller `StatefulSet` is now scaled using `OrderedBy` instead of the `Parallel` strategy |
There was a problem hiding this comment.
| The controller `StatefulSet` is now scaled using `OrderedBy` instead of the `Parallel` strategy | |
| The controller `StatefulSet` is now scaled using `OrderedReady` instead of the `Parallel` strategy |
| On termination, a new `preStop` hook on the controller container (`kafka`) removes the pod from | ||
| the voter list before shutdown. | ||
| The property `controller.quorum.bootstrap.servers` now contains the headless service names | ||
| of all controller role groups instead of individual peer host names. This prevevents the |
There was a problem hiding this comment.
| of all controller role groups instead of individual peer host names. This prevevents the | |
| of all controller role groups instead of individual peer host names. This prevents the |
| "-c".to_string(), | ||
| format!( | ||
| "timeout 2 bash -c 'cat < /dev/null > /dev/tcp/localhost/{client_port}' || exit 1\n\ | ||
| state=$(curl -s --max-time 2 localhost:{metrics_port}/metrics | grep -oE 'kafka_server_raft_metrics_current_state\\{{state=\"[a-z]+\"\\}}' | grep -oE '\"[a-z]+\"' | tr -d '\"')\n\ |
There was a problem hiding this comment.
The quorum-manager loop logs Could not determine local Raft state. Missing the trailing comma.
| state=$(curl -s --max-time 2 localhost:{metrics_port}/metrics | grep -oE 'kafka_server_raft_metrics_current_state\\{{state=\"[a-z]+\"\\}}' | grep -oE '\"[a-z]+\"' | tr -d '\"')\n\ | |
| state=$(curl -s --max-time 2 localhost:{metrics_port}/metrics | grep -oE 'kafka_server_raft_metrics_current_state\\{{state=\"[a-z]+\",?\\}}' | grep -oE '\"[a-z]+\"' | tr -d '\"')\n\ |
| && cat /tmp/{controller_properties_file} {admin_client_config} > {add_controller_config}; then | ||
| echo "Starting KRaft voter admission loop against bootstrap servers: $BOOTSTRAP_SERVERS" | ||
| while true; do | ||
| state=$(curl -s --max-time 5 --connect-timeout 2 localhost:{metrics_port}/metrics | grep -oE 'kafka_server_raft_metrics_current_state\{{state="[a-z]+"\}}' | grep -oE '"[a-z]+"' | tr -d '"') |
There was a problem hiding this comment.
| state=$(curl -s --max-time 5 --connect-timeout 2 localhost:{metrics_port}/metrics | grep -oE 'kafka_server_raft_metrics_current_state\{{state="[a-z]+"\}}' | grep -oE '"[a-z]+"' | tr -d '"') | |
| state=$(curl -s --max-time 5 --connect-timeout 2 localhost:{metrics_port}/metrics | grep -oE 'kafka_server_raft_metrics_current_state\{{state="[a-z]+",?\}}' | grep -oE '"[a-z]+"' | tr -d '"') |
| image: | ||
| {% if test_scenario['values']['kafka-kraft'].find(",") > 0 %} | ||
| custom: "{{ test_scenario['values']['kafka-kraft'].split(',')[1] }}" | ||
| productVersion: "{{ test_scenario['values']['kafka-kraft'].split(',')[0] }}" | ||
| {% else %} | ||
| productVersion: "{{ test_scenario['values']['kafka-kraft'] }}" | ||
| {% endif %} | ||
| pullPolicy: IfNotPresent | ||
| clusterConfig: | ||
| metadataManager: kraft | ||
| {% if lookup('env', 'VECTOR_AGGREGATOR') %} | ||
| vectorAggregatorConfigMapName: vector-aggregator-discovery | ||
| {% endif %} | ||
| brokers: | ||
| config: | ||
| logging: | ||
| enableVectorAgent: {{ lookup('env', 'VECTOR_AGGREGATOR') | length > 0 }} | ||
| roleGroups: | ||
| default: | ||
| replicas: 3 | ||
| controllers: | ||
| config: | ||
| logging: | ||
| enableVectorAgent: {{ lookup('env', 'VECTOR_AGGREGATOR') | length > 0 }} | ||
| roleGroups: | ||
| default: | ||
| replicas: 3 |
There was a problem hiding this comment.
Should not be required.
| image: | |
| {% if test_scenario['values']['kafka-kraft'].find(",") > 0 %} | |
| custom: "{{ test_scenario['values']['kafka-kraft'].split(',')[1] }}" | |
| productVersion: "{{ test_scenario['values']['kafka-kraft'].split(',')[0] }}" | |
| {% else %} | |
| productVersion: "{{ test_scenario['values']['kafka-kraft'] }}" | |
| {% endif %} | |
| pullPolicy: IfNotPresent | |
| clusterConfig: | |
| metadataManager: kraft | |
| {% if lookup('env', 'VECTOR_AGGREGATOR') %} | |
| vectorAggregatorConfigMapName: vector-aggregator-discovery | |
| {% endif %} | |
| brokers: | |
| config: | |
| logging: | |
| enableVectorAgent: {{ lookup('env', 'VECTOR_AGGREGATOR') | length > 0 }} | |
| roleGroups: | |
| default: | |
| replicas: 3 | |
| controllers: | |
| config: | |
| logging: | |
| enableVectorAgent: {{ lookup('env', 'VECTOR_AGGREGATOR') | length > 0 }} | |
| roleGroups: | |
| default: | |
| replicas: 3 |
| set -uo pipefail | ||
| {derive_pod_index} | ||
| [ -n "$POD_INDEX" ] || exit 0 | ||
| {export_replica_id} | ||
| {extract_bootstrap_servers} | ||
| DEADLINE=$((SECONDS + {deadline_seconds})) | ||
| finished=false | ||
| while [ "$SECONDS" -lt "$DEADLINE" ]; do | ||
| describe=$(timeout --kill-after={cli_kill_after} {cli_timeout} {binary} --bootstrap-controller "$BOOTSTRAP_SERVERS" --command-config {config} describe --replication 2>/dev/null) | ||
| if [ -n "$describe" ]; then | ||
| voters=$(echo "$describe" | tail -n +2 | awk '$NF == "Leader" || $NF == "Follower"') | ||
| total_voters=$(echo "$voters" | grep -c .) | ||
| if [ "$total_voters" -gt 0 ]; then | ||
| remaining_after_removal=$(( total_voters - 1 )) | ||
| if [ "$remaining_after_removal" -ge 1 ]; then | ||
| directory_id=$(echo "$voters" | awk -v id="$REPLICA_ID" '$1 == id {{ print $2 }}') | ||
| if [ -n "$directory_id" ]; then | ||
| echo "Removing self (node $REPLICA_ID, directory $directory_id) from the voter set..." | ||
| if timeout --kill-after={cli_kill_after} {cli_timeout} {binary} --bootstrap-controller "$BOOTSTRAP_SERVERS" --command-config {config} remove-controller \ | ||
| --controller-id "$REPLICA_ID" --controller-directory-id "$directory_id"; then | ||
| finished=true | ||
| else | ||
| echo "remove-controller attempt failed, will retry if time remains" | ||
| fi | ||
| else | ||
| echo "Could not find own node $REPLICA_ID among current voters (already removed?), nothing to do" | ||
| finished=true | ||
| fi | ||
| else | ||
| echo "Removing self would leave zero voters, skipping (this can't become safe later during my own termination -- nothing else will add a voter for me)" | ||
| finished=true | ||
| fi | ||
| [ "$finished" = true ] && break | ||
| else | ||
| echo "Could not identify any voters in the describe output (unrecognized format), skipping removal for safety and retrying..." | ||
| fi | ||
| fi |
There was a problem hiding this comment.
This has a level of complexity and nesting (6 levels) we should not use in code like this. It is not really tested as well. I would try to early exit (resolve the nesting), add a couple of comments or extract a script from this. I would use formatdoc! as well rather than rawstring.
Description
Fixes #1009
See CHANGELOG for a high level view of everything that changed.
✅ : OKD integration tests work. I ran them many times but there was always one or two tests that failed due to flakiness. I added some cleanup steps so that
kuttldoesn't timeout during namespace cleanups.Definition of Done Checklist
Author
Reviewer
Acceptance
type/deprecationlabel & add to the deprecation scheduletype/experimentallabel & add to the experimental features tracker