Skip to content
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.

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

Fixed Cadence service monitor additional labels #1292

Merged
merged 2 commits into from
Sep 21, 2021

Conversation

pregnor
Copy link
Member

@pregnor pregnor commented Sep 18, 2021

Q A
Bug fix? yes
New feature? no
API breaks? no
Deprecations? no
Related tickets resolves #1291
License Apache 2.0

What's in this PR?

Fixed Cadence service monitor additional labels templating.

Why?

The labels substitution was broken.

Additional context

I used Helm local template generation for testing (helm template cadence cadence from the repository root).

Originally indent was used instead of nindent which together with the extra carriage return in the template and extra indentations broke the resulting YAML after substitution.

# Source: cadence/templates/server-service-monitor.yaml
apiVersion: monitoring.coreos.com/v1
kind: ServiceMonitor
metadata:
  name: cadence-history
  labels:
    app.kubernetes.io/name: cadence
    helm.sh/chart: cadence-0.21.1
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/instance: cadence
    app.kubernetes.io/version: 0.21.3
    app.kubernetes.io/component: history
    app.kubernetes.io/part-of: cadence    label-1: value-1
    label-2: value-2

I replaced indent with nindent which adds a new line as well before the substitution and that fixed the generated YAML.

# Source: cadence/templates/server-service-monitor.yaml
apiVersion: monitoring.coreos.com/v1
kind: ServiceMonitor
metadata:
  name: cadence-worker
  labels:
    app.kubernetes.io/name: cadence
    helm.sh/chart: cadence-0.21.1
    app.kubernetes.io/managed-by: Helm
    app.kubernetes.io/instance: cadence
    app.kubernetes.io/version: 0.21.3
    app.kubernetes.io/component: worker
    app.kubernetes.io/part-of: cadence
    label-1: value-1
    label-2: value-2

Checklist

  • Code meets the Developer Guide
  • User guide and development docs updated (if needed)
  • Related Helm chart(s) updated (if needed)

To Do

@pregnor pregnor force-pushed the fix/cadence-service-monitor-additional-labels branch from e496b6b to 0b154b2 Compare September 21, 2021 09:07
@pregnor pregnor merged commit 5707f13 into master Sep 21, 2021
@pregnor pregnor deleted the fix/cadence-service-monitor-additional-labels branch September 21, 2021 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Cadence] servicemonitor additional labels didn't work
4 participants