Skip to content

feat(examples,metrics,kube-state-metrics): extend configmap for 'KafkaConnect', 'KafkaConnector' and 'KafkaMirrorMaker2' #11354

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jun 19, 2025
Merged

feat(examples,metrics,kube-state-metrics): extend configmap for 'KafkaConnect', 'KafkaConnector' and 'KafkaMirrorMaker2' #11354

merged 5 commits into from
Jun 19, 2025

Conversation

sebastiangaiser
Copy link
Contributor

Part of #10276

Type of change

Select the type of your PR

  • Enhancement / new feature

Description

Please check the commit messages

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

@ppatierno ppatierno added this to the 0.47.0 milestone Apr 22, 2025
ppatierno
ppatierno requested changes May 5, 2025
View reviewed changes
@ppatierno ppatierno requested review from a team and katheris May 5, 2025 15:08
@sebastiangaiser
Copy link
Contributor Author

sebastiangaiser commented May 13, 2025 •
edited
Loading

Thanks for the review. I simply applied your suggestions 🚀

@sebastiangaiser sebastiangaiser requested a review from ppatierno May 13, 2025 13:42
im-konge
im-konge approved these changes Jun 5, 2025
View reviewed changes
Copy link
Member

@im-konge im-konge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I tried the changes locally (I played with the Ready and NotReady states), everything seems to work fine:

https://gist.github.com/im-konge/3a6793a7f10bb543b3f1dc3044504195

so LGTM, thanks for the PR @sebastiangaiser !

@im-konge
Copy link
Member

im-konge commented Jun 5, 2025

@strimzi/maintainers could you please have a look on this? It's opened for quite a long time. Thanks

katheris
katheris requested changes Jun 9, 2025
View reviewed changes
Copy link
Contributor

@katheris katheris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If my understanding is correct the metric mapping for KafkaMirrorMaker2 is incorrect since the field for the auto restart is different to KafkaConnector.

@sebastiangaiser
Copy link
Contributor Author

Good catch, thanks 👍🏻

@sebastiangaiser sebastiangaiser requested a review from katheris June 10, 2025 15:02
@sebastiangaiser
Copy link
Contributor Author

Is the failed test /home/runner/work/strimzi-kafka-operator/strimzi-kafka-operator/systemtest/src/test/java/io/strimzi/systemtest/connect/ConnectST.java:[565,9] cannot find symbol related to my changes? 🤔

@im-konge
Copy link
Member

@sebastiangaiser I merged big PR rewriting a lot of things in our STs. Did you do rebase? Also, did you have any conflicts or something?

@im-konge
Copy link
Member

Actually, #11198 broke the main because of the PR wasn't rebased IMHO before merge. I'm gonna open a new PR fixing the issue on main. Sorry for the issues.

@im-konge
Copy link
Member

@sebastiangaiser I opened fix for the build #11523
Once merged, I will tag you here. Thanks and sorry for the issues.

@im-konge
Copy link
Member

@sebastiangaiser PR is merged, could you please rebase this PR? Thanks

ppatierno
ppatierno approved these changes Jun 12, 2025
View reviewed changes
Copy link
Member

@ppatierno ppatierno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. @sebastiangaiser we just need it to be rebased to have the build passing. Thanks!

@im-konge
Copy link
Member

Hey @sebastiangaiser, could you please rebase this PR, so we can merge it eventually? The rebase should fix the error you see in the build pipeline, as I mentioned in previous comment. Thanks!

sebastiangaiser and others added 5 commits June 19, 2025 11:02
@sebastiangaiser
fix(examples,metrics,ksm): add missing resources to 'ClusterRole'
2b9ce71 
This is part of implementing https://github.com/strimzi/proposals/blob/main/087-monitoring-of-custom-resources.md

Signed-off-by: Sebastian Gaiser <[email protected]>
@sebastiangaiser
feat(examples,metrics,kube-state-metrics): extend configmap for 'Kafk…
47d7b2f 
…aConnect', 'KafkaConnector' and 'KafkaMirrorMaker2'

In order to implement https://github.com/strimzi/proposals/blob/main/087-monitoring-of-custom-resources.md the configmap is extended for 'KafkaConnect', 'KafkaConnector' and 'KafkaMirrorMaker2' resources.

Signed-off-by: Sebastian Gaiser <[email protected]>
@sebastiangaiser
feat(examples,metrics,ksm): add PrometheusRule for 'KafkaConnect', 'K…
097bb93 
…afkaConnector' and 'KafkaMirrorMaker2'

Signed-off-by: Sebastian Gaiser <[email protected]>
@sebastiangaiser @ppatierno
Apply suggestions from code review
b37e660 
Co-authored-by: Paolo Patierno <[email protected]>
Signed-off-by: Sebastian Gaiser <[email protected]>
@sebastiangaiser @katheris
Apply suggestions from code review
de66f7a 
Co-authored-by: Kate Stanley <[email protected]>
Signed-off-by: Sebastian Gaiser <[email protected]>
@sebastiangaiser
Copy link
Contributor Author

Rebased, sorry for the delay. Didn't had a laptop with me over the last week.

@im-konge
Copy link
Member

Rebased, sorry for the delay. Didn't had a laptop with me over the last week.

No issues, sorry for two pings 🙂 we can merge this once the build pipeline is green. Thanks a lot once more!

@im-konge im-konge merged commit a81d2b2 into strimzi:main Jun 19, 2025
13 checks passed
@im-konge
Copy link
Member

Thanks for the PR @sebastiangaiser !

fvaleri pushed a commit to OwenCorrigan76/strimzi-kafka-operator that referenced this pull request Jun 24, 2025
@sebastiangaiser @ppatierno
@katheris @fvaleri
feat(examples,metrics,kube-state-metrics): extend configmap for 'Kafk…
2b7efed 
…aConnect', 'KafkaConnector' and 'KafkaMirrorMaker2' (strimzi#11354)

Signed-off-by: Sebastian Gaiser <[email protected]>
Co-authored-by: Paolo Patierno <[email protected]>
Co-authored-by: Kate Stanley <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ppatierno ppatierno ppatierno approved these changes

@katheris katheris katheris approved these changes

@im-konge im-konge im-konge approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.47.0
Development

Successfully merging this pull request may close these issues.
4 participants
@sebastiangaiser @im-konge @ppatierno @katheris