[bitnami/redis] use hostnames in sentinel

[tomislater]

Description of the change

Because it is a chart for Kubernetes, we should use hostnames instead of IPs (https://redis.io/topics/sentinel#ip-addresses-and-dns-names). I added more information about my case in #1682 and #5418.

Benefits

It should works better in dynamic environment.

Possible drawbacks

I don't know yet. Maybe there is a bug somewhere. I have tested on my clusters and everything works correctly. But still...

Applicable issues

• fixes [bitnami/redis] Sentinel return wrong IP #1682
• fixes [bitnami/redis] After failover, there will be two masters (with simple reproducible experiments included!) #5418

Checklist

• Chart version bumped in Chart.yaml according to semver.
• Variables are documented in the README.md
• Title of the PR starts with chart name (e.g. [bitnami/<name_of_the_chart>])

After this change sentinel.conf looks like:

cat opt/bitnami/redis-sentinel/etc/sentinel.conf
cat opt/bitnami/redis-sentinel/etc/sentinel.conf
dir "/tmp"
port 26379
sentinel monitor mymaster my-redis-node-1.my-redis-headless.my-redis.svc.cluster.local 6379 2
sentinel down-after-milliseconds mymaster 5000
sentinel failover-timeout mymaster 18000

# User-supplied sentinel configuration:
sentinel resolve-hostnames yes
sentinel announce-hostnames yes
# End of sentinel configuration
sentinel myid 475427d0cefa44aff518ab6b888c3510314d882c
sentinel announce-ip "my-redis-node-1.my-redis-headless.my-redis.svc.cluster.local"

sentinel known-sentinel mymaster my-redis-node-2.my-redis-headless.my-redis.svc.cluster.local 26379 80d3b99129c4f9bb62cdcec2399499f2c4666789

sentinel known-replica mymaster my-redis-node-0.my-redis-headless.my-redis.svc.cluster.local 6379

sentinel known-sentinel mymaster my-redis-node-0.my-redis-headless.my-redis.svc.cluster.local 26379 cae9f9b5ed951299f4bdc7de1da8e996e7b7197c

sentinel known-replica mymaster my-redis-node-2.my-redis-headless.my-redis.svc.cluster.local 6379
# Generated by CONFIG REWRITE
protected-mode no
user default on nopass ~* &* +@all
sentinel config-epoch mymaster 1
sentinel leader-epoch mymaster 1
sentinel current-epoch 1

[tomislater]

After deleting different pods x times (to force re-election and try to find potential bugs) :

cat opt/bitnami/redis-sentinel/etc/sentinel.conf
dir "/tmp"
port 26379
sentinel monitor mymaster my-redis-node-1.my-redis-headless.my-redis.svc.cluster.local 6379 2
sentinel down-after-milliseconds mymaster 5000
sentinel failover-timeout mymaster 18000

# User-supplied sentinel configuration:
sentinel resolve-hostnames yes
sentinel announce-hostnames yes
# End of sentinel configuration
sentinel myid 80d3b99129c4f9bb62cdcec2399499f2c4666789
sentinel announce-ip "my-redis-node-2.my-redis-headless.my-redis.svc.cluster.local"

sentinel known-sentinel mymaster my-redis-node-0.my-redis-headless.my-redis.svc.cluster.local 26379 cae9f9b5ed951299f4bdc7de1da8e996e7b7197c

sentinel known-replica mymaster my-redis-node-0.my-redis-headless.my-redis.svc.cluster.local 6379

sentinel known-sentinel mymaster my-redis-node-1.my-redis-headless.my-redis.svc.cluster.local 26379 475427d0cefa44aff518ab6b888c3510314d882c

sentinel known-replica mymaster my-redis-node-2.my-redis-headless.my-redis.svc.cluster.local 6379
# Generated by CONFIG REWRITE
protected-mode no
user default on nopass ~* &* +@all
sentinel config-epoch mymaster 3
sentinel leader-epoch mymaster 3
sentinel current-epoch 4

[tomislater]

I also tested it with haproxy as a "fronted" and this also works.

[juan131]

Hi @tomislater

Thanks so much for this great contribution!!!

I agree that we shouldn't be use IPs on a Kubernetes environment since they're ephemeral, and we should rely on stable network identifiers such as the hostname. Therefore, I'm fine with your approach.

I'd like @rafariossaa to double-check this, since he has more experience with Redis & Sentinel configuration. He's currently on PTO so please expect a few days delay before he comes back to you.

[github-actions]

This Pull Request has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thank you for your contribution.

[tomislater]

👋 what about this PR? It works on staging and prod since then and I don’t see any problems.

I had one problem because I had not changed masterSet (both deployments had the same masterSet) and two separated sentinels (in different namespaces) were thinking that they are one cluster 🤝, but it is another issue.

[juan131]

Sorry @tomislater I lost the track of this one!! Please @rafariossaa take a look to this PR when possible.

[rafariossaa]

Hi,
I am taking a look to it. For me it makes sense.
Please, allow me a couple of days to do some testing in my side before merging.

[tomislater]

⚠️ Hmm, strange. I have two deployments (with sentinels) in different namespaces with different masterSets.
Sometimes, when I delete two pods at the same time (one in namspace A and one in namespace B) sentinel from namespace A thinks that replica from namespace B belongs to him. And vice cersa.

It is very hard to reproduce. Only twice I have seen this.

Maybe is it something wrong with announce-hostnames or replica-announce-ip 🤔

[rafariossaa]

Hi,
In that case I will wait until this is clarified before merging it.

[tomislater]

Yeah, please do not merge it. I see that there were some new changes on master.

[tomislater]

Okay, my changes are not required anymore. We can close this PR. Similar changes have been added here: 722dead
15.4.0 uses hostnames instead of names.

But, I have noticed the same problems. Auto-discovery seems broken. I am going to create an issue for this.

[rafariossaa]

Hi,
In that case I am closing it.
Thanks

[vfiset]

Hi @tomislater could you please expand on what you've refered to as:

Auto-discovery seems broken

in your last comment?

I am still having some issues from time to time even when using the version containing #7461

thanks

[tomislater]

I referred to this problem: #7428 (comment) but it was visible only on my branch. With current version everything works correctly.

https://github.com/bitnami/charts/tree/master/bitnami/redis#redis-sentinel-configuration-parameters remember about masterSet setting.