Update Kafka PDB to use live Pods instead of the CR Spec

[alungu]

Q	A
Bug fix?	yes
New feature?	no
API breaks?	no
Deprecations?	no
Related tickets	#769
License	Apache 2.0

What's in this PR?

The Kafka PDB logic is update to consider the total number of healthy Kafka brokers as being the number of Running Kafka pods (instead of the number of Kafka brokers specified in the Kafka CR)

Why?

The previous implementation lead to a faulty PDB during scaling (both in and out) since adding or removing brokers requires some time.

Additional context

Checklist

• Implementation tested
• Error handling code meets the guideline
• Logging code meets the guideline
• User guide and development docs updated (if needed)

[bartam1]

Thank you for the PR!

[stoader]

@alungu relying on the number of Kafka pods might not be the best approach. For example if you have a 6 broker cluster and all 6 broker pods are up and running thus the PDB:min available will be 5. When a broker pod is deleted for whatever reason outside of koperator that event will trigger a reconcile inkoperator which will update the PDB:min available to 4 as there are only 5 running broker pods found before the missing pod is recreated. After the missing pod comes up in the next reconcile flow PDB:min available will be set back to 5.

If the PDB handling https://github.com/banzaicloud/koperator/blob/master/pkg/resources/kafka/kafka.go#L162 is moved as a last step after https://github.com/banzaicloud/koperator/blob/master/pkg/resources/kafka/kafka.go#L287 than it not might be an issue.

What I'm thinking of is to determine the number of brokers for PDB handling from Status as that contains the list of running brokers and brokers pending graceful upscale/downscale. In this case the PDB handling could stay where it is now: https://github.com/banzaicloud/koperator/blob/master/pkg/resources/kafka/kafka.go#L162

What do you think?

[alungu]

@alungu relying on the number of Kafka pods might not be the best approach. For example if you have a 6 broker cluster and all 6 broker pods are up and running thus the PDB:min available will be 5. When a broker pod is deleted for whatever reason outside of koperator that event will trigger a reconcile inkoperator which will update the PDB:min available to 4 as there are only 5 running broker pods found before the missing pod is recreated. After the missing pod comes up in the next reconcile flow PDB:min available will be set back to 5.

If the PDB handling https://github.com/banzaicloud/koperator/blob/master/pkg/resources/kafka/kafka.go#L162 is moved as a last step after https://github.com/banzaicloud/koperator/blob/master/pkg/resources/kafka/kafka.go#L287 than it not might be an issue.

What I'm thinking of is to determine the number of brokers for PDB handling from Status as that contains the list of running brokers and brokers pending graceful upscale/downscale. In this case the PDB handling could stay where it is now: https://github.com/banzaicloud/koperator/blob/master/pkg/resources/kafka/kafka.go#L162

What do you think?

@stoader Thank you for mentioning this issue!
I performed a few more tests with all the options (Spec vs Status vs Pod List). All of them have blind spots, but I agree that on the issue you highlighted with the "Pod List" option.
As such, I will patch this PR to use the KafkaCluster Status instead of the "List Kafka Pods".

[alungu]

@bartam1 @stoader could you review the new solution, please? Thanks!