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



src/docker/docker.hpp (line 152)
<https://reviews.apache.org/r/37505/#comment152735>

    The interface is confusing to me as I would assume it's checkin that the 
version of docker matches exactly the expectedVersion, but actually is making 
sure it's above the minimumVersion.
    You should rename expectVersion to minVersion.
    Also please add comments above.



src/tests/health_check_tests.cpp (line 573)
<https://reviews.apache.org/r/37505/#comment152736>

    Docker tests should inherit from MesosTest which inherits from 
TemporrayDirectoryTest already, which means the current work directory is a 
temporary directory that you can use.
    
    Just create the file in your current working directory and pass that path 
in.



src/tests/health_check_tests.cpp (line 626)
<https://reviews.apache.org/r/37505/#comment152737>

    You don't need to clean up temp file if you just stick in the os::getcwd() 
child path.


- Timothy Chen


On Aug. 30, 2015, 7:32 a.m., haosdent huang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37505/
> -----------------------------------------------------------
> 
> (Updated Aug. 30, 2015, 7:32 a.m.)
> 
> 
> Review request for mesos, Adam B and Timothy Chen.
> 
> 
> Bugs: MESOS-3136
>     https://issues.apache.org/jira/browse/MESOS-3136
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Fix broken health check in docker executor.
> 
> 
> Diffs
> -----
> 
>   src/docker/docker.hpp 6086710fff32a25e46197a69ae1063074317221b 
>   src/docker/docker.cpp 12dc0505c9ec4bd380e817d44da2c4e8d1b0d5f5 
>   src/docker/executor.cpp 256d53d59d5cda63bbeb8c987ce0019e24b9fb77 
>   src/slave/containerizer/docker.cpp a17e4f21e7f5a1dfd47699ec84c7a48fd82294ad 
>   src/tests/health_check_tests.cpp 157a56aa06677d8b7a2cef53b29ed05cb4b5d8ea 
> 
> Diff: https://reviews.apache.org/r/37505/diff/
> 
> 
> Testing
> -------
> 
> # Add two new test cases, HealthCheckTest.ROOT_DOCKER_DockerHealthyTask and 
> HealthCheckTest.ROOT_DOCKER_DockerHealthStatusChange
> # Docker health check command is run through "docker exec"
> sudo ./bin/mesos-tests.sh 
> --gtest_filter="HealthCheckTest.ROOT_DOCKER_DockerHealthyTask" --verbose
> sudo ./bin/mesos-tests.sh 
> --gtest_filter="HealthCheckTest.ROOT_DOCKER_DockerHealthStatusChange" 
> --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Reply via email to