From e803858a2b042929e274472e8baf5e3561258308 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Boros?= Date: Fri, 3 Sep 2021 07:12:02 +0200 Subject: [PATCH] [FAL-2031] Add Redis support for Ocim provisioned instances (#822) Add support for provisioning a redis acl for use by instances as an alternative to rabbitmq. Co-authored-by: Samuel Walladge --- .env.e2e | 1 + .env.test | 1 + documentation/configuration.md | 6 + documentation/development/docker.md | 27 ++- instance/admin.py | 6 + .../0142_add_cache_db_to_openedxinstance.py | 18 ++ instance/migrations/0143_add_redis_server.py | 58 ++++++ .../0144_make_redis_username_unique.py | 37 ++++ instance/models/mixins/openedx_database.py | 59 +++++- instance/models/mixins/redis.py | 184 ++++++++++++++++++ instance/models/openedx_instance.py | 12 +- instance/models/redis_server.py | 120 ++++++++++++ instance/serializers/openedx_instance.py | 1 + instance/static/html/instance/details.html | 1 + instance/tests/api/test_openedx_appserver.py | 4 + .../models/factories/RedisServerFactory.py | 26 +++ .../models/test_openedx_database_mixins.py | 57 ++++++ .../tests/models/test_openedx_instance.py | 39 ++++ instance/tests/utils.py | 3 + opencraft/settings.py | 5 +- requirements/base.in | 2 +- requirements/base.txt | 2 +- requirements/dev.txt | 2 +- requirements/test.txt | 2 +- 24 files changed, 661 insertions(+), 12 deletions(-) create mode 100644 instance/migrations/0142_add_cache_db_to_openedxinstance.py create mode 100644 instance/migrations/0143_add_redis_server.py create mode 100644 instance/migrations/0144_make_redis_username_unique.py create mode 100644 instance/models/mixins/redis.py create mode 100644 instance/models/redis_server.py create mode 100644 instance/tests/models/factories/RedisServerFactory.py diff --git a/.env.e2e b/.env.e2e index a6aa61ea7..9e7e3b270 100644 --- a/.env.e2e +++ b/.env.e2e @@ -21,6 +21,7 @@ DEFAULT_LOAD_BALANCING_SERVER='ubuntu@haproxy-test.fake.domain' LOAD_BALANCER_FRAGMENT_NAME_PREFIX='opencraft-' DEFAULT_RABBITMQ_API_URL='https://admin:password@rabbitmq.example.com:15671' DEFAULT_INSTANCE_RABBITMQ_URL='amqps://rabbitmq.example.com:5671' +DEFAULT_INSTANCE_REDIS_URL='rediss://admin:pass@example.com:6397/0' CONSUL_ENABLED=false CONSUL_SERVERS=haproxy-integration.net.opencraft.hosting DISABLE_LOAD_BALANCER_CONFIGURATION=false diff --git a/.env.test b/.env.test index b9cad1dd6..90e4b03c4 100644 --- a/.env.test +++ b/.env.test @@ -19,6 +19,7 @@ DEFAULT_LOAD_BALANCING_SERVER='ubuntu@haproxy-test.fake.domain' LOAD_BALANCER_FRAGMENT_NAME_PREFIX='opencraft-' DEFAULT_RABBITMQ_API_URL='https://admin:password@rabbitmq.example.com:15671' DEFAULT_INSTANCE_RABBITMQ_URL='amqps://rabbitmq.example.com:5671' +DEFAULT_INSTANCE_REDIS_URL='rediss://admin:pass@example.com:6397/0' CONSUL_ENABLED=false CONSUL_SERVERS=haproxy-integration.net.opencraft.hosting DISABLE_LOAD_BALANCER_CONFIGURATION=false diff --git a/documentation/configuration.md b/documentation/configuration.md index 940c02022..baa5477a8 100644 --- a/documentation/configuration.md +++ b/documentation/configuration.md @@ -210,6 +210,12 @@ Required settings: * `DEFAULT_INSTANCE_RABBITMQ_URL`: The RabbitMQ AMQPS URI to be used by instances. E.g., `amqps://rabbitmq.example.com:5671` +### Redis settings +* `DEFAULT_INSTANCE_REDIS_URL` The Redis URI (including the protocol, port, db, and basic auth) + to be used by instances. E.g., `rediss://admin:admin_password@redis.example.com:6397/0`. + Similarly to `DEFAULT_RABBITMQ_API_URL` this setting is saved in the database for new instances + using `RedisServerManager._create_default`, where the setting is not explicity overridden. + ### DNS settings * `DEFAULT_INSTANCE_BASE_DOMAIN`: Instances are created as subdomains of this domain, diff --git a/documentation/development/docker.md b/documentation/development/docker.md index 16e0049ee..5763df446 100644 --- a/documentation/development/docker.md +++ b/documentation/development/docker.md @@ -6,8 +6,30 @@ This describes how to create an environment for development using Docker. ## Running OCIM in a docker container -First, set your `.env` file as you would normally, -but set the following vars: +First, set your `.env` file as you would normally, but first set the following vars: +```env +DEBUG=True +OPENSTACK_USER='username' +OPENSTACK_PASSWORD='password' +OPENSTACK_TENANT='tenant-name' +OPENSTACK_AUTH_URL='https://auth.cloud.ovh.net/v2.0' +OPENSTACK_REGION='BHS1' +OPENSTACK_SANDBOX_SSH_KEYNAME='keypair-name' +DEFAULT_INSTANCE_BASE_DOMAIN='example.com' +GANDI_API_KEY='api-key' +GITHUB_ACCESS_TOKEN='github-token' +SECRET_KEY='tests' +DEFAULT_INSTANCE_MYSQL_URL=... +DEFAULT_RABBITMQ_API_URL=... +DEFAULT_INSTANCE_RABBITMQ_URL=... +DEFAULT_INSTANCE_REDIS_URL=... +DEFAULT_MONGO_REPLICA_SET_USER=... +DEFAULT_MONGO_REPLICA_SET_PASSWORD=... +DEFAULT_MONGO_REPLICA_SET_NAME=... +DEFAULT_MONGO_REPLICA_SET_PRIMARY=... +DEFAULT_MONGO_REPLICA_SET_HOSTS=... +REDIS_URL=... +``` ```sh ALLOWED_HOSTS='["*"]' @@ -43,3 +65,4 @@ For example: ```sh docker-compose logs -f ocim ``` + diff --git a/instance/admin.py b/instance/admin.py index 683fdb736..e38c713ec 100644 --- a/instance/admin.py +++ b/instance/admin.py @@ -33,6 +33,7 @@ from instance.models.openedx_deployment import OpenEdXDeployment from instance.models.openedx_instance import OpenEdXInstance from instance.models.rabbitmq_server import RabbitMQServer +from instance.models.redis_server import RedisServer from instance.models.server import OpenStackServer @@ -109,6 +110,10 @@ class RabbitMQServerAdmin(admin.ModelAdmin): # pylint: disable=missing-docstring list_display = ('name', 'description', 'api_url', 'instance_host', 'instance_port') +class RedisServerAdmin(admin.ModelAdmin): # pylint: disable=missing-docstring + list_display = ('name', 'description', 'instance_host', 'instance_port', 'instance_db') + + class LoadBalancingServerAdmin(admin.ModelAdmin): # pylint: disable=missing-docstring list_display = ('domain', 'ssh_username') @@ -132,5 +137,6 @@ def has_changes(self, obj): admin.site.register(MongoDBServer, MongoDBServerAdmin) admin.site.register(MongoDBReplicaSet, MongoDBReplicaSetAdmin) admin.site.register(RabbitMQServer, RabbitMQServerAdmin) +admin.site.register(RedisServer, RedisServerAdmin) admin.site.register(LoadBalancingServer, LoadBalancingServerAdmin) admin.site.register(OpenEdXDeployment, OpenEdXDeploymentAdmin) diff --git a/instance/migrations/0142_add_cache_db_to_openedxinstance.py b/instance/migrations/0142_add_cache_db_to_openedxinstance.py new file mode 100644 index 000000000..1e4348c7f --- /dev/null +++ b/instance/migrations/0142_add_cache_db_to_openedxinstance.py @@ -0,0 +1,18 @@ +# Generated by Django 2.2.24 on 2021-08-13 08:24 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('instance', '0141_features_to_features_extras'), + ] + + operations = [ + migrations.AddField( + model_name='openedxinstance', + name='cache_db', + field=models.CharField(choices=[('redis', 'redis'), ('rabbit_mq', 'rabbit_mq')], default='rabbit_mq', max_length=9), + ), + ] diff --git a/instance/migrations/0143_add_redis_server.py b/instance/migrations/0143_add_redis_server.py new file mode 100644 index 000000000..c1b1fc6c3 --- /dev/null +++ b/instance/migrations/0143_add_redis_server.py @@ -0,0 +1,58 @@ +# Generated by Django 2.2.24 on 2021-08-13 08:53 + +from django.db import migrations, models +import django.db.models.deletion +import django_extensions.db.fields +import instance.models.mixins.redis +import instance.models.utils + + +class Migration(migrations.Migration): + + dependencies = [ + ('instance', '0142_add_cache_db_to_openedxinstance'), + ] + + operations = [ + migrations.CreateModel( + name='RedisServer', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created', django_extensions.db.fields.CreationDateTimeField(auto_now_add=True, verbose_name='created')), + ('modified', django_extensions.db.fields.ModificationDateTimeField(auto_now=True, verbose_name='modified')), + ('name', models.CharField(max_length=250)), + ('description', models.CharField(blank=True, max_length=250)), + ('admin_username', models.CharField(max_length=64)), + ('admin_password', models.CharField(max_length=128)), + ('instance_host', models.CharField(max_length=128)), + ('instance_port', models.PositiveIntegerField(default=5671)), + ('instance_db', models.PositiveIntegerField(default=0)), + ('use_ssl_connections', models.BooleanField(default=True)), + ('accepts_new_clients', models.BooleanField(default=False)), + ], + options={ + 'verbose_name': 'Redis Server', + }, + bases=(instance.models.utils.ValidateModelMixin, models.Model), + ), + migrations.AddField( + model_name='openedxinstance', + name='redis_password', + field=models.CharField(max_length=64, null=True), + ), + migrations.AddField( + model_name='openedxinstance', + name='redis_provisioned', + field=models.BooleanField(default=False), + ), + migrations.AddField( + model_name='openedxinstance', + name='redis_username', + field=models.CharField(max_length=32, null=True), + ), + migrations.AddField( + model_name='openedxinstance', + name='redis_server', + field=models.ForeignKey(blank=True, default=instance.models.mixins.redis.select_random_redis_server, null=True, on_delete=django.db.models.deletion.PROTECT, to='instance.RedisServer'), + ), + ] diff --git a/instance/migrations/0144_make_redis_username_unique.py b/instance/migrations/0144_make_redis_username_unique.py new file mode 100644 index 000000000..4104dc28d --- /dev/null +++ b/instance/migrations/0144_make_redis_username_unique.py @@ -0,0 +1,37 @@ +# Generated by Django 2.2.24 on 2021-08-13 08:53 + +from django.db import migrations, models +import django.db.models.deletion +import django_extensions.db.fields +import instance.models.mixins.redis +import instance.models.utils + + +def generate_credentials(apps, schema_editor): + Model = apps.get_model('instance', 'OpenEdXInstance') + for row in Model.objects.all(): + row.redis_username = instance.models.mixins.redis.random_username() + row.redis_password = instance.models.mixins.redis.random_password() + row.save(update_fields=['redis_username', 'redis_password']) + + + +class Migration(migrations.Migration): + + dependencies = [ + ('instance', '0143_add_redis_server'), + ] + + operations = [ + migrations.RunPython(generate_credentials, reverse_code=migrations.RunPython.noop), + migrations.AlterField( + model_name='openedxinstance', + name='redis_password', + field=models.CharField(default=instance.models.mixins.redis.random_password, max_length=64), + ), + migrations.AlterField( + model_name='openedxinstance', + name='redis_username', + field=models.CharField(default=instance.models.mixins.redis.random_username, max_length=32, unique=True), + ), + ] diff --git a/instance/models/mixins/openedx_database.py b/instance/models/mixins/openedx_database.py index c8bbf2063..bbc842526 100644 --- a/instance/models/mixins/openedx_database.py +++ b/instance/models/mixins/openedx_database.py @@ -22,11 +22,13 @@ import hashlib import hmac +from django.db import models import MySQLdb as mysql import yaml from instance.models.mixins.database import MySQLInstanceMixin, MongoDBInstanceMixin from instance.models.mixins.rabbitmq import RabbitMQInstanceMixin +from instance.models.mixins.redis import RedisInstanceMixin FORUM_MONGO_REPLICA_SET_URL = ( "mongodb://{{ FORUM_MONGO_USER }}:{{ FORUM_MONGO_PASSWORD }}@" @@ -41,13 +43,25 @@ # Classes ##################################################################### -class OpenEdXDatabaseMixin(MySQLInstanceMixin, MongoDBInstanceMixin, RabbitMQInstanceMixin): +class OpenEdXDatabaseMixin(MySQLInstanceMixin, MongoDBInstanceMixin, RabbitMQInstanceMixin, RedisInstanceMixin): """ Mixin that provides functionality required for the database backends that an OpenEdX Instance uses + TODO: ElasticSearch? """ + + REDIS = "redis" + RABBIT_MQ = "rabbit_mq" + CACHE_DBS = ( + (REDIS, REDIS), + (RABBIT_MQ, RABBIT_MQ), + ) + + # TODO: When every server is using REDIS, use REDIS as default + cache_db = models.CharField(max_length=9, choices=CACHE_DBS, default=RABBIT_MQ) + class Meta: abstract = True @@ -475,6 +489,39 @@ def _get_rabbitmq_settings(self): "EDXAPP_CELERY_BROKER_USE_SSL": True } + def _get_redis_settings(self): + """ + Return dictionary of Redis settings + """ + + redis_hostname = "{}:{}".format( + self.redis_server.instance_host, + self.redis_server.instance_port + ) + + redis_transport_options = { + "CELERY_BROKER_TRANSPORT_OPTIONS": { + "global_keyprefix": self.redis_key_prefix + } + } + + return { + "EDXAPP_REDIS_HOSTNAME": redis_hostname, + "EDXAPP_CELERY_BROKER_TRANSPORT": "redis", + "EDXAPP_CELERY_USER": self.redis_username, + "EDXAPP_CELERY_PASSWORD": self.redis_password, + "EDXAPP_CELERY_BROKER_HOSTNAME": redis_hostname, + "EDXAPP_CELERY_BROKER_VHOST": str(self.redis_server.instance_db), + "EDXAPP_CELERY_BROKER_USE_SSL": self.redis_server.use_ssl_connections, + + "EDXAPP_LMS_ENV_EXTRA": { + **redis_transport_options + }, + "EDXAPP_CMS_ENV_EXTRA": { + **redis_transport_options + } + } + def get_database_settings(self): """ Get configuration_database_settings to pass to a new AppServer @@ -489,7 +536,13 @@ def get_database_settings(self): if self.mongodb_replica_set or self.mongodb_server: new_settings.update(self._get_mongo_settings()) - # RabbitMQ: - new_settings.update(self._get_rabbitmq_settings()) + if self.cache_db == self.REDIS: + # Redis: + new_settings.update(self._get_redis_settings()) + elif self.cache_db == self.RABBIT_MQ: + # RabbitMQ: + new_settings.update(self._get_rabbitmq_settings()) + else: + raise NotImplementedError(f"{self.cache_db} does not load any cache DB settings") return yaml.dump(new_settings, default_flow_style=False) diff --git a/instance/models/mixins/redis.py b/instance/models/mixins/redis.py new file mode 100644 index 000000000..bcc28d90e --- /dev/null +++ b/instance/models/mixins/redis.py @@ -0,0 +1,184 @@ +# -*- coding: utf-8 -*- +# +# OpenCraft -- tools to aid developing and hosting free software projects +# Copyright (C) 2015-2021 OpenCraft +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . +# +""" +Instance app model mixins - Redis +""" + +# Imports ##################################################################### + +from functools import lru_cache +import string +import redis + +from django.db import models +from django.utils.crypto import get_random_string + +from instance.models.redis_server import RedisServer + + +# Functions ################################################################### + +def random_username(): + """ + Generate a random username. + + Only ASCII lowercase chars are used to make operations easier. + """ + return get_random_string(32, allowed_chars=string.ascii_lowercase) + + +def random_password(): + """ + Generate a random password. + """ + return get_random_string(64) + + +def select_random_redis_server(): + """ + Helper for the field default of `redis_server`. + + The random server selection does not make sense in case if one RedisServer + is available at a time, though if multiple servers are registered it serves + as a very basic "load balancer". + + It selects a random server that accepts new connections. If the default + Redis instance's URL is set, it also creates a Redis server based on it. + """ + return RedisServer.objects.select_random().pk + + +# Classes ##################################################################### + +class RedisInstanceMixin(models.Model): + """ + An instance that uses a Redis db with a set of users. + """ + + redis_server = models.ForeignKey( + RedisServer, + null=True, + blank=True, + default=select_random_redis_server, + on_delete=models.PROTECT, + ) + + redis_username = models.CharField(max_length=32, default=random_username, unique=True) + redis_password = models.CharField(max_length=64, default=random_password) + redis_provisioned = models.BooleanField(default=False) + + class Meta: + abstract = True + + @lru_cache() + def _redis_client(self, **kwargs): + """ + Generic method for sending an HTTP request to the Redis API. + Raises a RedisAPIError if the response isn't OK. + """ + + return redis.Redis( + username=self.redis_server.admin_username, + password=self.redis_server.admin_password, + host=self.redis_server.instance_host, + port=self.redis_server.instance_port, + db=self.redis_server.instance_db, + ssl=self.redis_server.use_ssl_connections, + **kwargs + ) + + @property + def redis_key_prefix(self) -> str: + """ + Return the key prefix for the user. + + Username used to prefix redis keys and set ACLs on the Redis server. + """ + return f"{self.redis_username}_" + + def delete_redis_acl(self, client: redis.Redis): + """ + Delete ACL record for user. + """ + client.acl_deluser(self.redis_username) + + def create_redis_acl(self, client: redis.Redis): + """ + Create ACL record for user. + """ + client.acl_setuser( + username=self.redis_username, + passwords=[f"+{self.redis_password}"], + keys=[f"{self.redis_key_prefix}*"], + commands=["+@all"], + enabled=True + ) + + def provision_redis(self): + """ + Creates the Redis users. + """ + if self.redis_provisioned: + self.logger.info( + 'Redis is already provisioned for %s. No provisioning needed.', + self.redis_username + ) + + return + + self.logger.info('Provisioning Redis started.') + + client = self._redis_client() + self.create_redis_acl(client) + + self.redis_provisioned = True + self.save() + + def deprovision_redis(self, ignore_errors=False): + """ + Deletes the Redis vhost and users. + """ + if not self.redis_provisioned: + self.logger.info( + 'Redis is not provisioned for %s. No deprovisioning needed.', + self.redis_username + ) + + return + + self.logger.info('Deprovisioning Redis started.') + self.logger.info('Deleting redis user: %s', self.redis_username) + + try: + client = self._redis_client() + self.delete_redis_acl(client) + except Exception as exc: # pylint: disable=broad-except + self.logger.exception( + 'Cannot delete redis user: %s. %s', + self.redis_username, + exc + ) + + if not ignore_errors: + raise exc + + self.redis_provisioned = False + self.save() + + self.logger.info('Deprovisioning Redis finished.') diff --git a/instance/models/openedx_instance.py b/instance/models/openedx_instance.py index 00bf71c29..f4ddf7864 100644 --- a/instance/models/openedx_instance.py +++ b/instance/models/openedx_instance.py @@ -310,8 +310,16 @@ def _spawn_appserver(self, deployment_id=None): elif self.storage_type == self.S3_STORAGE: self.logger.info('Provisioning S3 bucket...') self.provision_s3() - self.logger.info('Provisioning RabbitMQ vhost...') - self.provision_rabbitmq() + + if self.cache_db == OpenEdXDatabaseMixin.REDIS: + self.logger.info('Provisioning Redis user ACL...') + self.provision_redis() + + elif self.cache_db == OpenEdXDatabaseMixin.RABBIT_MQ: + self.logger.info('Provisioning RabbitMQ vhost...') + self.provision_rabbitmq() + else: + raise NotImplementedError(f"{self.cache_db} does not provision any cache DBs") return self._create_owned_appserver(deployment_id=deployment_id) diff --git a/instance/models/redis_server.py b/instance/models/redis_server.py new file mode 100644 index 000000000..265213fd7 --- /dev/null +++ b/instance/models/redis_server.py @@ -0,0 +1,120 @@ +# -*- coding: utf-8 -*- +# +# OpenCraft -- tools to aid developing and hosting free software projects +# Copyright (C) 2015-2021 OpenCraft +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . +# +""" +Instance app models - Redis Server model and manager +""" + +# Imports ##################################################################### + +import logging +from urllib.parse import urlparse + +from django.conf import settings +from django.core.exceptions import ImproperlyConfigured +from django.db import models +from django_extensions.db.models import TimeStampedModel + +from instance.models.shared_server import SharedServerManager +from instance.models.utils import ValidateModelMixin + + +# Logging ##################################################################### + +logger = logging.getLogger(__name__) + + +# Constants ################################################################### + +DEFAULT_INSTANCE_REDIS_PORT = 5671 +DEFAULT_INSTANCE_REDIS_DB = 0 + + +# Models ###################################################################### + +class RedisServerManager(SharedServerManager): + """ + Custom manager for the RedisServer model. + """ + + def _create_default(self): + """ + Create the default Redis server configured in the Django settings, if any. + """ + if settings.DEFAULT_INSTANCE_REDIS_URL: + conn_params = urlparse(settings.DEFAULT_INSTANCE_REDIS_URL) + + defaults = { + 'admin_username': conn_params.username, + 'admin_password': conn_params.password, + 'instance_host': conn_params.hostname, + 'instance_port': conn_params.port, + 'instance_db': conn_params.path.replace("/", ""), + 'accepts_new_clients': True, + } + + if not all(defaults.values()): + raise ImproperlyConfigured( + "Found DEFAULT_INSTANCE_REDIS_URL, but it must contain " + "basic auth credentials, hostname, port and db." + ) + + logger.info("Creating RedisServer %s", defaults['instance_host']) + + defaults['name'] = defaults['instance_host'] + defaults['instance_db'] = int(defaults['instance_db']) + defaults['use_ssl_connections'] = conn_params.scheme == "rediss" + + server, created = self.get_or_create(defaults=defaults) + + if not created and not all(getattr(server, name) == default for name, default in defaults.items()): + logger.warning( + "RedisServer for %s already exists, and its settings do not match " + "the Django settings: " + ", ".join(["%s vs %s"] * len(defaults)), + *sum(sorted(defaults.items()), ()), + ) + + +class RedisServer(ValidateModelMixin, TimeStampedModel): + """ + A model representing a configured Redis server. + """ + name = models.CharField(max_length=250, blank=False) + description = models.CharField(max_length=250, blank=True) + + # Credentials for accessing the Redis server. + # User identified by these credentials must have permission to manage ACLs. + admin_username = models.CharField(max_length=64) + admin_password = models.CharField(max_length=128) + + instance_host = models.CharField(max_length=128) + instance_port = models.PositiveIntegerField(default=DEFAULT_INSTANCE_REDIS_PORT) + instance_db = models.PositiveIntegerField(default=DEFAULT_INSTANCE_REDIS_DB) + + use_ssl_connections = models.BooleanField(default=True) + accepts_new_clients = models.BooleanField(default=False) + + objects = RedisServerManager() + + + class Meta: + verbose_name = 'Redis Server' + + def __str__(self) -> str: + description = f' ({self.description})' if self.description else '' + return f'{self.name}{description}' diff --git a/instance/serializers/openedx_instance.py b/instance/serializers/openedx_instance.py index 3de4fa1a6..57b2034a8 100644 --- a/instance/serializers/openedx_instance.py +++ b/instance/serializers/openedx_instance.py @@ -117,6 +117,7 @@ class Meta: 'mongo_pass', 'mongo_provisioned', 'rabbitmq_provisioned', + 'redis_provisioned', 'dns_records_updated', diff --git a/instance/static/html/instance/details.html b/instance/static/html/instance/details.html index fedc96c03..500e5a2cd 100644 --- a/instance/static/html/instance/details.html +++ b/instance/static/html/instance/details.html @@ -139,6 +139,7 @@

Status

MySQL provisioned{{ instance.mysql_provisioned }} Mongo provisioned{{ instance.mongo_provisioned }} RabbitMQ provisioned{{ instance.rabbitmq_provisioned }} + Redis provisioned{{ instance.redis_provisioned }} DNS records set at{{ instance.dns_records_updated | date:'yyyy-MM-dd HH:mm:ssZ' }} diff --git a/instance/tests/api/test_openedx_appserver.py b/instance/tests/api/test_openedx_appserver.py index c65053001..f5f1e921a 100644 --- a/instance/tests/api/test_openedx_appserver.py +++ b/instance/tests/api/test_openedx_appserver.py @@ -237,7 +237,9 @@ def test_spawn_appserver(self, mocks, mock_consul, mock_provision): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.data, {'status': 'Instance provisioning started'}) self.assertEqual(mock_provision.call_count, 1) + # TODO: update this when redis becomes the default self.assertEqual(mocks.mock_provision_rabbitmq.call_count, 1) + self.assertEqual(mocks.mock_provision_redis.call_count, 0) instance.refresh_from_db() self.assertEqual(instance.appserver_set.count(), 1) @@ -268,7 +270,9 @@ def test_spawn_appserver_break_on_success( spawn_appserver(instance.ref.pk, mark_active_on_success=mark_active, num_attempts=4) self.assertEqual(mock_provision.call_count, 1) + # TODO: update this when redis becomes the default self.assertEqual(mocks.mock_provision_rabbitmq.call_count, 1) + self.assertEqual(mocks.mock_provision_redis.call_count, 0) @ddt.data( (None, 'Authentication credentials were not provided.'), diff --git a/instance/tests/models/factories/RedisServerFactory.py b/instance/tests/models/factories/RedisServerFactory.py new file mode 100644 index 000000000..0b2ec2a44 --- /dev/null +++ b/instance/tests/models/factories/RedisServerFactory.py @@ -0,0 +1,26 @@ +""" +Factories related to RabbitMQ. +""" +import factory +import factory.django as django_factory + +from instance.models.redis_server import RedisServer + + +class RedisServerFactory(django_factory.DjangoModelFactory): + """ + Factory for RabbitMQServer. + """ + + class Meta: + model = RedisServer + + name = factory.Sequence(lambda n: f"Redis {n}") + admin_username = "admin" + admin_password = "admin" + + instance_host = factory.Sequence(lambda n: f"redis-{n}.test") + instance_port = 6379 + instance_db = 0 + + use_ssl_connections = False diff --git a/instance/tests/models/test_openedx_database_mixins.py b/instance/tests/models/test_openedx_database_mixins.py index 42f8e2fb4..b4f39141b 100644 --- a/instance/tests/models/test_openedx_database_mixins.py +++ b/instance/tests/models/test_openedx_database_mixins.py @@ -41,6 +41,7 @@ ) from instance.models.mixins.rabbitmq import RabbitMQAPIError from instance.models.rabbitmq_server import RabbitMQServer +from instance.models.redis_server import RedisServer from instance.tests.base import TestCase from instance.tests.models.factories.openedx_appserver import make_test_appserver from instance.tests.models.factories.openedx_instance import OpenEdXInstanceFactory @@ -881,3 +882,59 @@ def test_mismatch_warning(self, mock_logger, mock_consul): 'instance_host', 'rabbitmq.example.com', 'instance_port', 5671, ) + + +@patch( + 'instance.tests.models.factories.openedx_instance.OpenEdXInstance._write_metadata_to_consul', + return_value=(1, True) +) +class RedisServerManagerTestCase(TestCase): + """ + Tests for RedisServerManager. + """ + + @override_settings(DEFAULT_INSTANCE_REDIS_URL=None) + def test_no_redis_server_available(self, mock_consul): + """ + Test that get_random() raises an exception when no rabbitmq servers are available. + """ + RedisServer.objects.all().delete() + with self.assertRaises(RedisServer.DoesNotExist): + RedisServer.objects.select_random() + + @override_settings(DEFAULT_INSTANCE_REDIS_URL="http://doesnotexist.example.com:12345") + def test_invalid_redis_server(self, mock_consul): + """ + Verify that an exception gets raised when the credentials are missing from from the + setting for the default redis API url server. + """ + with self.assertRaises(ImproperlyConfigured): + RedisServer.objects.select_random() + + @patch('instance.models.redis_server.logger') + def test_mismatch_warning(self, mock_logger, mock_consul): + """ + Test that a warning is logged when trying to spawn the default, but a default already + exists and contains mismatching parameters with the given settings. + """ + urls = [ + 'http://user:pass@doesnotexist.example.com:12345/0', + 'http://user2:pass2@doesnotexist.example.com:12345/0' + ] + + for url in urls: + with override_settings(DEFAULT_INSTANCE_REDIS_URL=url): + RedisServer.objects._create_default() + + mock_logger.warning.assert_called_with( + 'RedisServer for %s already exists, and its settings do not match the Django settings: ' + '%s vs %s, %s vs %s, %s vs %s, %s vs %s, %s vs %s, %s vs %s, %s vs %s, %s vs %s', + 'accepts_new_clients', True, + 'admin_password', 'pass2', + 'admin_username', 'user2', + 'instance_db', 0, + 'instance_host', 'doesnotexist.example.com', + 'instance_port', 12345, + 'name', 'doesnotexist.example.com', + 'use_ssl_connections', False + ) diff --git a/instance/tests/models/test_openedx_instance.py b/instance/tests/models/test_openedx_instance.py index 559af4a4b..422de2463 100644 --- a/instance/tests/models/test_openedx_instance.py +++ b/instance/tests/models/test_openedx_instance.py @@ -49,6 +49,7 @@ from instance.models.openedx_deployment import OpenEdXDeployment from instance.models.openedx_instance import OpenEdXInstance, OpenEdXAppConfiguration from instance.models.rabbitmq_server import RabbitMQServer +from instance.models.redis_server import RedisServer from instance.models.server import OpenStackServer, Server, Status as ServerStatus from instance.models.utils import WrongStateException from instance.tests.base import TestCase @@ -58,6 +59,7 @@ OpenEdXInstanceFactory, RabbitMQServerFactory ) +from instance.tests.models.factories.RedisServerFactory import RedisServerFactory from instance.tests.utils import patch_services, skip_unless_consul_running from registration.models import BetaTestApplication from registration.approval import ApplicationNotReady @@ -1643,3 +1645,40 @@ def test_assigns_available_server(self, mock_consul): instance = OpenEdXInstanceFactory() assert instance.rabbitmq_server == server + + +@ddt.ddt +# prevent default rabbitmq server from being created +@override_settings(DEFAULT_INSTANCE_REDIS_URL=None) +@patch( + 'instance.tests.models.factories.openedx_instance.OpenEdXInstance._write_metadata_to_consul', + return_value=(1, True) +) +class OpenEDXInstanceRedisTestCase(TestCase): + """ + Tests Redis related features in OpenEDXInstance + """ + + def setUp(self): + """ + Remove all redis servers to ensure test isolation + """ + RedisServer.objects.filter(accepts_new_clients=True).delete() + + def test_assign_redis_fails_when_no_available_servers(self, mock_consul): + """ + Test that creating a new OpenEdXInstance fails when there are suitable RabbitMQServer instances. + """ + with self.assertRaises(RedisServer.DoesNotExist): + OpenEdXInstanceFactory() + + def test_assigns_available_server(self, mock_consul): + """ + Test that creating a new OpenEdXInstance only assigns valid RedisServer instances. + """ + for _ in range(5): + RedisServerFactory(accepts_new_clients=False) + server = RedisServerFactory(accepts_new_clients=True) + + instance = OpenEdXInstanceFactory() + assert instance.redis_server == server diff --git a/instance/tests/utils.py b/instance/tests/utils.py index 484287179..45461fb5a 100644 --- a/instance/tests/utils.py +++ b/instance/tests/utils.py @@ -143,6 +143,9 @@ def check_sleep_count(_delay): mock_provision_rabbitmq=stack_patch( 'instance.models.mixins.rabbitmq.RabbitMQInstanceMixin.provision_rabbitmq', ), + mock_provision_redis=stack_patch( + 'instance.models.mixins.redis.RedisInstanceMixin.provision_redis', + ), mock_provision_swift=stack_patch( 'instance.models.mixins.openedx_storage.SwiftContainerInstanceMixin.provision_swift' ), diff --git a/opencraft/settings.py b/opencraft/settings.py index 5b6738c58..c293d639b 100644 --- a/opencraft/settings.py +++ b/opencraft/settings.py @@ -742,9 +742,12 @@ DEFAULT_MONGO_REPLICA_SET_USER = env('DEFAULT_MONGO_REPLICA_SET_USER', default=None) DEFAULT_MONGO_REPLICA_SET_PASSWORD = env('DEFAULT_MONGO_REPLICA_SET_PASSWORD', default=None) -# The RabbitMQ host must be accessible from both OpenCraft IM as well as well as any instances using it. +# The RabbitMQ host must be accessible from both OpenCraft IM and any instances using it DEFAULT_INSTANCE_RABBITMQ_URL = env('DEFAULT_INSTANCE_RABBITMQ_URL', default=None) +# The Redis host must be accessible from both OpenCraft IM and any instances using it +DEFAULT_INSTANCE_REDIS_URL = env('DEFAULT_INSTANCE_REDIS_URL', default=None) + # Limit the number of log entries fetched for each instance, for performance LOG_LIMIT = env.int('LOG_LIMIT', default=10000) diff --git a/requirements/base.in b/requirements/base.in index ebe9cf5c3..67a527c60 100644 --- a/requirements/base.in +++ b/requirements/base.in @@ -64,7 +64,7 @@ python-openstackclient python-swiftclient pytz==2019.1 PyYAML<5.5,>=5.4.1 -redis==3.2.1 +redis==3.5.3 requests requests-file==1.4.3 responses==0.10.6 diff --git a/requirements/base.txt b/requirements/base.txt index 867a8badb..39708a032 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -440,7 +440,7 @@ pyyaml==5.4.1 # oslo.config rcssmin==1.0.6 # via django-compressor -redis==3.2.1 +redis==3.5.3 # via # -r requirements/base.in # django-redis diff --git a/requirements/dev.txt b/requirements/dev.txt index 894450c5f..a550dc623 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -547,7 +547,7 @@ pyyaml==5.4.1 # prospector rcssmin==1.0.6 # via django-compressor -redis==3.2.1 +redis==3.5.3 # via # -r requirements/base.in # django-redis diff --git a/requirements/test.txt b/requirements/test.txt index 55b520162..2a4cf108f 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -515,7 +515,7 @@ pyyaml==5.4.1 # prospector rcssmin==1.0.6 # via django-compressor -redis==3.2.1 +redis==3.5.3 # via # -r requirements/base.in # django-redis