Fix servicemonitor relabeling

[kubroid]

Trying to deploy keda using this values file:

prometheus:
  operator:
    enabled: true
    port: 8080
    serviceMonitor:
      enabled: true
      relabellings:
      - regex: (rest_client_.*|go_.*|workqueue_.*)
        action: drop

Result is:

Error: YAML parse error on keda/templates/manager/servicemonitor.yaml: error converting YAML to JSON: yaml: line 20: did not find expected key

Checklist

• I have verified that my change is according to the deprecations & breaking changes policy
• Commits are signed with Developer Certificate of Origin (DCO - learn more)

Fixes #
Helm chart fixed to use right indents.

helm template -f a.yaml -n keda . | grep ServiceMonitor -A20
kind: ServiceMonitor
metadata:
  name: keda-operator
  annotations:
    {}
  labels:
    app.kubernetes.io/name: keda-operator    
    helm.sh/chart: keda-2.10.2
    app.kubernetes.io/component: operator
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/instance: release-name
    app.kubernetes.io/part-of: keda-operator
    app.kubernetes.io/version: 2.10.1
spec:
  endpoints:
  - port: metrics
    path: /metrics
    relabelings:
      - action: drop
        regex: (rest_client_.*|go_.*|workqueue_.*)
  namespaceSelector:

[JorTurFer]

Looking good, could you update the test values yaml to use service monitor on all the resources with a relabeling (to prevent this in the future)?
You can achieve it modifying this section: https://github.com/kedacore/charts/blob/main/.github/workflows/ci-core.yml#L107-L115

[kubroid]

Sure. Test data updated. Templating is ok, but helm deploy failing on timeout. Is it common?

[JorTurFer]

Is it common?

No it isn't. Metrics server pod is failing to start, which shouldn't happen. I don't think that it's related with your commit, let me check

[kubroid]

The only idea I have is an invalid regex expression. Had no chance to check it in real deployment.

[JorTurFer]

The only idea I have is an invalid regex expression. Had no chance to check it in real deployment.

I don't think that it's enough to break the metrics server startup as it's something related with Prometheus, not with the metrics server itself. I'll test it locally during the morning

[kubroid]

Thanks Jorge.
One more thing to discuss: here is inconsistency between relabellings and relabelings. Do we want to fix this?

[JorTurFer]

Thanks Jorge. One more thing to discuss: here is inconsistency between relabellings and relabelings. Do we want to fix this?

It'd be nice, but we should maintain backward compatibility, so we can't just rename it, we have to maintain both, the old one and the new one (adding a deprecation note on the old one)

[JorTurFer]

It's because there is a PR merged in core but still not merged here: #432
Let me push it

[JorTurFer]

Could you rebase your branch? The pending PR has been merged

[kubroid]

Done

[JorTurFer]

I'm on top of main:

* 0bd1bc1 - (18 hours ago) Fix servicemonitor relabeling - Alexey Kubrinsky (HEAD -> fix-servicemonitor, origin/fix-servicemonitor)                                                                                                                                                       
* c5fad71 - (5 days ago) fix(http-add-on): Fix resources variables for the http-add-on deploym… (#454) - Jocelyn Thode (origin/main, origin/HEAD, main)                                                                                                                                   

This is the commit that you need: 502980f

[kubroid]

Sorry, did pull from my fork instead of keda repo

[kubroid]

Jorge, please check last commit. Made relabelling deprecated. Is it good?

[JorTurFer]

I think so.
Do we need to create an issue for the deprecation @tomkerkhove ?

[tomkerkhove]

Yes, we should follow https://github.com/kedacore/governance/blob/main/DEPRECATIONS.md#introducing-new-deprecations.

I'll do this