Skip to content

Commit aa41d6c

Browse files
committed
feat(apps): validate routeability through deis-router upon application 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 #551
1 parent e9ee330 commit aa41d6c

13 files changed

Lines changed: 263 additions & 102 deletions

File tree

rootfs/api/models/app.py

Lines changed: 90 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,19 @@
33
import random
44
import re
55
import requests
6+
from requests_toolbelt import user_agent
67
import string
78
import time
9+
from urllib.parse import urljoin
810

911
from django.conf import settings
1012
from django.db import models
1113
from django.core.exceptions import ValidationError
1214
from jsonfield import JSONField
1315

16+
from deis import __version__ as deis_version
1417
from api.models import UuidAuditedModel, log_event, AlreadyExists
18+
1519
from api.utils import generate_app_name, app_build_type
1620
from api.models.release import Release
1721
from api.models.config import Config
@@ -304,7 +308,9 @@ def _scale_pods(self, scale_types):
304308
'replicas': scale_types[scale_type],
305309
'app_type': scale_type,
306310
'build_type': build_type,
307-
'healthcheck': release.config.healthcheck()
311+
'healthcheck': release.config.healthcheck(),
312+
# http://docs.deis.io/en/latest/using_deis/process-types/#web-vs-cmd-process-types
313+
'routable': True if scale_type in ['web', 'cmd'] else False
308314
}
309315

310316
command = self._get_command(scale_type)
@@ -348,7 +354,9 @@ def deploy(self, user, release):
348354
'version': version,
349355
'app_type': scale_type,
350356
'build_type': build_type,
351-
'healthcheck': release.config.healthcheck()
357+
'healthcheck': release.config.healthcheck(),
358+
# http://docs.deis.io/en/latest/using_deis/process-types/#web-vs-cmd-process-types
359+
'routable': True if scale_type in ['web', 'cmd'] else False
352360
}
353361

354362
command = self._get_command(scale_type)
@@ -360,6 +368,13 @@ def deploy(self, user, release):
360368
command=command,
361369
**kwargs
362370
)
371+
372+
# Wait until application is available in the router
373+
# Only run when there is no previous build / release
374+
old = release.previous()
375+
if old is None or old.build is None:
376+
self.verify_application_health(**kwargs)
377+
363378
except Exception as e:
364379
err = '{} (app::deploy): {}'.format(self._get_job_id(scale_type), e)
365380
log_event(self, err, logging.ERROR)
@@ -385,6 +400,79 @@ def _default_structure(self, release):
385400

386401
return structure
387402

403+
def verify_application_health(self, **kwargs):
404+
"""
405+
Verify an application is healthy via the router.
406+
This is only used in conjunction with the kubernetes health check system and should
407+
only run after kubernetes has reported all pods as healthy
408+
"""
409+
# Bail out early if the application is not routable
410+
if not kwargs.get('routable', False):
411+
return
412+
413+
app_type = kwargs.get('app_type')
414+
self.log(
415+
'Waiting for router to be ready to serve traffic to process type {}'.format(app_type),
416+
level=logging.DEBUG
417+
)
418+
419+
# Get the router host and append healthcheck path
420+
url = 'http://{}:{}'.format(settings.ROUTER_HOST, settings.ROUTER_PORT)
421+
422+
# if a health check url is available then 200 is the only acceptable status code
423+
if len(kwargs['healthcheck']):
424+
allowed = [200]
425+
url = urljoin(url, kwargs['healthcheck'].get('path'))
426+
req_timeout = kwargs['healthcheck'].get('timeout')
427+
else:
428+
allowed = set(range(200, 599))
429+
allowed.remove(404)
430+
req_timeout = 3
431+
432+
session = requests.Session()
433+
session.headers = {
434+
# https://toolbelt.readthedocs.org/en/latest/user-agent.html#user-agent-constructor
435+
'User-Agent': user_agent('Deis Controller', deis_version),
436+
# set the Host header for the application being checked - not used for actual routing
437+
'Host': '{}.{}.nip.io'.format(self.id, settings.ROUTER_HOST)
438+
}
439+
440+
# `mount` a custom adapter that retries failed connections for HTTP and HTTPS requests.
441+
# http://docs.python-requests.org/en/latest/api/#requests.adapters.HTTPAdapter
442+
session.mount('http://', requests.adapters.HTTPAdapter(max_retries=10))
443+
session.mount('https://', requests.adapters.HTTPAdapter(max_retries=10))
444+
445+
# Give the router max of 10 tries or max 30 seconds to become healthy
446+
# Uses time module to account for the timout value of 3 seconds
447+
start = time.time()
448+
for _ in range(10):
449+
# http://docs.python-requests.org/en/master/user/advanced/#timeouts
450+
response = session.get(url, timeout=req_timeout)
451+
452+
# 1 minute timeout
453+
if (time.time() - start) > (req_timeout * 10):
454+
break
455+
456+
# check response against the allowed pool
457+
if response.status_code in allowed:
458+
break
459+
460+
# a small sleep since router usually resolve within 10 seconds
461+
time.sleep(1)
462+
463+
# Endpoint did not report healthy in time
464+
if response.status_code == 404:
465+
self.log(
466+
'Router was not ready to serve traffic to process type {} in time'.format(app_type), # noqa
467+
level=logging.WARNING
468+
)
469+
return
470+
471+
self.log(
472+
'Router is ready to serve traffic to process type {}'.format(app_type),
473+
level=logging.DEBUG
474+
)
475+
388476
def logs(self, log_lines=str(settings.LOG_LINES)):
389477
"""Return aggregated log data for this application."""
390478
try:

rootfs/api/tests/__init__.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,41 @@
11
import logging
2+
import random
3+
import requests_mock
4+
import time
25

6+
from django.conf import settings
37
from django.test.runner import DiscoverRunner
48

59

10+
# Mock out router requests and add in some jitter
11+
# Used for application is available in router checks
12+
def fake_responses(request, context):
13+
responses = [
14+
# increasing the chance of 404
15+
{'text': 'Not Found', 'status_code': 404},
16+
{'text': 'Not Found', 'status_code': 404},
17+
{'text': 'Not Found', 'status_code': 404},
18+
{'text': 'Not Found', 'status_code': 404},
19+
{'text': 'OK', 'status_code': 200},
20+
{'text': 'Gateway timeout', 'status_code': 504},
21+
{'text': 'Bad gateway', 'status_code': 502},
22+
]
23+
random.shuffle(responses)
24+
response = responses.pop()
25+
26+
context.status_code = response['status_code']
27+
context.reason = response['text']
28+
# Random float x, 1.0 <= x < 4.0 for some sleep jitter
29+
time.sleep(random.uniform(1, 4))
30+
return response['text']
31+
32+
url = 'http://{}:{}'.format(settings.ROUTER_HOST, settings.ROUTER_PORT)
33+
adapter = requests_mock.Adapter()
34+
adapter.register_uri('GET', url + '/', text=fake_responses)
35+
adapter.register_uri('GET', url + '/health', text=fake_responses)
36+
adapter.register_uri('GET', url + '/healthz', text=fake_responses)
37+
38+
639
class SilentDjangoTestSuiteRunner(DiscoverRunner):
740
"""Prevents api log messages from cluttering the console during tests."""
841

rootfs/api/tests/test_app.py

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,15 @@
1414

1515
from api.models import App
1616

17+
from . import adapter
18+
import requests_mock
19+
1720

1821
def mock_none(*args, **kwargs):
1922
return None
2023

2124

25+
@requests_mock.Mocker(real_http=True, adapter=adapter)
2226
class AppTest(APITestCase):
2327
"""Tests creation of applications"""
2428

@@ -33,7 +37,7 @@ def tearDown(self):
3337
# make sure every test has a clean slate for k8s mocking
3438
cache.clear()
3539

36-
def test_app(self):
40+
def test_app(self, mock_requests):
3741
"""
3842
Test that a user can create, read, update and delete an application
3943
"""
@@ -54,7 +58,7 @@ def test_app(self):
5458
response = self.client.delete(url)
5559
self.assertEqual(response.status_code, 204)
5660

57-
def test_response_data(self):
61+
def test_response_data(self, mock_requests):
5862
"""Test that the serialized response contains only relevant data."""
5963
body = {'id': 'test'}
6064
response = self.client.post('/v2/apps', body)
@@ -67,7 +71,7 @@ def test_response_data(self):
6771
}
6872
self.assertDictContainsSubset(expected, response.data)
6973

70-
def test_app_override_id(self):
74+
def test_app_override_id(self, mock_requests):
7175
body = {'id': 'myid'}
7276
response = self.client.post('/v2/apps', body)
7377
self.assertEqual(response.status_code, 201)
@@ -77,7 +81,7 @@ def test_app_override_id(self):
7781
return response
7882

7983
@mock.patch('requests.get')
80-
def test_app_actions(self, mock_get):
84+
def test_app_actions(self, mock_requests, mock_get):
8185
url = '/v2/apps'
8286
body = {'id': 'autotest'}
8387
response = self.client.post(url, body)
@@ -121,7 +125,7 @@ def test_app_actions(self, mock_get):
121125
# TODO: test run needs an initial build
122126

123127
@mock.patch('api.models.logger')
124-
def test_app_release_notes_in_logs(self, mock_logger):
128+
def test_app_release_notes_in_logs(self, mock_requests, mock_logger):
125129
"""Verifies that an app's release summary is dumped into the logs."""
126130
url = '/v2/apps'
127131
body = {'id': 'autotest'}
@@ -132,7 +136,7 @@ def test_app_release_notes_in_logs(self, mock_logger):
132136
exp_log_call = mock.call(logging.INFO, exp_msg)
133137
mock_logger.log.has_calls(exp_log_call)
134138

135-
def test_app_errors(self):
139+
def test_app_errors(self, mock_requests):
136140
app_id = 'autotest-errors'
137141
url = '/v2/apps'
138142
body = {'id': 'camelCase'}
@@ -155,7 +159,7 @@ def test_app_errors(self):
155159
response = self.client.get(url)
156160
self.assertEqual(response.status_code, 404)
157161

158-
def test_app_reserved_names(self):
162+
def test_app_reserved_names(self, mock_requests):
159163
"""Nobody should be able to create applications with names which are reserved."""
160164
url = '/v2/apps'
161165
reserved_names = ['foo', 'bar']
@@ -168,7 +172,7 @@ def test_app_reserved_names(self):
168172
'{} is a reserved name.'.format(name),
169173
status_code=400)
170174

171-
def test_app_structure_is_valid_json(self):
175+
def test_app_structure_is_valid_json(self, mock_requests):
172176
"""Application structures should be valid JSON objects."""
173177
url = '/v2/apps'
174178
response = self.client.post(url)
@@ -185,7 +189,7 @@ def test_app_structure_is_valid_json(self):
185189
self.assertEqual(response.data['structure'], {"web": 1})
186190

187191
@mock.patch('api.models.logger')
188-
def test_admin_can_manage_other_apps(self, mock_logger):
192+
def test_admin_can_manage_other_apps(self, mock_requests, mock_logger):
189193
"""Administrators of Deis should be able to manage all applications.
190194
"""
191195
# log in as non-admin user and create an app
@@ -213,7 +217,7 @@ def test_admin_can_manage_other_apps(self, mock_logger):
213217
response = self.client.delete(url)
214218
self.assertEqual(response.status_code, 204)
215219

216-
def test_admin_can_see_other_apps(self):
220+
def test_admin_can_see_other_apps(self, mock_requests):
217221
"""If a user creates an application, the administrator should be able
218222
to see it.
219223
"""
@@ -231,7 +235,7 @@ def test_admin_can_see_other_apps(self):
231235
response = self.client.get(url)
232236
self.assertEqual(response.data['count'], 1)
233237

234-
def test_run_without_release_should_error(self):
238+
def test_run_without_release_should_error(self, mock_requests):
235239
"""
236240
A user should not be able to run a one-off command unless a release
237241
is present.
@@ -253,7 +257,7 @@ def _mock_run(*args, **kwargs):
253257
@mock.patch('api.models.App.run', _mock_run)
254258
@mock.patch('api.models.App.deploy', mock_none)
255259
@mock.patch('api.models.Release.publish', mock_none)
256-
def test_run(self):
260+
def test_run(self, mock_requests):
257261
"""
258262
A user should be able to run a one off command
259263
"""
@@ -276,7 +280,7 @@ def test_run(self):
276280
self.assertEqual(response.data['rc'], 0)
277281
self.assertEqual(response.data['output'], 'mock')
278282

279-
def test_unauthorized_user_cannot_see_app(self):
283+
def test_unauthorized_user_cannot_see_app(self, mock_requests):
280284
"""
281285
An unauthorized user should not be able to access an app's resources.
282286
@@ -307,7 +311,7 @@ def test_unauthorized_user_cannot_see_app(self):
307311
response = self.client.delete(url)
308312
self.assertEqual(response.status_code, 403)
309313

310-
def test_app_info_not_showing_wrong_app(self):
314+
def test_app_info_not_showing_wrong_app(self, mock_requests):
311315
app_id = 'autotest'
312316
base_url = '/v2/apps'
313317
body = {'id': app_id}
@@ -316,7 +320,7 @@ def test_app_info_not_showing_wrong_app(self):
316320
response = self.client.get(url)
317321
self.assertEqual(response.status_code, 404)
318322

319-
def test_app_transfer(self):
323+
def test_app_transfer(self, mock_requests):
320324
owner = User.objects.get(username='autotest2')
321325
owner_token = Token.objects.get(user=owner).key
322326
self.client.credentials(HTTP_AUTHORIZATION='Token ' + owner_token)
@@ -364,7 +368,7 @@ def test_app_transfer(self):
364368
self.assertEqual(response.status_code, 200)
365369
self.assertEqual(response.data['owner'], self.user.username)
366370

367-
def test_app_exists_in_kubernetes(self):
371+
def test_app_exists_in_kubernetes(self, mock_requests):
368372
"""
369373
Create an app that has the same namespace as an existing kubernetes namespace
370374
"""

0 commit comments

Comments
 (0)