Merge pull request #557 from helgi/ticket_551

feat(apps): validate routeability through deis-router upon application creation

Author: helgi

rootfs/api/models/app.py

@@ -3,15 +3,19 @@
 import random
 import re
 import requests
+from requests_toolbelt import user_agent
 import string
 import time
+from urllib.parse import urljoin
 
 from django.conf import settings
 from django.db import models
 from django.core.exceptions import ValidationError
 from jsonfield import JSONField
 
+from deis import __version__ as deis_version
 from api.models import UuidAuditedModel, log_event, AlreadyExists
+
 from api.utils import generate_app_name, app_build_type
 from api.models.release import Release
 from api.models.config import Config
@@ -304,7 +308,9 @@ def _scale_pods(self, scale_types):
                 'replicas': scale_types[scale_type],
                 'app_type': scale_type,
                 'build_type': build_type,
-                'healthcheck': release.config.healthcheck()
+                'healthcheck': release.config.healthcheck(),
+                # http://docs.deis.io/en/latest/using_deis/process-types/#web-vs-cmd-process-types
+                'routable': True if scale_type in ['web', 'cmd'] else False
             }
 
             command = self._get_command(scale_type)
@@ -348,7 +354,9 @@ def deploy(self, user, release):
                 'version': version,
                 'app_type': scale_type,
                 'build_type': build_type,
-                'healthcheck': release.config.healthcheck()
+                'healthcheck': release.config.healthcheck(),
+                # http://docs.deis.io/en/latest/using_deis/process-types/#web-vs-cmd-process-types
+                'routable': True if scale_type in ['web', 'cmd'] else False
             }
 
             command = self._get_command(scale_type)
@@ -360,6 +368,13 @@ def deploy(self, user, release):
                     command=command,
                     **kwargs
                 )
+
+                # Wait until application is available in the router
+                # Only run when there is no previous build / release
+                old = release.previous()
+                if old is None or old.build is None:
+                    self.verify_application_health(**kwargs)
+
             except Exception as e:
                 err = '{} (app::deploy): {}'.format(self._get_job_id(scale_type), e)
                 log_event(self, err, logging.ERROR)
@@ -385,6 +400,79 @@ def _default_structure(self, release):
 
         return structure
 
+    def verify_application_health(self, **kwargs):
+        """
+        Verify an application is healthy via the router.
+        This is only used in conjunction with the kubernetes health check system and should
+        only run after kubernetes has reported all pods as healthy
+        """
+        # Bail out early if the application is not routable
+        if not kwargs.get('routable', False):
+            return
+
+        app_type = kwargs.get('app_type')
+        self.log(
+            'Waiting for router to be ready to serve traffic to process type {}'.format(app_type),
+            level=logging.DEBUG
+        )
+
+        # Get the router host and append healthcheck path
+        url = 'http://{}:{}'.format(settings.ROUTER_HOST, settings.ROUTER_PORT)
+
+        # if a health check url is available then 200 is the only acceptable status code
+        if len(kwargs['healthcheck']):
+            allowed = [200]
+            url = urljoin(url, kwargs['healthcheck'].get('path'))
+            req_timeout = kwargs['healthcheck'].get('timeout')
+        else:
+            allowed = set(range(200, 599))
+            allowed.remove(404)
+            req_timeout = 3
+
+        session = requests.Session()
+        session.headers = {
+            # https://toolbelt.readthedocs.org/en/latest/user-agent.html#user-agent-constructor
+            'User-Agent': user_agent('Deis Controller', deis_version),
+            # set the Host header for the application being checked - not used for actual routing
+            'Host': '{}.{}.nip.io'.format(self.id, settings.ROUTER_HOST)
+        }
+
+        # `mount` a custom adapter that retries failed connections for HTTP and HTTPS requests.
+        # http://docs.python-requests.org/en/latest/api/#requests.adapters.HTTPAdapter
+        session.mount('http://', requests.adapters.HTTPAdapter(max_retries=10))
+        session.mount('https://', requests.adapters.HTTPAdapter(max_retries=10))
+
+        # Give the router max of 10 tries or max 30 seconds to become healthy
+        # Uses time module to account for the timout value of 3 seconds
+        start = time.time()
+        for _ in range(10):
+            # http://docs.python-requests.org/en/master/user/advanced/#timeouts
+            response = session.get(url, timeout=req_timeout)
+
+            # 1 minute timeout
+            if (time.time() - start) > (req_timeout * 10):
+                break
+
+            # check response against the allowed pool
+            if response.status_code in allowed:
+                break
+
+            # a small sleep since router usually resolve within 10 seconds
+            time.sleep(1)
+
+        # Endpoint did not report healthy in time
+        if response.status_code == 404:
+            self.log(
+                'Router was not ready to serve traffic to process type {} in time'.format(app_type),  # noqa
+                level=logging.WARNING
+            )
+            return
+
+        self.log(
+            'Router is ready to serve traffic to process type {}'.format(app_type),
+            level=logging.DEBUG
+        )
+
     def logs(self, log_lines=str(settings.LOG_LINES)):
         """Return aggregated log data for this application."""
         try:

rootfs/api/tests/__init__.py

@@ -1,8 +1,41 @@
 import logging
+import random
+import requests_mock
+import time
 
+from django.conf import settings
 from django.test.runner import DiscoverRunner
 
 
+# Mock out router requests and add in some jitter
+# Used for application is available in router checks
+def fake_responses(request, context):
+    responses = [
+        # increasing the chance of 404
+        {'text': 'Not Found', 'status_code': 404},
+        {'text': 'Not Found', 'status_code': 404},
+        {'text': 'Not Found', 'status_code': 404},
+        {'text': 'Not Found', 'status_code': 404},
+        {'text': 'OK', 'status_code': 200},
+        {'text': 'Gateway timeout', 'status_code': 504},
+        {'text': 'Bad gateway', 'status_code': 502},
+    ]
+    random.shuffle(responses)
+    response = responses.pop()
+
+    context.status_code = response['status_code']
+    context.reason = response['text']
+    # Random float x, 1.0 <= x < 4.0 for some sleep jitter
+    time.sleep(random.uniform(1, 4))
+    return response['text']
+
+url = 'http://{}:{}'.format(settings.ROUTER_HOST, settings.ROUTER_PORT)
+adapter = requests_mock.Adapter()
+adapter.register_uri('GET', url + '/', text=fake_responses)
+adapter.register_uri('GET', url + '/health', text=fake_responses)
+adapter.register_uri('GET', url + '/healthz', text=fake_responses)
+
+
 class SilentDjangoTestSuiteRunner(DiscoverRunner):
     """Prevents api log messages from cluttering the console during tests."""
 

rootfs/api/tests/test_app.py

@@ -14,11 +14,15 @@
 
 from api.models import App
 
+from . import adapter
+import requests_mock
+
 
 def mock_none(*args, **kwargs):
     return None
 
 
+@requests_mock.Mocker(real_http=True, adapter=adapter)
 class AppTest(APITestCase):
     """Tests creation of applications"""
 
@@ -33,7 +37,7 @@ def tearDown(self):
         # make sure every test has a clean slate for k8s mocking
         cache.clear()
 
-    def test_app(self):
+    def test_app(self, mock_requests):
         """
         Test that a user can create, read, update and delete an application
         """
@@ -54,7 +58,7 @@ def test_app(self):
         response = self.client.delete(url)
         self.assertEqual(response.status_code, 204)
 
-    def test_response_data(self):
+    def test_response_data(self, mock_requests):
         """Test that the serialized response contains only relevant data."""
         body = {'id': 'test'}
         response = self.client.post('/v2/apps', body)
@@ -67,7 +71,7 @@ def test_response_data(self):
         }
         self.assertDictContainsSubset(expected, response.data)
 
-    def test_app_override_id(self):
+    def test_app_override_id(self, mock_requests):
         body = {'id': 'myid'}
         response = self.client.post('/v2/apps', body)
         self.assertEqual(response.status_code, 201)
@@ -77,7 +81,7 @@ def test_app_override_id(self):
         return response
 
     @mock.patch('requests.get')
-    def test_app_actions(self, mock_get):
+    def test_app_actions(self, mock_requests, mock_get):
         url = '/v2/apps'
         body = {'id': 'autotest'}
         response = self.client.post(url, body)
@@ -121,7 +125,7 @@ def test_app_actions(self, mock_get):
         # TODO: test run needs an initial build
 
     @mock.patch('api.models.logger')
-    def test_app_release_notes_in_logs(self, mock_logger):
+    def test_app_release_notes_in_logs(self, mock_requests, mock_logger):
         """Verifies that an app's release summary is dumped into the logs."""
         url = '/v2/apps'
         body = {'id': 'autotest'}
@@ -132,7 +136,7 @@ def test_app_release_notes_in_logs(self, mock_logger):
         exp_log_call = mock.call(logging.INFO, exp_msg)
         mock_logger.log.has_calls(exp_log_call)
 
-    def test_app_errors(self):
+    def test_app_errors(self, mock_requests):
         app_id = 'autotest-errors'
         url = '/v2/apps'
         body = {'id': 'camelCase'}
@@ -155,7 +159,7 @@ def test_app_errors(self):
             response = self.client.get(url)
             self.assertEqual(response.status_code, 404)
 
-    def test_app_reserved_names(self):
+    def test_app_reserved_names(self, mock_requests):
         """Nobody should be able to create applications with names which are reserved."""
         url = '/v2/apps'
         reserved_names = ['foo', 'bar']
@@ -168,7 +172,7 @@ def test_app_reserved_names(self):
                     '{} is a reserved name.'.format(name),
                     status_code=400)
 
-    def test_app_structure_is_valid_json(self):
+    def test_app_structure_is_valid_json(self, mock_requests):
         """Application structures should be valid JSON objects."""
         url = '/v2/apps'
         response = self.client.post(url)
@@ -185,7 +189,7 @@ def test_app_structure_is_valid_json(self):
         self.assertEqual(response.data['structure'], {"web": 1})
 
     @mock.patch('api.models.logger')
-    def test_admin_can_manage_other_apps(self, mock_logger):
+    def test_admin_can_manage_other_apps(self, mock_requests, mock_logger):
         """Administrators of Deis should be able to manage all applications.
         """
         # log in as non-admin user and create an app
@@ -213,7 +217,7 @@ def test_admin_can_manage_other_apps(self, mock_logger):
         response = self.client.delete(url)
         self.assertEqual(response.status_code, 204)
 
-    def test_admin_can_see_other_apps(self):
+    def test_admin_can_see_other_apps(self, mock_requests):
         """If a user creates an application, the administrator should be able
         to see it.
         """
@@ -231,7 +235,7 @@ def test_admin_can_see_other_apps(self):
         response = self.client.get(url)
         self.assertEqual(response.data['count'], 1)
 
-    def test_run_without_release_should_error(self):
+    def test_run_without_release_should_error(self, mock_requests):
         """
         A user should not be able to run a one-off command unless a release
         is present.
@@ -253,7 +257,7 @@ def _mock_run(*args, **kwargs):
     @mock.patch('api.models.App.run', _mock_run)
     @mock.patch('api.models.App.deploy', mock_none)
     @mock.patch('api.models.Release.publish', mock_none)
-    def test_run(self):
+    def test_run(self, mock_requests):
         """
         A user should be able to run a one off command
         """
@@ -276,7 +280,7 @@ def test_run(self):
         self.assertEqual(response.data['rc'], 0)
         self.assertEqual(response.data['output'], 'mock')
 
-    def test_unauthorized_user_cannot_see_app(self):
+    def test_unauthorized_user_cannot_see_app(self, mock_requests):
         """
         An unauthorized user should not be able to access an app's resources.
 
@@ -307,7 +311,7 @@ def test_unauthorized_user_cannot_see_app(self):
         response = self.client.delete(url)
         self.assertEqual(response.status_code, 403)
 
-    def test_app_info_not_showing_wrong_app(self):
+    def test_app_info_not_showing_wrong_app(self, mock_requests):
         app_id = 'autotest'
         base_url = '/v2/apps'
         body = {'id': app_id}
@@ -316,7 +320,7 @@ def test_app_info_not_showing_wrong_app(self):
         response = self.client.get(url)
         self.assertEqual(response.status_code, 404)
 
-    def test_app_transfer(self):
+    def test_app_transfer(self, mock_requests):
         owner = User.objects.get(username='autotest2')
         owner_token = Token.objects.get(user=owner).key
         self.client.credentials(HTTP_AUTHORIZATION='Token ' + owner_token)
@@ -364,7 +368,7 @@ def test_app_transfer(self):
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data['owner'], self.user.username)
 
-    def test_app_exists_in_kubernetes(self):
+    def test_app_exists_in_kubernetes(self, mock_requests):
         """
         Create an app that has the same namespace as an existing kubernetes namespace
         """