[bitnami/redis] Add Redis Sentinel Exporter #4916

ricoberger · 2021-01-07T20:43:11Z

Description of the change

Add support to get Prometheus metrics via the Redis Sentinel Exporter. For that the sentinel configuration contains a new section metrics, which adds an additional sidecar with the Redis Sentinel Exporter, when enabled.

Benefits

This allows a user to monitor Redis Sentinel within the Helm chart.

Applicable issues

closes [bitnami/redis] Add support for Redis Sentinel metrics #4909

Additional information

Currently this uses the following image from Docker Hub: https://hub.docker.com/r/leominov/redis_sentinel_exporter. The corresponding GitHub repository can be found here: https://github.com/leominov/redis_sentinel_exporter.

As mentioned by @marcosbc in #4909 let me know when the Bitnami image is ready, so that I can adjust the values file.

Checklist

[ x] Chart version bumped in Chart.yaml according to semver.
[ x] Variables are documented in the README.md
[ x] Title of the PR starts with chart name (e.g. [bitnami/chart])
If the chart contains a values-production.yaml apart from values.yaml, ensure that you implement the changes in both files

Add support to get Prometheus metrics via the Redis Sentinel Exporter. For that the sentinel configuration contains a new section metrics, which adds an additional sidecar with the Redis Sentinel Exporter, when enabled.

juan131

Thanks so much for your PR! Please check my comments

bitnami/redis/README.md

bitnami/redis/templates/metrics-sentinel-prometheus.yaml

bitnami/redis/templates/metrics-sentinel-svc.yaml

bitnami/redis/values.yaml

bitnami/redis/templates/metrics-sentinel-svc.yaml

bitnami/redis/templates/redis-node-statefulset.yaml

bitnami/redis/values.yaml

ricoberger · 2021-01-08T09:23:24Z

Hi @juan131, thanks for your fast review, I will adjust the PR.

The Bitnami image isn't available yet. The creation of the image is tracked in #4909 and I would adjust the PR once it is ready.

juan131 · 2021-01-11T09:59:48Z

The Bitnami image isn't available yet. The creation of the image is tracked in #4909 and I would adjust the PR once it is ready.

Great! Thanks so much

github-actions · 2021-03-23T15:55:51Z

Great PR! Please pay attention to the following items before merging:

Files matching bitnami/*/values.yaml:

Is the PR adding a new container? Please reviewer, add it to the models (internal process)
Is the PR adding a new parameter? Please, ensure it’s documented in the README.md

This is an automatically generated QA checklist based on modified files

migruiz4 · 2021-03-23T16:44:27Z

Hi @ricoberger ,

The bitnami/redis-sentinel-exporter image is now ready so we can merge this PR.

@juan131 would you mind re-reviewing it?

bitnami/redis/templates/metrics-sentinel-svc.yaml

ricoberger · 2021-03-23T20:54:16Z

Hi @ricoberger ,

The bitnami/redis-sentinel-exporter image is now ready so we can merge this PR.

@juan131 would you mind re-reviewing it?

Awesome thank you very much 🙂🎉

juan131

Some minor suggestions

bitnami/redis/README.md

bitnami/redis/templates/metrics-sentinel-prometheus.yaml

bitnami/redis/values.yaml

ricoberger · 2021-03-24T20:03:00Z

Some minor suggestions

@juan131 thank you, I applied your suggestions.

Edit: Regarding my comment with the port name for the Sentinel Exporter: I would come up with an adjustment in a following PR, because I think this should also be changed for the Redis Exporter, if this is ok.

miguelaeh

Thank you very much for the PR!

miguelaeh · 2021-03-25T10:38:58Z

bitnami/redis/templates/metrics-sentinel-prometheus.yaml

+ app: {{ template "redis.name" . }}
+ chart: {{ template "redis.chart" . }}
+ release: {{ .Release.Name }}
+ heritage: {{ .Release.Service }}
+ {{- range $key, $value := .Values.sentinel.metrics.serviceMonitor.selector }}
+ {{ $key }}: {{ $value | quote }}
+ {{- end }}


These are the old labels, please use the common.labels function

miguelaeh · 2021-03-25T10:39:16Z

bitnami/redis/templates/metrics-sentinel-svc.yaml

+ app: {{ template "redis.name" . }}
+ chart: {{ template "redis.chart" . }}
+ release: {{ .Release.Name }}
+ heritage: {{ .Release.Service }}
+ app.kubernetes.io/component: "sentinel-metrics"
+ {{- if .Values.sentinel.metrics.service.labels -}}
+ {{- toYaml .Values.sentinel.metrics.service.labels | nindent 4 }}
+ {{- end -}}


miguelaeh · 2021-03-25T10:39:42Z

bitnami/redis/templates/metrics-sentinel-svc.yaml

+ app: {{ template "redis.name" . }}
+ release: {{ .Release.Name }}


After changing the labels you will need to change the selector as well

bitnami/redis/templates/redis-node-statefulset.yaml

ricoberger · 2021-03-25T12:02:07Z

Thank you very much for the PR!

@miguelaeh before I add your suggestions, I just want to make sure that this is really the way to go, because this would increase the inconsistent in this chart even more.

migruiz4 · 2021-03-25T12:15:54Z

Hi @miguelaeh ,

In this case I agree with @ricoberger.

Please notice that all the changes you suggested are imported from the metrics (redis-exporter) configuration:

charts/bitnami/redis/templates/metrics-prometheus.yaml

Lines 12 to 18 in e105226

 app: {{ template "redis.name" . }} 

 chart: {{ template "redis.chart" . }} 

 release: {{ .Release.Name }} 

 heritage: {{ .Release.Service }} 

 {{- range $key, $value := .Values.metrics.serviceMonitor.selector }} 

 {{ $key }}: {{ $value | quote }} 

 {{- end }}

charts/bitnami/redis/templates/redis-node-statefulset.yaml

Lines 353 to 359 in e105226

 {{- if and .Values.usePassword (not .Values.usePasswordFile) }} 

 - name: REDIS_PASSWORD 

 valueFrom: 

 secretKeyRef: 

 name: {{ template "redis.secretName" . }} 

 key: {{ template "redis.secretPasswordKey" . }} 

 {{- end }}

charts/bitnami/redis/templates/metrics-svc.yaml

Lines 1 to 34 in e105226

 {{- if .Values.metrics.enabled }} 

 apiVersion: v1 

 kind: Service 

 metadata: 

 name: {{ template "redis.fullname" . }}-metrics 

 namespace: {{ .Release.Namespace | quote }} 

 labels: 

 app: {{ template "redis.name" . }} 

 chart: {{ template "redis.chart" . }} 

 release: {{ .Release.Name }} 

 heritage: {{ .Release.Service }} 

 app.kubernetes.io/component: "metrics" 

 {{- if .Values.metrics.service.labels -}} 

 {{- toYaml .Values.metrics.service.labels | nindent 4 }} 

 {{- end -}} 

 {{- if .Values.metrics.service.annotations }} 

 annotations: {{- toYaml .Values.metrics.service.annotations | nindent 4 }} 

 {{- end }} 

 spec: 

 type: {{ .Values.metrics.service.type }} 

 {{ if eq .Values.metrics.service.type "LoadBalancer" }} 

 externalTrafficPolicy: {{ .Values.metrics.service.externalTrafficPolicy }} 

 {{- end }} 

 {{ if and (eq .Values.metrics.service.type "LoadBalancer") .Values.metrics.service.loadBalancerIP }} 

 loadBalancerIP: {{ .Values.metrics.service.loadBalancerIP }} 

 {{- end }} 

 ports: 

 - name: metrics 

 port: 9121 

 targetPort: metrics 

 selector: 

 app: {{ template "redis.name" . }} 

 release: {{ .Release.Name }} 

 {{- end }}

migruiz4 · 2021-03-25T12:18:17Z

We could adapt those things in this PR, but I'd incline for being consistent at the moment with the content of the chart and then make a second PR to standardize the Redis chart with a major release.

halradaideh · 2021-03-28T15:32:23Z

when it is expected to be merged ?

bitnami/redis/values.yaml

bitnami/redis/templates/redis-node-statefulset.yaml

bitnami/redis/templates/metrics-sentinel-svc.yaml

miguelaeh · 2021-03-30T09:39:10Z

Hi guys, sorry I didn't notice about we were still using the old labels on this chart. I thought we updated them in the last major change. Forget about those labels comments

ricoberger · 2021-03-30T15:03:19Z

@miguelaeh thanks for your review. I applied your suggestions, hopefully I didn't forgot sth.

miguelaeh

Hi @ricoberger,
I added just some minor comments again, after that it look good to me

bitnami/redis/Chart.yaml

bitnami/redis/templates/metrics-sentinel-prometheus.yaml

bitnami/redis/templates/metrics-sentinel-svc.yaml

bitnami/redis/templates/redis-node-statefulset.yaml

miguelaeh

LGTM!

This reverts commit b27be24.

Add Redis Sentinel Exporter

91383b7

Add support to get Prometheus metrics via the Redis Sentinel Exporter. For that the sentinel configuration contains a new section metrics, which adds an additional sidecar with the Redis Sentinel Exporter, when enabled.

ricoberger mentioned this pull request Jan 7, 2021

[bitnami/redis] Add support for Redis Sentinel metrics #4909

Closed

Remove trailing spaces

3927462

juan131 requested changes Jan 8, 2021

View reviewed changes

ricoberger added 4 commits January 8, 2021 10:32

Apply suggestions from the PR review

a053f0d

Merge branch 'master' into add-redis-sentinel-exporter

8fa4e2c

Fix linter error

708c3cc

Fix component name for Redis Sentinel metrics

da2c3fc

juan131 added the on-hold Issues or Pull Requests with this label will never be considered stale label Jan 11, 2021

ricoberger and others added 4 commits February 4, 2021 10:12

Merge branch 'master' into add-redis-sentinel-exporter

549901d

Merge branch 'master' into add-redis-sentinel-exporter

85a0bd7

Merge branch 'master' into add-redis-sentinel-exporter

40a64e8

Update PR with bitnami/redis-sentinel-exporter

4c8c2f6

Miguel Ruiz added 2 commits March 23, 2021 17:37

Redis Sentinel metrics service port configurable

e523bba

Update README.md with default port 9355

c30b2d0

migruiz4 requested a review from juan131 March 23, 2021 16:44

ricoberger commented Mar 23, 2021

View reviewed changes

bitnami/redis/templates/metrics-sentinel-svc.yaml Show resolved Hide resolved

juan131 reviewed Mar 24, 2021

View reviewed changes

ricoberger added 2 commits March 24, 2021 20:58

Apply review suggestions

4be251e

Apply review suggestions

db47a8d

miguelaeh suggested changes Mar 25, 2021

View reviewed changes

miguelaeh suggested changes Mar 30, 2021

View reviewed changes

ricoberger added 4 commits March 30, 2021 16:34

Apply review suggestions

e5839da

Apply review suggestions

0a8f646

Merge branch 'master' into add-redis-sentinel-exporter

2d01431

Apply review suggestions

ca7462b

miguelaeh suggested changes Mar 31, 2021

View reviewed changes

ricoberger added 2 commits March 31, 2021 11:05

Apply review suggestions

bafe862

Merge branch 'master' into add-redis-sentinel-exporter

df63b7d

miguelaeh approved these changes Mar 31, 2021

View reviewed changes

miguelaeh merged commit b27be24 into bitnami:master Mar 31, 2021

miguelaeh added a commit that referenced this pull request Mar 31, 2021

Revert "[bitnami/redis] Add Redis Sentinel Exporter (#4916)"

78b3372

This reverts commit b27be24.

FraPazGal mentioned this pull request Apr 5, 2021

[bitnami/redis] Updating redis version from 6.0 to 6.2, bumping major version of the chart #5990

Merged

3 tasks

ekampf mentioned this pull request Apr 8, 2021

[bitnami/redis] Fix for NetworkPolicy blocking sentinel metrics exporter #6050

Merged

2 tasks

carrodher removed the on-hold Issues or Pull Requests with this label will never be considered stale label Dec 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[bitnami/redis] Add Redis Sentinel Exporter #4916

[bitnami/redis] Add Redis Sentinel Exporter #4916

ricoberger commented Jan 7, 2021

juan131 left a comment

ricoberger commented Jan 8, 2021

juan131 commented Jan 11, 2021

github-actions bot commented Mar 23, 2021 •

edited

Loading

migruiz4 commented Mar 23, 2021

ricoberger commented Mar 23, 2021

juan131 left a comment

ricoberger commented Mar 24, 2021 •

edited

Loading

miguelaeh left a comment

miguelaeh Mar 25, 2021

miguelaeh Mar 25, 2021

miguelaeh Mar 25, 2021

ricoberger commented Mar 25, 2021

migruiz4 commented Mar 25, 2021

migruiz4 commented Mar 25, 2021 •

edited

Loading

halradaideh commented Mar 28, 2021

miguelaeh commented Mar 30, 2021

ricoberger commented Mar 30, 2021

miguelaeh left a comment

miguelaeh left a comment

		app: {{ template "redis.name" . }}
		release: {{ .Release.Name }}

[bitnami/redis] Add Redis Sentinel Exporter #4916

[bitnami/redis] Add Redis Sentinel Exporter #4916

Conversation

ricoberger commented Jan 7, 2021

juan131 left a comment

Choose a reason for hiding this comment

ricoberger commented Jan 8, 2021

juan131 commented Jan 11, 2021

github-actions bot commented Mar 23, 2021 • edited Loading

migruiz4 commented Mar 23, 2021

ricoberger commented Mar 23, 2021

juan131 left a comment

Choose a reason for hiding this comment

ricoberger commented Mar 24, 2021 • edited Loading

miguelaeh left a comment

Choose a reason for hiding this comment

miguelaeh Mar 25, 2021

Choose a reason for hiding this comment

miguelaeh Mar 25, 2021

Choose a reason for hiding this comment

miguelaeh Mar 25, 2021

Choose a reason for hiding this comment

ricoberger commented Mar 25, 2021

migruiz4 commented Mar 25, 2021

migruiz4 commented Mar 25, 2021 • edited Loading

halradaideh commented Mar 28, 2021

miguelaeh commented Mar 30, 2021

ricoberger commented Mar 30, 2021

miguelaeh left a comment

Choose a reason for hiding this comment

miguelaeh left a comment

Choose a reason for hiding this comment

github-actions bot commented Mar 23, 2021 •

edited

Loading

ricoberger commented Mar 24, 2021 •

edited

Loading

migruiz4 commented Mar 25, 2021 •

edited

Loading