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

Use k8s liveness and readiness probes (HTTP GET only for now) to replace the defunct publisher healthcheck system. For now those two are conflated into one to support the current documented deis healthcheck workflow and an assumption of port 80 is in the code, as well as HTTP (no ssl).

All healthcheck related tests were axed as they are no longer applicable and in fact not possible to test until there is a more robust k8s testing strategy for workflow.

http://kubernetes.io/v1.1/docs/user-guide/pod-states.html#container-probes has more information on what probes can do and what's available

Fixes #76

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'})