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

Proposed by Blake Rouse
Status: Merged
Approved by: Blake Rouse
Approved revision: 21b957c1336b04ece39cd605fda962589c2b4c01
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~blake-rouse/maas:revert-fix-1789315
Merge into: maas:master
Diff against target: 212 lines (+111/-12)
2 files modified
src/maasserver/tests/test_webapp.py (+73/-0)
src/maasserver/webapp.py (+38/-12)
Reviewer Review Type Date Requested Status
Andres Rodriguez (community) Approve
Review via email: mp+354726@code.launchpad.net

Commit message

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."

To post a comment you must log in.
Revision history for this message
Andres Rodriguez (andreserl) wrote :

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

Revision history for this message
Andres Rodriguez (andreserl) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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 @@
7__all__ = []7__all__ = []
88
9import random9import random
10from textwrap import dedent
10from unittest.mock import sentinel11from unittest.mock import sentinel
1112
12from django.core.handlers.wsgi import WSGIHandler13from django.core.handlers.wsgi import WSGIHandler
14from lxml import html
13from maasserver import (15from maasserver import (
14 eventloop,16 eventloop,
15 webapp,17 webapp,
@@ -20,8 +22,10 @@ from maasserver.websockets.protocol import WebSocketFactory
20from maastesting.factory import factory22from maastesting.factory import factory
21from maastesting.matchers import MockCalledOnceWith23from maastesting.matchers import MockCalledOnceWith
22from maastesting.testcase import MAASTestCase24from maastesting.testcase import MAASTestCase
25from maastesting.twisted import TwistedLoggerFixture
23from provisioningserver.utils.twisted import reducedWebLogFormatter26from provisioningserver.utils.twisted import reducedWebLogFormatter
24from testtools.matchers import (27from testtools.matchers import (
28 Equals,
25 Is,29 Is,
26 IsInstance,30 IsInstance,
27 MatchesStructure,31 MatchesStructure,
@@ -209,6 +213,25 @@ class TestWebApplicationService(MAASTestCase):
209 ))213 ))
210 self.assertThat(service.websocket, IsInstance(WebSocketFactory))214 self.assertThat(service.websocket, IsInstance(WebSocketFactory))
211215
216 def test__default_site_renders_starting_page(self):
217 service = self.make_webapp()
218 request = DummyRequest("any/where".split("/"))
219 resource = service.site.getResourceFor(request)
220 content = resource.render(request)
221 page = html.fromstring(content)
222 self.expectThat(
223 page.find(".//title").text_content(),
224 Equals("503 - MAAS is starting"))
225 self.expectThat(
226 page.find(".//h1").text_content(),
227 Equals("MAAS is starting"))
228 self.expectThat(
229 page.find(".//p").text_content(),
230 Equals("Please try again in a few seconds."))
231 self.assertEqual(
232 ['5'],
233 request.responseHeaders.getRawHeaders("retry-after"))
234
212 def test__startService_starts_application(self):235 def test__startService_starts_application(self):
213 service = self.make_webapp()236 service = self.make_webapp()
214 self.addCleanup(service.stopService)237 self.addCleanup(service.stopService)
@@ -217,6 +240,56 @@ class TestWebApplicationService(MAASTestCase):
217240
218 self.assertTrue(service.running)241 self.assertTrue(service.running)
219242
243 def test__error_when_starting_is_logged(self):
244 service = self.make_webapp()
245 self.addCleanup(service.stopService)
246
247 mock_prepare = self.patch_autospec(service, "prepareApplication")
248 mock_prepare.side_effect = factory.make_exception()
249
250 # The failure is logged.
251 with TwistedLoggerFixture() as logger:
252 service.startService()
253
254 self.assertDocTestMatches(
255 dedent("""\
256 MAAS web application failed to start
257 Traceback (most recent call last):
258 ...
259 maastesting.factory.TestException#...
260 """),
261 logger.output)
262
263 def test__error_when_starting_changes_page_to_error(self):
264 service = self.make_webapp()
265 self.addCleanup(service.stopService)
266
267 mock_prepare = self.patch_autospec(service, "prepareApplication")
268 mock_prepare.side_effect = factory.make_exception()
269
270 # No error is returned.
271 with TwistedLoggerFixture():
272 service.startService()
273
274 # The site's page (for any path) shows the error.
275 request = DummyRequest("any/where".split("/"))
276 resource = service.site.getResourceFor(request)
277 content = resource.render(request)
278 page = html.fromstring(content)
279 self.expectThat(
280 page.find(".//title").text_content(),
281 Equals("503 - MAAS failed to start"))
282 self.expectThat(
283 page.find(".//h1").text_content(),
284 Equals("MAAS failed to start"))
285 self.assertDocTestMatches(
286 dedent("""\
287 Traceback (most recent call last):
288 ...
289 maastesting.factory.TestException#...
290 """),
291 page.find(".//pre").text_content())
292
220 def test__successful_start_installs_wsgi_resource(self):293 def test__successful_start_installs_wsgi_resource(self):
221 service = self.make_webapp()294 service = self.make_webapp()
222 self.addCleanup(service.stopService)295 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__ = [
99
10import copy10import copy
11from functools import partial11from functools import partial
12from http.client import SERVICE_UNAVAILABLE
12import re13import re
1314
14from django.conf import settings15from django.conf import settings
16from lxml import html
15from maasserver import concurrency17from maasserver import concurrency
16from maasserver.utils.threads import deferToDatabase18from maasserver.utils.threads import deferToDatabase
17from maasserver.utils.views import WebApplicationHandler19from maasserver.utils.views import WebApplicationHandler
@@ -30,8 +32,10 @@ from provisioningserver.utils.twisted import (
30from twisted.application.internet import StreamServerEndpointService32from twisted.application.internet import StreamServerEndpointService
31from twisted.internet import reactor33from twisted.internet import reactor
32from twisted.internet.defer import inlineCallbacks34from twisted.internet.defer import inlineCallbacks
35from twisted.python import failure
33from twisted.web.error import UnsupportedMethod36from twisted.web.error import UnsupportedMethod
34from twisted.web.resource import (37from twisted.web.resource import (
38 ErrorPage,
35 NoResource,39 NoResource,
36 Resource,40 Resource,
37)41)
@@ -47,6 +51,27 @@ from twisted.web.wsgi import WSGIResource
47log = LegacyLogger()51log = LegacyLogger()
4852
4953
54class StartPage(ErrorPage, object):
55 def __init__(self):
56 super(StartPage, self).__init__(
57 status=int(SERVICE_UNAVAILABLE), brief="MAAS is starting",
58 detail="Please try again in a few seconds.")
59
60 def render(self, request):
61 request.setHeader(b"Retry-After", b"5")
62 return super(StartPage, self).render(request)
63
64
65class StartFailedPage(ErrorPage, object):
66
67 def __init__(self, failure):
68 traceback = html.Element("pre")
69 traceback.text = failure.getTraceback()
70 super(StartFailedPage, self).__init__(
71 status=int(SERVICE_UNAVAILABLE), brief="MAAS failed to start",
72 detail=html.tostring(traceback, encoding=str))
73
74
50class CleanPathRequest(Request, object):75class CleanPathRequest(Request, object):
51 """A request that supports '/+' in the path.76 """A request that supports '/+' in the path.
5277
@@ -159,11 +184,8 @@ class WebApplicationService(StreamServerEndpointService):
159 """184 """
160185
161 def __init__(self, endpoint, listener, status_worker):186 def __init__(self, endpoint, listener, status_worker):
162 # Start with an empty `Resource`, `installApplication` will configure
163 # the root resource. This must be seperated because Django must be
164 # start from inside a thread with database access.
165 self.site = OverlaySite(187 self.site = OverlaySite(
166 Resource(), logFormatter=reducedWebLogFormatter, timeout=None)188 StartPage(), logFormatter=reducedWebLogFormatter, timeout=None)
167 self.site.requestFactory = CleanPathRequest189 self.site.requestFactory = CleanPathRequest
168 super(WebApplicationService, self).__init__(endpoint, self.site)190 super(WebApplicationService, self).__init__(endpoint, self.site)
169 self.websocket = WebSocketFactory(listener)191 self.websocket = WebSocketFactory(listener)
@@ -220,21 +242,25 @@ class WebApplicationService(StreamServerEndpointService):
220 self.site.resource = root242 self.site.resource = root
221 self.site.underlay = underlay_site243 self.site.underlay = underlay_site
222244
245 def installFailed(self, failure):
246 """Display a page explaining why the web app could not start."""
247 self.site.resource = StartFailedPage(failure)
248 log.err(failure, "MAAS web application failed to start")
249
223 @inlineCallbacks250 @inlineCallbacks
224 def startApplication(self):251 def startApplication(self):
225 """Start the Django application, and install it."""252 """Start the Django application, and install it."""
226 application = yield deferToDatabase(self.prepareApplication)253 try:
227 self.startWebsocket()254 application = yield deferToDatabase(self.prepareApplication)
228 self.installApplication(application)255 self.startWebsocket()
256 self.installApplication(application)
257 except:
258 self.installFailed(failure.Failure())
229259
230 @asynchronous(timeout=30)260 @asynchronous(timeout=30)
231 @inlineCallbacks
232 def startService(self):261 def startService(self):
233 # Start the application first before starting the service. This ensures
234 # that the application is running correctly before any requests
235 # can be handled.
236 yield self.startApplication()
237 super(WebApplicationService, self).startService()262 super(WebApplicationService, self).startService()
263 return self.startApplication()
238264
239 @asynchronous(timeout=30)265 @asynchronous(timeout=30)
240 def stopService(self):266 def stopService(self):

Subscribers

People subscribed via source and target branches