Rename private reconcile method to reconcileKafkaRebalance

[steffen-karlsson]

Type of change

Select the type of your PR

• Refactoring

Description

Fixing issue #10420
Renaming for less confusion as AbstractOperator as well exposes a reconcile method.

Checklist

Please go through this checklist and make sure all applicable tasks have been done

• Write tests
• Make sure all tests pass
• Update documentation
• Check RBAC rights for Kubernetes / OpenShift roles
• Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally
• Reference relevant issue(s) and close them after merging
• Update CHANGELOG.md
• Supply screenshots for visual changes, such as Grafana dashboards

[scholzj]

LGTM. Thansk for the PR.

[scholzj]

/azp run regression

[ppatierno]

@steffen-karlsson thanks for the PR, while I know that the current naming is misleading and bad as pointed out by Jakub, I am not convinced the proposed name is the best one. Right now the calls chain is made by ... (where KRAO is our KafkaRebalanceAssemblyOperator)

AbstractOperator.reconcile() --> KRAO.createOrUpdate() --> reconcileRebalance(Reconciliation, KafkaRebalance) --> reconcile(Reconciliation, Host, CC API client, KafkaRebalance, KafkaRebalanceStatus)

We are trying to rename the last reconcile and the current proposal is reconcileKafkaRebalance which could make sense if there wasn't the existing reconcileRebalance that could be renamed the same way.
My opinion is that the current reconcileRebalance could be renamed as reconcileKafkaRebalance (because it's the first entry point from the overall reconcile, and its goal is to actually reconciling the KafkaRebalance custom resource).
Now coming back to the reconcile, this is the method which is going to interact with CC (through the FSM) to get optimization proposals, running the rebalancing (by calling requestRebalance in both cases), verifying its status and returning back an updated one. So what do you think about something like processRebalancing, processRebalance, handleRebalance but not mentioning reconcile at all?

@scholzj @katheris because you already approved, wdyt about my reasoning for changing the proposal?

[scholzj]

@scholzj @katheris because you already approved, wdyt about my reasoning for changing the proposal?

I do not have a problem with it being changed as you suggested.

[katheris]

@scholzj @katheris because you already approved, wdyt about my reasoning for changing the proposal?

The reasoning makes sense to me, I think handleRebalance works best of the ones you suggested. Might also be useful to add some javadoc to that method, but that can be done in a separate PR.

[ppatierno]

Cool! Thanks folds ... so @steffen-karlsson if you agree I would go this way ...

reconcileRebalance renamed into reconcileKafkaRebalance
reconcile renamed into handleRebalance

Wdyt?

[katheris]

LGTM

[ppatierno]

/azp run regression

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[ppatierno]

LGTM. Thanks!