diff --git a/instance/gandi.py b/instance/gandi.py index 7c8f940bf..a42e7f70c 100644 --- a/instance/gandi.py +++ b/instance/gandi.py @@ -99,23 +99,19 @@ def set_zone_version(self, zone_id, zone_version_id): """ return self.client_zone.version.set(self.api_key, zone_id, zone_version_id) - def set_dns_record(self, domain, attempts=4, retry_delay=1, **record): + def _dns_operation(self, callback, domain, log_msg, attempts=4, retry_delay=1): """ - Set a DNS record - Automatically create a new version, update with the change & activate - This method takes the mandatory `domain` parameter to be able to support multiple domains, - handled by the same Gandi account + Encapsulate logic that is common to high-level DNS operations: grab the global lock, get the + zone_id for a domain, create a new zone version, activate the zone version after successful + update, and retry the whole procedure multiple times if necessary. """ - if 'ttl' not in record.keys(): - record['ttl'] = 1200 - with cache.lock('gandi_set_dns_record'): # Only do one DNS update at a time for i in range(1, attempts + 1): try: - logger.info('Setting DNS record: %s (attempt %d out of %d)', record, i, attempts) + logger.info('%s (attempt %d out of %d)', log_msg, i, attempts) zone_id = self.get_zone_id(domain) new_zone_version = self.create_new_zone_version(zone_id) - self.delete_dns_record(zone_id, new_zone_version, record['name']) - returned_record = self.add_dns_record(zone_id, new_zone_version, record) + result = callback(zone_id, new_zone_version) self.set_zone_version(zone_id, new_zone_version) break except xmlrpc.client.Fault: @@ -123,4 +119,41 @@ def set_dns_record(self, domain, attempts=4, retry_delay=1, **record): raise time.sleep(retry_delay) retry_delay *= 2 - return returned_record + return result + + def set_dns_record(self, domain, **record): + """ + Set a DNS record. This method takes the mandatory `domain` parameter to be able to support + multiple domains, handled by the same Gandi account. + """ + if 'ttl' not in record.keys(): + record['ttl'] = 1200 + + def set_dns_record_callback(zone_id, zone_version): + """ + Callback to be passed to _dns_operation(). + """ + self.delete_dns_record(zone_id, zone_version, record['name']) + return self.add_dns_record(zone_id, zone_version, record) + + self._dns_operation( + callback=set_dns_record_callback, + domain=domain, + log_msg='Setting DNS record: {}'.format(record), + ) + + def remove_dns_record(self, domain, name): + """ + Remove the given name for the domain. + """ + def remove_dns_record_callback(zone_id, zone_version): + """ + Callback to be passed to _dns_operation(). + """ + self.delete_dns_record(zone_id, zone_version, name) + + self._dns_operation( + callback=remove_dns_record_callback, + domain=domain, + log_msg='Deleting DNS record: {}'.format(name), + ) diff --git a/instance/models/load_balancer.py b/instance/models/load_balancer.py index fce890168..912f76c79 100644 --- a/instance/models/load_balancer.py +++ b/instance/models/load_balancer.py @@ -210,3 +210,10 @@ def deconfigure(self): self.run_playbook( "FRAGMENT_NAME: {fragment_name}\nREMOVE_FRAGMENT: True".format(fragment_name=fragment_name) ) + + def delete(self, *args, **kwargs): + """ + Delete the LoadBalancingServer from the database. + """ + self.deconfigure() + super().delete(*args, **kwargs) # pylint: disable=no-member diff --git a/instance/models/mixins/load_balanced.py b/instance/models/mixins/load_balanced.py index 5e9aa7465..78f48e6d6 100644 --- a/instance/models/mixins/load_balanced.py +++ b/instance/models/mixins/load_balanced.py @@ -68,7 +68,6 @@ def set_dns_records(self): """ 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, @@ -77,6 +76,14 @@ def set_dns_records(self): value=load_balancer_domain, ) + def remove_dns_records(self): + """ + Delete the DNS records for this instance. + """ + for domain in self.get_managed_domains(): + domain_parts = tldextract(domain) + gandi.remove_dns_record(domain_parts.registered_domain, domain_parts.subdomain) + def get_preliminary_page_config(self, primary_key): """ Return a load balancer configuration for the preliminary page. diff --git a/instance/models/openedx_instance.py b/instance/models/openedx_instance.py index 10a1e8ffc..b1497ee41 100644 --- a/instance/models/openedx_instance.py +++ b/instance/models/openedx_instance.py @@ -272,12 +272,7 @@ def delete(self, *args, **kwargs): """ Delete this Open edX Instance and its associated AppServers, and deprovision external databases and storage. """ - self.disable_monitoring() - for appserver in self.appserver_set.all(): - appserver.terminate_vm() - self.deprovision_mysql() - self.deprovision_mongo() - self.deprovision_swift() + self.shut_down() super().delete(*args, **kwargs) def get_load_balancer_configuration(self): @@ -415,12 +410,18 @@ def terminate_obsolete_appservers(self, days=2): def shut_down(self): """ Shut down this instance. - - This process consists of two steps: - - 1) Disable New Relic monitors. - 2) Terminate all app servers belonging to this instance. """ - self.disable_monitoring() - for appserver in self.appserver_set.all(): + if self.load_balancing_server is not None: + load_balancer = self.load_balancing_server + self.load_balancing_server = None + self.save() + if self.active_appserver is None: + # If an appserver is active, reconfiguring the load_balancer happens + # implicitly when terminate_vm() is called further down. + load_balancer.reconfigure() + self.remove_dns_records() + self.deprovision_mysql() + self.deprovision_mongo() + self.deprovision_swift() + for appserver in self.appserver_set.iterator(): appserver.terminate_vm() diff --git a/instance/tests/integration/base.py b/instance/tests/integration/base.py index 1f8374605..b22cc51c5 100644 --- a/instance/tests/integration/base.py +++ b/instance/tests/integration/base.py @@ -51,14 +51,15 @@ def setUp(self): patcher.start() def tearDown(self): - for appserver in OpenEdXAppServer.objects.iterator(): - appserver.terminate_vm() + # Trigger clean-up operations for load-balancers and instances. To avoid reconfiguring the + # load balancing server multiple time, we first remove the configured load balancer from all + # instances, the delete the load balancers, the delete the instances. for instance in OpenEdXInstance.objects.iterator(): - instance.deprovision_swift() - instance.deprovision_mongo() - instance.deprovision_mysql() - for load_balancer in LoadBalancingServer.objects.iterator(): # pylint: disable=no-member - load_balancer.deconfigure() + instance.load_balancing_server = None + instance.save() + LoadBalancingServer.objects.delete() # pylint: disable=no-member + OpenEdXInstance.objects.delete() + super().tearDown() # All VMs should be terminated at this point, but check just in case: diff --git a/instance/tests/models/test_openedx_instance.py b/instance/tests/models/test_openedx_instance.py index 8187f436a..d27fd2482 100644 --- a/instance/tests/models/test_openedx_instance.py +++ b/instance/tests/models/test_openedx_instance.py @@ -473,9 +473,9 @@ def test_get_load_balancer_config_ext_domains(self): @patch('instance.models.mixins.database.MySQLInstanceMixin.deprovision_mysql') @patch('instance.models.mixins.database.MongoDBInstanceMixin.deprovision_mongo') @patch('instance.models.mixins.storage.SwiftContainerInstanceMixin.deprovision_swift') + @patch('instance.models.mixins.load_balanced.LoadBalancedInstance.remove_dns_records') def test_delete_instance( - self, mocks, delete_by_ref, - mock_deprovision_swift, mock_deprovision_mongo, mock_deprovision_mysql, mock_terminate_vm + self, mocks, delete_by_ref, mock_remove_dns_record, *mocked_methods ): """ Test that an instance can be deleted directly or by its InstanceReference, @@ -486,9 +486,7 @@ def test_delete_instance( instance_ref = instance.ref appserver = OpenEdXAppServer.objects.get(pk=instance.spawn_appserver()) - for mocked_method in ( - mock_terminate_vm, mock_deprovision_mysql, mock_deprovision_mongo, mock_deprovision_swift - ): + for mocked_method in mocked_methods: self.assertEqual(mocked_method.call_count, 0) # Now delete the instance, either using InstanceReference or the OpenEdXInstance class: @@ -497,9 +495,7 @@ def test_delete_instance( else: instance.delete() - for mocked_method in ( - mock_terminate_vm, mock_deprovision_mysql, mock_deprovision_mongo, mock_deprovision_swift - ): + for mocked_method in mocked_methods: self.assertEqual(mocked_method.call_count, 1) with self.assertRaises(OpenEdXInstance.DoesNotExist):