testcontainers
Feature/kafka add kraft support
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
- Closes #1347
- Another approach: #1348
The difference between implementations:
- Supports for KRaft in Confluent image too
- Throws an error if KRaft is not supported in provided image
- Does not require another class to use. All happens in the same class without breaking backward compatibility.
- Makes the project nullable.
- Tests different setups.
Deploy Preview for testcontainers-dotnet ready!
| Name | Link |
|---|---|
| Latest commit | 1cdd3be8f3cb553d55ab26d5323a23b224bd3cbd |
| Latest deploy log | https://app.netlify.com/projects/testcontainers-dotnet/deploys/68a05a53cb88090009b0968c |
| Deploy Preview | https://deploy-preview-1353--testcontainers-dotnet.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify project configuration.
![netlify[bot] avatar](https://avatars.githubusercontent.com/in/13473?v=4 "Published by a pub.dev verified publisher"){kind=small}
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 https://kafka.apache.org/blog#apache_kafka_400_release_announcement
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 can see some improvement we could make on the tests at least where we should definitely test the newer images with the major and maybe the behavior with actual incompatible image/protocols
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?
@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 👀 What's blocking ? Need any help on something ?
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 🙏.
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.