Merge pull request #242 from helgi/healthchecks_k8s

feat(healthcheck): use k8s liveness and readiness probes for app healthchecks

Author: helgi

docs/src/using-deis/configuring-an-application.md

@@ -78,7 +78,7 @@ appname to the old one:
 ## Custom Health Checks
 
 By default, Deis only checks that a container is running. You can add a healthcheck by configuring a
-URL, initial delay, and timeout value:
+URL, port, initial delay, and timeout value:
 
     $ deis config:set HEALTHCHECK_URL=/200.html
     === peachy-waxworks
@@ -92,12 +92,19 @@ URL, initial delay, and timeout value:
     HEALTHCHECK_TIMEOUT: 5
     HEALTHCHECK_INITIAL_DELAY: 5
     HEALTHCHECK_URL: /200.html
+    $ deis config:set HEALTHCHECK_PORT=5000
+    === peachy-waxworks
+    HEALTHCHECK_TIMEOUT: 5
+    HEALTHCHECK_INITIAL_DELAY: 5
+    HEALTHCHECK_URL: /200.html
+    HEALTHCHECK_PORT: 5000
 
 If a new release does not pass the healthcheck, the application will be rolled back to the previous
 release. Beyond that, if an application container responds to a heartbeat check with a different
-status than a 200 OK, the [router][] will mark that container as down and stop sending
+status than a 200-399, the [router][] will mark that container as down and stop sending
 requests to that container.
 
+The health checks are provided by [kubernetes health checks][]. Currently only HTTP GET is supported.
 
 ## Track Changes
 
@@ -135,3 +142,4 @@ Use `deis rollback` to revert to a previous release.
 [stores config in environment variables]: http://12factor.net/config
 [release]: ../reference-guide/terms.md#release
 [router]:  ../understanding-deis/components.md#router
+[kubernetes health checks]: http://kubernetes.io/v1.1/docs/user-guide/pod-states.html#container-probes

rootfs/api/models.py

@@ -11,7 +11,6 @@
 import importlib
 import logging
 import re
-import time
 import uuid
 import morph
 from threading import Thread
@@ -29,7 +28,7 @@
 import requests
 from rest_framework.authtoken.models import Token
 
-from api import utils, exceptions
+from api import utils
 from registry import publish_release
 from utils import dict_diff, dict_merge, fingerprint
 
@@ -322,13 +321,18 @@ def _scale_containers(self, scale_types, to_remove):
         for scale_type in scale_types:
             image = release.image
             version = "v{}".format(release.version)
-            kwargs = {'memory': release.config.memory,
-                      'cpu': release.config.cpu,
-                      'tags': release.config.tags,
-                      'envs': release.config.values,
-                      'version': version,
-                      'aname': self.id,
-                      'num': scale_types[scale_type]}
+            kwargs = {
+                'memory': release.config.memory,
+                'cpu': release.config.cpu,
+                'tags': release.config.tags,
+                'envs': release.config.values,
+                'version': version,
+                'aname': self.id,
+                'num': scale_types[scale_type],
+                'app_type': scale_type,
+                'healthcheck': release.config.healthcheck()
+            }
+
             job_id = self._get_job_id(scale_type)
             command = self._get_command(scale_type)
             try:
@@ -343,6 +347,7 @@ def _scale_containers(self, scale_types, to_remove):
                 # scheduler.scale creates the required service on apps:create
                 if not Domain.objects.filter(owner=self.owner, app=self, domain=self).exists():
                     Domain(owner=self.owner, app=self, domain=str(self)).save()
+
             except Exception as e:
                 err = '{} (scale): {}'.format(job_id, e)
                 log_event(self, err, logging.ERROR)
@@ -367,59 +372,6 @@ def _start_containers(self, to_add):
         if set([c.state for c in to_add]) != set(['up']):
             err = 'warning, some containers failed to start'
             log_event(self, err, logging.WARNING)
-        # if the user specified a health check, try checking to see if it's running
-        try:
-            config = self.config_set.latest()
-            if 'HEALTHCHECK_URL' in config.values.keys():
-                self._healthcheck(to_add, config.values)
-        except Config.DoesNotExist:
-            pass
-
-    def _healthcheck(self, containers, config):
-        # if at first it fails, back off and try again at 10%, 50% and 100% of INITIAL_DELAY
-        intervals = [1.0, 0.1, 0.5, 1.0]
-        # HACK (bacongobbler): we need to wait until publisher has a chance to publish each
-        # service to etcd, which can take up to 20 seconds.
-        time.sleep(20)
-        for i in xrange(len(intervals)):
-            delay = int(config.get('HEALTHCHECK_INITIAL_DELAY', 0))
-            try:
-                # sleep until the initial timeout is over
-                if delay > 0:
-                    time.sleep(delay * intervals[i])
-                to_healthcheck = [c for c in containers if c.type in ['web', 'cmd']]
-                self._do_healthcheck(to_healthcheck, config)
-                break
-            except exceptions.HealthcheckException as e:
-                try:
-                    next_delay = delay * intervals[i+1]
-                    msg = "{}; trying again in {} seconds".format(e, next_delay)
-                    log_event(self, msg, logging.WARNING)
-                except IndexError:
-                    log_event(self, e, logging.WARNING)
-        else:
-            self._destroy_containers(containers)
-            msg = "aborting, app containers failed to respond to health check"
-            log_event(self, msg, logging.ERROR)
-            raise RuntimeError(msg)
-
-    def _do_healthcheck(self, containers, config):
-        path = config.get('HEALTHCHECK_URL', '/')
-        timeout = int(config.get('HEALTHCHECK_TIMEOUT', 1))
-        if not _etcd_client:
-            raise exceptions.HealthcheckException('no etcd client available')
-        for container in containers:
-            try:
-                key = "/deis/services/{self}/{container.job_id}".format(**locals())
-                url = "http://{}{}".format(_etcd_client.get(key).value, path)
-                response = requests.get(url, timeout=timeout)
-                if response.status_code != requests.codes.OK:
-                    raise exceptions.HealthcheckException(
-                        "app failed health check (got '{}', expected: '200')".format(
-                            response.status_code))
-            except (requests.Timeout, requests.ConnectionError, KeyError) as e:
-                raise exceptions.HealthcheckException(
-                    'failed to connect to container ({})'.format(e))
 
     def _restart_containers(self, to_restart):
         """Restarts containers via the scheduler"""
@@ -479,13 +431,18 @@ def _deploy_app(self, scale_types, release, existing):
         for scale_type in scale_types:
             image = release.image
             version = "v{}".format(release.version)
-            kwargs = {'memory': release.config.memory,
-                      'cpu': release.config.cpu,
-                      'tags': release.config.tags,
-                      'envs': release.config.values,
-                      'aname': self.id,
-                      'num': 0,
-                      'version': version}
+            kwargs = {
+                'memory': release.config.memory,
+                'cpu': release.config.cpu,
+                'tags': release.config.tags,
+                'envs': release.config.values,
+                'aname': self.id,
+                'num': 0,
+                'version': version,
+                'app_type': scale_type,
+                'healthcheck': release.config.healthcheck()
+            }
+
             job_id = self._get_job_id(scale_type)
             command = self._get_command(scale_type)
             try:
@@ -793,6 +750,15 @@ class Meta:
     def __str__(self):
         return "{}-{}".format(self.app.id, str(self.uuid)[:7])
 
+    def healthcheck(self):
+        # Update healthcheck - Scheduler determines the app type
+        path = self.values.get('HEALTHCHECK_URL', '/')
+        timeout = int(self.values.get('HEALTHCHECK_TIMEOUT', 1))
+        delay = int(self.values.get('HEALTHCHECK_INITIAL_DELAY', 10))
+        port = int(self.values.get('HEALTHCHECK_PORT', 8080))
+
+        return {'path': path, 'timeout': timeout, 'delay': delay, 'port': port}
+
     def save(self, **kwargs):
         """merge the old config with the new"""
         try:
@@ -804,16 +770,19 @@ def save(self, **kwargs):
                     data = getattr(previous_config, attr).copy()
                 except AttributeError:
                     data = {}
+
                 try:
                     new_data = getattr(self, attr).copy()
                 except AttributeError:
                     new_data = {}
+
                 data.update(new_data)
                 # remove config keys if we provided a null value
                 [data.pop(k) for k, v in new_data.viewitems() if v is None]
                 setattr(self, attr, data)
         except Config.DoesNotExist:
             pass
+
         return super(Config, self).save(**kwargs)
 
 

rootfs/api/tests/test_config.py

@@ -8,16 +8,13 @@
 from __future__ import unicode_literals
 
 import json
-import logging
 import requests
 
 from django.contrib.auth.models import User
 from django.test import TransactionTestCase
-import etcd
 import mock
 from rest_framework.authtoken.models import Token
 
-import api.exceptions
 from api.models import App, Config
 from . import mock_status_ok
 
@@ -33,19 +30,6 @@ def mock_request_connection_error(*args, **kwargs):
     raise requests.exceptions.ConnectionError("connection error")
 
 
-class MockEtcdClient:
-
-    def __init__(self, app):
-        self.app = app
-
-    def get(self, key, *args, **kwargs):
-        node = {
-            'key': '/deis/services/{}/{}_v2.web.1'.format(self.app, self.app),
-            'value': '127.0.0.1:1234'
-        }
-        return etcd.EtcdResult(None, node)
-
-
 @mock.patch('api.models.publish_release', lambda *args: None)
 class ConfigTest(TransactionTestCase):
 
@@ -570,142 +554,3 @@ def test_unauthorized_user_cannot_modify_config(self):
         response = self.client.post(url, json.dumps(body), content_type='application/json',
                                     HTTP_AUTHORIZATION='token {}'.format(unauthorized_token))
         self.assertEqual(response.status_code, 403)
-
-    def _test_app_healthcheck(self):
-        # post a new build, expecting it to pass as usual
-        url = "/v2/apps/{self.app}/builds".format(**locals())
-        body = {'image': 'autotest/example'}
-        response = self.client.post(url, json.dumps(body), content_type='application/json',
-                                    HTTP_AUTHORIZATION='token {}'.format(self.token))
-        self.assertEqual(response.status_code, 201)
-        # mock out the etcd client
-        api.models._etcd_client = MockEtcdClient(self.app)
-        # set an initial healthcheck url.
-        url = "/v2/apps/{self.app}/config".format(**locals())
-        body = {'values': json.dumps({'HEALTHCHECK_URL': '/'})}
-        return self.client.post(url, json.dumps(body), content_type='application/json',
-                                HTTP_AUTHORIZATION='token {}'.format(self.token))
-
-    @mock.patch('requests.get', mock_status_ok)
-    @mock.patch('time.sleep', lambda func: func)
-    def test_app_healthcheck_good(self):
-        """
-        If a user deploys an app with a config value set for HEALTHCHECK_URL, the controller
-        should check that the application responds with a 200 OK.
-        """
-        response = self._test_app_healthcheck()
-        self.assertEqual(response.status_code, 201)
-        self.assertEqual(self.app.release_set.latest().version, 3)
-
-    @mock.patch('requests.get', mock_status_not_found)
-    @mock.patch('api.models.get_etcd_client', lambda func: func)
-    @mock.patch('time.sleep', lambda func: func)
-    @mock.patch('api.models.logger')
-    def test_app_healthcheck_bad(self, mock_logger):
-        """
-        If a user deploys an app with a config value set for HEALTHCHECK_URL, the controller
-        should check that the application responds with a 200 OK. If it's down, the app should be
-        rolled back.
-        """
-        response = self._test_app_healthcheck()
-        self.assertEqual(response.status_code, 503)
-        self.assertEqual(
-            response.data,
-            {'detail': 'aborting, app containers failed to respond to health check'})
-        # check that only the build and initial release exist
-        self.assertEqual(self.app.release_set.latest().version, 2)
-        # assert that the reason why the containers failed was because
-        # they failed the health check 4 times; we do this by looking
-        # at logs-- there may be a better way
-        exp_msg = "{}: app failed health check (got '404', expected: '200'); trying again in 0.0 \
-seconds".format(self.app.id)
-        exp_log_call = mock.call(logging.WARNING, exp_msg)
-        log_calls = mock_logger.log.mock_calls
-        self.assertEqual(log_calls.count(exp_log_call), 3)
-        exp_msg = "{}: app failed health check (got '404', expected: '200')".format(self.app.id)
-        exp_log_call = mock.call(logging.WARNING, exp_msg)
-        self.assertEqual(log_calls.count(exp_log_call), 1)
-
-    @mock.patch('requests.get', mock_status_not_found)
-    @mock.patch('api.models.get_etcd_client', lambda func: func)
-    @mock.patch('time.sleep')
-    def test_app_backoff_interval(self, mock_time):
-        """
-        Ensure that when a healthcheck fails, a backoff strategy is used before trying again.
-        """
-        # post a new build, expecting it to pass as usual
-        url = "/v2/apps/{self.app}/builds".format(**locals())
-        body = {'image': 'autotest/example'}
-        response = self.client.post(url, json.dumps(body), content_type='application/json',
-                                    HTTP_AUTHORIZATION='token {}'.format(self.token))
-        self.assertEqual(response.status_code, 201)
-        # mock out the etcd client
-        api.models._etcd_client = MockEtcdClient(self.app)
-        # set an initial healthcheck url.
-        url = "/v2/apps/{self.app}/config".format(**locals())
-        body = {'values': json.dumps({'HEALTHCHECK_URL': '/'})}
-        return self.client.post(url, json.dumps(body), content_type='application/json',
-                                HTTP_AUTHORIZATION='token {}'.format(self.token))
-        self.assertEqual(mock_time.call_count, 5)
-
-    @mock.patch('requests.get', mock_status_ok)
-    @mock.patch('time.sleep')
-    def test_app_healthcheck_initial_delay(self, mock_time):
-        """
-        Ensure that when an initial delay is set, the request will sleep for x seconds, where
-        x is the number of seconds in the initial timeout.
-        """
-        # post a new build, expecting it to pass as usual
-        url = "/v2/apps/{self.app}/builds".format(**locals())
-        body = {'image': 'autotest/example'}
-        response = self.client.post(url, json.dumps(body), content_type='application/json',
-                                    HTTP_AUTHORIZATION='token {}'.format(self.token))
-        self.assertEqual(response.status_code, 201)
-        # mock out the etcd client
-        api.models._etcd_client = MockEtcdClient(self.app)
-        # set an initial healthcheck url.
-        url = "/v2/apps/{self.app}/config".format(**locals())
-        body = {'values': json.dumps({'HEALTHCHECK_URL': '/'})}
-        return self.client.post(url, json.dumps(body), content_type='application/json',
-                                HTTP_AUTHORIZATION='token {}'.format(self.token))
-        # mock_time increments by one each time its called, so we should expect 2 calls to
-        # mock_time; one for the call in the code, and one for this invocation.
-        mock_time.assert_called_with(0)
-        app = App.objects.all()[0]
-        url = "/v2/apps/{app}/config".format(**locals())
-        body = {'values': json.dumps({'HEALTHCHECK_INITIAL_DELAY': 10})}
-        self.client.post(url, json.dumps(body), content_type='application/json',
-                         HTTP_AUTHORIZATION='token {}'.format(self.token))
-        mock_time.assert_called_with(10)
-
-    @mock.patch('requests.get')
-    @mock.patch('time.sleep', lambda func: func)
-    def test_app_healthcheck_timeout(self, mock_request):
-        """
-        Ensure when a timeout value is set, the controller respects that value
-        when making a request.
-        """
-        self._test_app_healthcheck()
-        app = App.objects.all()[0]
-        url = "/v2/apps/{app}/config".format(**locals())
-        body = {'values': json.dumps({'HEALTHCHECK_TIMEOUT': 10})}
-        self.client.post(url, json.dumps(body), content_type='application/json',
-                         HTTP_AUTHORIZATION='token {}'.format(self.token))
-        mock_request.assert_called_with('http://127.0.0.1:1234/', timeout=10)
-
-    @mock.patch('requests.get', mock_request_connection_error)
-    @mock.patch('time.sleep', lambda func: func)
-    def test_app_healthcheck_connection_error(self):
-        """
-        If a user deploys an app with a config value set for HEALTHCHECK_URL but the app
-        returns a connection error, the controller should continue checking until either the app
-        responds or the app fails to respond within the timeout.
-
-        NOTE (bacongobbler): the Docker userland proxy listens for connections and returns a
-        ConnectionError, hence the unit test.
-        """
-        response = self._test_app_healthcheck()
-        self.assertEqual(response.status_code, 503)
-        self.assertEqual(
-            response.data,
-            {'detail': 'aborting, app containers failed to respond to health check'})