[FAL-2031] Add Redis support for Ocim provisioned instances #822

gabor-boros · 2021-07-01T14:12:39Z

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.

~~Dependencies~~ Related 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:

Checkout the changes on Ocim stage
Create a Redis user and ACL for Ocim if no user was created (or use the default user) and double-check the connection to the Redis server
Set the necessary environment variables in the .env file (ie. DEFAULT_INSTANCE_REDIS_URL )
Restart Ocim
Provision a new app server
Login with edX user
Go to instructor/download data tab
Download a grades report (as that uses celery, that would use Redis now)
Validate the instructor task is finished successfully

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

instance/models/redis.py

documentation/configuration.md

instance/models/mixins/openedx_database.py

instance/models/mixins/redis.py

instance/models/redis_server.py

instance/models/mixins/redis.py

instance/models/mixins/openedx_database.py

samuelallan72 · 2021-08-16T03:34:25Z

@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.

gabor-boros · 2021-08-16T04:20:28Z

@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 global_keyprefix in the Redis transport options, which is done here.

samuelallan72 · 2021-08-16T06:04:48Z

@gabor-boros ah got it, thanks 👍

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.

samuelallan72

@gabor-boros 👍

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's configuration-secure repository.

instance/models/openedx_instance.py

instance/tests/models/test_redis_mixin.py

pomegranited · 2021-09-03T02:59:28Z

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.

gabor-boros self-assigned this Jul 1, 2021