From 33f3bbe313ad5d1a6bd35b8e4daf9708a3896c7b Mon Sep 17 00:00:00 2001 From: Sven Marnach Date: Mon, 31 Oct 2016 11:06:25 +0100 Subject: [PATCH] Move DNS management methods to the LoadBalancedInstance mixin. --- instance/models/mixins/load_balanced.py | 53 +++++++++++--- instance/models/openedx_instance.py | 59 ++++------------ instance/tests/api/test_openedx_appserver.py | 2 +- .../tests/models/test_load_balanced_mixin.py | 70 +++++++++++++++++++ .../tests/models/test_openedx_instance.py | 36 +--------- instance/tests/utils.py | 2 +- 6 files changed, 132 insertions(+), 90 deletions(-) create mode 100644 instance/tests/models/test_load_balanced_mixin.py diff --git a/instance/models/mixins/load_balanced.py b/instance/models/mixins/load_balanced.py index 5c0018d85..d0babd861 100644 --- a/instance/models/mixins/load_balanced.py +++ b/instance/models/mixins/load_balanced.py @@ -24,20 +24,19 @@ from django.db import models from django.conf import settings +from tldextract import TLDExtract +from instance.gandi import GandiAPI from instance.models.load_balancer import LoadBalancingServer -# Functions ################################################################### +# Constants ################################################################### -def get_preliminary_page_config(primary_key, domain_names): - """Return a load balancer configuration for the preliminary page.""" - if not settings.PRELIMINARY_PAGE_SERVER_IP: - return [], [] - backend_name = "be-preliminary-page-{}".format(primary_key) - config = " server preliminary-page {}:80".format(settings.PRELIMINARY_PAGE_SERVER_IP) - backend_map = [(domain, backend_name) for domain in domain_names if domain] - return backend_map, [(backend_name, config)] +gandi = GandiAPI() + +# By default, tldextract will make an http request to fetch an updated list of +# TLDs on first invocation. Passing suffix_list_urls=None here prevents this. +tldextract = TLDExtract(suffix_list_urls=None) # Classes ##################################################################### @@ -50,3 +49,39 @@ class LoadBalancedInstance(models.Model): class Meta: abstract = True + + def get_load_balanced_domains(self): # pylint: disable=no-self-use + """ + Return an iterable of domains that should be handled by the load balancer. + """ + return [] + + def get_managed_domains(self): # pylint: disable=no-self-use + """ + Return an iterable of domains that we manage DNS entries for. + """ + return [] + + def set_dns_records(self): + """ + Create CNAME records for the domain names of this instance pointing to the load balancer. + """ + load_balancer_domain = self.load_balancing_server.domain.rstrip(".") + "." + for domain in self.get_managed_domains(): + self.logger.info('Updating DNS: %s...', domain) # pylint: disable=no-member + domain_parts = tldextract(domain) + gandi.set_dns_record( + domain_parts.registered_domain, + type='CNAME', + name=domain_parts.subdomain, + value=load_balancer_domain, + ) + + def get_preliminary_page_config(self, primary_key): + """Return a load balancer configuration for the preliminary page.""" + if not settings.PRELIMINARY_PAGE_SERVER_IP: + return [], [] + backend_name = "be-preliminary-page-{}".format(primary_key) + config = " server preliminary-page {}:80".format(settings.PRELIMINARY_PAGE_SERVER_IP) + backend_map = [(domain, backend_name) for domain in self.get_load_balanced_domains()] + return backend_map, [(backend_name, config)] diff --git a/instance/models/openedx_instance.py b/instance/models/openedx_instance.py index a9dde08d1..1fd2ad70a 100644 --- a/instance/models/openedx_instance.py +++ b/instance/models/openedx_instance.py @@ -26,29 +26,19 @@ from django.db import models, transaction from django.db.backends.utils import truncate_name from django.template import loader -from tldextract import TLDExtract -from instance.gandi import GandiAPI from instance.logging import log_exception from instance.models.appserver import Status as AppServerStatus from instance.models.load_balancer import LoadBalancingServer from instance.models.server import Status as ServerStatus from instance.utils import sufficient_time_passed from .instance import Instance -from .mixins.load_balanced import LoadBalancedInstance, get_preliminary_page_config +from .mixins.load_balanced import LoadBalancedInstance from .mixins.openedx_database import OpenEdXDatabaseMixin from .mixins.openedx_monitoring import OpenEdXMonitoringMixin from .mixins.openedx_storage import OpenEdXStorageMixin from .openedx_appserver import OpenEdXAppConfiguration, OpenEdXAppServer, DEFAULT_EDX_PLATFORM_REPO_URL -# Constants ################################################################### - -gandi = GandiAPI() - -# By default, tldextract will make an http request to fetch an updated list of -# TLDs on first invocation. Passing suffix_list_urls=None here prevents this. -tldextract = TLDExtract(suffix_list_urls=None) - # Functions ################################################################### @@ -152,6 +142,18 @@ def studio_domain(self): else: return self.internal_studio_domain + def get_load_balanced_domains(self): + domain_names = [ + self.external_lms_domain, self.external_lms_preview_domain, self.external_studio_domain, + self.internal_lms_domain, self.internal_lms_preview_domain, self.internal_studio_domain, + ] + return [name for name in domain_names if name] + + def get_managed_domains(self): + return [ + self.internal_lms_domain, self.internal_lms_preview_domain, self.internal_studio_domain + ] + @property def studio_domain_nginx_regex(self): """ @@ -277,43 +279,12 @@ def delete(self, *args, **kwargs): self.deprovision_swift() super().delete(*args, **kwargs) - def set_dns_records(self): - """ - Create CNAME records for the domain names of this instance pointing to the load balancer. - """ - load_balancer_domain = self.load_balancing_server.domain.rstrip(".") + "." - - self.logger.info('Updating DNS: LMS at %s...', self.internal_lms_domain) - lms_domain = tldextract(self.internal_lms_domain) - gandi.set_dns_record( - lms_domain.registered_domain, - type='CNAME', name=lms_domain.subdomain, value=load_balancer_domain - ) - - self.logger.info('Updating DNS: LMS preview at %s...', self.internal_lms_preview_domain) - lms_preview_domain = tldextract(self.internal_lms_preview_domain) - gandi.set_dns_record( - lms_preview_domain.registered_domain, - type='CNAME', name=lms_preview_domain.subdomain, value=load_balancer_domain - ) - - self.logger.info('Updating DNS: Studio at %s...', self.internal_studio_domain) - studio_domain = tldextract(self.internal_studio_domain) - gandi.set_dns_record( - studio_domain.registered_domain, - type='CNAME', name=studio_domain.subdomain, value=load_balancer_domain - ) - def get_load_balancer_configuration(self): """ Return the haproxy configuration fragment and backend map for this instance. """ - domain_names = [ - self.external_lms_domain, self.external_lms_preview_domain, self.external_studio_domain, - self.internal_lms_domain, self.internal_lms_preview_domain, self.internal_studio_domain, - ] if not self.active_appserver: - return get_preliminary_page_config(self.ref.pk, domain_names) + return self.get_preliminary_page_config(self.ref.pk) backend_name = "be-{}".format(self.active_appserver.server.name) server_name = "appserver-{}".format(self.active_appserver.pk) ip_address = self.active_appserver.server.public_ip @@ -323,7 +294,7 @@ def get_load_balancer_configuration(self): server_name=server_name, ip_address=ip_address, )) - backend_map = [(domain, backend_name) for domain in domain_names if domain] + backend_map = [(domain, backend_name) for domain in self.get_load_balanced_domains()] return backend_map, [(backend_name, config)] @property diff --git a/instance/tests/api/test_openedx_appserver.py b/instance/tests/api/test_openedx_appserver.py index 7dec72dd4..0fce83fc1 100644 --- a/instance/tests/api/test_openedx_appserver.py +++ b/instance/tests/api/test_openedx_appserver.py @@ -164,7 +164,7 @@ def test_view_name(self): self.assertIn("Open edX App Server Details", str(response.content)) @patch('instance.models.openedx_instance.OpenEdXAppServer.provision', return_value=True) - @patch('instance.models.openedx_instance.gandi.set_dns_record') + @patch('instance.models.mixins.load_balanced.gandi.set_dns_record') def test_spawn_appserver(self, mock_set_dns_record, mock_provision): """ POST /api/v1/openedx_appserver/ - Spawn a new OpenEdXAppServer for the given instance. diff --git a/instance/tests/models/test_load_balanced_mixin.py b/instance/tests/models/test_load_balanced_mixin.py new file mode 100644 index 000000000..830f32393 --- /dev/null +++ b/instance/tests/models/test_load_balanced_mixin.py @@ -0,0 +1,70 @@ +# -*- coding: utf-8 -*- +# +# OpenCraft -- tools to aid developing and hosting free software projects +# Copyright (C) 2015-2016 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 . +# +""" +Load-balanced instance mixin - tests +""" + +# Imports ##################################################################### + +from unittest.mock import patch, call + +from instance.tests.base import TestCase +from instance.tests.models.factories.openedx_instance import OpenEdXInstanceFactory + + +# Tests ####################################################################### + +class LoadBalancedInstanceTestCase(TestCase): + """ + Tests for OpenEdXStorageMixin + """ + + @patch('instance.models.mixins.load_balanced.gandi.set_dns_record') + def test_set_dns_records(self, mock_set_dns_record): + """ + Test set_dns_records() without external domains. + """ + instance = OpenEdXInstanceFactory(internal_lms_domain='test.dns.opencraft.com', + use_ephemeral_databases=True) + instance.set_dns_records() + lb_domain = instance.load_balancing_server.domain + "." # pylint: disable=no-member + self.assertEqual(mock_set_dns_record.mock_calls, [ + call('opencraft.com', name='test.dns', type='CNAME', value=lb_domain), + call('opencraft.com', name='preview-test.dns', type='CNAME', value=lb_domain), + call('opencraft.com', name='studio-test.dns', type='CNAME', value=lb_domain), + ]) + + @patch('instance.models.mixins.load_balanced.gandi.set_dns_record') + def test_set_dns_records_external_domain(self, mock_set_dns_record): + """ + Test set_dns_records() with custom external domains. + Ensure that the DNS records are only created for the internal domains. + """ + instance = OpenEdXInstanceFactory(internal_lms_domain='test.dns.opencraft.hosting', + external_lms_domain='courses.myexternal.org', + external_lms_preview_domain='preview.myexternal.org', + external_studio_domain='studio.myexternal.org', + use_ephemeral_databases=True) + instance.set_dns_records() + lb_domain = instance.load_balancing_server.domain + "." # pylint: disable=no-member + self.assertEqual(mock_set_dns_record.mock_calls, [ + call('opencraft.hosting', name='test.dns', type='CNAME', value=lb_domain), + call('opencraft.hosting', name='preview-test.dns', type='CNAME', value=lb_domain), + call('opencraft.hosting', name='studio-test.dns', type='CNAME', value=lb_domain), + ]) diff --git a/instance/tests/models/test_openedx_instance.py b/instance/tests/models/test_openedx_instance.py index aa4e5842b..0b0e231e7 100644 --- a/instance/tests/models/test_openedx_instance.py +++ b/instance/tests/models/test_openedx_instance.py @@ -47,7 +47,7 @@ # Tests ####################################################################### -@ddt.ddt # pylint: disable=too-many-public-methods +@ddt.ddt class OpenEdXInstanceTestCase(TestCase): """ Test cases for OpenEdXInstance models @@ -290,40 +290,6 @@ def test_set_appserver_active(self, mocks): self.assertEqual(instance.active_appserver.pk, appserver_id) self.assertEqual(mocks.mock_load_balancer_run_playbook.call_count, 1) - @patch('instance.models.openedx_instance.gandi.set_dns_record') - def test_set_dns_records(self, mock_set_dns_record): - """ - Test set_dns_records() without external domains. - """ - instance = OpenEdXInstanceFactory(internal_lms_domain='test.dns.opencraft.com', - use_ephemeral_databases=True) - instance.set_dns_records() - lb_domain = instance.load_balancing_server.domain + "." # pylint: disable=no-member - self.assertEqual(mock_set_dns_record.mock_calls, [ - call('opencraft.com', name='test.dns', type='CNAME', value=lb_domain), - call('opencraft.com', name='preview-test.dns', type='CNAME', value=lb_domain), - call('opencraft.com', name='studio-test.dns', type='CNAME', value=lb_domain), - ]) - - @patch('instance.models.openedx_instance.gandi.set_dns_record') - def test_set_dns_records_external_domain(self, mock_set_dns_record): - """ - Test set_dns_records() with custom external domains. - Ensure that the DNS records are only created for the internal domains. - """ - instance = OpenEdXInstanceFactory(internal_lms_domain='test.dns.opencraft.hosting', - external_lms_domain='courses.myexternal.org', - external_lms_preview_domain='preview.myexternal.org', - external_studio_domain='studio.myexternal.org', - use_ephemeral_databases=True) - instance.set_dns_records() - lb_domain = instance.load_balancing_server.domain + "." # pylint: disable=no-member - self.assertEqual(mock_set_dns_record.mock_calls, [ - call('opencraft.hosting', name='test.dns', type='CNAME', value=lb_domain), - call('opencraft.hosting', name='preview-test.dns', type='CNAME', value=lb_domain), - call('opencraft.hosting', name='studio-test.dns', type='CNAME', value=lb_domain), - ]) - @patch_services @patch('instance.models.openedx_instance.OpenEdXAppServer.provision', return_value=True) def test_spawn_appserver_names(self, mocks, mock_provision): diff --git a/instance/tests/utils.py b/instance/tests/utils.py index 2c6eec358..06b25807d 100644 --- a/instance/tests/utils.py +++ b/instance/tests/utils.py @@ -65,7 +65,7 @@ def check_sleep_count(_delay): 'instance.models.server.openstack.create_server', side_effect=new_servers, ), mock_sleep=mock_sleep, - mock_set_dns_record=stack_patch('instance.models.openedx_instance.gandi.set_dns_record'), + mock_set_dns_record=stack_patch('instance.models.mixins.load_balanced.gandi.set_dns_record'), mock_run_ansible_playbooks=stack_patch( 'instance.models.mixins.ansible.AnsibleAppServerMixin.run_ansible_playbooks', return_value=([], 0),