DandyDeveloper / charts

Various helm charts migrated from [helm/stable] due to deprecation
https://dandydeveloper.github.io/charts
Apache License 2.0
156 stars 143 forks source link

[stable/redis-ha] Fix service-monitor labels #166

Closed GMartinez-Sisti closed 2 years ago

GMartinez-Sisti commented 2 years ago

What this PR does / why we need it:

The labels for service monitors (redis+haproxy) are not working as expected. There are global missing labels and helm fails when service monitor labels are provided along with extraLabels.

Which issue this PR fixes

Helm template output for current version shows the extra label key outside of labels and duplicated namespace, this fails the deploy:

 → helm template . \
    --set=replicas=1 \
    --set=exporter.enabled=true \
    --set=exporter.serviceMonitor.enabled=true \
    --set=extraLabels.extralabel-key=extralabel-value \
    --set=exporter.serviceMonitor.labels.svcmon-key=svcmon-value \
    --set=exporter.serviceMonitor.namespace=monitor-namespace \
    --api-versions=monitoring.coreos.com/v1

[...]
---
# Source: redis-ha/templates/redis-ha-servicemonitor.yaml
apiVersion: monitoring.coreos.com/v1
kind: ServiceMonitor
metadata:
  labels:
    svcmon-key: svcmon-value
  name: RELEASE-NAME-redis-ha
  namespace: "default"
  namespace: "monitor-namespace"
  extralabel-key: "extralabel-value"
spec:
  endpoints:
  - targetPort: 9121
  jobLabel: RELEASE-NAME-redis-ha
  namespaceSelector:
    matchNames:
    - "default"
  selector:
    matchLabels:
      app: redis-ha
      release: RELEASE-NAME
      exporter: enabled
[...]

With this PR the result will include the missing global labels and also optional labels with correct indentation:


 → helm template . \
    --set=replicas=1 \
    --set=exporter.enabled=true \
    --set=exporter.serviceMonitor.enabled=true \
    --set=extraLabels.extralabel-key=extralabel-value \
    --set=exporter.serviceMonitor.labels.svcmon-key=svcmon-value \
    --set=exporter.serviceMonitor.namespace=monitor-namespace \
    --api-versions=monitoring.coreos.com/v1

[...]
---
# Source: redis-ha/templates/redis-ha-servicemonitor.yaml
apiVersion: monitoring.coreos.com/v1
kind: ServiceMonitor
metadata:
  name: RELEASE-NAME-redis-ha
  namespace: "monitor-namespace"
  labels:
    app: redis-ha
    heritage: "Helm"
    release: "RELEASE-NAME"
    chart: redis-ha-4.14.8
    extralabel-key: "extralabel-value"
    svcmon-key: "svcmon-value"
spec:
  endpoints:
  - targetPort: 9121
  jobLabel: RELEASE-NAME-redis-ha
  namespaceSelector:
    matchNames:
    - "default"
  selector:
    matchLabels:
      app: redis-ha
      release: RELEASE-NAME
      exporter: enabled
[...]

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

GMartinez-Sisti commented 2 years ago

Hi @DandyDeveloper, is there anything missing until we can merge this PR? Thanks.