Extract Topic Operator's Cruise Control client

[fvaleri]

This is the first step in the direction of having a shared Cruise Control client based on Java HTTP client. For the moment changes are limited to the Topic Operator.

If we agree on the strategy, I can then move the Cruise Control client to operator-common and also use it in CruiseControlApi (different PR).

---

In my view, the shared Cruise Control client shouldn't be concerned with response handling. This client should simply send the request and return a response POJO. Much like Kafka admin and Kube clients do.

The response handling is specific to the functionality we are implementing (the same response could be processed in different ways), so it should be the responsibility of the caller (CruiseControlApi in CO, ReplicasChangeHandler in TO).

That way, each object in the call chain triggered by the controller is focused on doing one thing, and the code is easier to read and maintain.

This would leave a Vertx dependency in CruiseControlApi required by KafkaRebalanceAssemblyOperator. Of course we can also get rid of that, but I would leave it for a dedicated PR.

[ppatierno]

I left one comment but not sure the impact it has on the current CC client. I mean, I see just moving a header to a dedicated class and nothing more in the direction of "extract a cruise control client" as the PR name suggests. Maybe it's just a not that great title and PR description which makes me hard the purpose?

[fvaleri]

I see just moving a header to a dedicated class and nothing more in the direction of "extract a cruise control client" as the PR name suggests.

Here the long term objective is to have a shared CC thin client to be used by both cluster and topic operators. That was discussed in the RF change proposal. This is a first step in that direction, where I simply extract all CC client logic in the topic operator, so that I can later (in another PR) move it to operator-common and share with CruiseControlApi.

[scholzj]

I left some nits. I do not see this PR partiularly controversial. But I wonder if the overall idea should have some higher level discussion or proposal:

• While doing smaller steps is good, it is hard to review the PRs when not understanding the end state
• It seems it could clarify what and how should such client work and what should it do
• I wonder if the shared CruiseControl client should really live in operator-common instead of a separate module or maybe even a seprate subproject.

[fvaleri]

@scholzj @ppatierno thanks for the review.

This started as a simple refactoring, but I now agree on the necessity of a formal proposal before moving on with sharing the CC client with the cluster operator, and I will create one.

Current changes are mostly confined to the topic operator, so it would still be good to have them for better modularization.

I wonder if the shared CruiseControl client should really live in operator-common instead of a separate module or maybe even a seprate subproject.

That's an interesting question. A CruiseControl client could probably be useful in other contexts, so it may make sense to create a separate subproject.

[scholzj]

Current changes are mostly confined to the topic operator, so it would still be good to have them for better modularization.

TBH, I left some comments, but I personally do not find this particular PR somehow controversial. So I do not have problem to proceed with this one right now. My comment was meant more in general.

[scholzj]

LGTM. Should this be also reviewed by @tombentley as the TO SME?

[ppatierno]

LGTM. Approving because it doesn't have any big impact on what we have as CC client within the cluster operator. But to move to a new one, we should discuss everything in a proposal.

[scholzj]

/azp run regression

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[fvaleri]

@tombentley are you ok with the current state? I would need this to complete some other TO changes.

Keep in mind that I'll create a new proposal for sharing the CC client with the CO where we can discuss the API in more details.

[scholzj]

/azp run regression

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[tombentley]

Hi @fvaleri, this is looking pretty good, but I had a few more questions. Thanks!

[fvaleri]

@tombentley should be complete now, thanks for your suggestions.