Add sync actions for kafka connect artefacts

kafka-ops / julie

A solution to help you build automation and gitops in your Apache Kafka deployments. The Kafka gitops!

MIT License

417 stars 113 forks source link

Add sync actions for kafka connect artefacts #542

Closed jniebuhr closed 1 year ago

jniebuhr commented 1 year ago

Please check if the PR fulfills these requirements
[x] The commit messages are descriptive
[x] Tests for the changes have been added (for bug fixes / features)
[ ] Docs have been added / updated (for bug fixes / features)
[x] An issue has been created for the pull requests. Some issues might require previous discussion.
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...) Fixes #541
What is the current behavior? (You can also link to an open issue here) There are only creates or deletes on connectors, no updates.
What is the new behavior (if this is a feature change)? Connector artefacts get a hash of their config. This hash is calculated from the cluster state or persisted in the julie state. If an artefact already exists, the hash is compared and a sync action is added to the plan if necessary.
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?) No

purbon commented 1 year ago

Thanks a lot for your contribution @jniebuhr, it is certainly something required. during my first review I noticed there is no specific test for this new functionality, do you mind adding one? 🙏 it is important for the project to keep the test bed as good as possible.

Looking forward to land this

purbon commented 1 year ago

Screenshot 2022-09-27 at 10 47 14

Also it looks IT test does not pass, can you please take a look at well. Thanks a lot.

jniebuhr commented 1 year ago

@purbon Thanks for the review, I'm adding some tests and fixing the integration tests and will update the PR in a bit.

jniebuhr commented 1 year ago

@purbon I've repaired the failing tests and added some new ones. Also squashed it all into one commit. Could you review again?

purbon commented 1 year ago

Hi @jniebuhr, thanks again for your contributions and help. Sorry, I could not go back here faster, but last week I was wiped out in bed.

I see you started adding some tests, sadly the new functionality still breaks a few things like the still missing implementation for KsqlDB. But we're in a good direction.

There are a couple of other things, we could probably improve, but for now, I think I will take it from here and add a patch for KsqlDB, hopefully, that is ok for you.

This functionality will be available in the next release, check out the main page for info.

Thanks again for your help and valuable contributions.

jniebuhr commented 1 year ago

Hi @purbon, thanks for the feedback. Sure you can integrate it into the other merge request. I have not implemented it for anything other than Connect but it shouldn't throw any errors since there's no sync action generated for those.

purbon commented 1 year ago

I'm in the process of merging this manually now, as said it is going to be in upcoming release anytime.

purbon commented 1 year ago

released under https://github.com/kafka-ops/julie/releases/tag/v4.4.1