Move DNS management methods to the LoadBalancedInstance mixin.

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)]

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

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.

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 <contact@opencraft.com>
+#
+# 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 <http://www.gnu.org/licenses/>.
+#
+"""
+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),
+        ])

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):

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),