-
Notifications
You must be signed in to change notification settings - Fork 22
[FAL-2031] Add Redis support for Ocim provisioned instances #822
Conversation
2307843
to
13bf306
Compare
@gabor-boros somewhere we will need to set the acl policy for the key prefix for an instance. I'm not sure if it's been done already? I couldn't find it. |
@swalladge We are setting the key prefix based on the Redis username, and creating the ACL using the prefix. When the key prefix exists (we have a Redis username) and an ACL is created for that, the only thing left to do is setting the |
@gabor-boros ah got it, thanks 👍 |
eaed5a8
to
eea5363
Compare
The current version does not support other usernames than default and cannot set ACLs.
Add more information about how the setting is used and stored.
In case the instance configuration explicitly states to use redis or the instance has no prior app servers, let's use redis over rabbit.
Remove the `RedisUser` model and refactor the related provisioning function where necessary.
In case we provisioned a redis server over rabbit mq, ensure we use redis server over rabbit mq for the next time as well.
eea5363
to
af7bba7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I tested this: tested async tasks on stage appserver
- I read through the code
- [NA] I checked for accessibility issues
- Includes documentation
- I made sure any change in configuration variables is reflected in the corresponding
client'sconfiguration-secure
repository.
Fantastic work here @gabor-boros and @swalladge ! I'm happy with what's running on stage and the recent fixes, so this is ready to go live :) |
Remove the heuristics that switches to a redis cache_db automatically for new instances. We only want instances to provision and use redis if explicitly configured to.
The tests in this file duplicate those in instance/tests/models/test_openedx_theme_mixins.py Looks like a copy/paste error.
adabbf0
to
2156944
Compare
This Pull Request adds Redis cache support to Ocim. The changes won't take effect on already provisioned servers, but new ones or those that explicitly set to use Redis over RabbitMQ. This way we can slowly roll out changes for every server while we are not generating more work when new instances are created.
DependenciesRelated PRs:Screenshots:
Admin site integration of
RedisServer
Instance admin configuration
Redis cluster showing the Ocim created user and its ACL
Sandbox URL: Upon request, I can create an ngrok URL for my local Ocim
Merge deadline: None
Testing instructions:
.env
file (ie.DEFAULT_INSTANCE_REDIS_URL
)A note about staging env: The instance creation nowadays is hectic -- it seems for some instances the LB URL registration did not work.
Author notes and concerns:
N/A
Reviewers