-
-
Notifications
You must be signed in to change notification settings - Fork 340
feat(Kafka): Add KRaft support #1353
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
feat(Kafka): Add KRaft support #1353
Conversation
… support. Add support for an official apache/kafka image.
✅ Deploy Preview for testcontainers-dotnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Since last week a new major version of the confluentinc/cp-kafka image is available : 8.0. It uses KRaft as the DEFAULT and ONLY metadata management system, following Kafka 4.0 : https://docs.confluent.io/platform/current/release-notes/index.html#ccs-ak I wonder if we should try to validate the compatibility within this PR between the version of the image and the consensus support, if we should try to only support KRaft, or if we should keep it open and let the container fail at runtime while running the startup scripts. With the latest TestContainer package we were on the latest tag of the cp-kafka image and ran into this exact error once the new major was release. We have fixed the image version to 7.9.* but we are blocked from moving to the new major. |
|
I'm a bit behind on some PRs. I hope to review them after my vacation. I haven't forgotten. They were just delayed by some important fixes we had to do. @eddumelendez IIRC you know Kafka pretty well, right? Maybe you could help out here? |
|
Any news on this PR ? I might be able to take some time if the PR need some refinement or if we want to go towards a specific direction but for that we need to be clear on what we want probably. I will try to get the version from the PR to test on our setup to see if the PR solves the issue and give you some feedback, I assume there are no pre-release version currently up for this proposition ? |
|
@PierreBougon I'll try to review and test it by the end of this week. Have you looked at it? Would it make sense to extract the part that ensures compatibility with version 8 and above into a separate, smaller PR? That might be easier to review and then review KRaft support? |
KRaft is mandatory for confluent kafka images starting from 8.0. Maybe better way is to make separate brand-new KafkaV8Builder or KafkaKraftBuilder or something like this ? |
|
Hi there, I'm very interested by what's happening here 👀 |
…y/feature/kafka-add-kraft-support
|
I started today taking a look at this PR. |
|
Finally, I had some time to fully review the PR. I made a few changes to align it with the repo standards, reduced some if-else blocks, and added documentation. Please take a look at the changes and let me know what you think. I'll check the tests next, and then we can merge. The PR already looked really good. I'm looking forward to merging the changes and publishing a new release that includes them. Thanks 🙏. |
HofmeisterAn
left a comment
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.
If you're good with the changes, I'm happy to merge the PR. Thanks for the work!
|
I'm going to merge this PR so I can start preparing the next release. I'll be busier next weeks, and if there's anything we need to address, we can handle it in a follow-up PR. Thanks again for the contribution. |
65a7078
into
testcontainers:develop
What does this PR do?
Adds support for KRaft mode using Confluent images. Add support for and official Apache Kafka images (apache/kafka and apache/kafka-native).
Why is it important?
Current implementation does not allow the end user to use images other than confluent one. It also does not support KRaft mode, but it is supported in Go, Java and Python implementations.
The goal was to enable missing functionality.
Related issues
The difference between implementations: