feat: Sentinel - support hostname resolve and annonce

[mkl262]

Description
This feature enables support for hostname resolution and announcing for sentinel configuration, for a more flexible configuration

Type of change

• New feature (non-breaking change which adds functionality)

Checklist

• Tests have been added/modified and all tests pass.
• Functionality/bugs have been confirmed to be unchanged or fixed.
• I have performed a self-review of my own code.
• Documentation has been updated or added where necessary.

Additional Context

An additional pull request was created in redis docker image repo to support new environment variables.
OT-CONTAINER-KIT/redis#94

[nicogigi92]

+1 for this

I tested this PR, and it works well on my side. This is a feature we would love to see included in the operator. Is it planned to be merged?

Just few comment if I may:

• It would be great to be able to add the cluster's internal domain to the sentinels' hostname. For performance reasons, it's sometimes better to avoid iterating over search domains by using FQDNs.

• I also noticed one missing piece: currently, only the cluster discovery happens via hostname, but the runtime still relies on IPs. The following needs to be added to ensure full hostname-based communication:
  sentinel announce-ip $HOSTNAME
  Without this, we end up with IP addresses in the Sentinel config instead of hostnames:
  sentinel known-sentinel myMaster 10.244.1.9 26379 17cfaeaa4102e39fdcb8336d7a992fedf13db4a1

• Last but not least, is there a planned PR to implement the same behavior for replication so that they communicate and announce using hostnames? By doing this, we could avoid having to push IP/HOSTNAME env var changes through the reconciliation loop. We could rely solely on hostnames, and Redis Sentinel would handle the master <-> slave failover on its own thanks to DNS resolution.

[drivebyer]

Fix the lint check plz @mkl262

[mkl262]

@nicogigi92

I am working on improving the redundancy of the redis replication after some internal test.
I am planing to add a service with a clusterIP for each of the pods and the service DNS or its IP will be used for replication between redis pods. This will allow each pod to have a static ip address.
I will also pass the list of services into sentinel and add them as known replicas.

@drivebyer should be fixed

[codecov]

Codecov Report

Attention: Patch coverage is 56.09756% with 18 lines in your changes missing coverage. Please review.

Project coverage is 34.97%. Comparing base (60ef5a2) to head (ecbf821).
Report is 50 commits behind head on master.

Files with missing lines	Patch %	Lines
pkg/k8sutils/redis-sentinel.go	56.09%	15 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1247      +/-   ##
==========================================
+ Coverage   30.20%   34.97%   +4.76%     
==========================================
  Files          55       49       -6     
  Lines        6294     5573     -721     
==========================================
+ Hits         1901     1949      +48     
+ Misses       4200     3452     -748     
+ Partials      193      172      -21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[drivebyer]

lgtm

[nicogigi92]

Thank you for the clarification @mkl262 !

However, I still don't understand why additional services are being added and why the operator is given control over the master. The replicas only need to announce their hostname (headless service) via announce-ip, and the failover will be handled by Sentinel without requiring an environment variable push (which would trigger a StatefulSet rollout).

Moreover, I still don't see the use of the announce-ip option in the Sentinel configuration, which, according to the official Redis documentation, seems necessary to avoid mixing hostnames and IP addresses.

When I ran tests, I found that using announce-ip <headless-svc> without the operator improved the high availability of a replication + Sentinel system because it eliminates the rollouts triggered by the operator while maintaining cluster synchronization over DNS.

Please let me know if I'm missing something.