Consolidating the KafkaRebalance API for extensibility by kyguy · Pull Request #216 · strimzi/proposals

kyguy · 2026-04-08T22:29:08Z

This proposal addresses the increasing complexity and limited extensibility of the KafkaRebalance custom resource API.

Signed-off-by: Kyle Liberti <kliberti.us@gmail.com>

ppatierno · 2026-04-15T13:23:44Z

@kyguy is this ready to review? Should we add maintainers or are you still working on it?

Signed-off-by: Kyle Liberti <kliberti.us@gmail.com>

kyguy · 2026-04-15T14:59:06Z

@ppatierno yes, this is now ready for review! I'll request the maintainers as reviewers now

ppatierno · 2026-04-20T15:53:24Z

+# Consolidating the KafkaRebalance API for extensibility
+
+ This proposal addresses the increasing complexity and limited extensibility of the `KafkaRebalance` custom resource API. 
+ It introduces a `spec.parameters` map that replaces mode-specific primitive fields with their upstream Cruise Control key-value equivalents allowing users to consult Cruise Control documentation directly and enabling support for new parameters without Strimzi API changes. 


what about using spec.config as we have for other custom resources (i.e. Kafka, KafkaBridge, ...) ?

TBH in the original draft I had used spec.config for the sake of consistency with other custom resource configurations but then changed it to spec.parameters. I felt that using spec.parameters better emphasizes the distinction between configuration file properties and REST API query parameters. It should also help users avoid confusing these query parameters with the spec.cruiseControl.config options in the Kafka resource. WDYT?

The KafkaRebalance is an abstraction on the top of the CC REST API so I can see your point of having a sort of 1:1 matching with the query parameters on the REST call ... but still not sure if having a more friendly and well-known name like "config" would be better for users in terms of UX. Let's see what others think before making any changes.

I would personally go with .spec.config and have it consistent across all resources. Maybe it's just me, but I don't feel like we need to reflect the naming what we are using internally - and I think that users will not care that those are "parameters" for the REST API. So having parameters feels like it would be just something for us.

Fair points, I switched it back to spec.config just to show what it would look like (I can easily change it back in a later commit) I'll leave this comment unresolved for now so people can see this conversation and add their thoughts whether it be for or against

Signed-off-by: Kyle Liberti <kliberti.us@gmail.com>

im-konge

Thanks for the proposal. I left few comments.

im-konge · 2026-04-21T18:21:20Z

+# Consolidating the KafkaRebalance API for extensibility
+
+ This proposal addresses the increasing complexity and limited extensibility of the `KafkaRebalance` custom resource API. 
+ It introduces a `spec.parameters` map that replaces mode-specific primitive fields with their upstream Cruise Control key-value equivalents allowing users to consult Cruise Control documentation directly and enabling support for new parameters without Strimzi API changes. 


I would personally go with .spec.config and have it consistent across all resources. Maybe it's just me, but I don't feel like we need to reflect the naming what we are using internally - and I think that users will not care that those are "parameters" for the REST API. So having parameters feels like it would be just something for us.

im-konge · 2026-04-21T18:30:14Z

+   - Strimzi will log a warning and write an error in the `KafkaRebalance` status when `brokers` or `volumes` are provided but irrelevant to the selected mode.
+
+2. **Parameter field validation**:
+   - Strimzi does not pre-validate parameters entries.


Maybe completely stupid question, but today we are pre-validating the parameters specified in the CR, right?
How it would work when you have some fields specified under old structure and some in new structure? Will it automatically skip the validation from now on or it will be semi-validated? (As I said, maybe completely stupid question, sorry).

Not a stupid question at all.

Right now we only do loose validation on some of primitive fields that we’re planning to deprecate in favor of the new config. For example, we might enforce simple constraints like minimum values for fields such as concurrentPartitionMovementsPerBroker.

If a CR includes a mix of old and new structure, it won’t skip the validation of the deprecated primitive fields if they are included. That being said the parameters included as part of the new config will take preference. For example, if a primitive field is included with a valid value and it's equivalent field in the new config is included with an incorrect value, the KafkaRebalance resource will be put in the NotReady state and a condition with the error will be added.

Does that answer the question?

Sorry I realized I didn't reply here - yeah it answers my question, thanks :)

Signed-off-by: Kyle Liberti <kliberti.us@gmail.com>

PaulRMellor

The proposal is clear and well justified. I’ve added a few minor suggestions as I read through

PaulRMellor · 2026-04-23T10:54:45Z

+
+2. **Parameter field validation**:
+   - Strimzi won't pre-validate new config parameter entries.
+   Strimzi will pass the config parameters as-is to the Cruise Control REST API.


Any security considerations to mention here? Or how compatibility is handled between Strimzi and Cruise Control versions?

Thanks for raising this, there actually should be some parameter validation to mitigate users accidentally breaking the KafkaRebalance state machine and status. I have just added some notes on filtering parameters in the latest commit.

The parameter filtering should address any potential misuse or security concerns. As for compatibility between Strimzi and Cruise Control versions, a parameter that is unsupported by the bundle Cruise Control version will be rejected by Cruise Control at runtime and Strimzi will surface the error by transitioning the KafkaRebalance resource.

Did I understand/answer the question correctly @PaulRMellor?

Co-authored-by: PaulRMellor <47596553+PaulRMellor@users.noreply.github.com> Signed-off-by: Kyle Liberti <kliberti@redhat.com>

Signed-off-by: Kyle Liberti <kliberti.us@gmail.com>

kyguy · 2026-05-04T12:37:11Z

This is ready for another round of reviews whenever people have time of course!

ppatierno · 2026-05-04T14:58:46Z

+1. **Introduce the new `config` map and `volumes` field** while maintaining backward compatibility:
+   - Accept both old top-level primitive fields and new `config` map.
+   - Accept both `moveReplicasOffVolumes` and `volumes` (mapped to the same internal representation)
+   - If both old and new forms are provided, the new form takes precedence


I think we could even log a warning that they are both set but the old form will be ignored.

Also what happens in the following scenario?

spec: concurrentPartitionMovementsPerBroker: 10 # old config: concurrent_leader_movements: "5" # new but different field

Is the logic going to ignore the old concurrentPartitionMovementsPerBroker because the resource has a spec.config field or it's going to make a merge because they are referring to two different fields?

It would make a merge, respecting both fields and values but a warning would still be logged for the usage of the old concurrentPartitionMovementsPerBroker form.

Just updated some text here in to address the first comment in the latest commit, let me know what you think!

I would expect a merge yes, but at the same time, if our goal is discourage the usage of the current properties in favor of the config section, we could also decide that if you set the config section, it's going to override everything else coming from the dedicated fields. It could be counterintuitive though. In the past we had something similar with templates across Kafka and KafkaNodePool and we moved from an override approach to a merge. Let's see what others think about this.

Signed-off-by: Kyle Liberti <kliberti.us@gmail.com>

Restructuring the KafkaRebalance API for extensibility

550a8a1

Signed-off-by: Kyle Liberti <kliberti.us@gmail.com>

kyguy mentioned this pull request Apr 8, 2026

Add support for broker demotion #191

Open

kyguy force-pushed the kr-restructure branch from f669ad0 to 66d248e Compare April 14, 2026 00:13

kyguy marked this pull request as ready for review April 14, 2026 00:15

kyguy force-pushed the kr-restructure branch from 66d248e to 131abf2 Compare April 14, 2026 00:20

Consolidating the KafkaRebalance API for extensibility

06ab0d6

Signed-off-by: Kyle Liberti <kliberti.us@gmail.com>

kyguy force-pushed the kr-restructure branch from 131abf2 to 06ab0d6 Compare April 15, 2026 14:57

kyguy changed the title ~~Restructuring the KafkaRebalance API for extensibility~~ Consolidating the KafkaRebalance API for extensibility Apr 15, 2026

kyguy requested a review from a team April 15, 2026 14:59

ppatierno requested review from Frawless, PaulRMellor, im-konge, katheris, ppatierno, scholzj, see-quick and tinaselenge and removed request for a team April 20, 2026 15:42

ppatierno reviewed Apr 20, 2026

View reviewed changes

Addressing some feedback - pp

0e9df60

Signed-off-by: Kyle Liberti <kliberti.us@gmail.com>

im-konge reviewed Apr 21, 2026

View reviewed changes

Addressing feedback - lk

94af087

Signed-off-by: Kyle Liberti <kliberti.us@gmail.com>

PaulRMellor approved these changes Apr 23, 2026

View reviewed changes

kyguy and others added 2 commits April 23, 2026 14:06

Addressing feedback - pm

cdeff18

Co-authored-by: PaulRMellor <47596553+PaulRMellor@users.noreply.github.com> Signed-off-by: Kyle Liberti <kliberti@redhat.com>

Addressing more feedback - pm

890c969

Signed-off-by: Kyle Liberti <kliberti.us@gmail.com>

kyguy requested a review from fvaleri May 4, 2026 12:39

ppatierno reviewed May 4, 2026

View reviewed changes

Addressing feedback - pp

4e73db4

Signed-off-by: Kyle Liberti <kliberti.us@gmail.com>

Conversation

kyguy commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ppatierno commented Apr 15, 2026

Uh oh!

kyguy commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kyguy Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

im-konge left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

PaulRMellor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kyguy commented May 4, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kyguy May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ppatierno May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kyguy commented Apr 8, 2026 •

edited

Loading

kyguy commented Apr 15, 2026 •

edited

Loading

kyguy Apr 21, 2026 •

edited

Loading

kyguy May 4, 2026 •

edited

Loading

ppatierno May 5, 2026 •

edited

Loading