-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Allow configuring advertised.listeners for controllers with Kafka version 3.9.0 #10530
Conversation
0387df3
to
dd0da6c
Compare
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.
Thanks. Just some quick comments ...
...tor/src/test/java/io/strimzi/operator/cluster/model/KafkaBrokerConfigurationBuilderTest.java
Outdated
Show resolved
Hide resolved
...perator/src/main/java/io/strimzi/operator/cluster/model/KafkaBrokerConfigurationBuilder.java
Outdated
Show resolved
Hide resolved
…sion 3.9 Signed-off-by: Gantigmaa Selenge <tina.selenge@gmail.com>
dd0da6c
to
306f08c
Compare
@scholzj should this PR wait until Kafka 3.9 is released or can it be pushed before that? I think it doesn't have to wait. |
@tinaselenge I think we can proceed with it. Let me run the regression. Can you mark it as ready for review if you don't plan any more changes? |
/azp run kraft-regression |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run upgrade |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run migration |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run regression |
Azure Pipelines successfully started running 1 pipeline(s). |
thanks @scholzj. I've marked it as ready and regression tests passed. |
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.
One more nit. LGTM otherwise.
...perator/src/main/java/io/strimzi/operator/cluster/model/KafkaBrokerConfigurationBuilder.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Gantigmaa Selenge <tina.selenge@gmail.com>
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.
Thanks for the PR
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.
LGTM. Thanks @tinaselenge!
Type of change
Select the type of your PR
Description
When Kafka 3.9.0 is released and supported with Strimzi, the advertised.listeners will have to be configured for KRaft controllers, which is a forbidden configuration for the previous versions. While we still support an older version such as 3.8.0, we need to gate this configuration using the Kafka version.
Related issue: #10458.
Checklist
Please go through this checklist and make sure all applicable tasks have been done