Re: Review Request 51497: Avoided blocking calls in `Docker::validateVersion`.

[Joris Van Remoortere]

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



Closing this review due to inactivity. Please see our 
[guidelines](https://github.com/apache/mesos/blob/master/docs/reopening-reviews.md)
 for reopening reviews.

- Joris Van Remoortere


On Aug. 29, 2016, 5:05 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51497/
> ---
> 
> (Updated Aug. 29, 2016, 5:05 p.m.)
> 
> 
> Review request for mesos and Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added a `version` member variable in `Docker` class to avoid
> the blocking command line call in `Docker::validateVersion`.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
>   src/docker/docker.cpp 16abf64f98cc833ea4bee28bc08951eb2ff6d995 
>   src/tests/container_logger_tests.cpp 
> e8f934106510fe02b8b92be19c918a1e5c0b78fd 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 28cd3fa66886dbdbae3fdeca77707147faafcb7a 
>   src/tests/containerizer/docker_tests.cpp 
> e9a214a8973b3dfac69d59e90ce08473baa7eba4 
>   src/tests/health_check_tests.cpp 50a252b5267cbc3b21ddc75e1da86c4975f6faf1 
>   src/tests/hook_tests.cpp d864ef3d6153dedab8cc47403b1b9f76d75aeb5c 
>   src/tests/mesos.hpp 4cae54b4df906d4b7e8fe8d40d5b0ad59d260e6f 
>   src/tests/mesos.cpp f5034f9f8de7040182e10f51be125a87b29fdd24 
> 
> 
> Diff: https://reviews.apache.org/r/51497/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 51497: Avoided blocking calls in `Docker::validateVersion`.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [39939, 51497]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Aug. 29, 2016, 5:05 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51497/
> ---
> 
> (Updated Aug. 29, 2016, 5:05 p.m.)
> 
> 
> Review request for mesos and Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added a `version` member variable in `Docker` class to avoid
> the blocking command line call in `Docker::validateVersion`.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
>   src/docker/docker.cpp 16abf64f98cc833ea4bee28bc08951eb2ff6d995 
>   src/tests/container_logger_tests.cpp 
> e8f934106510fe02b8b92be19c918a1e5c0b78fd 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 28cd3fa66886dbdbae3fdeca77707147faafcb7a 
>   src/tests/containerizer/docker_tests.cpp 
> e9a214a8973b3dfac69d59e90ce08473baa7eba4 
>   src/tests/health_check_tests.cpp 50a252b5267cbc3b21ddc75e1da86c4975f6faf1 
>   src/tests/hook_tests.cpp d864ef3d6153dedab8cc47403b1b9f76d75aeb5c 
>   src/tests/mesos.hpp 4cae54b4df906d4b7e8fe8d40d5b0ad59d260e6f 
>   src/tests/mesos.cpp f5034f9f8de7040182e10f51be125a87b29fdd24 
> 
> Diff: https://reviews.apache.org/r/51497/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 51497: Avoided blocking calls in `Docker::validateVersion`.

[Zhitao Li]

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




src/docker/docker.hpp (line 223)


Can we make tihs a non-static virtual function so that we can simply 
override it in `MockDocker`?



src/docker/docker.cpp (lines 111 - 122)


Does this mean if docker daemon is not available, Mesos agent will not 
start either because DockerContainerizer will fail to initialize?

This seems okay behavior to me, but we might want to document this behavior 
in UPGRADE and/or containerizer.md



src/tests/mesos.cpp (lines 652 - 669)


Is there a way to avoid actually calling docker daemon to prepare the mock?

I think we can either override the `getVersion()` call, or override the 
`_version()` call by directly returning the `version` in this class.


- Zhitao Li


On Aug. 29, 2016, 5:05 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51497/
> ---
> 
> (Updated Aug. 29, 2016, 5:05 p.m.)
> 
> 
> Review request for mesos and Zhitao Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added a `version` member variable in `Docker` class to avoid
> the blocking command line call in `Docker::validateVersion`.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
>   src/docker/docker.cpp 16abf64f98cc833ea4bee28bc08951eb2ff6d995 
>   src/tests/container_logger_tests.cpp 
> e8f934106510fe02b8b92be19c918a1e5c0b78fd 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 28cd3fa66886dbdbae3fdeca77707147faafcb7a 
>   src/tests/containerizer/docker_tests.cpp 
> e9a214a8973b3dfac69d59e90ce08473baa7eba4 
>   src/tests/health_check_tests.cpp 50a252b5267cbc3b21ddc75e1da86c4975f6faf1 
>   src/tests/hook_tests.cpp d864ef3d6153dedab8cc47403b1b9f76d75aeb5c 
>   src/tests/mesos.hpp 4cae54b4df906d4b7e8fe8d40d5b0ad59d260e6f 
>   src/tests/mesos.cpp f5034f9f8de7040182e10f51be125a87b29fdd24 
> 
> Diff: https://reviews.apache.org/r/51497/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 51497: Avoided blocking calls in `Docker::validateVersion`.

[haosdent huang]

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

Review request for mesos and Zhitao Li.


Repository: mesos


Description
---

This patch added a `version` member variable in `Docker` class to avoid
the blocking command line call in `Docker::validateVersion`.


Diffs
-

  src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
  src/docker/docker.cpp 16abf64f98cc833ea4bee28bc08951eb2ff6d995 
  src/tests/container_logger_tests.cpp e8f934106510fe02b8b92be19c918a1e5c0b78fd 
  src/tests/containerizer/docker_containerizer_tests.cpp 
28cd3fa66886dbdbae3fdeca77707147faafcb7a 
  src/tests/containerizer/docker_tests.cpp 
e9a214a8973b3dfac69d59e90ce08473baa7eba4 
  src/tests/health_check_tests.cpp 50a252b5267cbc3b21ddc75e1da86c4975f6faf1 
  src/tests/hook_tests.cpp d864ef3d6153dedab8cc47403b1b9f76d75aeb5c 
  src/tests/mesos.hpp 4cae54b4df906d4b7e8fe8d40d5b0ad59d260e6f 
  src/tests/mesos.cpp f5034f9f8de7040182e10f51be125a87b29fdd24 

Diff: https://reviews.apache.org/r/51497/diff/


Testing
---


Thanks,

haosdent huang