-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Allow dynamic configuration update for controllers #11999
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
Signed-off-by: Gantigmaa Selenge <[email protected]>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #11999 +/- ##
============================================
- Coverage 74.90% 74.90% -0.01%
- Complexity 6441 6457 +16
============================================
Files 343 343
Lines 24324 24339 +15
Branches 3196 3209 +13
============================================
+ Hits 18221 18232 +11
- Misses 4829 4832 +3
- Partials 1274 1275 +1
🚀 New features to boost your workflow:
|
|
/gha run pipeline=regression |
|
⏳ System test verification started: link The following 6 job(s) will be executed:
Tests will start after successful build completion. |
|
❌ System test verification failed: link |
...ator/src/main/java/io/strimzi/operator/cluster/operator/resource/KafkaConfigurationDiff.java
Outdated
Show resolved
Hide resolved
|
@tinaselenge One more question ... what are the version limitations for this? Is it 4.0+? Also, what happens during upgrade from for example Kafka 3.8? Is this part skipped because for example container image changed and the pod is rolled anyway? |
Yes, it is limited to 4.0+ as it builds on #11838. And yes I think so... because we apply dynamic updates only if we don't need to restart the node. If the container image has changed, it will be restarted before getting to this config part. |
...ator/src/main/java/io/strimzi/operator/cluster/operator/resource/KafkaConfigurationDiff.java
Show resolved
Hide resolved
cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/resource/KafkaRoller.java
Outdated
Show resolved
Hide resolved
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.
Few more nits. LGTM overall.
...ator/src/main/java/io/strimzi/operator/cluster/operator/resource/KafkaConfigurationDiff.java
Outdated
Show resolved
Hide resolved
...ator/src/main/java/io/strimzi/operator/cluster/operator/resource/KafkaConfigurationDiff.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Gantigmaa Selenge <[email protected]>
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 assuming the tests pass. Thanks.
|
/gha run pipeline=regression,upgrade |
|
⏳ System test verification started: link The following 10 job(s) will be executed:
Tests will start after successful build completion. |
|
/azp run regression |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run upgrade |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
🎉 System test verification passed: link |
Type of change
Select the type of your PR
Description
Currently, we restart controller-only nodes if there is any controller relevant configuration changes detected via Pod annotation. The operator can now talk to controller-only nodes directly via Admin API (#11838).
This PR checks configurations of controllers and apply dynamic configurations via Admin API (same as how it's done for broker and mixed nodes) instead of restarting them based on annotation.
closes #10900
Checklist
Please go through this checklist and make sure all applicable tasks have been done