Skip to content

Commit

Permalink
feat(apps): validate routeability through deis-router upon applicatio…
Browse files Browse the repository at this point in the history
…n creation

Hits the router IP via python-requests with a Host header (uses xip.io) to emulate a request to the application. Sends a Deis Controller UA to make debugging easier
HTTP mocking is required for this to work in tests and all tests now get peppered with mock_requests object, which they can use for other purposes as well if required

Depends on a change to the requests_mock python package for testing only, pulling that from git currently

Tests take 2x the time because of added timing jitter to emulate router being slow to respond at times

Fixes deis#551
  • Loading branch information
helgi committed Mar 29, 2016
1 parent e9ee330 commit aa41d6c
Show file tree
Hide file tree
Showing 13 changed files with 263 additions and 102 deletions.
92 changes: 90 additions & 2 deletions rootfs/api/models/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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:
Expand Down
33 changes: 33 additions & 0 deletions rootfs/api/tests/__init__.py
Original file line number Diff line number Diff line change
@@ -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."""

Expand Down
36 changes: 20 additions & 16 deletions rootfs/api/tests/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"""

Expand All @@ -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
"""
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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'}
Expand All @@ -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'}
Expand All @@ -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']
Expand All @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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.
"""
Expand All @@ -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.
Expand All @@ -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
"""
Expand All @@ -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.
Expand Down Expand Up @@ -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}
Expand All @@ -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)
Expand Down Expand Up @@ -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
"""
Expand Down
Loading

0 comments on commit aa41d6c

Please sign in to comment.