aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dmitriy Shirchenko" <cald...@gmail.com>
Subject Re: Review Request 41154: Add support for performing health checks with a shell command.
Date Tue, 15 Dec 2015 03:56:36 GMT


> On Dec. 12, 2015, 12:06 a.m., Maxim Khutornenko wrote:
> > src/main/python/apache/aurora/config/schema/base.py, line 51
> > <https://reviews.apache.org/r/41154/diff/7/?file=1159782#file1159782line51>
> >
> >     Please, don't use 'type' for a field name. It's too generic and clashes with
python defaults.

Will be part of https://issues.apache.org/jira/browse/AURORA-1562


> On Dec. 12, 2015, 12:06 a.m., Maxim Khutornenko wrote:
> > src/main/python/apache/aurora/config/schema/base.py, line 50
> > <https://reviews.apache.org/r/41154/diff/7/?file=1159782#file1159782line50>
> >
> >     I second Zameer's concerns about logical conflict between the new `shell_command`
and the current http-based config values. I strongly suggest considering an approach similar
to `LifecycleConfig` to avoid contaminating health check config with logically conflicting
knobs. Something on the lines of (not focusing on names):
> >     
> >     ```
> >     HttpEndpointConfig(Struct):
> >       endpoint = ...
> >       expected_response = ...
> >       expected_response_code = ...
> >       
> >     ShellEndpointConfig(Struct):
> >       shell_command = ...
> >       
> >     HealthEndpointConfig(struct):
> >       http = HttpEndpointConfig
> >       shell = ShellEndpointConfig
> >       
> >     DefaultEndpointConfig = HealthEndpointConfig(http = HttpEndpointConfig())
> >     
> >     HealthCheckConfig(Struct):
> >       ...
> >       endpoint_config = Default(HealthEndpointConfig, DefaultEndpointConfig)
> >     ```
> >     
> >     Now, the above would require deprecating the existing fields in the HealthCheckConfig
(those that will move into HttpEndpointConfig) but longer term (e.g. in 0.12.0) we will be
able to remove (or alias) those and have a much cleaner schema open for adding any kinds of
other health checkers.
> 
> Bill Farner wrote:
>     Maxim - are you willing to shepherd those changes in a follow-up review, or make
them yourself in a follow-up?  Dmitriy has already been through the ringer a bit here, it
would be good to let him get some incremental progress.  We don't have to worry about compatibility
with this schema until we release, so a follow-up to focus on the schema seems appropriate.
> 
> Maxim Khutornenko wrote:
>     I am ok with a follow-up patch as long as this does not escape the release gates.
Happy to shepherd the design. Dmitriy, if you accept this, please file a blocking ticket for
0.11.0 release (AURORA-1525).

Files as a blocker for 0.11.0 release here:
https://issues.apache.org/jira/browse/AURORA-1562


- Dmitriy


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41154/#review110040
-----------------------------------------------------------


On Dec. 11, 2015, 7:25 a.m., Dmitriy Shirchenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41154/
> -----------------------------------------------------------
> 
> (Updated Dec. 11, 2015, 7:25 a.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1551
>     https://issues.apache.org/jira/browse/AURORA-1551
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Adding support for non-HTTP health checks.
> 
> 
> Diffs
> -----
> 
>   3rdparty/python/requirements.txt cfef18ee66b0f92d83c53dacb6d9376fc2e50445 
>   docs/configuration-reference.md 364292998bebb233d300fe59c9ea42b216deee81 
>   src/main/python/apache/aurora/client/config.py 2fc12559016d406c347adb416a5166cca31c961e

>   src/main/python/apache/aurora/common/BUILD 5fce3d0d29d2a38c6563b4d9be963532e595ee19

>   src/main/python/apache/aurora/common/health_check/__init__.py PRE-CREATION 
>   src/main/python/apache/aurora/common/health_check/shell.py PRE-CREATION 
>   src/main/python/apache/aurora/common/http_signaler.py a3193f3259276ec23d37f45839afe3c387cff6b1

>   src/main/python/apache/aurora/config/schema/base.py 398f737bed9ef02ce4a5636896d6587bce26501e

>   src/main/python/apache/aurora/executor/common/health_checker.py 03fdf0afef120c365c6ffad09e152780eed7e351

>   src/main/python/apache/aurora/executor/http_lifecycle.py 6d578cceb56375425ccac1cbfbbcd0add60f20e9

>   src/test/python/apache/aurora/client/BUILD 84c5c845d9b1c8078ae8b47242d0f2ffc00ef6dc

>   src/test/python/apache/aurora/client/test_config.py b1a3c1865819899ef19173be0f861783a2631d0a

>   src/test/python/apache/aurora/common/BUILD 2556c32842b3cf7040cb3c41172a0d9c365cb649

>   src/test/python/apache/aurora/common/health_check/BUILD PRE-CREATION 
>   src/test/python/apache/aurora/common/health_check/__init__.py PRE-CREATION 
>   src/test/python/apache/aurora/common/health_check/test_shell.py PRE-CREATION 
>   src/test/python/apache/aurora/common/test_http_signaler.py f68c71a6765f7f0b93c8c50662515b5742344f35

>   src/test/python/apache/aurora/executor/common/test_health_checker.py 27c71711d52f757ed1552db4accda671a6bdafdd

> 
> Diff: https://reviews.apache.org/r/41154/diff/
> 
> 
> Testing
> -------
> 
> Added unit tests. 
> Ran e2e test.
> Tested expected behavior on virtual Mesos cluster.
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message