Merge lp:~sinzui/charmworld/heartbeat into lp:~juju-jitsu/charmworld/trunk

[Curtis Hovey]

Add /heartbeat to verify the app is operational.

RULES

    * A review of API3 noted that the "latest" routes were updated and
      that they were moved. The routes were added for webops who were confused
      when APIs were removed. The 'latest' hack does not server webops well
      because they don't care about the API. They care about the app's
      operational health
    * We want a page that allows webops to see that the site is up
      * The page can list the checks it performed with pass and fail
        marks.
      * Developers can add checks as the site grows features
      * Webops can adapt the nagios checks to verify the site is operational.

QA

    * Visit http://staging.charmworld.com/heartbeat
      * Verify all the checks pass.
        * They might not pass if there are no featured charms!
      * Verify the response headers state the data is not cacheable.

    * See http://people.canonical.com/~curtis/heartbeat.png

[Richard Harding (rharding)]

A couple of notes. First, thanks for this. I think it's a great step.

This actually has me wondering if we can't move the logic into a charmworld/lib/health.py and keep the logic of adding/checking there make it more reusable. I'm actually thinking I'd love to provide a cli tool to sanity check your charmworld install for devs as well.

In the failing cases where we have on charm, for #IS I'd think that's a failing condition. So in #97 and the other checks like it should that set the status to fail?

Finally, if we're going to FAIL for no featured charms, should we adjust our bootstrap to auto select at least one charm to be featured automatically. This would help with the 'new dev setup' story as well as they could be sure to have featured already and be able to point the GUI at the dev instance without issue.

Sorry, this grew.

tl;dr
1. Looks good!
2. I think it'd be nice if the health checks were a module vs in the view.
3. I think we should auto select a single featured charm during install/setup?
4. Should 0 charms be failing for IS purposes?

review: Approve

[lp:~sinzui/charmworld/heartbeat]

332. By Curtis Hovey on 2013-07-24

Incomplete data is a fail condition.

333. By Curtis Hovey on 2013-07-24

DRY code.

334. By Curtis Hovey on 2013-07-24

Extract the three checks to the health module so that they are portable.

335. By Curtis Hovey on 2013-07-24

Duplicated the heartbeat tests as health tests. Next step is to make
both test modules test only the concerns.

336. By Curtis Hovey on 2013-07-24

Updated health tests to directly check the health functions.

337. By Curtis Hovey on 2013-07-24

Simplify the heartbeat tests to focus of the list of checks.

338. By Curtis Hovey on 2013-07-24

DRY code.

=== added file 'charmworld/health.py'
--- charmworld/health.py	1970-01-01 00:00:00 +0000
+++ charmworld/health.py	2013-07-24 16:11:42 +0000
@@ -0,0 +1,64 @@
+# Copyright 2013 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from collections import namedtuple
+
+
+Check = namedtuple('check', 'name status remark')
+PASS = 'Pass'
+FAIL = 'Fail'
+
+
+def check_mongodb(request):
+    """Return a Check of the mongodb charms collection."""
+    status = FAIL
+    try:
+        count = request.db.charms.count()
+        if count > 0:
+            remark = '{0} charms found'.format(count)
+            status = PASS
+        else:
+            remark = 'There are no charms. Is ingest running?'
+    except:
+        remark = 'The mongodb charm collection is not available.'
+    finally:
+        check = Check('charms collection', status, remark)
+    return check
+
+
+def check_elasticsearch(request):
+    """Return a Check of the elasticsearch charms index."""
+    status = FAIL
+    try:
+        results = request.index_client.search(terms='')
+        if results['charm_total'] > 0:
+            remark = '{0} matches found'.format(results['charm_total'])
+            status = PASS
+        else:
+            remark = 'There are no matches. Is ingest running?'
+    except:
+        remark = 'The elasticsearch charm index is not available.'
+    finally:
+        check = Check('charms index', status, remark)
+    return check
+
+
+def check_api(request):
+    """Return a Check of the API charms interesting end point."""
+    status = FAIL
+    try:
+        from charmworld.views.api import API2
+        api = API2(request)
+        interesting = api.charms(path=['interesting'])['result']
+        if (interesting['new']
+                and interesting['popular']
+                and interesting['featured']):
+            status = PASS
+            remark = 'Interesting has new, popular, and featured.'
+        else:
+            remark = 'Interesting is missing new, popular, or featured.'
+    except:
+        remark = 'The API is not available.'
+    finally:
+        check = Check('API interesting', status, remark)
+    return check

=== modified file 'charmworld/routes.py'
--- charmworld/routes.py	2013-07-19 13:28:20 +0000
+++ charmworld/routes.py	2013-07-24 16:11:42 +0000
@@ -83,20 +83,11 @@
     config.add_route('api-obsolete-0', '/api/0/*remaining')
     config.add_route('api-obsolete-1', '/api/1/*remaining')
 
-    # Provided for MONITORING purposes only! Not documented in the api docs so
-    # that people don't try to use this as the calls/results could change over
-    # time. Please be aware that IS is using this to verify that services are
-    # up and running. They may need to be consulted when this is changed to a
-    # later API version.
-    config.add_route('api_latest_single', '/api/latest/{endpoint}')
-    config.add_view('charmworld.views.api.API2',
-                    route_name='api_latest_single')
-    config.add_route('api_latest', '/api/latest/{endpoint}/*remainder')
-    config.add_view('charmworld.views.api.API2', route_name='api_latest')
-
     config.add_route('api_2_single', '/api/2/{endpoint}')
     config.add_view('charmworld.views.api.API2', route_name='api_2_single')
     config.add_route('api_2', '/api/2/{endpoint}/*remainder')
     config.add_view('charmworld.views.api.API2', route_name='api_2')
 
+    # Monitoring support.
+    config.add_route('heartbeat', '/heartbeat')
     return config

=== added file 'charmworld/templates/heartbeat.pt'
--- charmworld/templates/heartbeat.pt	1970-01-01 00:00:00 +0000
+++ charmworld/templates/heartbeat.pt	2013-07-24 16:11:42 +0000
@@ -0,0 +1,28 @@
+<html tal:define="main load: main.pt" metal:use-macro="main.macros['page']">
+<metal:block fill-slot="css_include">
+    <style type="text/css">
+        .checks th {
+            text-align: left;
+        }
+    </style>
+</metal:block>
+
+<metal:block fill-slot="content">
+    <h1>Heartbeat application checks</h1>
+
+    <p>
+        This app is operational if all checks pass,
+        but that does not mean the site is fully operational.
+        This app, and clients like juju-gui, are impaired though usable,
+        when data is missing.
+    </p>
+
+    <table class="checks table table-striped">
+        <tr tal:repeat="check checks">
+            <th>${check.name}</th>
+            <td>${check.status}</td>
+            <td>${check.remark}</td>
+        </tr>
+    </table>
+</metal:block>
+</html>

=== added file 'charmworld/tests/test_health.py'
--- charmworld/tests/test_health.py	1970-01-01 00:00:00 +0000
+++ charmworld/tests/test_health.py	2013-07-24 16:11:42 +0000
@@ -0,0 +1,94 @@
+# Copyright 2013 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from mock import patch
+
+from charmworld.health import (
+    check_api,
+    check_elasticsearch,
+    check_mongodb,
+)
+from charmworld.testing import (
+    factory,
+    ViewTestBase,
+)
+
+
+class TestCheckMixin:
+
+    def assertCheck(self, check, name, status, remark=None):
+        self.assertEqual(name, check.name)
+        self.assertEqual(status, check.status)
+        if remark is not None:
+            self.assertEqual(remark, check.remark)
+
+    @staticmethod
+    def makeFailCallable():
+        def failing(*args, **kwargs):
+            raise ValueError
+        return failing
+
+
+class HealthTestCase(ViewTestBase, TestCheckMixin):
+
+    def test_check_mongodb_pass(self):
+        factory.makeCharm(self.db)
+        check = check_mongodb(self.getRequest())
+        remark = '1 charms found'
+        self.assertCheck(check, 'charms collection', 'Pass', remark)
+
+    def test_check_mongodb_missing_data_fail(self):
+        check = check_mongodb(self.getRequest())
+        remark = 'There are no charms. Is ingest running?'
+        self.assertCheck(check, 'charms collection', 'Fail', remark)
+
+    def test_check_mongodb_no_collection_fail(self):
+        request = self.getRequest()
+        with patch.object(request, 'db', new_callable=self.makeFailCallable):
+            check = check_mongodb(request)
+        self.assertCheck(check, 'charms collection', 'Fail')
+
+    def test_check_elasticsearch_pass(self):
+        ignore, charm_data = factory.makeCharm(self.db)
+        self.use_index_client()
+        self.index_client.index_charm(charm_data)
+        check = check_elasticsearch(self.getRequest())
+        remark = '1 matches found'
+        self.assertCheck(check, 'charms index', 'Pass', remark)
+
+    def test_check_elasticsearch_missing_data_fail(self):
+        self.use_index_client()
+        request = self.getRequest()
+        check = check_elasticsearch(request)
+        remark = 'There are no matches. Is ingest running?'
+        self.assertCheck(check, 'charms index', 'Fail', remark)
+
+    def test_check_elasticsearch_no_index_fail(self):
+        request = self.getRequest()
+        # The index is not setup.
+        check = check_elasticsearch(request)
+        self.assertCheck(check, 'charms index', 'Fail')
+
+    def test_check_api_pass(self):
+        ignore, charm_data = factory.makeCharm(
+            self.db, downloads=5, is_featured=True)
+        self.use_index_client()
+        self.index_client.index_charm(charm_data)
+        check = check_api(self.getRequest())
+        remark = 'Interesting has new, popular, and featured.'
+        self.assertCheck(check, 'API interesting', 'Pass', remark)
+
+    def test_check_api_missing_data_fail(self):
+        ignore, charm_data = factory.makeCharm(self.db)
+        self.use_index_client()
+        self.index_client.index_charm(charm_data)
+        # There are no featured or downloaded charms.
+        check = check_api(self.getRequest())
+        remark = 'Interesting is missing new, popular, or featured.'
+        self.assertCheck(check, 'API interesting', 'Fail', remark)
+
+    def test_check_api_no_matches_fail(self):
+        request = self.getRequest()
+        # The index is not setup for the interesting search.
+        check = check_api(request)
+        self.assertCheck(check, 'API interesting', 'Fail')

=== modified file 'charmworld/views/misc.py'
--- charmworld/views/misc.py	2013-02-12 11:36:14 +0000
+++ charmworld/views/misc.py	2013-07-24 16:11:42 +0000
@@ -4,6 +4,11 @@
 from pyramid.view import view_config
 
 from charmworld import cached_view_config
+from charmworld.health import (
+    check_api,
+    check_elasticsearch,
+    check_mongodb,
+)
 
 
 # Basic app views
@@ -36,3 +41,16 @@
     owners.sort(lambda x, y: cmp(y[1], x[1]))
     owners.pop(0)
     return {"owners": owners}
+
+
+@view_config(
+    route_name="heartbeat",
+    http_cache=(0, {'public': True}),
+    renderer="charmworld:templates/heartbeat.pt")
+def heartbeat(request):
+    """Return a dict summarising the operational state of charmworld."""
+    checks = []
+    checks.append(check_mongodb(request))
+    checks.append(check_elasticsearch(request))
+    checks.append(check_api(request))
+    return {'checks': checks}

=== modified file 'charmworld/views/tests/test_misc.py'
--- charmworld/views/tests/test_misc.py	2013-02-15 17:37:23 +0000
+++ charmworld/views/tests/test_misc.py	2013-07-24 16:11:42 +0000
@@ -10,7 +10,13 @@
 from paste.deploy import loadapp
 from paste.deploy.config import PrefixMiddleware
 
-from charmworld.testing import WebTestBase
+from charmworld.testing import (
+    factory,
+    ViewTestBase,
+    WebTestBase,
+)
+from charmworld.views.misc import heartbeat
+from charmworld.tests.test_health import TestCheckMixin
 
 
 class TestAppConfig(unittest.TestCase):
@@ -81,3 +87,48 @@
             http_count += 1
             self.assertEqual('http', match.group(1))
         self.assertTrue(https_count == http_count)
+
+
+class HeartbeatWebTestCase(WebTestBase):
+
+    def test_heartbeat_cache_control(self):
+        # Never cache heartbeats.
+        response = self.app.get('/heartbeat', status=200)
+        body = response.body
+        # No data was setup, so heartbeat knows the site is not operational.
+        self.assertIn("Fail", body)
+        self.assertIn(
+            ('Cache-Control',
+             'max-age=0, must-revalidate, no-cache, no-store, public'),
+            response.headerlist)
+
+
+class HeartbeatViewTestCase(ViewTestBase, TestCheckMixin):
+
+    def test_all_checks_pass(self):
+        # All checks pass when the services are available and data is returned.
+        ignore, charm_data = factory.makeCharm(
+            self.db, downloads=5, is_featured=True)
+        self.use_index_client()
+        self.index_client.index_charm(charm_data)
+        response = heartbeat(self.getRequest())
+        checks = response['checks']
+        remark = '1 charms found'
+        self.assertCheck(checks[0], 'charms collection', 'Pass', remark)
+        remark = '1 matches found'
+        self.assertCheck(checks[1], 'charms index', 'Pass', remark)
+        remark = 'Interesting has new, popular, and featured.'
+        self.assertCheck(checks[2], 'API interesting', 'Pass', remark)
+
+    def test_checks_fail(self):
+        # When services or data are not available, the checks fail.
+        # No setup is performed, creating a case where services and data
+        # are not available
+        response = heartbeat(self.getRequest())
+        checks = response['checks']
+        remark = 'There are no charms. Is ingest running?'
+        self.assertCheck(checks[0], 'charms collection', 'Fail', remark)
+        remark = 'The elasticsearch charm index is not available.'
+        self.assertCheck(checks[1], 'charms index', 'Fail', remark)
+        remark = 'The API is not available.'
+        self.assertCheck(checks[2], 'API interesting', 'Fail', remark)