Skip to content
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

Support Redis Unix sockets #15962

Closed
tacerus opened this issue May 6, 2024 · 6 comments · Fixed by #16227
Closed

Support Redis Unix sockets #15962

tacerus opened this issue May 6, 2024 · 6 comments · Fixed by #16227
Assignees
Labels
complexity: medium Requires a substantial but not unusual amount of effort to implement status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application

Comments

@tacerus
Copy link

tacerus commented May 6, 2024

NetBox version

3.7.5

Feature type

Change to existing functionality

Proposed functionality

Support Redis connectivity through Unix sockets

Use case

Binding to Redis servers on localhost or sidecars without exposing a TCP listener.

Database changes

No response

External dependencies

No response

@tacerus tacerus added status: needs triage This issue is awaiting triage by a maintainer type: feature Introduction of new functionality to the application labels May 6, 2024
@tacerus
Copy link
Author

tacerus commented May 6, 2024

#15955
#4377
#15590

@arthanson
Copy link
Collaborator

From previous tickets:

Proposed Functionality

Implement unix-socket support for Redis cache and webhook

Use Case

When Redis is installed on the same Server as netbox, then the possible usage of unix sockets for redis would be nice. When using unix sockets, there is no need for localhost loopback with the whole IP-Stack. The usage with unix sockets can improve the performance too (even if it doesn't matter).

Database Changes

No DB-Changes needed. Just some small changes in settings.py and configuration.py

@arthanson arthanson removed their assignment May 6, 2024
@arthanson arthanson added status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation and removed status: needs triage This issue is awaiting triage by a maintainer labels May 6, 2024
@minijackson
Copy link
Contributor

Hello, I have a patch that I'm willing to upstream that was made to support Redis Unix sockets.

(Note: the linked patch also modifies STATIC_ROOT, but this modification is not part of the proposed changes)

This patch defines new REDIS['tasks']['URL'] and REDIS['caching']['URL'] settings, which take priority if defined by the user. The user can then set this config if they want to use Unix sockets:

REDIS = {
    "tasks": {"URL": "unix:///run/redis-netbox/redis.sock?db=0", "SSL": False},
    "caching": {"URL": "unix:///run/redis-netbox/redis.sock?db=1", "SSL": False},
}

In the case of the tasks Redis database, if and only if URL is defined, NetBox would use it.

In the case of the caching Redis database, URL is always used, but defaults to the value that was used before the patch.

This should be backwards compatible.

@jeremystretch
Copy link
Member

@minijackson sounds good, I'll assign this to you for a PR. Thanks!

@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation labels May 7, 2024
@jeremystretch
Copy link
Member

@minijackson are you still interested in working on this?

@jeremystretch jeremystretch added the complexity: medium Requires a substantial but not unusual amount of effort to implement label May 21, 2024
@minijackson
Copy link
Contributor

Yes, sorry. Went on a quick vacation, then that task fell through the cracks. I'll submit a PR soon.

minijackson added a commit to minijackson/netbox that referenced this issue May 21, 2024
jeremystretch added a commit that referenced this issue May 21, 2024
* Fixes #15962: support Redis Unix sockets

* Clean up language & remove obsolete note

---------

Co-authored-by: Jeremy Stretch <jstretch@netboxlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complexity: medium Requires a substantial but not unusual amount of effort to implement status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants