Skip to content

fix redis sentinel client handling to solve authentication error with password protected sentinel - #7718

Closed
RndName wants to merge 2 commits into
BerriAI:mainfrom
RndName:fix_redis_sentinel
Closed

fix redis sentinel client handling to solve authentication error with password protected sentinel#7718
RndName wants to merge 2 commits into
BerriAI:mainfrom
RndName:fix_redis_sentinel

Conversation

@RndName

@RndName RndName commented Jan 12, 2025

Copy link
Copy Markdown

Title

Implement correct redis sentinel client handling to fix a bug with sentinel_password not working as intended (authentication error when sentinel is using requirepass) as no password would be used to authenticate against sentinel.

In the previous implementation sentinel_password was passed directly as "password" which will be used by sentinel as connection_kwargs to connect to the redis master and not for authentication of the sentinel node. For sentinel authentication the sentinel_kwargs dict will be used instead.

The fix adopts the implementation to match the redis-py documentation for the sentinel Client (https://redis-py.readthedocs.io/en/v5.0.0/connections.html#sentinel-client) and allows the user to set REDIS_PASSWORD and REDIS_SENTINEL_PASSWORD correctly. It is also possible now to handle situations where users use different passwords for redis and sentinel nodes.

Cite from the manual:

sentinel_kwargs is a dictionary of connection arguments used when connecting to sentinel instances. Any argument that can be passed to a normal Redis connection can be specified here. If sentinel_kwargs is not specified, any socket_timeout and socket_keepalive options specified in connection_kwargs will be used.

connection_kwargs are keyword arguments that will be used when establishing a connection to a Redis server.

Relevant issues

#6154

Type

🐛 Bug Fix

Changes

  • sentinel_password is now passed to sentinel_kwargs instead connection_kwargs
  • added additional connection_kwargs as it was done with redis_cluster implementation to support more redis parameters

[REQUIRED] Testing - Attach a screenshot of any new tests passing locally

Tested with a redis-sentinel cluster with 3 sentinels and 3 redis nodes in a kubernetes deployment

- sentinel_password is now passed to sentinel_kwargs correctly
- added additional connection_kwargs as it was done with redis_cluster implementation
- this allows users to set redis_password, sentinel_password and all other parameters as expected
@vercel

vercel Bot commented Jan 12, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
litellm ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 12, 2025 10:05pm

@RndName RndName changed the title fix redis sentinal client handling to solve authentication error with password protected sentinel fix redis sentinel client handling to solve authentication error with password protected sentinel Jan 12, 2025
@upfeatdev

Copy link
Copy Markdown

i just spent a day trying to figure out why sentinel wasnt working with my setup as well, so thanks for this

@sheng-kenbun

Copy link
Copy Markdown

this is very helpful also for enterprise use cases and should be merged asap.

@rocketb

rocketb commented Apr 7, 2025

Copy link
Copy Markdown

The current implementation will cause an error (got multiple values for keyword argument) if the parameter socket_timeout is passed within redis_kwargs.

To resolve this issue, we have two possible solutions:

  1. Explicitly remove socket_timeout from sentinel_kwargs by adding:
sentinel_kwargs.pop('socket_timeout')

or

  1. Directly use connection_kwargs when calling the master_for method:
return sentinel.master_for(service_name, **connection_kwargs)

Either approach will prevent the duplication of the socket_timeout argument and avoid the related runtime error.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@RndName RndName closed this Apr 23, 2025
@xfenix

xfenix commented Jun 16, 2025

Copy link
Copy Markdown

This is very actual issue

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.

6 participants