Allow reconciliation to continue when scale-down is blocked due to brokers still in use

[scholzj]

Type of change

• Enhancement / new feature

Description

Currently, when a user tries to scale down the Kafka cluster but the brokers that would be removed are still in use, we just throw an exception and fail the reconciliation. This works fine in general. But it makes it hard to build any automation on top of it since it essentially blocks any other operation tasks. So for example, if use by mistake scales down the Kafka cluster, the operator starts throwing errors and will not proceed with for example renewing certificates.

This PR tries to address it by reverting the scale-down changes and allowing the reconciliation to proceed as usual. It takes a pragmatic approach and reverts all scale-downs even if some of the brokers would be empty and it would be possible to shut them down. Since Kafka has no fencing, this could mean the brokers that were empty get new topics assigned. But it helps to simplify the logic significantly because it does not need to deal with situations such as when user tries to scale down brokers 3 and 4 where broker 3 is empty and broker 4 is still in use. In such case, we cannot simply correct the replication factor by 1 as that would mean broker 3 (the empty one) is kept and broker 4 is removed => so it will be still blocked with an exception.

From the technical perspective, this PR introduces a new class KafkaClusterCreator that contains the logic. This class is used to:

• Create the Kafka cluster model
• Check if it has any scale-down issues
• If it has any scale-down issues, it tries to fix them and recursively creates the Kafka cluster model again (but this time, any scale-down issues are just raised as an error -> in case they were not fixed for the first time)
• It is written in a way to make it easier to extend it in the next steps to check against removing the broker role and possibly other checks (we might consider moving the storage checks to it from the KafkaCluster.fromCrd(...) method).

The KafkaClusterCreator is used by the KafkaAssemblyOperator and the created KafkaCluster instance is used to create the KafkaRecocniler. I considered some other options as well, but rejected them at the end:

• Doing it in the constructor of KafkaReconciler => I rejected it because it seemed to push a lot of logic into the constructor and also had issues with asynchronous code. The advantage of the current approach is that it keeps the constructor clean with only assignments of parameters to object fields.
• Doing it as the first step of the KafkaReconciler. This would work quite nicely. But it would mean that the KafkaCluster object is not final in the KafkaReconciler class. That seemed like something that can be tricky to maintain in the future and is something to avoid.
• Keeping the original KafkaCluster object and just modifying the node assignments inside it. This would probably be easier for the initial implementation. But it might be harder to maintain as it would mean the fields inside the KafkaCluster will not be final, will be subject to change and we will need to be careful what parts are changed and where and when are the used. Creating a brand new KafkaCluster object seemed like a better approach.

This is currently a draft sued for a preliminary review of the code. It does not contain any new tests and it does not update existing tests to the new code.

This should resolve #9232.

Checklist

• Write tests
• Make sure all tests pass
• Update documentation
• 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

[scholzj]

@ppatierno Would you mind having a quick look if the solution for this makes sense to you before I write all the tests etc.?

[ppatierno]

The overall logic seems to be reasonable to me. I left a couple of questions.
I will start crying about conflicts I see with my migration PR later ...

[scholzj]

/azp run regression

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[scholzj]

/azp run regression

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[scholzj]

/azp run feature-gates-regression

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[scholzj]

/azp run kraft-regression

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[ppatierno]

LGTM. I saw multiple TODO which should be removed.

[im-konge]

LGTM, thanks

[scholzj]

/azp run regression

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).