[occm] Fix serviceMonitor labelSelector and remove deprecated bearerTokenFile auth - #3039
[occm] Fix serviceMonitor labelSelector and remove deprecated bearerTokenFile auth#3039claudia-lola wants to merge 10 commits into
Conversation
|
Welcome @claudia-lola! |
|
Hi @claudia-lola. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @kayrus @stephenfin and @zetaab what is the estimate for a review of this PR please? |
|
Hi, @claudia-lola |
|
@gouthampacha Hi, i saw you are looking into this repo, can i friendly ask you for adding ok-to-test tag? |
|
/ok-to-test |
jihyun-huh
left a comment
There was a problem hiding this comment.
Hi, @claudia-lola I added some small reviews.
apart from that, the approach looks reasonable to me.
| metadata: | ||
| name: {{ include "occm.name" . }} | ||
| labels: | ||
| labels: |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Co-authored-by: Jihyun Hur <huhji.elha@gmail.com>
|
Thanks @claudia-lola, Hi, @kayrus I don’t see any remaining blockers from my side. Looks good to merge. |
|
Hi @claudia-lola , one small follow-up after #3080 was merged, the OCCM chart version on master is now Could you please rebase on the latest master and update the chart version to 2.36.2?
cc. @gouthampacha @kayrus |
|
@gouthampacha, would you be able to take a reviewer pass and add /lgtm, if this looks good? cc. @kayrus for approval after the reviewer pass. This fixes the out-of-the-box ServiceMonitor issue described in #3144. |
|
/lgtm looks good, thanks @claudia-lola |
What this PR does / why we need it:
The labels on the Service did not match the ServiceMonitor selector. Prometheus was unable to discover the target. This change sets the labels so the ServiceMonitor selects the correct Service.
bearerTokenFileis deprecated and no longer works. ServiceMonitor was updated to use a supported authentication method.authorization-always-allow-paths="/metrics"was added to ensure metrics can be scraped.Which issue this PR fixes(if applicable):
fixes #3144
Special notes for reviewers:
Release note: