-
-
Notifications
You must be signed in to change notification settings - Fork 7
Support KRaft for consensus building #889
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a first round of reading. Have some comments/suggestions.
Testing a bit more now.
/// Supporting Kraft alongside Zookeeper requires a couple of CRD checks | ||
/// - If Kafka 4 and higher is used, no zookeeper config map ref has to be provided | ||
/// - Configuring the controller role means no zookeeper config map ref has to be provided | ||
pub fn check_kraft_vs_zookeeper(&self, product_version: &str) -> Result<(), Error> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of this implicit decision a dedicated field could be used that explicitly puts the cluster in Zookeeper or Kraft mode.
It is possible to create a Kafka cluster in Kraft mode without any spec.controllers
property.
Being explicit about the metadata manager/handler will be needed later to support migrations when both Zookeeper and controllers must be running at the same time.
My suggestion for the field would be kraft_mode: [OFF|COMBINED|MIGRATING|ON]
This PR can ignore COMBINED
and MIGRATING
.
tests/test-definition.yaml
Outdated
@@ -62,17 +62,21 @@ dimensions: | |||
- "cluster-internal" | |||
- "external-stable" | |||
- "external-unstable" | |||
- name: use-kraft-controller |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed on Slack, this is too long. Maybe rename to just kraft
similarly to openshift
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now using zookeeper
and zookeeper-latest
dimension for kraft with value "false".
Co-authored-by: Razvan-Daniel Mihai <[email protected]>
Description
This PR add a new role
controller
to configure KRaft instead of ZooKeeper. This is prep work for Kafka 4.0, where ZooKeeper is removed.fixes #690
BREAKING Changes:
server.properties
tobroker.properties
andcontroller.properties
respectively (this affects e.g. config overrides)What was done?
AnyConfig
to better handle different config typesnode.id
cluster-id
is set to KafkaClustermetadata.uid
use-kraft-controller
dimension in almost all tests to split between Kraft / zkWhat was not done?
TODO
Definition of Done Checklist
Author
Reviewer
Acceptance
type/deprecation
label & add to the deprecation scheduletype/experimental
label & add to the experimental features tracker