Repository: aurora Updated Branches: refs/heads/master 7e3e7c9ad -> d92de5d18
Revert "Make health check configurable" This reverts commit 27a602d2c9efdd1cd2591c9c754f086c04ad0eb9. Bugs closed: AURORA-1266 Reviewed at https://reviews.apache.org/r/33026/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/d92de5d1 Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/d92de5d1 Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/d92de5d1 Branch: refs/heads/master Commit: d92de5d1862ca694fec3df4f85b8e33c4284dc71 Parents: 7e3e7c9 Author: Bill Farner <[email protected]> Authored: Thu Apr 9 12:22:09 2015 -0700 Committer: Bill Farner <[email protected]> Committed: Thu Apr 9 12:22:36 2015 -0700 ---------------------------------------------------------------------- docs/configuration-reference.md | 5 +- .../apache/aurora/common/http_signaler.py | 29 ++++------ .../python/apache/aurora/config/schema/base.py | 3 - .../aurora/executor/common/health_checker.py | 5 +- .../apache/aurora/common/test_http_signaler.py | 59 ++++---------------- .../executor/common/test_health_checker.py | 22 ++++---- 6 files changed, 37 insertions(+), 86 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/d92de5d1/docs/configuration-reference.md ---------------------------------------------------------------------- diff --git a/docs/configuration-reference.md b/docs/configuration-reference.md index fb753ea..af332f2 100644 --- a/docs/configuration-reference.md +++ b/docs/configuration-reference.md @@ -359,11 +359,8 @@ Parameters for controlling a task's health checks via HTTP. | ------- | :-------: | -------- | ```initial_interval_secs``` | Integer | Initial delay for performing an HTTP health check. (Default: 15) | ```interval_secs``` | Integer | Interval on which to check the task's health via HTTP. (Default: 10) -| ```max_consecutive_failures``` | Integer | Maximum number of consecutive failures that tolerated before considering a task unhealthy (Default: 0) | ```timeout_secs``` | Integer | HTTP request timeout. (Default: 1) -| ```endpoint``` | String | HTTP endpoint to check (Default: /health) -| ```expected_response``` | String | If not empty, fail the health check if the response differs. Case insensitive. (Default: ok) -| ```expected_response_code``` | Integer | If not zero, fail the health check if the response code differs. (Default: 0) +| ```max_consecutive_failures``` | Integer | Maximum number of consecutive failures that tolerated before considering a task unhealthy (Default: 0) ### Announcer Objects http://git-wip-us.apache.org/repos/asf/aurora/blob/d92de5d1/src/main/python/apache/aurora/common/http_signaler.py ---------------------------------------------------------------------- diff --git a/src/main/python/apache/aurora/common/http_signaler.py b/src/main/python/apache/aurora/common/http_signaler.py index 531f1fe..e3e819d 100644 --- a/src/main/python/apache/aurora/common/http_signaler.py +++ b/src/main/python/apache/aurora/common/http_signaler.py @@ -39,7 +39,7 @@ class HttpSignaler(object): def __init__(self, port, host='localhost', timeout_secs=None): self._host = host - self._url_base = 'http://%s:%d' % (host, port) + self._url_base = 'http://%s:%d/' % (host, port) if timeout_secs is None: env_timeout = os.getenv('AURORA_HTTP_SIGNALER_TIMEOUT_SECS') if env_timeout is not None: @@ -70,42 +70,37 @@ class HttpSignaler(object): try: with contextlib.closing( self.opener(url, data, timeout=self._timeout_secs)) as fp: - return (fp.read(), fp.getcode()) + return fp.read() except (HTTPException, SocketTimeout) as e: # the type of an HTTPException is typically more useful than its contents (since for example # BadStatusLines are often empty). likewise with socket.timeout. raise_error('Error within %s' % e.__class__.__name__) - except HTTPError as e: - return ('', e.code) - except URLError as e: + except (URLError, HTTPError) as e: raise_error(e) except Exception as e: raise_error('Unexpected error: %s' % e) - def __call__(self, endpoint, use_post_method=False, expected_response=None, - expected_response_code=None): + def __call__(self, endpoint, use_post_method=False, expected_response=None): """Returns a (boolean, string|None) tuple of (call success, failure reason)""" try: - response, response_code = self.query(endpoint, '' if use_post_method else None) - response = response.strip().lower() - if expected_response and response != expected_response.lower(): - reason = 'Response differs from expected response (expected "%s", got "%s")' + response = self.query(endpoint, '' if use_post_method else None).strip().lower() + if expected_response is not None and response != expected_response: def shorten(string): return (string if len(string) < self.FAILURE_REASON_LENGTH else "%s..." % string[:self.FAILURE_REASON_LENGTH - 3]) + reason = 'Response differs from expected response (expected "%s", got "%s")' log.warning(reason % (expected_response, response)) return (False, reason % (shorten(str(expected_response)), shorten(str(response)))) - elif expected_response_code and response_code != expected_response_code: - reason = 'Response code differs from expected response (expected %i, got %i)' - log.warning(reason % (expected_response_code, response_code)) - return (False, reason % (expected_response_code, response_code)) else: return (True, None) except self.QueryError as e: return (False, str(e)) + def health(self): + return self('health', use_post_method=False, expected_response='ok') + def quitquitquit(self): - return self('/quitquitquit', use_post_method=True) + return self('quitquitquit', use_post_method=True) def abortabortabort(self): - return self('/abortabortabort', use_post_method=True) + return self('abortabortabort', use_post_method=True) http://git-wip-us.apache.org/repos/asf/aurora/blob/d92de5d1/src/main/python/apache/aurora/config/schema/base.py ---------------------------------------------------------------------- diff --git a/src/main/python/apache/aurora/config/schema/base.py b/src/main/python/apache/aurora/config/schema/base.py index ec9f983..a87524a 100644 --- a/src/main/python/apache/aurora/config/schema/base.py +++ b/src/main/python/apache/aurora/config/schema/base.py @@ -40,9 +40,6 @@ class HealthCheckConfig(Struct): interval_secs = Default(Float, 10.0) timeout_secs = Default(Float, 1.0) max_consecutive_failures = Default(Integer, 0) - endpoint = Default(String, '/health') - expected_response = Default(String, 'ok') - expected_response_code = Default(Integer, 0) class Announcer(Struct): http://git-wip-us.apache.org/repos/asf/aurora/blob/d92de5d1/src/main/python/apache/aurora/executor/common/health_checker.py ---------------------------------------------------------------------- diff --git a/src/main/python/apache/aurora/executor/common/health_checker.py b/src/main/python/apache/aurora/executor/common/health_checker.py index 03fdf0a..cfc29c3 100644 --- a/src/main/python/apache/aurora/executor/common/health_checker.py +++ b/src/main/python/apache/aurora/executor/common/health_checker.py @@ -211,10 +211,7 @@ class HealthCheckerProvider(StatusCheckerProvider): portmap['health'], timeout_secs=health_check_config.get('timeout_secs')) health_checker = HealthChecker( - lambda: http_signaler( - endpoint=health_check_config.get('endpoint'), - expected_response=health_check_config.get('expected_response'), - expected_response_code=health_check_config.get('expected_response_code')), + http_signaler.health, sandbox, interval_secs=health_check_config.get('interval_secs'), initial_interval_secs=health_check_config.get('initial_interval_secs'), http://git-wip-us.apache.org/repos/asf/aurora/blob/d92de5d1/src/test/python/apache/aurora/common/test_http_signaler.py ---------------------------------------------------------------------- diff --git a/src/test/python/apache/aurora/common/test_http_signaler.py b/src/test/python/apache/aurora/common/test_http_signaler.py index c6a2170..f5f8419 100644 --- a/src/test/python/apache/aurora/common/test_http_signaler.py +++ b/src/test/python/apache/aurora/common/test_http_signaler.py @@ -25,20 +25,7 @@ if Compatibility.PY3: else: import urllib2 as urllib_request - -class OpenedURL(object): - def __init__(self, content, code=200): - self.content = content - self.code = code - - def read(self): - return self.content - - def close(self): - pass - - def getcode(self): - return self.code +StringIO = Compatibility.StringIO class TestHttpSignaler(unittest.TestCase): @@ -54,51 +41,29 @@ class TestHttpSignaler(unittest.TestCase): def test_all_calls_ok(self): self._mox.StubOutWithMock(urllib_request, 'urlopen') urllib_request.urlopen( - 'http://localhost:%s/quitquitquit' % self.PORT, '', timeout=1.0).AndReturn(OpenedURL('')) + 'http://localhost:%s/health' % self.PORT, None, timeout=1.0).AndReturn(StringIO('ok')) + urllib_request.urlopen( + 'http://localhost:%s/quitquitquit' % self.PORT, '', timeout=1.0).AndReturn(StringIO('')) urllib_request.urlopen( - 'http://localhost:%s/abortabortabort' % self.PORT, '', timeout=1.0).AndReturn(OpenedURL('')) + 'http://localhost:%s/abortabortabort' % self.PORT, '', timeout=1.0).AndReturn(StringIO('')) self._mox.ReplayAll() signaler = HttpSignaler(self.PORT) + assert signaler.health() == (True, None) assert signaler.quitquitquit() == (True, None) assert signaler.abortabortabort() == (True, None) - def test_health_checks(self): + def test_health_not_ok(self): self._mox.StubOutWithMock(urllib_request, 'urlopen') urllib_request.urlopen( - 'http://localhost:%s/health' % self.PORT, None, timeout=1.0).AndReturn(OpenedURL('ok')) - urllib_request.urlopen( - 'http://localhost:%s/health' % self.PORT, None, timeout=1.0).AndReturn(OpenedURL('not ok')) - urllib_request.urlopen( - 'http://localhost:%s/health' % self.PORT, None, timeout=1.0).AndReturn( - OpenedURL('not ok', code=200)) - urllib_request.urlopen( - 'http://localhost:%s/health' % self.PORT, None, timeout=1.0).AndReturn( - OpenedURL('ok', code=400)) - urllib_request.urlopen( - 'http://localhost:%s/health' % self.PORT, None, timeout=1.0).AndRaise( - urllib_request.HTTPError('', 501, '', None, None)) - urllib_request.urlopen( - 'http://localhost:%s/health' % self.PORT, None, timeout=1.0).AndReturn( - OpenedURL('ok', code=200)) - urllib_request.urlopen( - 'http://localhost:%s/random/endpoint' % self.PORT, None, timeout=1.0).AndReturn( - OpenedURL('ok')) + 'http://localhost:%s/health' % self.PORT, None, timeout=1.0).AndReturn(StringIO('not ok')) self._mox.ReplayAll() - signaler = HttpSignaler(self.PORT) - assert signaler('/health', expected_response='ok') == (True, None) - assert signaler('/health', expected_response='ok') == ( - False, 'Response differs from expected response (expected "ok", got "not ok")') - assert signaler('/health', expected_response_code=200) == (True, None) - assert signaler('/health', expected_response_code=200) == ( - False, 'Response code differs from expected response (expected 200, got 400)') - assert signaler('/health', expected_response_code=200) == ( - False, 'Response code differs from expected response (expected 200, got 501)') - assert signaler('/health', expected_response='ok', expected_response_code=200) == (True, None) - assert signaler('/random/endpoint', expected_response='ok') == (True, None) + health, reason = HttpSignaler(self.PORT).health() + assert not health + assert reason.startswith('Response differs from expected response') def test_exception(self): self._mox.StubOutWithMock(urllib_request, 'urlopen') @@ -108,4 +73,4 @@ class TestHttpSignaler(unittest.TestCase): self._mox.ReplayAll() - assert not HttpSignaler(self.PORT)('/health', expected_response='ok')[0] + assert not HttpSignaler(self.PORT).health()[0] http://git-wip-us.apache.org/repos/asf/aurora/blob/d92de5d1/src/test/python/apache/aurora/executor/common/test_health_checker.py ---------------------------------------------------------------------- diff --git a/src/test/python/apache/aurora/executor/common/test_health_checker.py b/src/test/python/apache/aurora/executor/common/test_health_checker.py index 27c7171..1b4423a 100644 --- a/src/test/python/apache/aurora/executor/common/test_health_checker.py +++ b/src/test/python/apache/aurora/executor/common/test_health_checker.py @@ -44,7 +44,7 @@ class TestHealthChecker(unittest.TestCase): self.fake_health_checks = [] def mock_health_check(): return self.fake_health_checks.pop(0) - self._checker.health = mock.Mock(spec=self._checker.__call__) + self._checker.health = mock.Mock(spec=self._checker.health) self._checker.health.side_effect = mock_health_check def append_health_checks(self, status, num_calls=1): @@ -209,8 +209,8 @@ class TestHealthCheckerProvider(unittest.TestCase): class TestThreadedHealthChecker(unittest.TestCase): def setUp(self): - self.health = mock.Mock() - self.health.return_value = (True, 'Fake') + self.signaler = mock.Mock(spec=HttpSignaler) + self.signaler.health.return_value = (True, 'Fake') self.sandbox = mock.Mock(spec_set=SandboxInterface) self.sandbox.exists.return_value = True @@ -222,14 +222,14 @@ class TestThreadedHealthChecker(unittest.TestCase): self.clock = mock.Mock(spec=time) self.clock.time.return_value = 1.0 self.health_checker = HealthChecker( - self.health, + self.signaler.health, None, self.interval_secs, self.initial_interval_secs, self.max_consecutive_failures, self.clock) self.health_checker_sandbox_exists = HealthChecker( - self.health, + self.signaler.health, self.sandbox, self.interval_secs, self.initial_interval_secs, @@ -238,29 +238,29 @@ class TestThreadedHealthChecker(unittest.TestCase): def test_perform_check_if_not_disabled_snooze_file_is_none(self): self.health_checker.threaded_health_checker.snooze_file = None - assert self.health.call_count == 0 + assert self.signaler.health.call_count == 0 assert self.health_checker_sandbox_exists.metrics.sample()['snoozed'] == 0 self.health_checker.threaded_health_checker._perform_check_if_not_disabled() - assert self.health.call_count == 1 + assert self.signaler.health.call_count == 1 assert self.health_checker_sandbox_exists.metrics.sample()['snoozed'] == 0 @mock.patch('os.path', spec_set=os.path) def test_perform_check_if_not_disabled_no_snooze_file(self, mock_os_path): mock_os_path.isfile.return_value = False - assert self.health.call_count == 0 + assert self.signaler.health.call_count == 0 assert self.health_checker_sandbox_exists.metrics.sample()['snoozed'] == 0 self.health_checker_sandbox_exists.threaded_health_checker._perform_check_if_not_disabled() - assert self.health.call_count == 1 + assert self.signaler.health.call_count == 1 assert self.health_checker_sandbox_exists.metrics.sample()['snoozed'] == 0 @mock.patch('os.path', spec_set=os.path) def test_perform_check_if_not_disabled_snooze_file_exists(self, mock_os_path): mock_os_path.isfile.return_value = True - assert self.health.call_count == 0 + assert self.signaler.health.call_count == 0 assert self.health_checker_sandbox_exists.metrics.sample()['snoozed'] == 0 result = ( self.health_checker_sandbox_exists.threaded_health_checker._perform_check_if_not_disabled()) - assert self.health.call_count == 0 + assert self.signaler.health.call_count == 0 assert self.health_checker_sandbox_exists.metrics.sample()['snoozed'] == 1 assert result == (True, None)
