Merge pull request #802 from open-craft/giovanni/deprecated-exception

BB-4198: Fix issues with package upgrades.

instance/api/openedx_appserver.py

@@ -37,7 +37,7 @@
     OpenEdXAppServerSerializer,
     SpawnAppServerSerializer,
 )
-from instance.tasks import make_appserver_active
+from instance.tasks import make_appserver_active, terminate_appserver
 from instance.utils import create_new_deployment
 from .filters import IsOrganizationOwnerFilterBackendAppServer, IsOrganizationOwnerFilterBackendInstance
 from ..models.deployment import DeploymentType
@@ -135,12 +135,15 @@ def make_inactive(self, request, pk):
     @action(detail=True, methods=['post'])
     def terminate(self, request, pk):
         """
-        Terminate the VM running the provided AppServer.
+        Trigger worker task to terminate the VM running the provided AppServer.
+
+        Note that this might fail silently without any errors appearing on the Ocim UI,
+        but it's acceptable given the current use case.
         """
         app_server = self.get_object()
         if app_server.is_active:
             return Response({
                 'error': 'Cannot terminate an active app server.'
             }, status=status.HTTP_400_BAD_REQUEST)
-        app_server.terminate_vm()
+        terminate_appserver(app_server.id)
         return Response({'status': 'App server termination initiated.'})

instance/models/server.py

@@ -364,9 +364,7 @@ def update_status(self):
                 # server must have been terminated.
                 self.logger.debug('Server does not exist anymore: %s', self)
                 self._status_to_terminated()
-            except (requests.RequestException,
-                    novaclient.exceptions.ClientException,
-                    novaclient.exceptions.EndpointNotFound) as exc:
+            except (requests.RequestException, novaclient.exceptions.ClientException) as exc:
                 self.logger.debug('Could not reach the OpenStack API due to %s', exc)
                 if self.status != Status.Unknown:
                     self._status_to_unknown()
@@ -397,7 +395,7 @@ def start(self,
                 key_name=key_name,
                 **kwargs
             )
-        except (novaclient.exceptions.ClientException, novaclient.exceptions.EndpointNotFound) as exc:
+        except novaclient.exceptions.ClientException as exc:
             self.logger.error('Failed to start server: %s', exc)
             self._status_to_build_failed()
         else:
@@ -456,9 +454,7 @@ def terminate(self):
         except novaclient.exceptions.NotFound:
             self.logger.error('Error while attempting to terminate server: could not find OS server')
             self._status_to_terminated()
-        except (requests.RequestException,
-                novaclient.exceptions.ClientException,
-                novaclient.exceptions.EndpointNotFound) as exc:
+        except (requests.RequestException, novaclient.exceptions.ClientException) as exc:
             self.logger.error('Unable to reach the OpenStack API due to %s', exc)
             if self.status != Status.Unknown:
                 self._status_to_unknown()

instance/tasks.py

@@ -248,6 +248,19 @@ def terminate_obsolete_appservers_all_instances():
             instance.logger.exception('Error deleting the obsolete appservers for instance %s', instance.domain)
 
 
+@db_task()
+def terminate_appserver(appserver_id):
+    """
+    Terminate a appserver on the background (in worker thread).
+    """
+    logger.info("Terminating appserver %s.", appserver_id)
+    try:
+        app_server = OpenEdXAppServer.objects.get(id=appserver_id)
+        app_server.terminate_vm()
+    except Exception as exc:  # pylint:disable=broad-except
+        logger.exception('Error terminating appserver %s: %s', appserver_id, exc)
+
+
 @db_periodic_task(crontab(day='*/1', hour='1', minute='0'))
 def clean_up():
     """

instance/tests/models/test_server.py

@@ -395,7 +395,6 @@ def raise_not_found():
     @data(
         requests.RequestException('Error'),
         novaclient.exceptions.ClientException('Error'),
-        novaclient.exceptions.EndpointNotFound('Error'),
     )
     def test_terminate_server_openstack_api_error(self, exception):
         """

instance/tests/test_tasks.py

@@ -798,3 +798,56 @@ def test_clean_up_betatest_user(self):
         tasks.cleanup_old_betatest_users()
         assert BetaTestApplication.objects.all().count() == 4
         assert get_user_model().objects.filter(username='user1').count() == 0
+
+
+class TerminateAppserverTestCase(TestCase):
+    """
+    Test cases for tasks.terminate_appserver, which wraps tasks.terminate_vm.
+    """
+    def setUp(self):
+        consul_patcher = patch(
+            'instance.tests.models.factories.openedx_instance.OpenEdXInstance._write_metadata_to_consul'
+        )
+        self.mock_consul = consul_patcher.start()
+        self.mock_consul.return_value = (1, True)
+        self.addCleanup(consul_patcher.stop)
+
+        terminate_appserver_patcher = patch(
+            'instance.models.openedx_appserver.OpenEdXAppServer.terminate_vm'
+        )
+        self.mock_terminate_appserver = terminate_appserver_patcher.start()
+        self.addCleanup(terminate_appserver_patcher.stop)
+
+        self.instance = OpenEdXInstanceFactory()
+        server = ReadyOpenStackServerFactory()
+        self.appserver = make_test_appserver(instance=self.instance, server=server)
+
+    def test_terminate_appserver(self):
+        """
+        Test that `terminate_appserver` calls `appserver.terminate_vm`.
+        """
+        with self.assertLogs(level='INFO') as logs:
+            tasks.terminate_appserver(self.appserver.id)
+            self.assertIn('INFO:instance.tasks:Terminating appserver', logs.output[0])
+
+        self.assertEqual(self.mock_terminate_appserver.call_count, 1)
+
+    def test_terminate_appserver_fails_no_id(self):
+        """
+        Test that `terminate_appserver` fails an invalid `appserver_id` is passed.
+        """
+        with self.assertLogs(level='ERROR') as logs:
+            tasks.terminate_appserver(3000)
+            self.assertIn('OpenEdXAppServer.DoesNotExist', logs.output[0])
+            self.assertEqual(self.mock_terminate_appserver.call_count, 0)
+
+    def test_terminate_appserver_logs_failure(self):
+        """
+        Test that `terminate_appserver` logs when any exception happens.
+        """
+        self.mock_terminate_appserver.side_effect = Exception('Some random generic error.')
+
+        with self.assertLogs(level='ERROR') as logs:
+            tasks.terminate_appserver(self.appserver.id)
+            self.assertIn('Exception: Some random generic error.', logs.output[0])
+            self.assertEqual(self.mock_terminate_appserver.call_count, 1)