Merge ~blake-rouse/maas:revert-fix-1789315 into maas:master

[Blake Rouse]

Revert 868d6ce2c7be21babc2afb04c5828512206f9a5f.

"Fix LP: #1789315 - Change webapp start-up to start the django application before opening the socket. Remove the start-up and failure pages."

[Andres Rodriguez (andreserl)]

lgtm, please reference the commit rev on the new commit message.

[Andres Rodriguez (andreserl)]

review: Approve

diff --git a/src/maasserver/tests/test_webapp.py b/src/maasserver/tests/test_webapp.py
index 6edf28a..bbf11bd 100644
--- a/src/maasserver/tests/test_webapp.py
+++ b/src/maasserver/tests/test_webapp.py
@@ -7,9 +7,11 @@
 __all__ = []
 
 import random
+from textwrap import dedent
 from unittest.mock import sentinel
 
 from django.core.handlers.wsgi import WSGIHandler
+from lxml import html
 from maasserver import (
     eventloop,
     webapp,
@@ -20,8 +22,10 @@ from maasserver.websockets.protocol import WebSocketFactory
 from maastesting.factory import factory
 from maastesting.matchers import MockCalledOnceWith
 from maastesting.testcase import MAASTestCase
+from maastesting.twisted import TwistedLoggerFixture
 from provisioningserver.utils.twisted import reducedWebLogFormatter
 from testtools.matchers import (
+    Equals,
     Is,
     IsInstance,
     MatchesStructure,
@@ -209,6 +213,25 @@ class TestWebApplicationService(MAASTestCase):
         ))
         self.assertThat(service.websocket, IsInstance(WebSocketFactory))
 
+    def test__default_site_renders_starting_page(self):
+        service = self.make_webapp()
+        request = DummyRequest("any/where".split("/"))
+        resource = service.site.getResourceFor(request)
+        content = resource.render(request)
+        page = html.fromstring(content)
+        self.expectThat(
+            page.find(".//title").text_content(),
+            Equals("503 - MAAS is starting"))
+        self.expectThat(
+            page.find(".//h1").text_content(),
+            Equals("MAAS is starting"))
+        self.expectThat(
+            page.find(".//p").text_content(),
+            Equals("Please try again in a few seconds."))
+        self.assertEqual(
+            ['5'],
+            request.responseHeaders.getRawHeaders("retry-after"))
+
     def test__startService_starts_application(self):
         service = self.make_webapp()
         self.addCleanup(service.stopService)
@@ -217,6 +240,56 @@ class TestWebApplicationService(MAASTestCase):
 
         self.assertTrue(service.running)
 
+    def test__error_when_starting_is_logged(self):
+        service = self.make_webapp()
+        self.addCleanup(service.stopService)
+
+        mock_prepare = self.patch_autospec(service, "prepareApplication")
+        mock_prepare.side_effect = factory.make_exception()
+
+        # The failure is logged.
+        with TwistedLoggerFixture() as logger:
+            service.startService()
+
+        self.assertDocTestMatches(
+            dedent("""\
+            MAAS web application failed to start
+            Traceback (most recent call last):
+            ...
+            maastesting.factory.TestException#...
+            """),
+            logger.output)
+
+    def test__error_when_starting_changes_page_to_error(self):
+        service = self.make_webapp()
+        self.addCleanup(service.stopService)
+
+        mock_prepare = self.patch_autospec(service, "prepareApplication")
+        mock_prepare.side_effect = factory.make_exception()
+
+        # No error is returned.
+        with TwistedLoggerFixture():
+            service.startService()
+
+        # The site's page (for any path) shows the error.
+        request = DummyRequest("any/where".split("/"))
+        resource = service.site.getResourceFor(request)
+        content = resource.render(request)
+        page = html.fromstring(content)
+        self.expectThat(
+            page.find(".//title").text_content(),
+            Equals("503 - MAAS failed to start"))
+        self.expectThat(
+            page.find(".//h1").text_content(),
+            Equals("MAAS failed to start"))
+        self.assertDocTestMatches(
+            dedent("""\
+            Traceback (most recent call last):
+            ...
+            maastesting.factory.TestException#...
+            """),
+            page.find(".//pre").text_content())
+
     def test__successful_start_installs_wsgi_resource(self):
         service = self.make_webapp()
         self.addCleanup(service.stopService)
diff --git a/src/maasserver/webapp.py b/src/maasserver/webapp.py
index 04a5b83..ae57a20 100644
--- a/src/maasserver/webapp.py
+++ b/src/maasserver/webapp.py
@@ -9,9 +9,11 @@ __all__ = [
 
 import copy
 from functools import partial
+from http.client import SERVICE_UNAVAILABLE
 import re
 
 from django.conf import settings
+from lxml import html
 from maasserver import concurrency
 from maasserver.utils.threads import deferToDatabase
 from maasserver.utils.views import WebApplicationHandler
@@ -30,8 +32,10 @@ from provisioningserver.utils.twisted import (
 from twisted.application.internet import StreamServerEndpointService
 from twisted.internet import reactor
 from twisted.internet.defer import inlineCallbacks
+from twisted.python import failure
 from twisted.web.error import UnsupportedMethod
 from twisted.web.resource import (
+    ErrorPage,
     NoResource,
     Resource,
 )
@@ -47,6 +51,27 @@ from twisted.web.wsgi import WSGIResource
 log = LegacyLogger()
 
 
+class StartPage(ErrorPage, object):
+    def __init__(self):
+        super(StartPage, self).__init__(
+            status=int(SERVICE_UNAVAILABLE), brief="MAAS is starting",
+            detail="Please try again in a few seconds.")
+
+    def render(self, request):
+        request.setHeader(b"Retry-After", b"5")
+        return super(StartPage, self).render(request)
+
+
+class StartFailedPage(ErrorPage, object):
+
+    def __init__(self, failure):
+        traceback = html.Element("pre")
+        traceback.text = failure.getTraceback()
+        super(StartFailedPage, self).__init__(
+            status=int(SERVICE_UNAVAILABLE), brief="MAAS failed to start",
+            detail=html.tostring(traceback, encoding=str))
+
+
 class CleanPathRequest(Request, object):
     """A request that supports '/+' in the path.
 
@@ -159,11 +184,8 @@ class WebApplicationService(StreamServerEndpointService):
     """
 
     def __init__(self, endpoint, listener, status_worker):
-        # Start with an empty `Resource`, `installApplication` will configure
-        # the root resource. This must be seperated because Django must be
-        # start from inside a thread with database access.
         self.site = OverlaySite(
-            Resource(), logFormatter=reducedWebLogFormatter, timeout=None)
+            StartPage(), logFormatter=reducedWebLogFormatter, timeout=None)
         self.site.requestFactory = CleanPathRequest
         super(WebApplicationService, self).__init__(endpoint, self.site)
         self.websocket = WebSocketFactory(listener)
@@ -220,21 +242,25 @@ class WebApplicationService(StreamServerEndpointService):
         self.site.resource = root
         self.site.underlay = underlay_site
 
+    def installFailed(self, failure):
+        """Display a page explaining why the web app could not start."""
+        self.site.resource = StartFailedPage(failure)
+        log.err(failure, "MAAS web application failed to start")
+
     @inlineCallbacks
     def startApplication(self):
         """Start the Django application, and install it."""
-        application = yield deferToDatabase(self.prepareApplication)
-        self.startWebsocket()
-        self.installApplication(application)
+        try:
+            application = yield deferToDatabase(self.prepareApplication)
+            self.startWebsocket()
+            self.installApplication(application)
+        except:
+            self.installFailed(failure.Failure())
 
     @asynchronous(timeout=30)
-    @inlineCallbacks
     def startService(self):
-        # Start the application first before starting the service. This ensures
-        # that the application is running correctly before any requests
-        # can be handled.
-        yield self.startApplication()
         super(WebApplicationService, self).startService()
+        return self.startApplication()
 
     @asynchronous(timeout=30)
     def stopService(self):