Skip to content

Allow Redis Connections to Use Sentinel #3984

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
dstarner opened this issue Jan 22, 2020 · 2 comments · Fixed by #4161
Closed

Allow Redis Connections to Use Sentinel #3984

dstarner opened this issue Jan 22, 2020 · 2 comments · Fixed by #4161

Comments

@dstarner
Copy link

dstarner commented Jan 22, 2020

Environment

  • Python version: 3.7.3
  • NetBox version: 2.7.2

Proposed Functionality

My proposal is to allow SENTINELS and SERVICE as optional keys to the REDIS dictionaries, like so. If these are both provided and non-empty, then the application would attempt to use the Redis Sentinel connection information. Note that the current way of configuring normal Redis would not change.

# Using the current method 
REDIS = {
    'webhooks': {
        'HOST': 'webhooks_redis',
        'PORT': 6379,
        'PASSWORD': '',
        'DATABASE': 0,
        'DEFAULT_TIMEOUT': 300,
        'SSL': False,
    },
    'caching': {
        'HOST': 'cache_redis',
        'PORT': 6379,
        'PASSWORD': '',
        'DATABASE': 0,
        'DEFAULT_TIMEOUT': 300,
        'SSL': False,
    }
}

# Using a Sentinel instance
REDIS = {
    'webhooks': {
        'SENTINELS': (
            ('sentinel.redis.example.com', 1234),  # Each is defined as (HOST, PORT) tuple
        ),
        'SERVICE': 'webhooks_redis',                   # Sentinel service to connect to
        'PASSWORD': '',
        'DATABASE': 0,
        'DEFAULT_TIMEOUT': 300,
        'SSL': False,
    },
    'caching': {
        'SENTINELS': (
            ('sentinel.redis.example.com', 1234),  # Each is defined as (HOST, PORT) tuple
        ),
        'SERVICE': 'webhooks_redis',                   # Sentinel service to connect to
        'PASSWORD': '',
        'DATABASE': 0,
        'DEFAULT_TIMEOUT': 300,
        'SSL': False,
    }
}

Use Case

It was nice that the webhooks and cache Redis connection settings were separated, but it would be ever nicer to be able to configure one (or both) to use Redis Sentinel connections for the sake of High Availability. Not only may this be preferred, but in some environments, it might be necessary due to the availability of developer environments.

Database Changes

None.

External Dependencies

None.

@kobayashi
Copy link
Contributor

Both of django-cacheops and django-rq support redis sentinel. So I would accept this FR. Let me start to figure out how to add them to settings.py, then make PR when accepted by other maintainers.

configuration.py is the same as @dstarner described and default comment out

# REDIS_SENTINEL = {
#     'webhooks': {
#         'SENTINELS': (
#             ('sentinel.redis.example.com', 1234),  # Each is defined as (HOST, PORT) tuple
#         ),
#         'MASTER': 'webhooks_redis',                   # Sentinel service to connect to
#         'PASSWORD': '',
#         'DATABASE': 0,
#         'SOCKET_TIMEOUT': 300,
#         'SSL': False,
#     },
#     'caching': {
#         'SENTINELS': (
#             ('sentinel.redis.example.com', 1234),  # Each is defined as (HOST, PORT) tuple
#         ),
#         'MASTER': 'webhooks_redis',                   # Sentinel service to connect to
#         'PASSWORD': '',
#         'DATABASE': 0,
#         'SOCKET_TIMEOUT': 300,
#         'SSL': False,
#     }
# }

settings.py

REDIS_SENTINEL = getattr(configuration, 'REDIS_SENTINEL', None)
if REDIS_SENTINEL:
    WEBHOOK_SENTINEL = REDIS_SENTINEL.get('webhooks', None)
    WEBHOOK_SENTINEL_SENTINELS = WEBHOOK_SENTINEL.get('SENTINELS', None)
    if not WEBHOOK_SENTINEL_SENTINELS:
        raise ImproperlyConfigured(
            "REDIS_SENTINEL section in configuration.py is missing SENTINELS subsection."
        )
    WEBHOOK_SENTINEL_MASTER = WEBHOOK_SENTINEL.get('MASTER', None)
    if not WEBHOOK_SENTINEL_MASTER:
        raise ImproperlyConfigured(
            "REDIS_SENTINEL section in configuration.py is missing MASTER subsection of webhooks."
        )
    WEBHOOK_SENTINEL_DB = WEBHOOK_SENTINEL.get('DATABASE', 0)
    WEBHOOK_SENTINEL_PASSWORD = WEBHOOK_SENTINEL.get('PASSWORD', None)
    WEBHOOK_SENTINEL_SOCKET_TIMEOUT = WEBHOOK_SENTINEL.get('SOCKET_TIMEOUT', None)
    WEBHOOK_SENTINEL_SSL = WEBHOOK_SENTINEL.get('SSL', False)

    RQ_QUEUES.extend({
        'SENTINELS': WEBHOOK_SENTINEL_SENTINELS,
        'MASTER_NAME': WEBHOOK_SENTINEL_MASTER,
        'DB': WEBHOOK_SENTINEL_DB,
        'SOCKET_TIMEOUT': WEBHOOK_SENTINEL_SOCKET_TIMEOUT,
        'PASSWORD': WEBHOOK_SENTINEL_PASSWORD,
        'SSL': WEBHOOK_SENTINEL_SSL,
    })
    
    CACHING_SENTINEL = REDIS_SENTINEL.get('caching', None)
    if CACHING_SENTINEL:
        # CACHEOPS_REDIS needs to be None for cacheops to enable redis sentinel
        CACHEOPS_REDIS = None
        CACHING_SENTINEL_LOCATIONS = CACHING_SENTINEL.get('SENTINELS', None)
        if not CACHEOPS_SENTINEL_LOCATIONS:
            raise ImproperlyConfigured(
                "REDIS_SENTINEL section in configuration.py is missing SENTINELS subsection of cacheops.")
        CACHING_SENTINEL_SERVICE_NAME = CACHING_SENTINEL.get('MASTER', None)
        if not CACHEOPS_SENTINEL_SERVICE_NAME:
            raise ImproperlyConfigured(
                "REDIS_SENTINEL section in configuration.py is missing MASTER subsection of cacheops.")
        CACHING_SENTINEL_SOCKET_TIMEOUT = CACHING_SENTINEL.get('SOCKET_TIMEOUT', None)
        CACHING_SENTINEL_DB = CACHING_SENTINEL.get('DATABASE', 0)
        CACHING_SENTINEL_PASSWORD = CACHING_SENTINEL.get('PASSWORD', None)
        CACHING_SENTINEL_SSL = CACHING_SENTINEL.get('SSL', False)
        CACHEOPS_SENTINEL = {
            'locations': [CACHING_SENTINEL_LOCATIONS],
            'service_name': CACHING_SENTINEL_SERVICE_NAME,
            'db': CACHING_SENTINEL_DB,
            'socket_timeout': CACHING_SENTINEL_SOCKET_TIMEOUT,
            'password': CACHING_SENTINEL_PASSWORD,
            'ssl': CACHING_SENTINEL_SSL,
        }

@dstarner
Copy link
Author

dstarner commented Feb 5, 2020

@kobayashi I can probably turn my hack into the solution you have above. I already have something similar implemented, I would just need to clean it up and upstream it into the main project.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants