Conversation
Signed-off-by: Richard Boldiš <richard@boldis.dev>
Signed-off-by: Richard Boldiš <richard@boldis.dev>
a4af4d1 to
da69b98
Compare
|
Could you please bump the helm chart version? Another PR was merged before yours that bumped the version to 3.5.0, so if you could bump to 3.5.1, this would resolve the conflict. Thank you and thanks for this contribution! :) |
Signed-off-by: Richard Boldiš <richard@boldis.dev>
Signed-off-by: Richard Boldiš <richard@boldis.dev>
|
Hello @jessebot , I bumped the version to |
Signed-off-by: JesseBot <jessebot@linux.com>
Signed-off-by: Richard Boldiš <richard@boldis.dev>
84f3ce9 to
133923d
Compare
Signed-off-by: JesseBot <jessebot@linux.com>
Signed-off-by: JesseBot <jessebot@linux.com>
| - {{ .Values.metrics.serviceMonitor.namespace | quote }} | ||
| {{- else }} | ||
| - {{ .Release.Namespace | quote }} | ||
| {{- end }} |
There was a problem hiding this comment.
with this whole block, it might need to be moved left two spaces to line up with matchNames.
matchNames:
{{- if .Values.metrics.serviceMonitor.namespace }}
- {{ .Values.metrics.serviceMonitor.namespace | quote }}
{{- else }}
- {{ .Release.Namespace | quote }}
{{- end }}This would match the way we're doing the gotemplates everywhere else like:
- port: metrics
path: "/"
{{- if .Values.metrics.serviceMonitor.interval }}
interval: {{ .Values.metrics.serviceMonitor.interval }}
{{- end }}
{{- if .Values.metrics.serviceMonitor.scrapeTimeout }}
scrapeTimeout: {{ .Values.metrics.serviceMonitor.scrapeTimeout }}
{{- end }}It's been a long week, @provokateurin can you just confirm I'm making sense here? I don't have time to test the helm chart directly for this specific issue right this second, but I will be tackling metrics probably in another month or two for a work project.
There was a problem hiding this comment.
Hello @jessebot,
I offset the list by two fewer spaces. At the moment, it seems that it is not uniform across the project. I created another MR to tweak this.
There was a problem hiding this comment.
I missed this ages ago and now your other PR makes sense :) this was also before I realized you have a bit more freedom when using gotemplating :)
wrenix
left a comment
There was a problem hiding this comment.
I miss a documentation in the values.yaml
| jobLabel: {{ .Values.metrics.serviceMonitor.jobLabel | quote }} | ||
| namespaceSelector: | ||
| matchNames: | ||
| {{- if .Values.metrics.serviceMonitor.namespace }} |
There was a problem hiding this comment.
{{- with .Values.metrics.serviceMonitor.namespaces }}
{{- toYaml . | nindent 6 }}
There was a problem hiding this comment.
- with is easy reable
- so multiple namespaces are configurable
|
please see wrenix's comment, bump the chart version, and sign off your commits and then this can be merged. |
|
i believe we could close this PR now ;) |
|
You're right |
Pull Request
Description of the change
The current configuration does not work when the monitored service runs outside of the
serviceMonitor's namespace.Other helm charts, such as bitnami/redis use
namespaceSelectorto fix this [1].Benefits
The
serviceMonitoris now able to find the monitored service.Possible drawbacks
Applicable issues
Additional information
Example of
metricssection fromvalues.yaml:Checklist
Chart.yamlaccording to semver.