launchpad-reviewers team mailing list archive

["Francis J. Lacoste" <francis.lacoste@xxxxxxxxxx>]

Francis J. Lacoste has proposed merging lp:~flacoste/launchpad/bug-721324 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #721324 Make haproxy integration result code configurable
  https://bugs.launchpad.net/bugs/721324

For more details, see:
https://code.launchpad.net/~flacoste/launchpad/bug-721324/+merge/50365

This branch makes the result code to return when the app server is going down
configurable through a config variable.

That's for the +haproxy monitoring URL. We were returning 500, but it's
possible that 404 results in better behavior from HAProxy. We have to test to
make sure, so I'm making this result code configurable.

I didn't use a feature flag to control this because we don't want the +haproxy
URL to hit the database. (It gets a special DB policy and a
NullFeatureController.)

Let me know if you have any questions.

-- 
https://code.launchpad.net/~flacoste/launchpad/bug-721324/+merge/50365
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~flacoste/launchpad/bug-721324 into lp:launchpad.

=== modified file 'lib/canonical/config/schema-lazr.conf'
--- lib/canonical/config/schema-lazr.conf	2011-02-17 14:07:09 +0000
+++ lib/canonical/config/schema-lazr.conf	2011-02-18 17:44:00 +0000
@@ -941,6 +941,13 @@
 # datatype: int
 port: 11371
 
+[haproxy_status_view]
+# Configuration for the +haproxy monitoring URL
+
+# Result code to return when the app server is going down.
+# datatype: int
+going_down_status: 500
+
 [initialisedistroseries]
 dbuser: initialisedistroseries
 

=== modified file 'lib/canonical/launchpad/webapp/haproxy.py'
--- lib/canonical/launchpad/webapp/haproxy.py	2010-12-17 20:58:27 +0000
+++ lib/canonical/launchpad/webapp/haproxy.py	2011-02-18 17:44:00 +0000
@@ -11,6 +11,8 @@
     'switch_going_down_flag',
     ]
 
+from canonical.config import config
+
 # This is the global flag, when this is True, the HAProxy view
 # will return 500, it returns 200 otherwise.
 going_down_flag = False
@@ -46,6 +48,10 @@
     restart them. The probe URL will then return 500 and the app server will
     be taken out of rotation. Once HAProxy reports that all connections to the
     app servers are finished, we can restart the server safely.
+
+    The returned result code when the server is going down can be configured
+    through the haproxy_status_view.going_down_status configuration variable.
+    It defaults to 500.
     """
 
     def __init__(self, context, request):
@@ -55,8 +61,10 @@
     def __call__(self):
         """Return 200 or 500 depending on the global flag."""
         global going_down_flag
+
         if going_down_flag:
-            self.request.response.setStatus(500)
+            self.request.response.setStatus(
+                config.haproxy_status_view.going_down_status)
             return u"May day! May day! I'm going down. Stop the flood gate."
         else:
             self.request.response.setStatus(200)

=== modified file 'lib/canonical/launchpad/webapp/tests/test_haproxy.py'
--- lib/canonical/launchpad/webapp/tests/test_haproxy.py	2010-12-17 20:58:27 +0000
+++ lib/canonical/launchpad/webapp/tests/test_haproxy.py	2011-02-18 17:44:00 +0000
@@ -6,6 +6,9 @@
 __metaclass__ = type
 __all__ = []
 
+from textwrap import dedent
+
+from canonical.config import config
 from canonical.testing.layers import FunctionalLayer
 from canonical.launchpad.webapp import haproxy
 from canonical.launchpad.webapp.dbpolicy import (
@@ -15,6 +18,7 @@
 from canonical.launchpad.webapp.servers import LaunchpadTestRequest
 
 from zope.app.testing.functional import HTTPCaller
+
 from lp.testing import TestCase
 
 
@@ -47,3 +51,14 @@
         self.assertEquals(False, haproxy.going_down_flag)
         haproxy.switch_going_down_flag()
         self.assertEquals(True, haproxy.going_down_flag)
+
+    def test_HAProxyStatusView_configured_404(self):
+        config.push('change_haproxy_status_code', dedent('''
+            [haproxy_status_view]
+            going_down_status: 404
+            '''))
+        self.addCleanup(config.pop, 'change_haproxy_status_code')
+        haproxy.set_going_down_flag(True)
+        result = self.http(u'GET /+haproxy HTTP/1.0', handle_errors=False)
+        self.assertEquals(404, result.getStatus())
+