Skip to content

Remove duplicate relabings and metricRelabelings#1830

Merged
fedepaol merged 1 commit intometallb:mainfrom
GuillaumeSmaha:patch-1
Mar 20, 2023
Merged

Remove duplicate relabings and metricRelabelings#1830
fedepaol merged 1 commit intometallb:mainfrom
GuillaumeSmaha:patch-1

Conversation

@GuillaumeSmaha
Copy link
Copy Markdown
Contributor

When I am trying to use relabelings on ServiceMonitor, I get:

Error: unable to build kubernetes objects from release manifest: error validating \"\": error validating data: ValidationError(ServiceMonitor.spec): unknown field \"relabelings\" in com.coreos.monitoring.v1.ServiceMonitor.spec

On https://raw.githubusercontent.com/prometheus-operator/prometheus-operator/v0.63.0/example/prometheus-operator-crd/monitoring.coreos.com_servicemonitors.yaml, relabings and metricRelabelings are only defined at one place.

And there are already used on line 21

selector:
matchLabels:
name: {{ template "metallb.fullname" . }}-speaker-monitor-service
{{- if .Values.prometheus.serviceMonitor.metricRelabelings }}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, metricRelabelings and relabelings are ServiceMonitor .spec.endpoints property and not .spec fields itself.

matchLabels:
name: {{ template "metallb.fullname" . }}-speaker-monitor-service
{{- if .Values.prometheus.serviceMonitor.metricRelabelings }}
metricRelabelings:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure 👍

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is not yet moved under endpoints as similar to speaker-monitor.

Copy link
Copy Markdown
Contributor Author

@GuillaumeSmaha GuillaumeSmaha Feb 20, 2023

Choose a reason for hiding this comment

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

Ohh ok sorry, I didn't understand you wanted to move it 🤦

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

no problem, it looks fine now. just noticed you've 3 commits for same kind of changes, can you just do it with a single commit ?

@GuillaumeSmaha
Copy link
Copy Markdown
Contributor Author

It is done

@pperiyasamy
Copy link
Copy Markdown
Contributor

LGTM

@fedepaol
Copy link
Copy Markdown
Member

@GuillaumeSmaha thanks for the PR! Mind fixing the commit message according to the ci failure?

@fedepaol
Copy link
Copy Markdown
Member

I think what's missing is just an empty line after the subj

Move metricRelabelings & relabelings for controller-monitor

Error when trying to use `relabelings` on ServiceMonitor:
```
Error: unable to build kubernetes objects from release manifest: error validating \"\": error validating data: ValidationError(ServiceMonitor.spec): unknown field \"relabelings\" in com.coreos.monitoring.v1.ServiceMonitor.spec
```

On https://raw.githubusercontent.com/prometheus-operator/prometheus-operator/v0.63.0/example/prometheus-operator-crd/monitoring.coreos.com_servicemonitors.yaml, `relabings` and `metricRelabelings` are only defined at one place.

Remove other part with relabings and metricRelabelings
@fedepaol fedepaol added this pull request to the merge queue Mar 20, 2023
@fedepaol fedepaol merged commit 0946b0e into metallb:main Mar 20, 2023
@GuillaumeSmaha GuillaumeSmaha deleted the patch-1 branch March 21, 2023 10:30
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.

3 participants