Skip to content

Added missing labels in the KSM deployment file#12663

Open
ShubhamRwt wants to merge 1 commit intostrimzi:mainfrom
ShubhamRwt:fixKSM
Open

Added missing labels in the KSM deployment file#12663
ShubhamRwt wants to merge 1 commit intostrimzi:mainfrom
ShubhamRwt:fixKSM

Conversation

@ShubhamRwt
Copy link
Copy Markdown
Contributor

Type of change

  • Bugfix

Description

This commit adds the missing app.kubernetes.io/name and app.kubernetes.io/instance labels to the strimzi-kube-state-metrics Service resource in the KSM.yaml of metrics example configuration. The ServiceMonitor seems to look for the label and since it is not able find them in the Service, the metrics are not scraped properly and therefore they don't appear in the prometheus metrics. This PR fixes the issue

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

Signed-off-by: ShubhamRwt <[email protected]>
@ShubhamRwt ShubhamRwt requested a review from ppatierno April 21, 2026 09:59
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.08%. Comparing base (42522ab) to head (4144753).

Additional details and impacted files
@@            Coverage Diff            @@
##               main   #12663   +/-   ##
=========================================
  Coverage     75.07%   75.08%           
- Complexity     6513     6514    +1     
=========================================
  Files           377      377           
  Lines         25092    25092           
  Branches       3268     3268           
=========================================
+ Hits          18838    18840    +2     
+ Misses         4914     4912    -2     
  Partials       1340     1340           

see 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Member

@scholzj scholzj left a comment

Choose a reason for hiding this comment

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

These labels are a mess and everyone expects something else from them. I wonder if you should use a completely different labels instead.

@ShubhamRwt
Copy link
Copy Markdown
Contributor Author

ShubhamRwt commented Apr 22, 2026

@scholzj I can do that but I was wondering that would mean more changes in the KSM yaml file. I think this is something which should have been there from the start. The ServiceMonitor specifically looks for these labels in the example yaml so I wonder if we somehow missed it or if that was the intention from the start.

@sebastiangaiser
Copy link
Copy Markdown
Contributor

@ShubhamRwt the kube-state-metrics was originally added by me. I used the kube-state-metrics Helm chart in the beginning to add this. Probably, I removed the labels for the service...
As the ServiceMonitor selects the Service via the labels, the example is not complete. So I would vote for adding them.

@scholzj
Copy link
Copy Markdown
Member

scholzj commented Apr 22, 2026

I think it is clear that the labels are missing.

The question is which labels we want to use. From our experience, app.kubernetes.io labels are badly defined and different users want different things from them. That is something where we burned ourself in the operator. Obviously, changing them here in the YAML files is easier, so maybe it matters less 🤷

@ShubhamRwt
Copy link
Copy Markdown
Contributor Author

@scholzj I got your point, do you have any suggestions for the same?

@scholzj
Copy link
Copy Markdown
Member

scholzj commented Apr 22, 2026

Let's see what others think about the labels first.

Copy link
Copy Markdown
Member

@Frawless Frawless left a comment

Choose a reason for hiding this comment

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

@ShubhamRwt the kube-state-metrics was originally added by me. I used the kube-state-metrics Helm chart in the beginning to add this. Probably, I removed the labels for the service... As the ServiceMonitor selects the Service via the labels, the example is not complete. So I would vote for adding them.

Sounds to me like it should be there so +1 for me.

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.

4 participants