Repository: aurora Updated Branches: refs/heads/master 3cdcd17a5 -> a8afa59fb
Capture health check output. Users really could really benefit from seeing the output of the shell health check failure, so plumbing through the output. Testing Done: added unit tests e2e tests screenshot attached. Bugs closed: AURORA-1881 Reviewed at https://reviews.apache.org/r/55902/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/a8afa59f Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/a8afa59f Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/a8afa59f Branch: refs/heads/master Commit: a8afa59fb6c96de81921bedffef04446ca022bfc Parents: 3cdcd17 Author: Dmitriy Shirchenko <cald...@gmail.com> Authored: Wed Jan 25 13:21:37 2017 -0800 Committer: Zameer Manji <zma...@apache.org> Committed: Wed Jan 25 13:21:46 2017 -0800 ---------------------------------------------------------------------- .../apache/aurora/common/health_check/shell.py | 11 ++++++-- .../aurora/common/health_check/test_shell.py | 29 ++++++++++++-------- 2 files changed, 26 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/a8afa59f/src/main/python/apache/aurora/common/health_check/shell.py ---------------------------------------------------------------------- diff --git a/src/main/python/apache/aurora/common/health_check/shell.py b/src/main/python/apache/aurora/common/health_check/shell.py index 58470a4..cbdb7f7 100644 --- a/src/main/python/apache/aurora/common/health_check/shell.py +++ b/src/main/python/apache/aurora/common/health_check/shell.py @@ -15,6 +15,8 @@ import os import sys +from subprocess32 import STDOUT + # Recommended pattern for Python 2 and 3 support from https://github.com/google/python-subprocess32 # Backport which adds bug fixes and timeout support for Python 2.7 if os.name == 'posix' and sys.version_info[0] < 3: @@ -35,6 +37,10 @@ class WrappedCalledProcessError(subprocess.CalledProcessError): self.returncode = error.returncode self.output = error.output + def __str__(self): + return "Command '%s' returned non-zero exit status %d with output '%s'" % ( + self.cmd, self.returncode, self.output) + class ShellHealthCheck(object): @@ -69,10 +75,11 @@ class ShellHealthCheck(object): :rtype tuple: """ try: - subprocess.check_call( + subprocess.check_output( self._wrapped_cmd, timeout=self._timeout_secs, - preexec_fn=self._preexec_fn) + preexec_fn=self._preexec_fn, + stderr=STDOUT) return True, None except subprocess.CalledProcessError as reason: # The command didn't return a 0 so provide reason for failure. http://git-wip-us.apache.org/repos/asf/aurora/blob/a8afa59f/src/test/python/apache/aurora/common/health_check/test_shell.py ---------------------------------------------------------------------- diff --git a/src/test/python/apache/aurora/common/health_check/test_shell.py b/src/test/python/apache/aurora/common/health_check/test_shell.py index 792ef40..6a090d8 100644 --- a/src/test/python/apache/aurora/common/health_check/test_shell.py +++ b/src/test/python/apache/aurora/common/health_check/test_shell.py @@ -17,6 +17,7 @@ import sys import unittest import mock +from subprocess32 import STDOUT from apache.aurora.common.health_check.shell import ShellHealthCheck @@ -31,58 +32,62 @@ else: class TestHealthChecker(unittest.TestCase): - @mock.patch('subprocess32.check_call', autospec=True) + @mock.patch('subprocess32.check_output', autospec=True) def test_health_check_ok(self, mock_popen): shell = ShellHealthCheck(raw_cmd='cmd', wrapped_cmd='wrapped-cmd', timeout_secs=30) success, msg = shell() self.assertTrue(success) self.assertIsNone(msg) - mock_popen.assert_called_once_with('wrapped-cmd', timeout=30, preexec_fn=mock.ANY) + mock_popen.assert_called_once_with( + 'wrapped-cmd', timeout=30, preexec_fn=mock.ANY, stderr=STDOUT) - @mock.patch('subprocess32.check_call', autospec=True) + @mock.patch('subprocess32.check_output', autospec=True) def test_health_check_failed(self, mock_popen): cmd = 'failed' wrapped_cmd = 'wrapped-failed' # Fail due to command returning a non-0 exit status. - mock_popen.side_effect = subprocess.CalledProcessError(1, wrapped_cmd) + mock_popen.side_effect = subprocess.CalledProcessError(1, wrapped_cmd, output='No file.') shell = ShellHealthCheck(raw_cmd=cmd, wrapped_cmd=wrapped_cmd, timeout_secs=30) success, msg = shell() - mock_popen.assert_called_once_with(wrapped_cmd, timeout=30, preexec_fn=mock.ANY) + mock_popen.assert_called_once_with(wrapped_cmd, timeout=30, preexec_fn=mock.ANY, stderr=STDOUT) self.assertFalse(success) - self.assertEqual(msg, "Command 'failed' returned non-zero exit status 1") + self.assertEqual(msg, "Command 'failed' returned non-zero exit status 1 with output 'No file.'") - @mock.patch('subprocess32.check_call', autospec=True) + @mock.patch('subprocess32.check_output', autospec=True) def test_health_check_timeout(self, mock_popen): # Fail due to command returning a non-0 exit status. mock_popen.side_effect = subprocess.TimeoutExpired('failed', timeout=30) shell = ShellHealthCheck(raw_cmd='cmd', wrapped_cmd='wrapped-cmd', timeout_secs=30) success, msg = shell() - mock_popen.assert_called_once_with('wrapped-cmd', timeout=30, preexec_fn=mock.ANY) + mock_popen.assert_called_once_with( + 'wrapped-cmd', timeout=30, preexec_fn=mock.ANY, stderr=STDOUT) self.assertFalse(success) self.assertEqual(msg, 'Health check timed out.') - @mock.patch('subprocess32.check_call', autospec=True) + @mock.patch('subprocess32.check_output', autospec=True) def test_health_check_os_error(self, mock_popen): # Fail due to command not existing. mock_popen.side_effect = OSError(1, 'failed') shell = ShellHealthCheck(raw_cmd='cmd', wrapped_cmd='wrapped-cmd', timeout_secs=30) success, msg = shell() - mock_popen.assert_called_once_with('wrapped-cmd', timeout=30, preexec_fn=mock.ANY) + mock_popen.assert_called_once_with( + 'wrapped-cmd', timeout=30, preexec_fn=mock.ANY, stderr=STDOUT) self.assertFalse(success) self.assertEqual(msg, 'OSError: failed') - @mock.patch('subprocess32.check_call', autospec=True) + @mock.patch('subprocess32.check_output', autospec=True) def test_health_check_value_error(self, mock_popen): # Invalid commmand passed in raises ValueError. mock_popen.side_effect = ValueError('Could not read command.') timeout = 10 shell = ShellHealthCheck(raw_cmd='cmd', wrapped_cmd='wrapped-cmd', timeout_secs=timeout) success, msg = shell() - mock_popen.assert_called_once_with('wrapped-cmd', timeout=timeout, preexec_fn=mock.ANY) + mock_popen.assert_called_once_with( + 'wrapped-cmd', timeout=timeout, preexec_fn=mock.ANY, stderr=STDOUT) self.assertFalse(success) self.assertEqual(msg, 'Invalid commmand.')
