Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve the separation between the Connect and MM2 operators #9955

Merged

Conversation

scholzj
Copy link
Member

@scholzj scholzj commented Apr 12, 2024

Type of change

  • Refactoring

Description

The AbstractConnectOperator, KafkaConnectAssemblyOperator and KafkaMirrorMaker2AssemblyOperator classes are currently badly designed in how they separate the functionality specific to Connect and MM2. The AbstractConnectOperator does not really contain only the abstract and shared parts. But it seems to contain also many parts related only to Connect. This applies specifically for the parts related to the KafkaConnector management and operations -> KafkaConnector resources are used only with Connect as MM2 has the connector configuration directly inside the KafkaMirrorMaker2 CR. These parts are then later overridden in the KafkaMirrorMaker2AssemblyOperator. This makes it very hard to see:

  • What methods are really shared
  • What should be the access modifiers for the different methods

This PR refactors these classes to improve the separation between them. The AbstractConnectOperator now contains only the parts that are really shared and does not contain any parts unique to either Connect or MM2. The other parts were moved to the corresponding assembly operator, in most cases to KafkaConnectAssemblyOperator. This refactoring should help us to better understand what is shared and what is not shared. And in the future it should allow us to follow up with another refactoring that will try to reuse more code between KafkaConnectAssemblyOperator and KafkaMirrorMaker2AssemblyOperator => these classes have now each its own reconciliation flow. But they really differ only in how the connectors are managed. So there should be space to further improve this. But this is not done in this PR to keep the PR scope under review and keep the reviews easier. Where possible, this PR also makes methods private or static.

Note: While this PR moves a lot of methods around, it usually only changes their placement, access modifiers etc. It doesn't change any content inside these methods with the exception of few places where very simple methods were inlined.

This contributes to #8158.

Checklist

  • Make sure all tests pass
  • Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally

Signed-off-by: Jakub Scholz <www@scholzj.com>
@scholzj scholzj added this to the 0.41.0 milestone Apr 12, 2024
@scholzj scholzj requested a review from ppatierno April 12, 2024 14:45
@scholzj
Copy link
Member Author

scholzj commented Apr 12, 2024

/azp run regression

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@scholzj scholzj merged commit b6172a5 into strimzi:main Apr 14, 2024
21 checks passed
@scholzj scholzj deleted the improve-separation-of-connect-and-mm2-operators branch April 14, 2024 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants